From a58be104727e664c765b94e19bfd942d870401e6 Mon Sep 17 00:00:00 2001 From: Aishwarya Subramanian Date: Mon, 15 Jun 2020 16:46:04 -0500 Subject: [PATCH] Listing expired yet active tokens When PAT expiry is not enforced, (ref: https://gitlab.com/gitlab-org/gitlab/-/issues/214723) the tokens that have expired yet continue to be usable are listed in the Active Personal Access Tokens section. These tokens have a help text to indicate that they have expired, yet the expiry is not enforced. And, are given an option to revoke the token. --- .../personal_access_tokens_controller.rb | 10 ++-- app/finders/personal_access_tokens_finder.rb | 2 + app/models/personal_access_token.rb | 2 + .../shared/access_tokens/_table.html.haml | 8 ++- .../personal_access_tokens_controller.rb | 16 ++++++ .../unreleased/active-yet-expired-tokens.yml | 5 ++ .../personal_access_tokens_controller_spec.rb | 53 +++++++++++++++++++ locale/gitlab.pot | 3 ++ .../personal_access_tokens_finder_spec.rb | 18 +++++++ spec/models/personal_access_token_spec.rb | 14 +++++ 10 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 ee/app/controllers/ee/profiles/personal_access_tokens_controller.rb create mode 100644 ee/changelogs/unreleased/active-yet-expired-tokens.yml create mode 100644 ee/spec/controllers/ee/profiles/personal_access_tokens_controller_spec.rb diff --git a/app/controllers/profiles/personal_access_tokens_controller.rb b/app/controllers/profiles/personal_access_tokens_controller.rb index f1c07cd9a1d1e9..30f25e8fdaa62b 100644 --- a/app/controllers/profiles/personal_access_tokens_controller.rb +++ b/app/controllers/profiles/personal_access_tokens_controller.rb @@ -40,14 +40,18 @@ def personal_access_token_params 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') diff --git a/app/finders/personal_access_tokens_finder.rb b/app/finders/personal_access_tokens_finder.rb index 7b15a3b0c1034e..e3d5f2ae8dec32 100644 --- a/app/finders/personal_access_tokens_finder.rb +++ b/app/finders/personal_access_tokens_finder.rb @@ -51,6 +51,8 @@ def by_state(tokens) tokens.active when 'inactive' tokens.inactive + when 'active_or_expired' + tokens.not_revoked.expired.or(tokens.active) else tokens end diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 7afee2a35cbd08..1061e2bd0c6692 100644 --- a/app/models/personal_access_token.rb +++ b/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) } diff --git a/app/views/shared/access_tokens/_table.html.haml b/app/views/shared/access_tokens/_table.html.haml index 5518c31cb0653c..33775090dc7472 100644 --- a/app/views/shared/access_tokens/_table.html.haml +++ b/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(', ') : _('') diff --git a/ee/app/controllers/ee/profiles/personal_access_tokens_controller.rb b/ee/app/controllers/ee/profiles/personal_access_tokens_controller.rb new file mode 100644 index 00000000000000..9748f63677eb05 --- /dev/null +++ b/ee/app/controllers/ee/profiles/personal_access_tokens_controller.rb @@ -0,0 +1,16 @@ +# 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 diff --git a/ee/changelogs/unreleased/active-yet-expired-tokens.yml b/ee/changelogs/unreleased/active-yet-expired-tokens.yml new file mode 100644 index 00000000000000..a7e7bf5f037175 --- /dev/null +++ b/ee/changelogs/unreleased/active-yet-expired-tokens.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to view tokens without expiry enforcement +merge_request: 34570 +author: +type: added diff --git a/ee/spec/controllers/ee/profiles/personal_access_tokens_controller_spec.rb b/ee/spec/controllers/ee/profiles/personal_access_tokens_controller_spec.rb new file mode 100644 index 00000000000000..0f44d0093595b2 --- /dev/null +++ b/ee/spec/controllers/ee/profiles/personal_access_tokens_controller_spec.rb @@ -0,0 +1,53 @@ +# 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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 04b3712e04b57b..5a69fd48fd1ba2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9251,6 +9251,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 "" diff --git a/spec/finders/personal_access_tokens_finder_spec.rb b/spec/finders/personal_access_tokens_finder_spec.rb index dde4f010e41068..94954f4153b586 100644 --- a/spec/finders/personal_access_tokens_finder_spec.rb +++ b/spec/finders/personal_access_tokens_finder_spec.rb @@ -218,6 +218,24 @@ def finder(options = {}) 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) } diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index a3f5eb38511ac5..6fe4bb443a43f4 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -187,6 +187,20 @@ 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 -- GitLab