Merge branch '214811-add-revoke-buttons-to-the-pat-tab-of-the-credential-inventory' into 'master'

Add revoke button to the PAT tab of the instance credentials inventory

See merge request gitlab-org/gitlab!39712

ee/app/controllers/admin/credentials_controller.rb

@@ -5,9 +5,9 @@ class Admin::CredentialsController < Admin::ApplicationController
   include CredentialsInventoryActions
   include Analytics::UniqueVisitsHelper
 
-  helper_method :credentials_inventory_path, :user_detail_path
+  helper_method :credentials_inventory_path, :user_detail_path, :personal_access_token_revoke_path, :revoke_button_available?
 
-  before_action :check_license_credentials_inventory_available!, only: [:index]
+  before_action :check_license_credentials_inventory_available!, only: [:index, :revoke]
 
   track_unique_visits :index, target_id: 'i_compliance_credential_inventory'
 
@@ -27,6 +27,16 @@ class Admin::CredentialsController < Admin::ApplicationController
     admin_user_path(user)
   end
 
+  override :personal_access_token_revoke_path
+  def personal_access_token_revoke_path(token)
+    revoke_admin_credential_path(token)
+  end
+
+  override :revoke_button_available?
+  def revoke_button_available?
+    true
+  end
+
   override :users
   def users
     nil

ee/app/controllers/concerns/credentials_inventory_actions.rb

@@ -14,6 +14,14 @@ module CredentialsInventoryActions
     end
   end
 
+  def revoke
+    personal_access_token = PersonalAccessTokensFinder.new({ user: users, impersonation: false }, current_user).find(params[:id])
+    service = PersonalAccessTokens::RevokeService.new(current_user, token: personal_access_token).execute
+    service.success? ? flash[:notice] = service.message : flash[:alert] = service.message
+
+    redirect_to credentials_inventory_path(page: params[:page])
+  end
+
   private
 
   def filter_credentials

ee/app/helpers/credentials_inventory_helper.rb

@@ -17,6 +17,10 @@ module CredentialsInventoryHelper
     License.feature_available?(:credentials_inventory)
   end
 
+  def revoke_button_available?
+    false
+  end
+
   def credentials_inventory_path(args)
     raise NotImplementedError, "#{self.class} does not implement #{__method__}"
   end
@@ -24,4 +28,8 @@ module CredentialsInventoryHelper
   def user_detail_path(user)
     raise NotImplementedError, "#{self.class} does not implement #{__method__}"
   end
+
+  def personal_access_token_revoke_path(token)
+    raise NotImplementedError, "#{self.class} does not implement #{__method__}"
+  end
 end

ee/app/views/shared/credentials_inventory/personal_access_tokens/_personal_access_token.html.haml

@@ -27,3 +27,5 @@
       - if personal_access_token.revoked?
         -# We're inferring the revoked date from the last updated_at, see https://gitlab.com/gitlab-org/gitlab/-/issues/218046#note_362875952
         = personal_access_token.updated_at.to_date
+      - elsif revoke_button_available? && personal_access_token.active?
+        = link_to _('Revoke'), personal_access_token_revoke_path(personal_access_token), method: :put, data: { confirm: _('Are you sure you want to revoke this personal access token? This action cannot be undone.') }, class: 'btn btn-danger btn-danger-secondary btn-md btn-secondary gl-button'

ee/changelogs/unreleased/214811-add-revoke-buttons-to-the-pat-tab-of-the-instance-credential-inventory.yml

+---
+title: Add Revoke buttons to the PAT tab of the instance credential inventory
+merge_request: 39712
+author:
+type: added

ee/config/routes/admin.rb

@@ -21,7 +21,11 @@ namespace :admin do
   resource :email, only: [:show, :create]
   resources :audit_logs, controller: 'audit_logs', only: [:index]
   resources :audit_log_reports, only: [:index], constraints: { format: :csv }
-  resources :credentials, only: [:index]
+  resources :credentials, only: [:index] do
+    member do
+      put :revoke
+    end
+  end
 
   resource :license, only: [:show, :new, :create, :destroy] do
     get :download, on: :member

ee/spec/controllers/admin/credentials_controller_spec.rb

@@ -91,4 +91,107 @@ RSpec.describe Admin::CredentialsController do
       end
     end
   end
+
+  describe 'PUT #revoke' do
+    context 'admin user' do
+      let_it_be(:current_user) { create(:admin) }
+
+      before do
+        sign_in(current_user)
+      end
+
+      context 'when `credentials_inventory` feature is enabled' do
+        before do
+          stub_licensed_features(credentials_inventory: true)
+        end
+
+        context 'non-existent personal access token specified' do
+          it 'returns 404' do
+            put :revoke, params: { id: 999999999999999999999999999999999 }
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+
+        describe 'with an existing personal access token' do
+          context 'does not have permissions to revoke the credential' do
+            let_it_be(:personal_access_token) { create(:personal_access_token) }
+
+            before do
+              expect(Ability).to receive(:allowed?).with(current_user, :log_in, :global) { true }
+              expect(Ability).to receive(:allowed?).with(current_user, :revoke_token, personal_access_token) { false }
+            end
+
+            it 'returns the flash error message' do
+              put :revoke, params: { id: personal_access_token.id }
+
+              expect(response).to redirect_to(admin_credentials_path)
+              expect(flash[:alert]).to eql 'Not permitted to revoke'
+            end
+          end
+
+          context 'personal access token is already revoked' do
+            let_it_be(:personal_access_token) { create(:personal_access_token, revoked: true) }
+
+            it 'returns the flash success message' do
+              put :revoke, params: { id: personal_access_token.id }
+
+              expect(response).to redirect_to(admin_credentials_path)
+              expect(flash[:notice]).to eql 'Revoked personal access token %{personal_access_token_name}!' % { personal_access_token_name: personal_access_token.name }
+            end
+          end
+
+          context 'personal access token is already expired' do
+            let_it_be(:personal_access_token) { create(:personal_access_token, expires_at: 5.days.ago) }
+
+            it 'returns the flash success message' do
+              put :revoke, params: { id: personal_access_token.id }
+
+              expect(response).to redirect_to(admin_credentials_path)
+              expect(flash[:notice]).to eql 'Revoked personal access token %{personal_access_token_name}!' % { personal_access_token_name: personal_access_token.name }
+            end
+          end
+
+          context 'personal access token is not revoked or expired' do
+            let_it_be(:personal_access_token) { create(:personal_access_token) }
+
+            it 'returns the flash success message' do
+              put :revoke, params: { id: personal_access_token.id }
+
+              expect(response).to redirect_to(admin_credentials_path)
+              expect(flash[:notice]).to eql 'Revoked personal access token %{personal_access_token_name}!' % { personal_access_token_name: personal_access_token.name }
+            end
+          end
+        end
+      end
+
+      context 'when `credentials_inventory` feature is disabled' do
+        let_it_be(:personal_access_token) { create(:personal_access_token) }
+
+        before do
+          stub_licensed_features(credentials_inventory: false)
+        end
+
+        it 'returns 404' do
+          put :revoke, params: { id: personal_access_token.id }
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+
+    context 'non-admin user' do
+      let_it_be(:personal_access_token) { create(:personal_access_token) }
+
+      before do
+        sign_in(create(:user))
+      end
+
+      it 'returns 404' do
+        put :revoke, params: { id: personal_access_token.id }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
 end

ee/spec/support/shared_examples/features/credentials_inventory_shared_examples.rb

@@ -24,6 +24,10 @@ RSpec.shared_examples_for 'credentials inventory personal access tokens' do |gro
       expect(first_row.text).to include('2019-12-10')
       expect(first_row.text).to include('Never')
       expect(first_row.text).not_to include('2020-06-22')
+
+      unless group_managed_account
+        expect(first_row).to have_selector('a.btn', text: 'Revoke')
+      end
     end
   end
 
@@ -67,6 +71,10 @@ RSpec.shared_examples_for 'credentials inventory personal access tokens' do |gro
 
     it 'shows the details with a revoked date', :aggregate_failures do
       expect(first_row.text).to include('2020-06-22')
+
+      unless group_managed_account
+        expect(first_row).not_to have_selector('a.btn', text: 'Revoke')
+      end
     end
   end
 end

ee/spec/views/shared/credentials_inventory/personal_access_tokens/_personal_access_token.html.haml_spec.rb

@@ -5,13 +5,22 @@ require 'spec_helper'
 RSpec.describe('shared/credentials_inventory/personal_access_tokens/_personal_access_token.html.haml') do
   let_it_be(:user) { create(:user) }
   let_it_be(:expiry_date) { 20.days.since }
-  let_it_be(:personal_access_token) { create(:personal_access_token, user: user, expires_at: expiry_date)}
+  let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user, expires_at: expiry_date)}
 
   before do
+    freeze_time
+
     allow(view).to receive(:user_detail_path).and_return('abcd')
+    allow(view).to receive(:personal_access_token_revoke_path).and_return('revoke')
+    allow(view).to receive(:revoke_button_available?).and_return(false)
+
     render 'shared/credentials_inventory/personal_access_tokens/personal_access_token', personal_access_token: personal_access_token
   end
 
+  after do
+    unfreeze_time
+  end
+
   it 'shows the users name' do
     expect(rendered).to have_text(user.name)
   end
@@ -27,19 +36,61 @@ RSpec.describe('shared/credentials_inventory/personal_access_tokens/_personal_ac
   context 'revoked date' do
     let_it_be(:updated_at_date) { 10.days.ago }
 
-    context 'when set' do
-      let_it_be(:personal_access_token) { create(:personal_access_token, user: user, updated_at: updated_at_date, revoked: true)}
+    context 'without revoke button available' do
+      context 'when revoked is set' do
+        let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user, updated_at: updated_at_date, revoked: true)}
+
+        it 'shows the revoked on date' do
+          expect(rendered).to have_text(updated_at_date.to_date.to_s)
+        end
 
-      it 'shows the last accessed on date' do
-        expect(rendered).to have_text(personal_access_token.updated_at.to_date.to_s)
+        it 'does not show the revoke button' do
+          expect(rendered).not_to have_css('a.btn', text: 'Revoke')
+        end
+      end
+
+      context 'when revoked is not set' do
+        let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user, updated_at: updated_at_date)}
+
+        it 'does not show the revoked on date' do
+          expect(rendered).not_to have_text(updated_at_date.to_date.to_s)
+        end
+
+        it 'does not show the revoke button' do
+          expect(rendered).not_to have_css('a.btn', text: 'Revoke')
+        end
       end
     end
 
-    context 'when not set' do
-      let_it_be(:personal_access_token) { create(:personal_access_token, user: user, updated_at: updated_at_date)}
+    context 'with revoke button available' do
+      before do
+        allow(view).to receive(:revoke_button_available?).and_return(true)
+
+        render 'shared/credentials_inventory/personal_access_tokens/personal_access_token', personal_access_token: personal_access_token
+      end
+
+      context 'when revoked is set' do
+        let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user, updated_at: updated_at_date, revoked: true)}
+
+        it 'shows the revoked on date' do
+          expect(rendered).to have_text(updated_at_date.to_date.to_s)
+        end
+
+        it 'does not show the revoke button' do
+          expect(rendered).not_to have_css('a.btn', text: 'Revoke')
+        end
+      end
+
+      context 'when revoked is not set' do
+        let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user, updated_at: updated_at_date)}
+
+        it 'does not show the revoked on date' do
+          expect(rendered).not_to have_text(updated_at_date.to_date.to_s)
+        end
 
-      it 'shows "Never" for the last accessed on date' do
-        expect(rendered).not_to have_text(updated_at_date.to_date.to_s)
+        it 'shows the revoke button' do
+          expect(rendered).to have_css('a.btn', text: 'Revoke')
+        end
       end
     end
   end
@@ -47,7 +98,7 @@ RSpec.describe('shared/credentials_inventory/personal_access_tokens/_personal_ac
   context 'scopes' do
     context 'when set' do
       let_it_be(:scopes) { %w(api read_user read_api) }
-      let_it_be(:personal_access_token) { create(:personal_access_token, user: user, scopes: scopes)}
+      let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user, scopes: scopes)}
 
       it 'shows the scopes' do
         expect(rendered).to have_text(personal_access_token.scopes.join(', '))
@@ -55,7 +106,7 @@ RSpec.describe('shared/credentials_inventory/personal_access_tokens/_personal_ac
     end
 
     context 'when not set' do
-      let_it_be(:personal_access_token) { create(:personal_access_token, user: user)}
+      let_it_be(:personal_access_token) { build_stubbed(:personal_access_token, user: user)}
 
       before do
         # Turns out on creation of a PersonalAccessToken we set some default scopes and you can't pass `nil`

locale/gitlab.pot

@@ -3321,6 +3321,9 @@ msgstr ""
 msgid "Are you sure you want to revoke this nickname?"
 msgstr ""
 
+msgid "Are you sure you want to revoke this personal access token? This action cannot be undone."
+msgstr ""
+
 msgid "Are you sure you want to stop this environment?"
 msgstr ""