Merge branch...

Merge branch '259665-project-access-tokens-not-working-when-fetching-updates-for-a-previously-checked-out' into 'master'

Fix project access token build authentication error

See merge request gitlab-org/gitlab!47247

app/policies/project_policy.rb

@@ -135,6 +135,10 @@ class ProjectPolicy < BasePolicy
     ::Feature.enabled?(:build_service_proxy, @subject)
   end
 
+  condition(:project_bot_is_member) do
+    user.project_bot? & team_member?
+  end
+
   with_scope :subject
   condition(:packages_disabled) { !@subject.packages_enabled }
 
@@ -608,6 +612,8 @@ class ProjectPolicy < BasePolicy
     enable :admin_resource_access_tokens
   end
 
+  rule { project_bot_is_member & ~blocked }.enable :bot_log_in
+
   private
 
   def user_is_user?

changelogs/unreleased/259665-project-access-tokens-not-working-when-fetching-updates-for-a-prev.yml

+---
+title: Fix project access token build authentication error
+merge_request: 47247
+author:
+type: fixed

lib/gitlab/auth.rb

@@ -196,11 +196,9 @@ module Gitlab
 
         return unless token
 
-        return if project && token.user.project_bot? && !project.bots.include?(token.user)
-
         return unless valid_scoped_token?(token, all_available_scopes)
 
-        if token.user.project_bot? || token.user.can?(:log_in)
+        if token.user.can?(:log_in) || token.user.can?(:bot_log_in, project)
           Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes))
         end
       end
@@ -285,7 +283,7 @@ module Gitlab
         return unless build.project.builds_enabled?
 
         if build.user
-          return unless build.user.can?(:log_in)
+          return unless build.user.can?(:log_in) || build.user.can?(:bot_log_in, build.project)
 
           # If user is assigned to build, use restricted credentials of user
           Gitlab::Auth::Result.new(build.user, build.project, :build, build_authentication_abilities)

spec/lib/gitlab/auth_spec.rb

@@ -364,20 +364,33 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
         let_it_be(:project_access_token) { create(:personal_access_token, user: project_bot_user) }
 
         context 'with valid project access token' do
-          before_all do
+          before do
             project.add_maintainer(project_bot_user)
           end
 
-          it 'succeeds' do
+          it 'successfully authenticates the project bot' do
             expect(gl_auth.find_for_git_client(project_bot_user.username, project_access_token.token, project: project, ip: 'ip'))
               .to eq(Gitlab::Auth::Result.new(project_bot_user, nil, :personal_access_token, described_class.full_authentication_abilities))
           end
         end
 
         context 'with invalid project access token' do
-          it 'fails' do
-            expect(gl_auth.find_for_git_client(project_bot_user.username, project_access_token.token, project: project, ip: 'ip'))
-              .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+          context 'when project bot is not a project member' do
+            it 'fails for a non-project member' do
+              expect(gl_auth.find_for_git_client(project_bot_user.username, project_access_token.token, project: project, ip: 'ip'))
+                .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+            end
+          end
+
+          context 'when project bot user is blocked' do
+            before do
+              project_bot_user.block!
+            end
+
+            it 'fails for a blocked project bot' do
+              expect(gl_auth.find_for_git_client(project_bot_user.username, project_access_token.token, project: project, ip: 'ip'))
+                .to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
+            end
           end
         end
       end

spec/policies/project_policy_spec.rb

@@ -401,6 +401,40 @@ RSpec.describe ProjectPolicy do
     end
   end
 
+  describe 'bot_log_in' do
+    let(:bot_user) { create(:user, :project_bot) }
+    let(:project) { private_project }
+
+    context 'when bot is in project and is not blocked' do
+      before do
+        project.add_maintainer(bot_user)
+      end
+
+      it 'is a valid project bot' do
+        expect(bot_user.can?(:bot_log_in, project)).to be_truthy
+      end
+    end
+
+    context 'when project bot is invalid' do
+      context 'when bot is not in project' do
+        it 'is not a valid project bot' do
+          expect(bot_user.can?(:bot_log_in, project)).to be_falsy
+        end
+      end
+
+      context 'when bot user is blocked' do
+        before do
+          project.add_maintainer(bot_user)
+          bot_user.block!
+        end
+
+        it 'is not a valid project bot' do
+          expect(bot_user.can?(:bot_log_in, project)).to be_falsy
+        end
+      end
+    end
+  end
+
   context 'support bot' do
     let(:current_user) { User.support_bot }