diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 54407ba694c..a2b4bbd9cd3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -7,6 +7,7 @@ class ApplicationController < ActionController::Base include VerifySpAttributesConcern include EffectiveUser include SecondMfaReminderConcern + include TwoFactorAuthenticatableMethods # Prevent CSRF attacks by raising an exception. # For APIs, you may want to use :null_session instead. @@ -380,7 +381,7 @@ def service_provider_mfa_policy @service_provider_mfa_policy ||= ServiceProviderMfaPolicy.new( user: current_user, service_provider: sp_from_sp_session, - auth_method: user_session[:auth_method], + auth_methods_session:, aal_level_requested: sp_session[:aal_level_requested], piv_cac_requested: sp_session[:piv_cac_requested], phishing_resistant_requested: sp_session[:phishing_resistant_requested], diff --git a/app/controllers/concerns/reauthentication_required_concern.rb b/app/controllers/concerns/reauthentication_required_concern.rb index 868c5a13bda..bea31e129d0 100644 --- a/app/controllers/concerns/reauthentication_required_concern.rb +++ b/app/controllers/concerns/reauthentication_required_concern.rb @@ -1,15 +1,13 @@ module ReauthenticationRequiredConcern include MfaSetupConcern + include TwoFactorAuthenticatableMethods def confirm_recently_authenticated_2fa - return unless user_fully_authenticated? - non_remembered_device_authentication = user_session[:auth_method].present? && - user_session[:auth_method] != 'remember_device' - return if recently_authenticated? && non_remembered_device_authentication + return if !user_fully_authenticated? || auth_methods_session.recently_authenticated_2fa? analytics.user_2fa_reauthentication_required( - auth_method: user_session[:auth_method], - authenticated_at: user_session[:authn_at], + auth_method: auth_methods_session.last_auth_event&.[](:auth_method), + authenticated_at: auth_methods_session.last_auth_event&.[](:at), ) prompt_for_second_factor @@ -17,13 +15,6 @@ def confirm_recently_authenticated_2fa private - def recently_authenticated? - return false if user_session.blank? - authn_at = user_session[:authn_at] - return false if authn_at.blank? - authn_at > Time.zone.now - IdentityConfig.store.reauthn_window - end - def prompt_for_second_factor store_location(request.url) user_session[:context] = 'reauthentication' diff --git a/app/controllers/concerns/remember_device_concern.rb b/app/controllers/concerns/remember_device_concern.rb index b24b8792afd..e684903bb65 100644 --- a/app/controllers/concerns/remember_device_concern.rb +++ b/app/controllers/concerns/remember_device_concern.rb @@ -53,9 +53,7 @@ def revoke_remember_device(user) private def expired_for_interval?(user, interval) - unless user_session[:auth_method] == TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE - return false - end + return false unless has_remember_device_auth_event? remember_cookie = remember_device_cookie return true if remember_cookie.nil? @@ -65,8 +63,13 @@ def expired_for_interval?(user, interval) ) end + def has_remember_device_auth_event? + auth_methods_session.auth_events.any? do |auth_event| + auth_event[:auth_method] == TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE + end + end + def handle_valid_remember_device_cookie(remember_device_cookie:) - user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE mark_user_session_authenticated( auth_method: TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE, authentication_type: :device_remembered, diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 5562a6eb231..b39536f9e65 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -149,13 +149,11 @@ def update_invalid_user end def handle_valid_verification_for_confirmation_context(auth_method:) - user_session[:auth_method] = auth_method mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa_confirmation) reset_second_factor_attempts_count end def handle_valid_verification_for_authentication_context(auth_method:) - user_session[:auth_method] = auth_method mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa) create_user_event(:sign_in_after_2fa) @@ -167,8 +165,6 @@ def reset_second_factor_attempts_count end def mark_user_session_authenticated(auth_method:, authentication_type:) - user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION] = false - user_session[:authn_at] = Time.zone.now auth_methods_session.authenticate!(auth_method) mark_user_session_authenticated_analytics(authentication_type) end diff --git a/app/policies/service_provider_mfa_policy.rb b/app/policies/service_provider_mfa_policy.rb index fdf62e5a694..661fc0e05b3 100644 --- a/app/policies/service_provider_mfa_policy.rb +++ b/app/policies/service_provider_mfa_policy.rb @@ -1,17 +1,17 @@ class ServiceProviderMfaPolicy - attr_reader :mfa_context, :auth_method, :service_provider + attr_reader :mfa_context, :auth_methods_session, :service_provider def initialize( user:, service_provider:, - auth_method:, + auth_methods_session:, aal_level_requested:, piv_cac_requested:, phishing_resistant_requested: ) @user = user @mfa_context = MfaContext.new(user) - @auth_method = auth_method + @auth_methods_session = auth_methods_session @service_provider = service_provider @aal_level_requested = aal_level_requested @piv_cac_requested = piv_cac_requested @@ -22,13 +22,18 @@ def user_needs_sp_auth_method_verification? # If the user needs to setup a new MFA method, return false so they go to # setup instead of verification return false if user_needs_sp_auth_method_setup? + return false if !piv_cac_required? && !phishing_resistant_required? + valid_auth_methods_for_sp_auth.blank? + end + def valid_auth_methods_for_sp_auth + all_auth_methods = auth_methods_session.auth_events.pluck(:auth_method) if piv_cac_required? - auth_method.to_s != TwoFactorAuthenticatable::AuthMethod::PIV_CAC + all_auth_methods & [TwoFactorAuthenticatable::AuthMethod::PIV_CAC] elsif phishing_resistant_required? - !TwoFactorAuthenticatable::AuthMethod.phishing_resistant?(auth_method) + all_auth_methods & TwoFactorAuthenticatable::AuthMethod::PHISHING_RESISTANT_METHODS.to_a else - false + all_auth_methods end end diff --git a/app/services/auth_methods_session.rb b/app/services/auth_methods_session.rb index 124bea8ed79..a1365e5f562 100644 --- a/app/services/auth_methods_session.rb +++ b/app/services/auth_methods_session.rb @@ -17,4 +17,15 @@ def authenticate!(auth_method) def auth_events user_session[:auth_events] || [] end + + def last_auth_event + auth_events.last + end + + def recently_authenticated_2fa? + auth_events.any? do |auth_event| + auth_event[:auth_method] != TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE && + auth_event[:at] > Time.zone.now - IdentityConfig.store.reauthn_window + end + end end diff --git a/spec/controllers/concerns/reauthentication_required_concern_spec.rb b/spec/controllers/concerns/reauthentication_required_concern_spec.rb index 54b1f979307..c13a50096ca 100644 --- a/spec/controllers/concerns/reauthentication_required_concern_spec.rb +++ b/spec/controllers/concerns/reauthentication_required_concern_spec.rb @@ -27,8 +27,15 @@ def index end context 'authenticated outside the authn window' do + let(:travel_time) { (IdentityConfig.store.reauthn_window + 1).seconds } + before do - controller.user_session[:authn_at] -= IdentityConfig.store.reauthn_window + controller.auth_methods_session.authenticate!(TwoFactorAuthenticatable::AuthMethod::TOTP) + travel travel_time + end + + around do |example| + freeze_time { example.run } end it 'redirects to 2FA options' do @@ -44,12 +51,11 @@ def index end it 'records analytics' do - controller.user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::TOTP stub_analytics expect(@analytics).to receive(:track_event).with( 'User 2FA Reauthentication Required', - authenticated_at: controller.user_session[:authn_at], - auth_method: 'totp', + authenticated_at: travel_time.ago, + auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP, ) get :index end diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 81f3eda6d9a..be937926e2c 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -832,7 +832,9 @@ def name_id_version(format_urn) it 'returns AAL3 authn_context when AAL3 is requested' do allow(controller).to receive(:user_session).and_return( - { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC }, + auth_events: [ + { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, at: Time.zone.now }, + ], ) user = create(:user, :with_piv_or_cac) auth_settings = saml_settings( @@ -849,7 +851,9 @@ def name_id_version(format_urn) it 'returns AAL3-HSPD12 authn_context when AAL3-HSPD12 is requested' do allow(controller).to receive(:user_session).and_return( - { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC }, + auth_events: [ + { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, at: Time.zone.now }, + ], ) user = create(:user, :with_piv_or_cac) auth_settings = saml_settings( @@ -866,7 +870,9 @@ def name_id_version(format_urn) it 'returns AAL2-HSPD12 authn_context when AAL2-HSPD12 is requested' do allow(controller).to receive(:user_session).and_return( - { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC }, + auth_events: [ + { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, at: Time.zone.now }, + ], ) user = create(:user, :with_piv_or_cac) auth_settings = saml_settings( @@ -883,7 +889,9 @@ def name_id_version(format_urn) it 'returns AAL2-phishing-resistant authn_context when AAL2-phishing-resistant requested' do allow(controller).to receive(:user_session).and_return( - { auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN }, + auth_events: [ + { auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, at: Time.zone.now }, + ], ) user = create(:user, :with_webauthn) auth_settings = saml_settings( diff --git a/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb index dd19c9bfc00..ca4b86a68ff 100644 --- a/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/backup_code_verification_controller_spec.rb @@ -43,9 +43,6 @@ post :create, params: payload - expect(subject.user_session[:auth_method]).to eq( - TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE, - ) expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE, @@ -146,7 +143,6 @@ expect(response).to render_template(:show) expect(flash[:error]).to eq t('two_factor_authentication.invalid_backup_code') - expect(subject.user_session[:auth_method]).to eq nil expect(subject.user_session[:auth_events]).to eq nil expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end diff --git a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb index f867135dcf9..3ee0b6446dd 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -173,7 +173,6 @@ end it 'does not set auth_method and requires 2FA' do - expect(controller.user_session[:auth_method]).to eq nil expect(controller.user_session[:auth_events]).to eq nil expect(controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end @@ -301,7 +300,6 @@ otp_delivery_preference: 'sms', } - expect(subject.user_session[:auth_method]).to eq TwoFactorAuthenticatable::AuthMethod::SMS expect(subject.user_session[:auth_events]).to eq( [ { diff --git a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb index a5d6be63c08..aac99a3f104 100644 --- a/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/personal_key_verification_controller_spec.rb @@ -71,9 +71,6 @@ freeze_time do post :create, params: payload - expect(subject.user_session[:auth_method]).to eq( - TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY, - ) expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY, @@ -138,7 +135,6 @@ post :create, params: payload - expect(subject.user_session[:auth_method]).to eq nil expect(subject.user_session[:auth_events]).to eq nil expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end diff --git a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb index ca11b28cb59..93d2529de60 100644 --- a/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/piv_cac_verification_controller_spec.rb @@ -69,9 +69,6 @@ freeze_time do get :show, params: { token: 'good-token' } - expect(subject.user_session[:auth_method]).to eq( - TwoFactorAuthenticatable::AuthMethod::PIV_CAC, - ) expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, @@ -163,7 +160,6 @@ end it 'does not set auth_method and requires 2FA' do - expect(subject.user_session[:auth_method]).to eq nil expect(subject.user_session[:auth_events]).to eq nil expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end diff --git a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb index 952a33adcd2..f711408c57f 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -24,9 +24,6 @@ post :create, params: { code: generate_totp_code(@secret) } expect(response).to redirect_to account_path - expect(subject.user_session[:auth_method]).to eq( - TwoFactorAuthenticatable::AuthMethod::TOTP, - ) expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP, @@ -112,7 +109,6 @@ end it 'does not set auth_method and still requires 2FA' do - expect(subject.user_session[:auth_method]).to eq nil expect(subject.user_session[:auth_events]).to eq nil expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end diff --git a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb index 9d7d50e50a2..1152a33361c 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -161,9 +161,6 @@ freeze_time do patch :confirm, params: params - expect(subject.user_session[:auth_method]).to eq( - TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, - ) expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, @@ -200,9 +197,6 @@ freeze_time do patch :confirm, params: params - expect(subject.user_session[:auth_method]).to eq( - TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM, - ) expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM, @@ -267,7 +261,6 @@ it 'redirects to webauthn show page' do patch :confirm, params: params expect(response).to redirect_to login_two_factor_webauthn_url(platform: true) - expect(subject.user_session[:auth_method]).to eq nil expect(subject.user_session[:auth_events]).to eq nil expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end diff --git a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb index 32c56dd516a..7151f319f8e 100644 --- a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb @@ -75,8 +75,6 @@ allow(PivCacService).to receive(:decode_token).with(bad_token) { bad_token_response } allow(subject).to receive(:user_session).and_return(piv_cac_nonce: nonce) subject.user_session[:piv_cac_nickname] = nickname - subject.user_session[:authn_at] = Time.zone.now - subject.user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::SMS end let(:nonce) { 'nonce' } diff --git a/spec/controllers/users/piv_cac_login_controller_spec.rb b/spec/controllers/users/piv_cac_login_controller_spec.rb index ce381a7fcb2..3210a464570 100644 --- a/spec/controllers/users/piv_cac_login_controller_spec.rb +++ b/spec/controllers/users/piv_cac_login_controller_spec.rb @@ -125,8 +125,6 @@ expect(controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]). to eq false - expect(controller.user_session[:authn_at]).to_not be nil - expect(controller.user_session[:authn_at].class).to eq ActiveSupport::TimeWithZone expect(controller.auth_methods_session.auth_events).to match( [ { diff --git a/spec/features/openid_connect/phishing_resistant_required_spec.rb b/spec/features/openid_connect/phishing_resistant_required_spec.rb index bb9bdc75fc6..68311eb0f73 100644 --- a/spec/features/openid_connect/phishing_resistant_required_spec.rb +++ b/spec/features/openid_connect/phishing_resistant_required_spec.rb @@ -100,6 +100,23 @@ expect(current_url).to eq(login_two_factor_webauthn_url) end + + context 'adding an ineligible method after authenticating with phishing-resistant' do + before do + signin_with_piv + within('.sidenav') { click_on t('account.navigation.add_phone_number') } + fill_in t('two_factor_authentication.phone_label'), with: '5135550100' + click_send_one_time_code + fill_in_code_with_last_phone_otp + click_submit_default + end + + it 'does not prompt the user to authenticate again' do + visit_idp_from_ial1_oidc_sp_requesting_aal3(prompt: 'select_account') + + expect(page).to have_current_path(sign_up_completed_path) + end + end end end @@ -155,6 +172,23 @@ expect(current_url).to eq(login_two_factor_webauthn_url) end + + context 'adding an ineligible method after authenticating with phishing-resistant' do + before do + signin_with_piv + within('.sidenav') { click_on t('account.navigation.add_phone_number') } + fill_in t('two_factor_authentication.phone_label'), with: '5135550100' + click_send_one_time_code + fill_in_code_with_last_phone_otp + click_submit_default + end + + it 'does not prompt the user to authenticate again' do + visit_idp_from_ial1_oidc_sp_requesting_phishing_resistant(prompt: 'select_account') + + expect(page).to have_current_path(sign_up_completed_path) + end + end end end @@ -210,6 +244,23 @@ expect(current_url).to eq(login_two_factor_webauthn_url) end + + context 'adding an ineligible method after authenticating with phishing-resistant' do + before do + signin_with_piv + within('.sidenav') { click_on t('account.navigation.add_phone_number') } + fill_in t('two_factor_authentication.phone_label'), with: '5135550100' + click_send_one_time_code + fill_in_code_with_last_phone_otp + click_submit_default + end + + it 'does not prompt the user to authenticate again' do + visit_idp_from_ial1_oidc_sp_defaulting_to_aal3(prompt: 'select_account') + + expect(page).to have_current_path(sign_up_completed_path) + end + end end end end diff --git a/spec/features/saml/phishing_resistant_required_spec.rb b/spec/features/saml/phishing_resistant_required_spec.rb index 414a5f84d31..b1c0e867b0a 100644 --- a/spec/features/saml/phishing_resistant_required_spec.rb +++ b/spec/features/saml/phishing_resistant_required_spec.rb @@ -114,6 +114,27 @@ expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) end end + + context 'adding an ineligible method after authenticating with phishing-resistant' do + before do + signin_with_piv + within('.sidenav') { click_on t('account.navigation.add_phone_number') } + fill_in t('two_factor_authentication.phone_label'), with: '5135550100' + click_send_one_time_code + fill_in_code_with_last_phone_otp + click_submit_default + end + + it 'does not prompt the user to authenticate again' do + visit_saml_authn_request_url( + overrides: { + authn_context: Saml::Idp::Constants::AAL2_PHISHING_RESISTANT_AUTHN_CONTEXT_CLASSREF, + }, + ) + + expect(page).to have_current_path(sign_up_completed_path) + end + end end end @@ -181,6 +202,27 @@ expect(current_url).to eq(login_two_factor_webauthn_url(platform: true)) end end + + context 'adding an ineligible method after authenticating with phishing-resistant' do + before do + signin_with_piv + within('.sidenav') { click_on t('account.navigation.add_phone_number') } + fill_in t('two_factor_authentication.phone_label'), with: '5135550100' + click_send_one_time_code + fill_in_code_with_last_phone_otp + click_submit_default + end + + it 'does not prompt the user to authenticate again' do + visit_saml_authn_request_url( + overrides: { + authn_context: Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF, + }, + ) + + expect(page).to have_current_path(sign_up_completed_path) + end + end end end @@ -259,6 +301,28 @@ expect(current_url).to eq(login_two_factor_webauthn_url) end + + context 'adding an ineligible method after authenticating with phishing-resistant' do + before do + signin_with_piv + within('.sidenav') { click_on t('account.navigation.add_phone_number') } + fill_in t('two_factor_authentication.phone_label'), with: '5135550100' + click_send_one_time_code + fill_in_code_with_last_phone_otp + click_submit_default + end + + it 'does not prompt the user to authenticate again' do + visit_saml_authn_request_url( + overrides: { + issuer: aal3_issuer, + authn_context: Saml::Idp::Constants::IAL1_AUTHN_CONTEXT_CLASSREF, + }, + ) + + expect(page).to have_current_path(sign_up_completed_path) + end + end end end end diff --git a/spec/policies/service_provider_mfa_policy_spec.rb b/spec/policies/service_provider_mfa_policy_spec.rb index 8a30d8a1ee7..81293c0f630 100644 --- a/spec/policies/service_provider_mfa_policy_spec.rb +++ b/spec/policies/service_provider_mfa_policy_spec.rb @@ -7,18 +7,23 @@ let(:aal_level_requested) { 1 } let(:piv_cac_requested) { false } let(:phishing_resistant_requested) { nil } + let(:auth_methods_session) { AuthMethodsSession.new(user_session: {}) } subject(:policy) do described_class.new( user: user, service_provider: service_provider, - auth_method: auth_method, + auth_methods_session: auth_methods_session, aal_level_requested: aal_level_requested, piv_cac_requested: piv_cac_requested, phishing_resistant_requested: phishing_resistant_requested, ) end + before do + auth_methods_session.authenticate!(auth_method) if auth_method + end + describe '#phishing_resistant_required?' do context 'AAL 3 requested' do let(:aal_level_requested) { 3 } @@ -82,6 +87,17 @@ it { expect(policy.user_needs_sp_auth_method_verification?).to eq(false) } end + context 'the user uses an eligible method and authenticates with another method afterward' do + let(:auth_method) { 'webauthn' } + + before do + setup_user_webauthn_token + auth_methods_session.authenticate!(TwoFactorAuthenticatable::AuthMethod::SMS) + end + + it { expect(policy.user_needs_sp_auth_method_verification?).to eq(false) } + end + context 'the user did not use an AAL3 method' do let(:auth_method) { TwoFactorAuthenticatable::AuthMethod::SMS } @@ -110,6 +126,14 @@ before { setup_user_piv } it { expect(policy.user_needs_sp_auth_method_verification?).to eq(false) } + + context 'the user authenticates with another method after using PIV/CAC' do + before do + auth_methods_session.authenticate!(TwoFactorAuthenticatable::AuthMethod::SMS) + end + + it { expect(policy.user_needs_sp_auth_method_verification?).to eq(false) } + end end context 'the user used webauthn' do @@ -141,6 +165,12 @@ end it { expect(policy.user_needs_sp_auth_method_verification?).to eq(false) } + + context 'user has not authenticated' do + let(:auth_method) { nil } + + it { expect(policy.user_needs_sp_auth_method_verification?).to eq(false) } + end end end @@ -194,6 +224,12 @@ before { setup_user_phone } it { expect(policy.user_needs_sp_auth_method_setup?).to eq(false) } + + context 'user has not authenticated' do + let(:auth_method) { nil } + + it { expect(policy.user_needs_sp_auth_method_setup?).to eq(false) } + end end end diff --git a/spec/services/auth_methods_session_spec.rb b/spec/services/auth_methods_session_spec.rb index dd7bf0ec7e8..1de85a733aa 100644 --- a/spec/services/auth_methods_session_spec.rb +++ b/spec/services/auth_methods_session_spec.rb @@ -79,4 +79,84 @@ end end end + + describe '#last_auth_event' do + subject(:last_auth_event) { auth_methods_session.last_auth_event } + + context 'no auth events' do + it { expect(last_auth_event).to be_nil } + end + + context 'with multiple auth events' do + let(:second_auth_event) { { auth_method: 'second', at: Time.zone.now } } + let(:session_auth_events) do + [ + { auth_method: 'first', at: 3.minutes.ago }, + second_auth_event, + ] + end + let(:user_session) { { auth_events: session_auth_events } } + + it 'returns the last auth event' do + expect(last_auth_event).to eq(second_auth_event) + end + end + end + + describe '#recently_authenticated_2fa?' do + subject(:recently_authenticated_2fa) { auth_methods_session.recently_authenticated_2fa? } + + context 'no auth events' do + it { expect(recently_authenticated_2fa).to eq(false) } + end + + context 'with remember device auth event' do + let(:user_session) do + { + auth_events: [ + { + auth_method: TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE, + at: Time.zone.now, + }, + ], + } + end + + it { expect(recently_authenticated_2fa).to eq(false) } + + context 'with non-remember device auth event' do + let(:user_session) do + { + auth_events: [ + { + auth_method: TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE, + at: 3.minutes.ago, + }, + { + auth_method: TwoFactorAuthenticatable::AuthMethod::SMS, + at: Time.zone.now, + }, + ], + } + end + + it { expect(recently_authenticated_2fa).to eq(true) } + end + end + + context 'with non-remember device auth event' do + let(:user_session) do + { + auth_events: [ + { + auth_method: TwoFactorAuthenticatable::AuthMethod::SMS, + at: Time.zone.now, + }, + ], + } + end + + it { expect(recently_authenticated_2fa).to eq(true) } + end + end end diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 19937fa3bb4..7dfa42ab378 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -16,9 +16,8 @@ def sign_in_before_2fa(user = create(:user, :fully_registered)) def stub_sign_in(user = build(:user, password: VALID_PASSWORD)) allow(request.env['warden']).to receive(:authenticate!).and_return(user) allow(request.env['warden']).to receive(:session).and_return(user: {}) - allow(controller).to receive(:user_session). - and_return({ authn_at: Time.zone.now }.with_indifferent_access) - controller.user_session[:auth_method] ||= TwoFactorAuthenticatable::AuthMethod::SMS + allow(controller).to receive(:user_session).and_return({}.with_indifferent_access) + controller.auth_methods_session.authenticate!(TwoFactorAuthenticatable::AuthMethod::SMS) allow(controller).to receive(:current_user).and_return(user) allow(controller).to receive(:confirm_two_factor_authenticated).and_return(true) allow(controller).to receive(:user_fully_authenticated?).and_return(true) diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index a80e0b391ea..cb903f583d8 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -203,8 +203,10 @@ def sign_in_with_warden(user, auth_method: nil) Warden.on_next_request do |proxy| session = proxy.env['rack.session'] - session['warden.user.user.session'] = { authn_at: Time.zone.now } - session['warden.user.user.session']['auth_method'] = auth_method if auth_method + session['warden.user.user.session'] = {}.with_indifferent_access + if auth_method + session['warden.user.user.session']['auth_events'] = [{ auth_method:, at: Time.zone.now }] + end end visit account_path end