Merge branch 'in-app-expiry-notification' into 'master'

In-app Personal Access Token expiry notification

See merge request gitlab-org/gitlab!34101

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 = () => {

app/models/user_callout_enums.rb

@@ -18,7 +18,8 @@ module UserCalloutEnums
       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

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"

ee/app/helpers/ee/user_callouts_helper.rb

@@ -14,6 +14,7 @@ module EE
     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 @@ module EE
       !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 @@ module EE
     def show_gold_trial_suitable_env?
       ::Gitlab.com? && !::Gitlab::Database.read_only?
     end
+
+    def token_expiration_enforced?
+      ::PersonalAccessToken.expiration_enforced?
+    end
   end
 end

ee/app/helpers/personal_access_tokens_helper.rb

@@ -23,6 +23,14 @@ module PersonalAccessTokensHelper
     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?

ee/app/models/ee/personal_access_token.rb

@@ -13,8 +13,12 @@ module EE
       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 @@ module EE
       false
     end
 
+    override :revoke!
+    def revoke!
+      clear_rotation_notification_cache
+
+      super
+    end
+
     private
 
     def expiration_policy_enabled?
@@ -95,5 +106,9 @@ module EE
     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

ee/app/services/personal_access_tokens/rotation_verifier_service.rb

+# 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

ee/app/views/layouts/header/_token_expiry_notification.html.haml

+- 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 }

ee/changelogs/unreleased/in-app-expiry-notification.yml

+---
+title: Add in-app notification for Personal Access Token expiry
+merge_request: 34101
+author:
+type: added

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 @@ RSpec.describe EE::UserCalloutsHelper do
   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 @@ RSpec.describe EE::UserCalloutsHelper do
   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 @@ RSpec.describe EE::UserCalloutsHelper do
   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 @@ RSpec.describe EE::UserCalloutsHelper do
     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? }
 

ee/spec/helpers/personal_access_tokens_helper_spec.rb

@@ -160,4 +160,22 @@ RSpec.describe PersonalAccessTokensHelper do
 
     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

ee/spec/models/ee/personal_access_token_spec.rb

@@ -252,4 +252,39 @@ RSpec.describe PersonalAccessToken do
       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

ee/spec/services/personal_access_tokens/rotation_verifier_service_spec.rb

+# 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

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 ""