Update CI_JOB_TOKEN error message on git repository requests

Add a new error message for job token case
Differenciate errors from user has no permissions and project is not in allowlist
Rename condition for better clarity
Update token_forbidden error message with suggested
Add more details on no permission error

Changelog: changed

app/policies/project_policy.rb

@@ -142,10 +142,15 @@ class ProjectPolicy < BasePolicy
   end
 
   desc "If user is authenticated via CI job token then the target project should be in scope"
-  condition(:project_allowed_for_job_token) do
+  condition(:project_allowed_for_job_token_by_scope) do
     !@user&.from_ci_job_token? || @user.ci_job_token_scope.accessible?(project)
   end
 
+  desc "Public, internal or project in the scope allowed via CI job token"
+  condition(:project_allowed_for_job_token) do
+    public_project? || internal_access? || project_allowed_for_job_token_by_scope?
+  end
+
   desc "If the user is via CI job token and project container registry visibility allows access"
   condition(:job_token_container_registry) { job_token_access_allowed_to?(:container_registry) }
 
@@ -745,10 +750,10 @@ class ProjectPolicy < BasePolicy
   end
 
   # If the project is private
-  rule { ~public_project & ~internal_access & ~project_allowed_for_job_token }.prevent_all
+  rule { ~project_allowed_for_job_token }.prevent_all
 
   # If this project is public or internal we want to prevent all aside from a few public policies
-  rule { public_or_internal & ~project_allowed_for_job_token }.policy do
+  rule { public_or_internal & ~project_allowed_for_job_token_by_scope }.policy do
     prevent :guest_access
     prevent :public_access
     prevent :reporter_access
@@ -757,7 +762,7 @@ class ProjectPolicy < BasePolicy
     prevent :owner_access
   end
 
-  rule { public_project & ~project_allowed_for_job_token }.policy do
+  rule { public_project & ~project_allowed_for_job_token_by_scope }.policy do
     prevent :public_user_access
   end
 

ee/app/policies/ee/project_policy.rb

@@ -801,7 +801,7 @@ module ProjectPolicy
         enable :admin_merge_request_approval_settings
       end
 
-      rule { security_policy_bot & project_allowed_for_job_token }.policy do
+      rule { security_policy_bot & project_allowed_for_job_token_by_scope }.policy do
         enable :create_pipeline
         enable :create_bot_pipeline
         enable :build_download_code

lib/gitlab/git_access.rb

@@ -23,6 +23,8 @@ class GitAccess
       deploy_key_upload: 'This deploy key does not have write access to this project.',
       no_repo: 'A repository for this project does not exist yet.',
       project_not_found: "The project you were looking for could not be found or you don't have permission to view it.",
+      auth_by_job_token_forbidden: 'Insufficient permissions to pull from the repository of project %{target_project_path}.',
+      auth_by_job_token_project_not_in_allowlist: 'Authentication by CI/CD job token not allowed from %{source_project_path} to %{target_project_path}.',
       command_not_allowed: "The command you're trying to execute is not allowed.",
       upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
       receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.',
@@ -242,7 +244,22 @@ def check_authentication_abilities!
     end
 
     def check_project_accessibility!
-      raise NotFoundError, not_found_message unless can_read_project?
+      return if can_read_project?
+
+      if user&.from_ci_job_token?
+
+        policy = ProjectPolicy.new(user, project)
+
+        if policy.project_allowed_for_job_token?
+          raise ForbiddenError, format(error_message(:auth_by_job_token_forbidden), target_project_path: project.path)
+        else
+          source_project = user.ci_job_token_scope.current_project
+
+          raise ForbiddenError, format(error_message(:auth_by_job_token_project_not_in_allowlist), source_project_path: source_project.path, target_project_path: project.path)
+        end
+      else
+        raise NotFoundError, not_found_message
+      end
     end
 
     def not_found_message

spec/lib/gitlab/git_access_spec.rb

@@ -1281,6 +1281,64 @@ def self.run_permission_checks(permissions_matrix)
       end
     end
 
+    describe 'when request is authorized with job token' do
+      let(:source_project) { create(:project) }
+      let(:auth_result_type) { :build }
+      let(:job) { build_stubbed(:ci_build, project: source_project, user: user) }
+      let(:scope) { user.set_ci_job_token_scope!(job) }
+
+      before do
+        accept_terms(user)
+
+        source_project.add_maintainer(user)
+        source_project.save!
+
+        allow(user).to receive(:ci_job_token_scope).and_return(scope)
+      end
+
+      context 'and target project in the allow list' do
+        let(:access) { access_class.new(user, project, 'web', authentication_abilities: [:download_code], repository_path: repository_path) }
+
+        before do
+          project.add_maintainer(user)
+          project.save!
+
+          allow(scope).to receive(:accessible?).with(project).and_return(true)
+        end
+
+        it 'does not block http pull' do
+          expect { pull_access_check }.not_to raise_error
+        end
+      end
+
+      context 'and target_project in the allow list, but user does not have permissions' do
+        let(:access) { access_class.new(user, project, 'web', authentication_abilities: [:download_code], repository_path: repository_path) }
+
+        before do
+          allow(scope).to receive(:accessible?).with(project).and_return(true)
+        end
+
+        it 'blocks http pull' do
+          expect { pull_access_check }.to raise_forbidden_by_job_token(project)
+        end
+      end
+
+      context 'and target project is not in the allow list' do
+        let(:access) { access_class.new(user, project, 'web', authentication_abilities: [:download_code], repository_path: repository_path) }
+
+        before do
+          project.add_maintainer(user)
+          project.save!
+
+          allow(scope).to receive(:accessible?).with(project).and_return(false)
+        end
+
+        it 'blocks http pull' do
+          expect { pull_access_check }.to raise_forbidden_by_job_token_allowlist(source_project, project)
+        end
+      end
+    end
+
     describe 'when request is made from CI' do
       let(:auth_result_type) { :build }
       let(:job) { build_stubbed(:ci_build, project: project, user: user) }
@@ -1359,6 +1417,16 @@ def raise_not_found
     raise_error(described_class::NotFoundError, described_class::ERROR_MESSAGES[:project_not_found])
   end
 
+  def raise_forbidden_by_job_token(target_project)
+    raise_error(described_class::ForbiddenError, format(described_class::ERROR_MESSAGES[:auth_by_job_token_forbidden],
+      target_project_path: target_project.path))
+  end
+
+  def raise_forbidden_by_job_token_allowlist(source_project, target_project)
+    raise_error(described_class::ForbiddenError, format(described_class::ERROR_MESSAGES[:auth_by_job_token_project_not_in_allowlist],
+      source_project_path: source_project.path, target_project_path: target_project.path))
+  end
+
   def build_authentication_abilities
     [
       :read_project,

spec/policies/project_policy_spec.rb

@@ -129,6 +129,45 @@ def set_access_level(access_level)
     end
   end
 
+  describe 'condition project_allowed_for_job_token' do
+    using RSpec::Parameterized::TableSyntax
+
+    subject { described_class.new(current_user, project).project_allowed_for_job_token? }
+
+    where(:project_visibility, :role, :project_in_allowlist, :allowed) do
+      :public   | :developer | true  | true
+      :public   | :developer | false | true
+      :public   | :owner     | true  | true
+      :public   | :owner     | false | true
+      :internal | :developer | true  | true
+      :internal | :developer | false | true
+      :internal | :owner     | true  | true
+      :internal | :owner     | false | true
+      :private  | :developer | true  | true
+      :private  | :developer | false | false
+      :private  | :owner     | true  | true
+      :private  | :owner     | false | false
+    end
+
+    with_them do
+      let(:current_user) { public_send(role) }
+      let(:scope_project) { public_project }
+      let(:project) { public_send("#{project_visibility}_project") }
+      let(:job) { build_stubbed(:ci_build, project: scope_project, user: current_user) }
+
+      before do
+        allow(current_user).to receive(:ci_job_token_scope).and_return(current_user.set_ci_job_token_scope!(job))
+        allow(current_user.ci_job_token_scope).to receive(:accessible?).with(project).and_return(project_in_allowlist)
+      end
+
+      if params[:allowed]
+        it { is_expected.to be_truthy }
+      else
+        it { is_expected.to be_falsey }
+      end
+    end
+  end
+
   context 'creating_merge_request_in' do
     context 'when the current_user can download_code' do
       before do

spec/requests/git_http_spec.rb

@@ -965,10 +965,10 @@ def pull
 
               it_behaves_like 'can download code only'
 
-              it 'downloads from other project get status 404' do
+              it 'downloads from other project get status 403' do
                 clone_get "#{other_project.full_path}.git", user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_gitlab_http_status(:not_found)
+                expect(response).to have_gitlab_http_status(:forbidden)
               end
 
               context 'when users password is expired' do
@@ -1566,10 +1566,10 @@ def attempt_login(include_password)
 
               it_behaves_like 'can download code only'
 
-              it 'downloads from other project get status 404' do
+              it 'downloads from other project get status 403' do
                 clone_get "#{other_project.full_path}.git", user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_gitlab_http_status(:not_found)
+                expect(response).to have_gitlab_http_status(:forbidden)
               end
 
               context 'when users password is expired' do