diff --git a/app/controllers/concerns/remember_device_concern.rb b/app/controllers/concerns/remember_device_concern.rb index 64b44c144f7..d1104ab875f 100644 --- a/app/controllers/concerns/remember_device_concern.rb +++ b/app/controllers/concerns/remember_device_concern.rb @@ -57,7 +57,9 @@ def revoke_remember_device(user) private def expired_for_interval?(user, interval) - return false unless user_session[:auth_method] == 'remember_device' + unless user_session[:auth_method] == TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE + return false + end remember_cookie = remember_device_cookie return true if remember_cookie.nil? @@ -68,7 +70,7 @@ def expired_for_interval?(user, interval) end def handle_valid_remember_device_cookie - user_session[:auth_method] = 'remember_device' + user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE mark_user_session_authenticated(:device_remembered) handle_valid_remember_device_analytics bypass_sign_in current_user diff --git a/app/controllers/concerns/two_factor_authenticatable.rb b/app/controllers/concerns/two_factor_authenticatable.rb index d705b0bc00b..250f2221c53 100644 --- a/app/controllers/concerns/two_factor_authenticatable.rb +++ b/app/controllers/concerns/two_factor_authenticatable.rb @@ -11,6 +11,28 @@ module TwoFactorAuthenticatable DIRECT_OTP_VALID_FOR_SECONDS = DIRECT_OTP_VALID_FOR_MINUTES * 60 REMEMBER_2FA_COOKIE = 'remember_tfa'.freeze + class AuthMethod + BACKUP_CODE = 'backup_code' + PERSONAL_KEY = 'personal_key' + PIV_CAC = 'piv_cac' + REMEMBER_DEVICE = 'remember_device' + SMS = 'sms' + TOTP = 'totp' + VOICE = 'voice' + WEBAUTHN = 'webauthn' + WEBAUTHN_PLATFORM = 'webauthn_platform' + + PHISHING_RESISTANT_METHODS = [ + WEBAUTHN, + WEBAUTHN_PLATFORM, + PIV_CAC, + ].to_set.freeze + + def self.phishing_resistant?(auth_method) + PHISHING_RESISTANT_METHODS.include?(auth_method) + end + end + included do # rubocop:disable Rails/LexicallyScopedActionFilter before_action :authenticate_user diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 1c62e4929d8..3cf5f71b37f 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -73,8 +73,11 @@ def check_already_authenticated def check_sp_required_mfa_bypass(auth_method:) return unless service_provider_mfa_policy.user_needs_sp_auth_method_verification? return if service_provider_mfa_policy.phishing_resistant_required? && - ServiceProviderMfaPolicy::PHISHING_RESISTANT_METHODS.include?(auth_method) - return if service_provider_mfa_policy.piv_cac_required? && auth_method == 'piv_cac' + TwoFactorAuthenticatable::AuthMethod.phishing_resistant?(auth_method) + if service_provider_mfa_policy.piv_cac_required? && + auth_method == TwoFactorAuthenticatable::AuthMethod::PIV_CAC + return + end prompt_to_verify_sp_required_mfa end 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 ad325e0ec94..805e1b96f03 100644 --- a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb +++ b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb @@ -61,7 +61,9 @@ def handle_invalid_backup_code def handle_result(result) if result.success? - handle_valid_otp_for_authentication_context(auth_method: 'backup_code') + handle_valid_otp_for_authentication_context( + auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE, + ) return handle_last_code if all_codes_used? handle_valid_backup_code else 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 90084452fe1..dbdcdeef8b9 100644 --- a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb +++ b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb @@ -89,7 +89,9 @@ def normalized_personal_key end def handle_valid_otp - handle_valid_otp_for_authentication_context(auth_method: 'personal_key') + handle_valid_otp_for_authentication_context( + auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY, + ) if current_user.identity_verified? || current_user.password_reset_profile.present? redirect_to manage_personal_key_url elsif MfaPolicy.new(current_user).two_factor_enabled? 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 16d51d2db52..28e1ccb2d00 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -50,7 +50,9 @@ def handle_valid_piv_cac presented: true, ) - handle_valid_otp_for_authentication_context(auth_method: 'piv_cac') + handle_valid_otp_for_authentication_context( + auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, + ) redirect_to after_otp_verification_confirmation_url reset_otp_session_data end diff --git a/app/controllers/users/piv_cac_login_controller.rb b/app/controllers/users/piv_cac_login_controller.rb index b3287b57008..ce1b809afdd 100644 --- a/app/controllers/users/piv_cac_login_controller.rb +++ b/app/controllers/users/piv_cac_login_controller.rb @@ -74,7 +74,10 @@ def process_valid_submission presented: true, ) - handle_valid_otp(next_url: next_step, auth_method: 'piv_cac') + handle_valid_otp( + next_url: next_step, + auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC, + ) end def next_step diff --git a/app/policies/service_provider_mfa_policy.rb b/app/policies/service_provider_mfa_policy.rb index 3f3c6d79a74..156bc9882a5 100644 --- a/app/policies/service_provider_mfa_policy.rb +++ b/app/policies/service_provider_mfa_policy.rb @@ -1,6 +1,4 @@ class ServiceProviderMfaPolicy - PHISHING_RESISTANT_METHODS = %w[webauthn webauthn_platform piv_cac].freeze - attr_reader :mfa_context, :auth_method, :service_provider def initialize( @@ -26,9 +24,9 @@ def user_needs_sp_auth_method_verification? return false if user_needs_sp_auth_method_setup? if piv_cac_required? - auth_method.to_s != 'piv_cac' + auth_method.to_s != TwoFactorAuthenticatable::AuthMethod::PIV_CAC elsif phishing_resistant_required? - !PHISHING_RESISTANT_METHODS.include?(auth_method.to_s) + !TwoFactorAuthenticatable::AuthMethod.phishing_resistant?(auth_method) else false end diff --git a/spec/controllers/concerns/reauthentication_required_concern_spec.rb b/spec/controllers/concerns/reauthentication_required_concern_spec.rb index 5084a2c9d57..0c398032379 100644 --- a/spec/controllers/concerns/reauthentication_required_concern_spec.rb +++ b/spec/controllers/concerns/reauthentication_required_concern_spec.rb @@ -89,7 +89,7 @@ def index end it 'records analytics' do - controller.user_session[:auth_method] = 'totp' + controller.user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::TOTP stub_analytics expect(@analytics).to receive(:track_event).with( 'User 2FA Reauthentication Required', diff --git a/spec/controllers/saml_idp_controller_spec.rb b/spec/controllers/saml_idp_controller_spec.rb index 6852da16925..279aeed87fe 100644 --- a/spec/controllers/saml_idp_controller_spec.rb +++ b/spec/controllers/saml_idp_controller_spec.rb @@ -827,7 +827,9 @@ def name_id_version(format_urn) end it 'returns AAL3 authn_context when AAL3 is requested' do - allow(controller).to receive(:user_session).and_return({ auth_method: 'piv_cac' }) + allow(controller).to receive(:user_session).and_return( + { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC }, + ) user = create(:user, :with_piv_or_cac) auth_settings = saml_settings( overrides: { authn_context: Saml::Idp::Constants::AAL3_AUTHN_CONTEXT_CLASSREF }, @@ -842,7 +844,9 @@ def name_id_version(format_urn) end it 'returns AAL3-HSPD12 authn_context when AAL3-HSPD12 is requested' do - allow(controller).to receive(:user_session).and_return({ auth_method: 'piv_cac' }) + allow(controller).to receive(:user_session).and_return( + { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC }, + ) user = create(:user, :with_piv_or_cac) auth_settings = saml_settings( overrides: { authn_context: Saml::Idp::Constants::AAL3_HSPD12_AUTHN_CONTEXT_CLASSREF }, @@ -857,7 +861,9 @@ def name_id_version(format_urn) end it 'returns AAL2-HSPD12 authn_context when AAL2-HSPD12 is requested' do - allow(controller).to receive(:user_session).and_return({ auth_method: 'piv_cac' }) + allow(controller).to receive(:user_session).and_return( + { auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC }, + ) user = create(:user, :with_piv_or_cac) auth_settings = saml_settings( overrides: { authn_context: Saml::Idp::Constants::AAL2_HSPD12_AUTHN_CONTEXT_CLASSREF }, @@ -872,7 +878,9 @@ def name_id_version(format_urn) end it 'returns AAL2-phishing-resistant authn_context when AAL2-phishing-resistant requested' do - allow(controller).to receive(:user_session).and_return({ auth_method: 'webauthn' }) + allow(controller).to receive(:user_session).and_return( + { auth_method: TwoFactorAuthenticatable::AuthMethod::WEBAUTHN }, + ) user = create(:user, :with_webauthn) auth_settings = saml_settings( overrides: { 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 cca080c12ab..138247b2ad3 100644 --- a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb @@ -76,7 +76,7 @@ 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] = 'phone' + subject.user_session[:auth_method] = TwoFactorAuthenticatable::AuthMethod::SMS end let(:nonce) { 'nonce' } diff --git a/spec/policies/service_provider_mfa_policy_spec.rb b/spec/policies/service_provider_mfa_policy_spec.rb index 22da8adb6a1..b33495c875f 100644 --- a/spec/policies/service_provider_mfa_policy_spec.rb +++ b/spec/policies/service_provider_mfa_policy_spec.rb @@ -3,7 +3,7 @@ describe ServiceProviderMfaPolicy do let(:user) { create(:user) } let(:service_provider) { create(:service_provider) } - let(:auth_method) { 'phone' } + let(:auth_method) { TwoFactorAuthenticatable::AuthMethod::SMS } let(:aal_level_requested) { 1 } let(:piv_cac_requested) { false } let(:phishing_resistant_requested) { nil } @@ -83,7 +83,7 @@ end context 'the user did not use an AAL3 method' do - let(:auth_method) { 'phone' } + let(:auth_method) { TwoFactorAuthenticatable::AuthMethod::SMS } before do setup_user_phone @@ -124,7 +124,7 @@ end context 'the user did not use an AAL3 method' do - let(:auth_method) { 'phone' } + let(:auth_method) { TwoFactorAuthenticatable::AuthMethod::SMS } before do setup_user_phone diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index 7383bc4fc1b..0f6d0fa7bfe 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -18,7 +18,7 @@ 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) - controller.user_session[:auth_method] ||= 'phone' + controller.user_session[:auth_method] ||= 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)