diff --git a/app/assets/javascripts/persistent_user_callouts.js b/app/assets/javascripts/persistent_user_callouts.js index 8dfbfea9b097b82be13e4a908afeaf1a322d4cf7..f4fe605f0a2d7634775969ddccdd67e6d8a6f4d0 100644 --- a/app/assets/javascripts/persistent_user_callouts.js +++ b/app/assets/javascripts/persistent_user_callouts.js @@ -6,6 +6,7 @@ const PERSISTENT_USER_CALLOUTS = [ '.js-admin-licensed-user-count-threshold', '.js-buy-pipeline-minutes-notification-callout', '.js-alerts-moved-alert', + '.js-token-expiry-callout', ]; const initCallouts = () => { diff --git a/app/models/user_callout_enums.rb b/app/models/user_callout_enums.rb index f36a955f4de88b0b22c5f0e3b5d93b512f4ecebe..c5dfd678021f6c2510570e296d7d53379713d736 100644 --- a/app/models/user_callout_enums.rb +++ b/app/models/user_callout_enums.rb @@ -18,7 +18,8 @@ def self.feature_names tabs_position_highlight: 10, webhooks_moved: 13, admin_integrations_moved: 15, - alerts_moved: 20 + alerts_moved: 20, + personal_access_token_expiry: 21 # EE-only } end end diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index d1cf83b2a9f8e28a0c1a1d13dddacaf1ad3a2ccf..72b88fa8f7fe3d4f31f86ea8af10fad8c6f52791 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -7,6 +7,7 @@ = render 'shared/outdated_browser' = render_if_exists 'layouts/header/users_over_license_banner' = render_if_exists "layouts/header/licensed_user_count_threshold" + = render_if_exists "layouts/header/token_expiry_notification" = render "layouts/broadcast" = render "layouts/header/read_only_banner" = render "layouts/nav/classification_level_banner" diff --git a/ee/app/helpers/ee/user_callouts_helper.rb b/ee/app/helpers/ee/user_callouts_helper.rb index 5949ea1b0cb555b79c6905cb022d3716748178c2..a8b3908c866c5421e01a8a06d42fc21352fb9fa5 100644 --- a/ee/app/helpers/ee/user_callouts_helper.rb +++ b/ee/app/helpers/ee/user_callouts_helper.rb @@ -14,6 +14,7 @@ module UserCalloutsHelper USERS_OVER_LICENSE_BANNER = 'users_over_license_banner' STANDALONE_VULNERABILITIES_INTRODUCTION_BANNER = 'standalone_vulnerabilities_introduction_banner' ACTIVE_USER_COUNT_THRESHOLD = 'active_user_count_threshold' + PERSONAL_ACCESS_TOKEN_EXPIRY = 'personal_access_token_expiry' def show_canary_deployment_callout?(project) !user_dismissed?(CANARY_DEPLOYMENT) && @@ -87,6 +88,12 @@ def show_standalone_vulnerabilities_introduction_banner? !user_dismissed?(STANDALONE_VULNERABILITIES_INTRODUCTION_BANNER) end + def show_token_expiry_notification? + !token_expiration_enforced? && + current_user.active? && + !user_dismissed?(PERSONAL_ACCESS_TOKEN_EXPIRY, 1.week.ago) + end + private def hashed_storage_enabled? @@ -126,5 +133,9 @@ def show_gold_trial?(user, callout = GOLD_TRIAL) def show_gold_trial_suitable_env? ::Gitlab.com? && !::Gitlab::Database.read_only? end + + def token_expiration_enforced? + ::PersonalAccessToken.expiration_enforced? + end end end diff --git a/ee/app/helpers/personal_access_tokens_helper.rb b/ee/app/helpers/personal_access_tokens_helper.rb index 789b9181a0f2ad1313a64d65b9cbbcba88e80580..2cf9ffaafbc9c731a90477e793067c3f8445d760 100644 --- a/ee/app/helpers/personal_access_tokens_helper.rb +++ b/ee/app/helpers/personal_access_tokens_helper.rb @@ -23,6 +23,14 @@ def enforce_pat_expiration_feature_available? PersonalAccessToken.enforce_pat_expiration_feature_available? end + def token_expiry_banner_message(user) + verifier = PersonalAccessTokens::RotationVerifierService.new(user) + + return _('At least one of your Personal Access Tokens is expired, but expiration enforcement is disabled. %{generate_new}') if verifier.expired? + + return _('At least one of your Personal Access Tokens will expire soon, but expiration enforcement is disabled. %{generate_new}') if verifier.expiring_soon? + end + private def instance_level_personal_access_token_expiration_policy_enabled? diff --git a/ee/app/models/ee/personal_access_token.rb b/ee/app/models/ee/personal_access_token.rb index 7d26ee57c6e23a9896ee3a655536bd7cf7a251f7..eb382fd8cd1c2216d78dcd92ec969c94a82c371f 100644 --- a/ee/app/models/ee/personal_access_token.rb +++ b/ee/app/models/ee/personal_access_token.rb @@ -13,8 +13,12 @@ module PersonalAccessToken include ::Gitlab::Utils::StrongMemoize include FromUnion + after_create :clear_rotation_notification_cache + scope :with_no_expires_at, -> { where(revoked: false, expires_at: nil) } scope :with_expires_at_after, ->(max_lifetime) { where(revoked: false).where('expires_at > ?', max_lifetime) } + scope :expires_in, ->(within) { not_revoked.where('expires_at > NOW() AND expires_at <= ?', within) } + scope :created_on_or_after, ->(date) { active.where('created_at >= ?', date) } with_options if: :expiration_policy_enabled? do validates :expires_at, presence: true @@ -56,6 +60,13 @@ def expired? false end + override :revoke! + def revoke! + clear_rotation_notification_cache + + super + end + private def expiration_policy_enabled? @@ -95,5 +106,9 @@ def group_level_expiration_policy_enabled? def group_level_max_expiry_date user.managing_group.max_personal_access_token_lifetime_from_now end + + def clear_rotation_notification_cache + ::PersonalAccessTokens::RotationVerifierService.new(user).clear_cache + end end end diff --git a/ee/app/services/personal_access_tokens/rotation_verifier_service.rb b/ee/app/services/personal_access_tokens/rotation_verifier_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..0120fe21705ef8ad55de9d5da7b02384548df997 --- /dev/null +++ b/ee/app/services/personal_access_tokens/rotation_verifier_service.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module PersonalAccessTokens + class RotationVerifierService + def initialize(user) + @user = user + end + + # If a new token has been created after we started notifying the user about the most recently EXPIRED token, + # rotation is NOT needed. + # For example: If the most recent token expired on 14th of June, and user created a token anytime on or after + # 7th of June (first notification date), no rotation is required. + def expired? + Rails.cache.fetch(expired_cache_key, expires_in: expires_in.minutes) do + most_recent_expires_at = tokens_without_impersonation.not_revoked.expired.maximum(:expires_at) + + if most_recent_expires_at.nil? + false + else + !tokens_without_impersonation.created_on_or_after(most_recent_expires_at - Expirable::DAYS_TO_EXPIRE).exists? + end + end + end + + # If a new token has been created after we started notifying the user about the most recently EXPIRING token, + # rotation is NOT needed. + # User is notified about an expiring token before `days_within` (7 days) of expiry + def expiring_soon? + Rails.cache.fetch(expiring_cache_key, expires_in: expires_in.minutes) do + most_recent_expires_at = tokens_without_impersonation.expires_in(Expirable::DAYS_TO_EXPIRE.days.from_now).maximum(:expires_at) + + if most_recent_expires_at.nil? + false + else + !tokens_without_impersonation.created_on_or_after(most_recent_expires_at - Expirable::DAYS_TO_EXPIRE).exists? + end + end + end + + def clear_cache + Rails.cache.delete(expired_cache_key) + Rails.cache.delete(expiring_cache_key) + end + + private + + attr_reader :user + + NUMBER_OF_MINUTES = 60 + + def expired_cache_key + ['users', user.id, 'token_expired_rotation'] + end + + def expiring_cache_key + ['users', user.id, 'token_expiring_rotation'] + end + + def tokens_without_impersonation + @tokens_without_impersonation ||= user + .personal_access_tokens + .without_impersonation + end + + # Expire the cache at the end of day + # Calculates the number of minutes remaining from now until end of day + def expires_in + (Time.current.at_end_of_day - Time.current) / NUMBER_OF_MINUTES + end + end +end diff --git a/ee/app/views/layouts/header/_token_expiry_notification.html.haml b/ee/app/views/layouts/header/_token_expiry_notification.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..944d12daf253f28a348201429c22a539ac86377a --- /dev/null +++ b/ee/app/views/layouts/header/_token_expiry_notification.html.haml @@ -0,0 +1,12 @@ +- return unless show_token_expiry_notification? +- message = token_expiry_banner_message(current_user) +- return unless message + +- link = link_to _('Generate new token'), profile_personal_access_tokens_path + +.gl-alert.gl-alert-danger.js-token-expiry-callout{ role: 'alert', data: { feature_id: "personal_access_token_expiry", dismiss_endpoint: user_callouts_path, defer_links: "true" } } + %button.js-close.gl-alert-dismiss.gl-cursor-pointer{ type: 'button', 'aria-label' => _('Dismiss') } + = sprite_icon('close', size: 16, css_class: 'gl-icon') + .gl-alert-body + = sprite_icon('warning', size: 16, css_class: 'vertical-align-text-top') + = message.html_safe % { generate_new: link } diff --git a/ee/changelogs/unreleased/in-app-expiry-notification.yml b/ee/changelogs/unreleased/in-app-expiry-notification.yml new file mode 100644 index 0000000000000000000000000000000000000000..a254d5d7bdf84f1c989270f9782007a1a961b3ce --- /dev/null +++ b/ee/changelogs/unreleased/in-app-expiry-notification.yml @@ -0,0 +1,5 @@ +--- +title: Add in-app notification for Personal Access Token expiry +merge_request: 34101 +author: +type: added diff --git a/ee/spec/helpers/ee/user_callouts_helper_spec.rb b/ee/spec/helpers/ee/user_callouts_helper_spec.rb index c3f111a63ff5bcb99c4d46bf88e8aa9ac9a1ea7c..cd851793bf71fbf1707fa0d4df5e71916db537d8 100644 --- a/ee/spec/helpers/ee/user_callouts_helper_spec.rb +++ b/ee/spec/helpers/ee/user_callouts_helper_spec.rb @@ -3,6 +3,8 @@ require "spec_helper" RSpec.describe EE::UserCalloutsHelper do + using RSpec::Parameterized::TableSyntax + describe '.render_enable_hashed_storage_warning' do context 'when we should show the enable warning' do it 'renders the enable warning' do @@ -171,8 +173,6 @@ end describe '#render_dashboard_gold_trial' do - using RSpec::Parameterized::TableSyntax - let_it_be(:namespace) { create(:namespace) } let_it_be(:gold_plan) { create(:gold_plan) } let(:user) { namespace.owner } @@ -236,8 +236,6 @@ end describe '#render_billings_gold_trial' do - using RSpec::Parameterized::TableSyntax - let(:namespace) { create(:namespace) } let_it_be(:free_plan) { create(:free_plan) } let_it_be(:silver_plan) { create(:silver_plan) } @@ -289,8 +287,6 @@ end describe '#render_account_recovery_regular_check' do - using RSpec::Parameterized::TableSyntax - let(:new_user) { create(:user) } let(:old_user) { create(:user, created_at: 4.months.ago )} let(:anonymous) { nil } @@ -348,6 +344,36 @@ end end + describe '.show_token_expiry_notification?' do + subject { helper.show_token_expiry_notification? } + + let_it_be(:user) { create(:user) } + + where(:expiration_enforced?, :dismissed_callout?, :active?, :result) do + true | true | true | false + true | true | false | false + true | false | true | false + false | true | true | false + true | false | false | false + false | false | true | true + false | true | false | false + false | false | false | false + end + + with_them do + before do + allow(helper).to receive(:current_user).and_return(user) + allow(user).to receive(:active?).and_return(active?) + allow(helper).to receive(:token_expiration_enforced?).and_return(expiration_enforced?) + allow(user).to receive(:dismissed_callout?).and_return(dismissed_callout?) + end + + it do + expect(subject).to be result + end + end + end + describe '.show_standalone_vulnerabilities_introduction_banner?' do subject { helper.show_standalone_vulnerabilities_introduction_banner? } diff --git a/ee/spec/helpers/personal_access_tokens_helper_spec.rb b/ee/spec/helpers/personal_access_tokens_helper_spec.rb index ea776776fa374681facac2197bdf962109f8c534..fc3b0a99eb76a4a82e2f80c64a86f437c8de898f 100644 --- a/ee/spec/helpers/personal_access_tokens_helper_spec.rb +++ b/ee/spec/helpers/personal_access_tokens_helper_spec.rb @@ -160,4 +160,22 @@ it_behaves_like 'feature availability' end + + describe '#token_expiry_banner_message' do + subject { helper.token_expiry_banner_message(user) } + + let_it_be(:user) { create(:user) } + + context 'when user has an expired token requiring rotation' do + let_it_be(:expired_pat) { create(:personal_access_token, :expired, user: user, created_at: 1.month.ago) } + + it { is_expected.to eq('At least one of your Personal Access Tokens is expired, but expiration enforcement is disabled. %{generate_new}') } + end + + context 'when user has an expiring token requiring rotation' do + let_it_be(:expiring_pat) { create(:personal_access_token, expires_at: 3.days.from_now, user: user, created_at: 1.month.ago) } + + it { is_expected.to eq('At least one of your Personal Access Tokens will expire soon, but expiration enforcement is disabled. %{generate_new}') } + end + end end diff --git a/ee/spec/models/ee/personal_access_token_spec.rb b/ee/spec/models/ee/personal_access_token_spec.rb index eaab1b6ac51d4fa9640dfd1d3d8f7dfa245a3104..e114f486853ed138a211c9b406b755347471691d 100644 --- a/ee/spec/models/ee/personal_access_token_spec.rb +++ b/ee/spec/models/ee/personal_access_token_spec.rb @@ -252,4 +252,39 @@ it { expect(subject).to be result } end end + + shared_context 'write to cache' do + let_it_be(:pat) { create(:personal_access_token) } + let_it_be(:cache_keys) { %w(token_expired_rotation token_expiring_rotation) } + + before do + cache_keys.each do |key| + Rails.cache.write(['users', pat.user.id, key], double) + end + end + end + + describe '#revoke', :use_clean_rails_memory_store_caching do + include_context 'write to cache' + + it 'clears cache on revoke access' do + pat.revoke! + + cache_keys.each do |key| + expect(Rails.cache.read(['users', pat.user.id, key])).to be_nil + end + end + end + + describe 'after create callback', :use_clean_rails_memory_store_caching do + include_context 'write to cache' + + it 'clears cache for the user' do + create(:personal_access_token, user_id: pat.user_id) + + cache_keys.each do |key| + expect(Rails.cache.read(['users', pat.user.id, key])).to be_nil + end + end + end end diff --git a/ee/spec/services/personal_access_tokens/rotation_verifier_service_spec.rb b/ee/spec/services/personal_access_tokens/rotation_verifier_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5acf8c722b05496060998ebf5927f47230e76d09 --- /dev/null +++ b/ee/spec/services/personal_access_tokens/rotation_verifier_service_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe PersonalAccessTokens::RotationVerifierService do + let_it_be(:user) { create(:user) } + let_it_be(:no_pat_user) { create(:user) } + let_it_be(:active_pat) { create(:personal_access_token, user: user, expires_at: 2.months.from_now, created_at: 1.month.ago) } + + shared_examples 'rotation required' do + it { is_expected.to be true } + end + + shared_examples 'rotation NOT required' do + it { is_expected.to be false } + end + + shared_examples 'stores in cache' do + it do + subject + + expect(Rails.cache.read(['users', user.id, key])).to eq(value) + end + end + + describe '#expired?' do + subject { described_class.new(user).expired? } + + let_it_be(:recent_expired_pat) { create(:personal_access_token, :expired, user: user, created_at: 1.month.ago) } + + context 'when no new token was created after notification for expired token started' do + it_behaves_like 'rotation required' + + context 'cache', :use_clean_rails_memory_store_caching do + let(:key) { 'token_expired_rotation' } + let(:value) { true } + + it_behaves_like 'stores in cache' + end + end + + context 'when token was created after notification for expired token started' do + before do + create(:personal_access_token, user: user, created_at: recent_expired_pat.expires_at + 1.day) + end + + it_behaves_like 'rotation NOT required' + + context 'cache', :use_clean_rails_memory_store_caching do + let(:key) { 'token_expired_rotation' } + let(:value) { false } + + it_behaves_like 'stores in cache' + end + end + + context 'with multiple expired tokens' do + let_it_be(:expired_pat1) { create(:personal_access_token, expires_at: 12.days.ago, user: user, created_at: 1.month.ago) } + + context 'when no new token was created after notification for expired token started' do + it_behaves_like 'rotation required' + end + + context 'when new token was created after notification for ONLY first expired token started' do + before do + create(:personal_access_token, user: user, created_at: expired_pat1.expires_at + 1.day) + end + + it_behaves_like 'rotation required' + end + + context 'when new token was created after notification for most recent expired token started' do + before do + create(:personal_access_token, user: user, created_at: recent_expired_pat.expires_at + 1.day) + end + + it_behaves_like 'rotation NOT required' + end + end + + context 'For user with no PATs' do + subject { described_class.new(no_pat_user).expired? } + + it_behaves_like 'rotation NOT required' + end + end + + describe '#expiring_soon?' do + subject { described_class.new(user).expiring_soon? } + + let_it_be(:recent_expiring_pat) { create(:personal_access_token, user: user, expires_at: 6.days.from_now, created_at: 1.month.ago) } + + context 'when no new token was created after notification for recent expiring token started' do + it_behaves_like 'rotation required' + + context 'cache', :use_clean_rails_memory_store_caching do + let(:key) { 'token_expiring_rotation' } + let(:value) { true } + + it_behaves_like 'stores in cache' + end + end + + context 'when token was created after notification for recent expiring token started' do + before do + create(:personal_access_token, user: user, created_at: recent_expiring_pat.expires_at - 2.days) + end + + it_behaves_like 'rotation NOT required' + + context 'cache', :use_clean_rails_memory_store_caching do + let(:key) { 'token_expiring_rotation' } + let(:value) { false } + + it_behaves_like 'stores in cache' + end + end + + context 'with multiple expiring tokens' do + let_it_be(:expiring_pat1) { create(:personal_access_token, expires_at: 4.days.ago, user: user, created_at: 1.month.ago) } + + context 'when no new token was created after notification for expiring token started' do + it_behaves_like 'rotation required' + end + + context 'when new token was created after notification for ONLY first expiring token started' do + before do + create(:personal_access_token, user: user, created_at: expiring_pat1.expires_at - 1.day) + end + + it_behaves_like 'rotation required' + end + + context 'when new token was created after notification for most recent expiring token started' do + before do + create(:personal_access_token, user: user, created_at: recent_expiring_pat.expires_at - 1.day) + end + + it_behaves_like 'rotation NOT required' + end + end + + context 'For user with no PATs' do + subject { described_class.new(no_pat_user).expiring_soon? } + + it_behaves_like 'rotation NOT required' + end + end + + describe '#clear_cache', :use_clean_rails_memory_store_caching do + let_it_be(:cache_keys) { %w(token_expired_rotation token_expiring_rotation) } + + before do + cache_keys.each do |key| + Rails.cache.write(['users', user.id, key], double) + end + end + + it 'clears cache' do + described_class.new(user).clear_cache + + cache_keys.each do |key| + expect(Rails.cache.read(['users', user.id, key])).to be_nil + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index bcc97ca5aab582d54bf853fbed71b55a4e20ba47..4993c61495bcc87b268532a717ebe089f36f368c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3154,6 +3154,12 @@ msgstr "" msgid "At least one of group_id or project_id must be specified" msgstr "" +msgid "At least one of your Personal Access Tokens is expired, but expiration enforcement is disabled. %{generate_new}" +msgstr "" + +msgid "At least one of your Personal Access Tokens will expire soon, but expiration enforcement is disabled. %{generate_new}" +msgstr "" + msgid "At risk" msgstr "" @@ -10348,6 +10354,9 @@ msgstr "" msgid "Generate new export" msgstr "" +msgid "Generate new token" +msgstr "" + msgid "GenericReports|Report" msgstr ""