Merge branch 'active-yet-expired-tokens' into 'master'

Display tokens that have do not have expiry enforced

See merge request gitlab-org/gitlab!34570

app/controllers/profiles/personal_access_tokens_controller.rb

@@ -40,14 +40,18 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController
     params.require(:personal_access_token).permit(:name, :expires_at, scopes: [])
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def set_index_vars
     @scopes = Gitlab::Auth.available_scopes_for(current_user)
 
     @inactive_personal_access_tokens = finder(state: 'inactive').execute
-    @active_personal_access_tokens = finder(state: 'active').execute.order(:expires_at)
+    @active_personal_access_tokens = active_personal_access_tokens
 
     @new_personal_access_token = PersonalAccessToken.redis_getdel(current_user.id)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
+
+  def active_personal_access_tokens
+    finder(state: 'active', sort: 'expires_at_asc').execute
+  end
 end
+
+Profiles::PersonalAccessTokensController.prepend_if_ee('EE::Profiles::PersonalAccessTokensController')

app/finders/personal_access_tokens_finder.rb

@@ -51,6 +51,8 @@ class PersonalAccessTokensFinder
       tokens.active
     when 'inactive'
       tokens.inactive
+    when 'active_or_expired'
+      tokens.not_revoked.expired.or(tokens.active)
     else
       tokens
     end

app/models/personal_access_token.rb

@@ -22,6 +22,8 @@ class PersonalAccessToken < ApplicationRecord
   scope :inactive, -> { where("revoked = true OR expires_at < NOW()") }
   scope :with_impersonation, -> { where(impersonation: true) }
   scope :without_impersonation, -> { where(impersonation: false) }
+  scope :revoked, -> { where(revoked: true) }
+  scope :not_revoked, -> { where(revoked: [false, nil]) }
   scope :for_user, -> (user) { where(user: user) }
   scope :preload_users, -> { preload(:user) }
   scope :order_expires_at_asc, -> { reorder(expires_at: :asc) }

app/views/shared/access_tokens/_table.html.haml

@@ -26,8 +26,12 @@
             %td= token.created_at.to_date.to_s(:medium)
             %td
               - if token.expires?
-                %span{ class: ('text-warning' if token.expires_soon?) }
-                  = _('In %{time_to_now}') % { time_to_now: distance_of_time_in_words_to_now(token.expires_at) }
+                - if token.expires_at.past? || token.expires_at.today?
+                  %span{ class: 'text-danger has-tooltip', title: _('Expiration not enforced') }
+                    = _('Expired')
+                - else
+                  %span{ class: ('text-warning' if token.expires_soon?) }
+                    = _('In %{time_to_now}') % { time_to_now: distance_of_time_in_words_to_now(token.expires_at) }
               - else
                 %span.token-never-expires-label= _('Never')
             %td= token.scopes.present? ? token.scopes.join(', ') : _('<no scopes selected>')

ee/app/controllers/ee/profiles/personal_access_tokens_controller.rb

+# frozen_string_literal: true
+
+module EE
+  module Profiles::PersonalAccessTokensController
+    extend ::Gitlab::Utils::Override
+
+    private
+
+    override :active_personal_access_tokens
+    def active_personal_access_tokens
+      return super if ::PersonalAccessToken.expiration_enforced?
+
+      finder(state: 'active_or_expired', sort: 'expires_at_asc').execute
+    end
+  end
+end

ee/changelogs/unreleased/active-yet-expired-tokens.yml

+---
+title: Add ability to view tokens without expiry enforcement
+merge_request: 34570
+author:
+type: added

ee/spec/controllers/ee/profiles/personal_access_tokens_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Profiles::PersonalAccessTokensController do
+  describe '#index' do
+    context 'expired yet active personal access token' do
+      subject { get :index }
+
+      let!(:user) { create(:user) }
+      let!(:expired_active_personal_access_token) { create(:personal_access_token, expires_at: 5.days.ago, user: user) }
+
+      before do
+        sign_in(user)
+        stub_licensed_features(enforce_pat_expiration: licensed)
+        stub_application_setting(enforce_pat_expiration: application_setting)
+      end
+
+      shared_examples 'does not include in list of active tokens' do
+        it do
+          subject
+
+          expect(assigns(:active_personal_access_tokens)).not_to include(expired_active_personal_access_token)
+        end
+      end
+
+      context 'when token expiry is enforced' do
+        using RSpec::Parameterized::TableSyntax
+
+        where(:licensed, :application_setting) do
+          true  | true
+          false | true
+          false | false
+        end
+
+        with_them do
+          it_behaves_like 'does not include in list of active tokens'
+        end
+      end
+
+      context 'when token expiry is NOT enforced' do
+        let(:licensed) { true }
+        let(:application_setting) { false }
+
+        it do
+          subject
+
+          expect(assigns(:active_personal_access_tokens)).to include(expired_active_personal_access_token)
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -9257,6 +9257,9 @@ msgstr ""
 msgid "Expiration date"
 msgstr ""
 
+msgid "Expiration not enforced"
+msgstr ""
+
 msgid "Expiration policy for the Container Registry is a perfect solution for keeping the Registry space down while still enjoying the full power of GitLab CI/CD."
 msgstr ""
 

spec/finders/personal_access_tokens_finder_spec.rb

@@ -218,6 +218,24 @@ RSpec.describe PersonalAccessTokensFinder do
         end
       end
 
+      describe 'with active or expired state' do
+        before do
+          params[:state] = 'active_or_expired'
+        end
+
+        it 'includes active tokens' do
+          is_expected.to include(active_personal_access_token, active_impersonation_token)
+        end
+
+        it 'includes expired tokens' do
+          is_expected.to include(expired_personal_access_token, expired_impersonation_token)
+        end
+
+        it 'does not include revoked tokens' do
+          is_expected.not_to include(revoked_personal_access_token, revoked_impersonation_token)
+        end
+      end
+
       describe 'with id' do
         subject { finder(params).find_by_id(active_personal_access_token.id) }
 

spec/models/personal_access_token_spec.rb

@@ -187,6 +187,20 @@ describe PersonalAccessToken do
         expect(described_class.without_impersonation).to contain_exactly(personal_access_token)
       end
     end
+
+    describe 'revoke scopes' do
+      let_it_be(:revoked_token) { create(:personal_access_token, :revoked) }
+      let_it_be(:non_revoked_token) { create(:personal_access_token, revoked: false) }
+      let_it_be(:non_revoked_token2) { create(:personal_access_token, revoked: nil) }
+
+      describe '.revoked' do
+        it { expect(described_class.revoked).to contain_exactly(revoked_token) }
+      end
+
+      describe '.not_revoked' do
+        it { expect(described_class.not_revoked).to contain_exactly(non_revoked_token, non_revoked_token2) }
+      end
+    end
   end
 
   describe '.simple_sorts' do