1 files changed, 51 insertions, 23 deletions

diff --git a/spec/services/resource_access_tokens/revoke_service_spec.rb b/spec/services/resource_access_tokens/revoke_service_spec.rb
index ffc06d770f8..af29ee2a721 100644
--- a/spec/services/resource_access_tokens/revoke_service_spec.rb
+++ b/spec/services/resource_access_tokens/revoke_service_spec.rb
@@ -8,17 +8,17 @@ RSpec.describe ResourceAccessTokens::RevokeService do
   let_it_be(:user) { create(:user) }
   let(:access_token) { create(:personal_access_token, user: resource_bot) }
 
-  describe '#execute' do
+  describe '#execute', :sidekiq_inline do
     # Created shared_examples as it will easy to include specs for group bots in https://gitlab.com/gitlab-org/gitlab/-/issues/214046
     shared_examples 'revokes access token' do
       it { expect(subject.success?).to be true }
 
-      it { expect(subject.message).to eq("Revoked access token: #{access_token.name}") }
+      it { expect(subject.message).to eq("Access token #{access_token.name} has been revoked and the bot user has been scheduled for deletion.") }
 
-      it 'revokes token access' do
-        subject
+      it 'calls delete user worker' do
+        expect(DeleteUserWorker).to receive(:perform_async).with(user.id, resource_bot.id, skip_authorization: true)
 
-        expect(access_token.reload.revoked?).to be true
+        subject
       end
 
       it 'removes membership of bot user' do
@@ -34,6 +34,12 @@ RSpec.describe ResourceAccessTokens::RevokeService do
 
         expect(issue.reload.author.ghost?).to be true
       end
+
+      it 'deletes project bot user' do
+        subject
+
+        expect(User.exists?(resource_bot.id)).to be_falsy
+      end
     end
 
     shared_examples 'rollback revoke steps' do
@@ -56,49 +62,71 @@ RSpec.describe ResourceAccessTokens::RevokeService do
 
         expect(issue.reload.author.ghost?).to be false
       end
+
+      it 'does not destroy project bot user' do
+        subject
+
+        expect(User.exists?(resource_bot.id)).to be_truthy
+      end
     end
 
     context 'when resource is a project' do
       let_it_be(:resource) { create(:project, :private) }
-      let_it_be(:resource_bot) { create(:user, :project_bot) }
+      let(:resource_bot) { create(:user, :project_bot) }
 
-      before_all do
+      before do
         resource.add_maintainer(user)
         resource.add_maintainer(resource_bot)
       end
 
       it_behaves_like 'revokes access token'
 
-      context 'when revoke fails' do
-        context 'invalid resource type' do
-          subject { described_class.new(user, resource, access_token).execute }
+      context 'revoke fails' do
+        let_it_be(:other_user) { create(:user) }
 
-          let_it_be(:resource) { double }
-          let_it_be(:resource_bot) { create(:user, :project_bot) }
+        context 'when access token does not belong to this project' do
+          it 'does not find the bot' do
+            other_access_token = create(:personal_access_token, user: other_user)
 
-          it 'returns error response' do
-            response = subject
+            response = described_class.new(user, resource, other_access_token).execute
 
             expect(response.success?).to be false
             expect(response.message).to eq("Failed to find bot user")
+            expect(access_token.reload.revoked?).to be false
           end
-
-          it { expect { subject }.not_to change(access_token.reload, :revoked) }
         end
 
-        context 'when migration to ghost user fails' do
-          before do
-            allow_next_instance_of(::Members::DestroyService) do |service|
-              allow(service).to receive(:execute).and_return(false)
+        context 'when user does not have permission to destroy bot' do
+          context 'when non-project member tries to delete project bot' do
+            it 'does not allow other user to delete bot' do
+              response = described_class.new(other_user, resource, access_token).execute
+
+              expect(response.success?).to be false
+              expect(response.message).to eq("#{other_user.name} cannot delete #{access_token.user.name}")
+              expect(access_token.reload.revoked?).to be false
             end
           end
 
-          it_behaves_like 'rollback revoke steps'
+          context 'when non-maintainer project member tries to delete project bot' do
+            let(:developer) { create(:user) }
+
+            before do
+              resource.add_developer(developer)
+            end
+
+            it 'does not allow developer to delete bot' do
+              response = described_class.new(developer, resource, access_token).execute
+
+              expect(response.success?).to be false
+              expect(response.message).to eq("#{developer.name} cannot delete #{access_token.user.name}")
+              expect(access_token.reload.revoked?).to be false
+            end
+          end
         end
 
-        context 'when migration to ghost user fails' do
+        context 'when deletion of bot user fails' do
           before do
-            allow_next_instance_of(::Users::MigrateToGhostUserService) do |service|
+            allow_next_instance_of(::ResourceAccessTokens::RevokeService) do |service|
               allow(service).to receive(:execute).and_return(false)
             end
           end