Merge branch '220388-project-access-tokens-bot-not-deleted-when-token-expires' into 'master'

Remove project bot user membership when project access token expires

See merge request gitlab-org/gitlab!43605

app/services/resource_access_tokens/create_service.rb

@@ -94,7 +94,7 @@ module ResourceAccessTokens
     end
 
     def provision_access(resource, user)
-      resource.add_maintainer(user)
+      resource.add_user(user, :maintainer, expires_at: params[:expires_at])
     end
 
     def error(message)

changelogs/unreleased/220388-project-access-tokens-bot-not-deleted-when-token-expires.yml

+---
+title: Remove project bot user membership when project access token expires
+merge_request: 43605
+author:
+type: fixed

spec/services/resource_access_tokens/create_service_spec.rb

@@ -24,6 +24,7 @@ RSpec.describe ResourceAccessTokens::CreateService do
       end
     end
 
+    # Remove this shared example when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43190 merges
     shared_examples 'fails on gitlab.com' do
       before do
         allow(Gitlab).to receive(:com?) { true }
@@ -68,8 +69,8 @@ RSpec.describe ResourceAccessTokens::CreateService do
       end
 
       context 'bot name' do
-        context 'when no value is passed' do
-          it 'uses default value' do
+        context 'when no name is passed' do
+          it 'uses default name' do
             response = subject
             access_token = response.payload[:access_token]
 
@@ -77,10 +78,10 @@ RSpec.describe ResourceAccessTokens::CreateService do
           end
         end
 
-        context 'when user provides value' do
+        context 'when user provides name' do
           let_it_be(:params) { { name: 'Random bot' } }
 
-          it 'overrides the default value' do
+          it 'overrides the default name value' do
             response = subject
             access_token = response.payload[:access_token]
 
@@ -112,7 +113,7 @@ RSpec.describe ResourceAccessTokens::CreateService do
         context 'when user provides scope explicitly' do
           let_it_be(:params) { { scopes: Gitlab::Auth::REPOSITORY_SCOPES } }
 
-          it 'overrides the default value' do
+          it 'overrides the default scope value' do
             response = subject
             access_token = response.payload[:access_token]
 
@@ -121,24 +122,44 @@ RSpec.describe ResourceAccessTokens::CreateService do
         end
 
         context 'expires_at' do
-          context 'when no value is passed' do
-            it 'uses default value' do
+          context 'when no expiration value is passed' do
+            it 'uses nil expiration value' do
               response = subject
               access_token = response.payload[:access_token]
 
               expect(access_token.expires_at).to eq(nil)
             end
+
+            context 'expiry of the project bot member' do
+              it 'project bot membership does not expire' do
+                response = subject
+                access_token = response.payload[:access_token]
+                project_bot = access_token.user
+
+                expect(project.members.find_by(user_id: project_bot.id).expires_at).to eq(nil)
+              end
+            end
           end
 
-          context 'when user provides value' do
+          context 'when user provides expiration value' do
             let_it_be(:params) { { expires_at: Date.today + 1.month } }
 
-            it 'overrides the default value' do
+            it 'overrides the default expiration value' do
               response = subject
               access_token = response.payload[:access_token]
 
               expect(access_token.expires_at).to eq(params[:expires_at])
             end
+
+            context 'expiry of the project bot member' do
+              it 'sets the project bot to expire on the same day as the token' do
+                response = subject
+                access_token = response.payload[:access_token]
+                project_bot = access_token.user
+
+                expect(project.members.find_by(user_id: project_bot.id).expires_at).to eq(params[:expires_at])
+              end
+            end
           end
 
           context 'when invalid scope is passed' do
@@ -155,7 +176,7 @@ RSpec.describe ResourceAccessTokens::CreateService do
 
       context 'when access provisioning fails' do
         before do
-          allow(resource).to receive(:add_maintainer).and_return(nil)
+          allow(resource).to receive(:add_user).and_return(nil)
         end
 
         it 'returns error' do