From 93d7277ba17f8a58cd96af7844ffc4efce30b1b8 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 10:02:44 -0500 Subject: [PATCH 01/12] Consolidate two factor verification and confirmation behavior changelog: Internal, Two-Factor Authentication, Consolidate two factor verification and confirmation behavior --- .../two_factor_authentication/totp_verification_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 033ea9cfd48..c1b06038ab6 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -23,7 +23,7 @@ def create irs_attempts_api_tracker.mfa_login_totp(success: result.success?) if result.success? - handle_valid_otp(next_url: nil, auth_method: 'authenticator') + handle_valid_otp(next_url: nil, auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP) else handle_invalid_otp(context: context, type: 'totp') end From 3c80fa5dc82680acac00e4133ca4c722ca090394 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 12:04:27 -0500 Subject: [PATCH 02/12] piv cac auth --- .../concerns/two_factor_authenticatable_methods.rb | 3 ++- app/controllers/users/piv_cac_login_controller.rb | 12 ++---------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index d5316e965ac..f9c694f2ce4 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -97,7 +97,6 @@ def handle_valid_otp(next_url:, auth_method: nil) handle_valid_otp_for_context(auth_method: auth_method) handle_remember_device next_url ||= after_otp_verification_confirmation_url - reset_otp_session_data redirect_to next_url end @@ -171,6 +170,7 @@ def update_invalid_user def handle_valid_verification_for_confirmation_context(auth_method:) user_session[:auth_method] = auth_method mark_user_session_authenticated(:valid_2fa_confirmation) + reset_otp_session_data reset_second_factor_attempts_count end @@ -179,6 +179,7 @@ def handle_valid_verification_for_authentication_context(auth_method:) mark_user_session_authenticated(:valid_2fa) create_user_event(:sign_in_after_2fa) + reset_otp_session_data reset_second_factor_attempts_count end diff --git a/app/controllers/users/piv_cac_login_controller.rb b/app/controllers/users/piv_cac_login_controller.rb index 80cce49ca71..5092c755ac5 100644 --- a/app/controllers/users/piv_cac_login_controller.rb +++ b/app/controllers/users/piv_cac_login_controller.rb @@ -74,10 +74,10 @@ def process_valid_submission presented: true, ) - handle_valid_otp( - next_url: next_step, + handle_valid_verification_for_authentication_context( auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, ) + redirect_to next_step end def next_step @@ -105,13 +105,5 @@ def process_invalid_submission def process_token_with_error redirect_to login_piv_cac_error_url(error: piv_cac_login_form.error_type) end - - def mark_user_session_authenticated(authentication_type) - user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION] = false - user_session[:authn_at] = Time.zone.now - analytics.user_marked_authed( - authentication_type: authentication_type, - ) - end end end From b2aabb9e4638cc2e4cfd510e77922adf15a02e56 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 12:11:04 -0500 Subject: [PATCH 03/12] totp auth --- .../totp_verification_controller.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index c1b06038ab6..5e86bb14d9e 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -23,7 +23,11 @@ def create irs_attempts_api_tracker.mfa_login_totp(success: result.success?) if result.success? - handle_valid_otp(next_url: nil, auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP) + handle_valid_otp_for_authentication_context( + auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP, + ) + handle_remember_device + redirect_to after_otp_verification_confirmation_url else handle_invalid_otp(context: context, type: 'totp') end From 85de8ffd22be58c0c61346f5a71bb8918c8826a3 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 12:19:06 -0500 Subject: [PATCH 04/12] phones --- .../otp_verification_controller.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 2c19adb8f19..ed9670ecaeb 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -21,8 +21,14 @@ def create result = otp_verification_form.submit post_analytics(result) if result.success? - handle_valid_confirmation_otp if UserSessionContext.confirmation_context?(context) - handle_valid_otp(next_url: nil, auth_method: params[:otp_delivery_preference]) + if UserSessionContext.confirmation_context?(context) + handle_valid_confirmation_otp + else + handle_valid_otp_for_authentication_context(auth_method: params[:otp_delivery_preference]) + end + + handle_remember_device + redirect_to after_otp_verification_confirmation_url else handle_invalid_otp(context: context, type: 'otp') end @@ -34,6 +40,9 @@ def handle_valid_confirmation_otp assign_phone track_mfa_added @next_mfa_setup_path = next_setup_path + handle_valid_verification_for_confirmation_context( + auth_method: params[:otp_delivery_preference], + ) flash[:success] = t('notices.phone_confirmed') end From 3a12baa692d9a5bac9b05cd09c85f15e1460643f Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 12:37:37 -0500 Subject: [PATCH 05/12] webauthn --- .../webauthn_verification_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index fe8cff30809..b1d9c2b1b4c 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -43,7 +43,6 @@ def handle_valid_webauthn handle_valid_verification_for_authentication_context(auth_method: 'webauthn') handle_remember_device redirect_to after_otp_verification_confirmation_url - reset_otp_session_data end def handle_invalid_webauthn From 59b2b6e23201d31781422f094dd7576d312c2569 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 12:39:19 -0500 Subject: [PATCH 06/12] rename method --- .../two_factor_authenticatable_methods.rb | 15 --------------- .../otp_verification_controller.rb | 4 +++- .../totp_verification_controller.rb | 2 +- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index f9c694f2ce4..6eb599049bb 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -93,26 +93,11 @@ def reset_attempt_count_if_user_no_longer_locked_out ).call end - def handle_valid_otp(next_url:, auth_method: nil) - handle_valid_otp_for_context(auth_method: auth_method) - handle_remember_device - next_url ||= after_otp_verification_confirmation_url - redirect_to next_url - end - def handle_remember_device save_user_opted_remember_device_pref save_remember_device_preference end - def handle_valid_otp_for_context(auth_method:) - if UserSessionContext.authentication_or_reauthentication_context?(context) - handle_valid_verification_for_authentication_context(auth_method: auth_method) - elsif UserSessionContext.confirmation_context?(context) - handle_valid_verification_for_confirmation_context(auth_method: auth_method) - end - end - # Method will be renamed in the next refactor. # You can pass in any "type" with a corresponding I18n key in # two_factor_authentication.invalid_#{type} diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index ed9670ecaeb..1b2f9676066 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -24,7 +24,9 @@ def create if UserSessionContext.confirmation_context?(context) handle_valid_confirmation_otp else - handle_valid_otp_for_authentication_context(auth_method: params[:otp_delivery_preference]) + handle_valid_verification_for_authentication_context( + auth_method: params[:otp_delivery_preference], + ) end handle_remember_device diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 5e86bb14d9e..ffff839eca9 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -23,7 +23,7 @@ def create irs_attempts_api_tracker.mfa_login_totp(success: result.success?) if result.success? - handle_valid_otp_for_authentication_context( + handle_valid_verification_for_authentication_context( auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP, ) handle_remember_device From ae3a556b9f0be57a14582d9efa21a2688ada4f19 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Mon, 15 May 2023 12:41:37 -0500 Subject: [PATCH 07/12] webauthn auth --- .../webauthn_verification_controller.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index b1d9c2b1b4c..09e5db5f516 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -40,7 +40,15 @@ def handle_webauthn_result(result) end def handle_valid_webauthn - handle_valid_verification_for_authentication_context(auth_method: 'webauthn') + if form.webauthn_configuration.platform_authenticator + handle_valid_verification_for_authentication_context( + auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, + ) + else + handle_valid_verification_for_authentication_context( + auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM, + ) + end handle_remember_device redirect_to after_otp_verification_confirmation_url end From 41d8266408b4b73888a786b2f6f8d69eb816c39e Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 25 May 2023 09:11:30 -0500 Subject: [PATCH 08/12] move reset_otp_session_data to phone otp controller --- app/controllers/concerns/remember_device_concern.rb | 1 - .../concerns/two_factor_authenticatable_methods.rb | 7 ------- .../backup_code_verification_controller.rb | 1 - .../otp_verification_controller.rb | 6 ++++++ .../personal_key_verification_controller.rb | 1 - .../piv_cac_verification_controller.rb | 1 - spec/controllers/users/piv_cac_login_controller_spec.rb | 8 -------- 7 files changed, 6 insertions(+), 19 deletions(-) diff --git a/app/controllers/concerns/remember_device_concern.rb b/app/controllers/concerns/remember_device_concern.rb index 86cfaf999e2..81adc2aff16 100644 --- a/app/controllers/concerns/remember_device_concern.rb +++ b/app/controllers/concerns/remember_device_concern.rb @@ -74,7 +74,6 @@ def handle_valid_remember_device_cookie(remember_device_cookie:) mark_user_session_authenticated(:device_remembered) handle_valid_remember_device_analytics(cookie_created_at: remember_device_cookie.created_at) redirect_to after_otp_verification_confirmation_url unless reauthn? - reset_otp_session_data end def handle_valid_remember_device_analytics(cookie_created_at:) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 6eb599049bb..59869d4e4d8 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -155,7 +155,6 @@ def update_invalid_user def handle_valid_verification_for_confirmation_context(auth_method:) user_session[:auth_method] = auth_method mark_user_session_authenticated(:valid_2fa_confirmation) - reset_otp_session_data reset_second_factor_attempts_count end @@ -164,7 +163,6 @@ def handle_valid_verification_for_authentication_context(auth_method:) mark_user_session_authenticated(:valid_2fa) create_user_event(:sign_in_after_2fa) - reset_otp_session_data reset_second_factor_attempts_count end @@ -172,11 +170,6 @@ def reset_second_factor_attempts_count UpdateUser.new(user: current_user, attributes: { second_factor_attempts_count: 0 }).call end - def reset_otp_session_data - user_session.delete(:unconfirmed_phone) - user_session[:context] = 'authentication' - end - def after_otp_verification_confirmation_url return @next_mfa_setup_path if @next_mfa_setup_path return account_url if @updating_existing_number diff --git a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb index 61cc1bf3147..66e0a01bebf 100644 --- a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb +++ b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb @@ -77,7 +77,6 @@ def backup_code_params def handle_valid_backup_code redirect_to after_otp_verification_confirmation_url - reset_otp_session_data end def check_sp_required_mfa diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 1b2f9676066..362f4ed4272 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -30,6 +30,7 @@ def create end handle_remember_device + reset_otp_session_data redirect_to after_otp_verification_confirmation_url else handle_invalid_otp(context: context, type: 'otp') @@ -258,5 +259,10 @@ def selected_otp_make_default_number def direct_otp_code current_user.direct_otp if FeatureManagement.prefill_otp_codes? end + + def reset_otp_session_data + user_session.delete(:unconfirmed_phone) + user_session[:context] = 'authentication' + end end end diff --git a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb index 98655c5a383..125effcc62a 100644 --- a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb +++ b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb @@ -99,7 +99,6 @@ def handle_valid_otp else redirect_to authentication_methods_setup_url end - reset_otp_session_data end end end diff --git a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb index 0f33a572a04..84c22eed08f 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -54,7 +54,6 @@ def handle_valid_piv_cac auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, ) redirect_to after_otp_verification_confirmation_url - reset_otp_session_data end def handle_invalid_piv_cac diff --git a/spec/controllers/users/piv_cac_login_controller_spec.rb b/spec/controllers/users/piv_cac_login_controller_spec.rb index 6f47682d06b..465c3150542 100644 --- a/spec/controllers/users/piv_cac_login_controller_spec.rb +++ b/spec/controllers/users/piv_cac_login_controller_spec.rb @@ -146,14 +146,6 @@ end describe 'it handles the otp_context' do - it 'sets the otp user_session' do - expect(controller.user_session[:auth_method]). - to eq TwoFactorAuthenticatable::AuthMethod::PIV_CAC - - expect(controller.user_session[:unconfirmed_phone]).to be nil - expect(controller.user_session[:context]).to eq 'authentication' - end - it 'tracks the user_marked_authed event' do expect(@analytics).to have_received(:track_event).with( 'User marked authenticated', From f2b7f546b83e74c53fd56d5a16859b56a9d5199a Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 25 May 2023 10:42:28 -0500 Subject: [PATCH 09/12] set NEED_AUTHENTICATION when signing in before 2FA in controller specs --- .../otp_verification_controller_spec.rb | 3 ++- spec/support/controller_helper.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) 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 ff23ba93060..daf62458deb 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -393,6 +393,7 @@ let(:user) { create(:user, :fully_registered) } before do sign_in_as_user(user) + subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION] = false subject.user_session[:unconfirmed_phone] = '+1 (703) 555-5555' subject.user_session[:context] = 'confirmation' @@ -417,7 +418,7 @@ @previous_phone = MfaContext.new(subject.current_user).phone_configurations.first&.phone end - context 'user has an existing phone number' do + context 'user is fully authenticated and has an existing phone number' do context 'user enters a valid code' do before do subject.user_session[:mfa_selections] = ['sms'] diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 903850b0cf3..e5cb0c4ad8b 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -4,14 +4,13 @@ module ControllerHelper def sign_in_as_user(user = create(:user, :fully_registered, password: VALID_PASSWORD)) @request.env['devise.mapping'] = Devise.mappings[:user] sign_in user + controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION] = true user end def sign_in_before_2fa(user = create(:user, :fully_registered)) sign_in_as_user(user) controller.current_user.create_direct_otp - allow(controller).to receive(:user_fully_authenticated?).and_return(false) - allow(controller).to receive(:signed_in_url).and_return(account_url) end def stub_sign_in(user = build(:user, password: VALID_PASSWORD)) @@ -32,6 +31,7 @@ def stub_sign_in_before_2fa(user = build(:user, password: VALID_PASSWORD)) allow(controller).to receive(:current_user).and_return(user) allow(controller).to receive(:user_fully_authenticated?).and_return(false) allow(controller).to receive(:signed_in_url).and_return(account_url) + controller.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION] = true end def stub_idv_steps_before_verify_step(user) From 8651fc4d427e4586d343291e7de63cf95cfa2c9e Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 25 May 2023 11:13:50 -0500 Subject: [PATCH 10/12] fix WEBAUTHN swap --- .../webauthn_verification_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 09e5db5f516..9996267b7f6 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -42,11 +42,11 @@ def handle_webauthn_result(result) def handle_valid_webauthn if form.webauthn_configuration.platform_authenticator handle_valid_verification_for_authentication_context( - auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, + auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM, ) else handle_valid_verification_for_authentication_context( - auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM, + auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, ) end handle_remember_device From d0499bacdf5df9b695d14d7da109c4c74901b5fd Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 25 May 2023 11:15:56 -0500 Subject: [PATCH 11/12] use constants for auth_method --- .../webauthn_verification_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 9996267b7f6..5db5cb2730f 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -103,9 +103,9 @@ def credential_ids def analytics_properties auth_method = if form&.webauthn_configuration&.platform_authenticator || params[:platform].to_s == 'true' - 'webauthn_platform' + TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM else - 'webauthn' + TwoFactorAuthenticatable::AuthMethod::WEBAUTHN end { context: context, From f7f3512f83b91cd12fb7386b1fee45b86ed712db Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 25 May 2023 11:28:12 -0500 Subject: [PATCH 12/12] test auth_method and NEED_AUTHENTICATION --- .../backup_code_verification_controller_spec.rb | 7 +++++++ .../otp_verification_controller_spec.rb | 8 ++++++++ .../personal_key_verification_controller_spec.rb | 8 ++++++++ .../piv_cac_verification_controller_spec.rb | 9 +++++++++ .../totp_verification_controller_spec.rb | 9 ++++++++- .../webauthn_verification_controller_spec.rb | 11 +++++++++++ 6 files changed, 51 insertions(+), 1 deletion(-) 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 34923ef1c08..64a3f6f597b 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 @@ -41,6 +41,11 @@ with(:mfa_login_backup_code, success: true) post :create, params: payload + + expect(subject.user_session[:auth_method]).to eq( + TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE, + ) + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end it 'tracks the valid authentication event when there are exisitng codes' do @@ -120,6 +125,8 @@ 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[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end it 'tracks the max attempts event' do 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 daf62458deb..cc03c67b446 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -161,6 +161,11 @@ it 'displays flash error message' do expect(flash[:error]).to eq t('two_factor_authentication.invalid_otp') end + + it 'does not set auth_method and requires 2FA' do + expect(subject.user_session[:auth_method]).to eq nil + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true + end end context 'when the user enters an invalid OTP during reauthentication context' do @@ -278,6 +283,9 @@ code: subject.current_user.reload.direct_otp, otp_delivery_preference: 'sms', } + + expect(subject.user_session[:auth_method]).to eq TwoFactorAuthenticatable::AuthMethod::SMS + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end it 'tracks the attempt event with reauthentication parameter true' do 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 d96fa740c17..a3ec63a5570 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 @@ -67,6 +67,11 @@ with('User marked authenticated', authentication_type: :valid_2fa) post :create, params: payload + + expect(subject.user_session[:auth_method]).to eq( + TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY, + ) + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end end @@ -136,6 +141,9 @@ expect(subject).to receive(:handle_invalid_otp).and_call_original post :create, params: payload + + expect(subject.user_session[:auth_method]).to eq nil + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end it 're-renders the personal key entry screen' do 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 ed2fd99941e..c15a8f957fa 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 @@ -68,6 +68,10 @@ get :show, params: { token: 'good-token' } + expect(subject.user_session[:auth_method]).to eq( + TwoFactorAuthenticatable::AuthMethod::PIV_CAC, + ) + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false expect(response).to redirect_to account_path expect(subject.user_session[:decrypted_x509]).to eq( { @@ -147,6 +151,11 @@ it 'resets the piv/cac session information' do expect(subject.user_session[:decrypted_x509]).to be_nil end + + it 'does not set auth_method and requires 2FA' do + expect(subject.user_session[:auth_method]).to eq nil + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true + end end context 'when the user presents a different piv/cac' do 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 9bb8a7dd935..3008b84a875 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -15,7 +15,7 @@ Db::AuthAppConfiguration.create(user, @secret, nil, 'foo') end - it 'redirects to the profile' do + it 'redirects to the profile and sets auth_method' do cfg = subject.current_user.auth_app_configurations.first expect(Db::AuthAppConfiguration).to receive(:authenticate).and_return(cfg) expect(subject.current_user.reload.second_factor_attempts_count).to eq 0 @@ -23,6 +23,8 @@ 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[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end it 'resets the second_factor_attempts_count' do @@ -74,6 +76,11 @@ it 'displays flash error message' do expect(flash[:error]).to eq t('two_factor_authentication.invalid_otp') 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[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true + end end context 'when the user has reached the max number of TOTP attempts' do 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 67eedb26e5d..edc18df1258 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -91,6 +91,11 @@ ) patch :confirm, params: params + + expect(subject.user_session[:auth_method]).to eq( + TwoFactorAuthenticatable::AuthMethod::WEBAUTHN, + ) + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end it 'tracks a valid platform authenticator submission' do @@ -117,6 +122,10 @@ ) patch :confirm, params: params + expect(subject.user_session[:auth_method]).to eq( + TwoFactorAuthenticatable::AuthMethod::WEBAUTHN_PLATFORM, + ) + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end it 'tracks an invalid submission' do @@ -173,6 +182,8 @@ 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[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true end it 'displays flash error message' do