diff --git a/app/controllers/concerns/second_mfa_reminder_concern.rb b/app/controllers/concerns/second_mfa_reminder_concern.rb index fd2f33282a4..c24f3058026 100644 --- a/app/controllers/concerns/second_mfa_reminder_concern.rb +++ b/app/controllers/concerns/second_mfa_reminder_concern.rb @@ -1,12 +1,19 @@ module SecondMfaReminderConcern def user_needs_second_mfa_reminder? return false unless IdentityConfig.store.second_mfa_reminder_enabled - return false if user_has_dismissed_second_mfa_reminder? || user_has_multiple_mfa_methods? + return false if user_has_dismissed_second_mfa_reminder? + return false if second_mfa_enrollment_may_downgrade_for_service_provider_mfa_requirement? + return false if user_has_multiple_mfa_methods? exceeded_sign_in_count_for_second_mfa_reminder? || exceeded_account_age_for_second_mfa_reminder? end private + def second_mfa_enrollment_may_downgrade_for_service_provider_mfa_requirement? + service_provider_mfa_policy.phishing_resistant_required? || + service_provider_mfa_policy.piv_cac_required? + end + def user_has_dismissed_second_mfa_reminder? current_user.second_mfa_reminder_dismissed_at.present? end diff --git a/spec/controllers/concerns/second_mfa_reminder_concern_spec.rb b/spec/controllers/concerns/second_mfa_reminder_concern_spec.rb index e79b4691a2a..b5bf49f719b 100644 --- a/spec/controllers/concerns/second_mfa_reminder_concern_spec.rb +++ b/spec/controllers/concerns/second_mfa_reminder_concern_spec.rb @@ -5,19 +5,51 @@ Class.new do include SecondMfaReminderConcern - attr_reader :current_user + attr_reader :current_user, :service_provider_mfa_policy - def initialize(current_user:) + def initialize(current_user:, service_provider_mfa_policy:) @current_user = current_user + @service_provider_mfa_policy = service_provider_mfa_policy end end end let(:user) { build(:user) } - let(:instance) { test_class.new(current_user: user) } + let(:phishing_resistant_required) { false } + let(:piv_cac_required) { false } + let(:service_provider_mfa_policy) do + instance_double( + ServiceProviderMfaPolicy, + phishing_resistant_required?: phishing_resistant_required, + piv_cac_required?: piv_cac_required, + ) + end + let(:instance) { test_class.new(current_user: user, service_provider_mfa_policy:) } describe '#user_needs_second_mfa_reminder?' do subject(:user_needs_second_mfa_reminder) { instance.user_needs_second_mfa_reminder? } + shared_examples 'second mfa reminder feature is disabled' do + before do + allow(IdentityConfig.store).to receive(:second_mfa_reminder_account_age_in_days). + and_return(10) + allow(IdentityConfig.store).to receive(:second_mfa_reminder_enabled).and_return(false) + end + + it { expect(user_needs_second_mfa_reminder).to eq(false) } + end + + shared_examples 'second mfa reminder with phishing-resistant required request' do + let(:phishing_resistant_required) { true } + + it { expect(user_needs_second_mfa_reminder).to eq(false) } + end + + shared_examples 'second mfa reminder with piv required request' do + let(:piv_cac_required) { true } + + it { expect(user_needs_second_mfa_reminder).to eq(false) } + end + context 'user has already dismissed second mfa reminder' do let(:user) { build(:user, second_mfa_reminder_dismissed_at: Time.zone.now) } @@ -47,6 +79,10 @@ def initialize(current_user:) end it { expect(user_needs_second_mfa_reminder).to eq(true) } + + it_behaves_like 'second mfa reminder feature is disabled' + it_behaves_like 'second mfa reminder with phishing-resistant required request' + it_behaves_like 'second mfa reminder with piv required request' end context 'user has exceeded account age threshold for reminder' do @@ -58,18 +94,10 @@ def initialize(current_user:) end it { expect(user_needs_second_mfa_reminder).to eq(true) } - end - - context 'user meets threshold but feature is disabled' do - let(:user) { build(:user, :with_phone, created_at: 11.days.ago) } - - before do - allow(IdentityConfig.store).to receive(:second_mfa_reminder_account_age_in_days). - and_return(10) - allow(IdentityConfig.store).to receive(:second_mfa_reminder_enabled).and_return(false) - end - it { expect(user_needs_second_mfa_reminder).to eq(false) } + it_behaves_like 'second mfa reminder feature is disabled' + it_behaves_like 'second mfa reminder with phishing-resistant required request' + it_behaves_like 'second mfa reminder with piv required request' end end end