diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index bcfee78cc87..fee8ddf02d2 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -12,16 +12,24 @@ def auth_methods_session end def handle_verification_for_authentication_context(result:, auth_method:, extra_analytics: nil) + increment_mfa_selection_attempt_count(auth_method) analytics.multi_factor_auth( **result.to_h, multi_factor_auth_method: auth_method, enabled_mfa_methods_count: mfa_context.enabled_mfa_methods_count, new_device: new_device?, **extra_analytics.to_h, + mfa_attempts: user_session[:mfa_attempts], + pii_like_keypaths: [ + [:mfa_attempts, :otp], + [:errors, :personal_key], + [:error_details, :personal_key], + ], ) if result.success? handle_valid_verification_for_authentication_context(auth_method:) + user_session.delete(:mfa_attempts) else handle_invalid_verification_for_authentication_context end @@ -124,6 +132,13 @@ def handle_remember_device_preference(remember_device_preference) save_remember_device_preference(remember_device_preference) end + def increment_mfa_selection_attempt_count(auth_method) + user_session[:mfa_attempts] ||= {} + user_session[:mfa_attempts][auth_method] ||= 0 + user_session[:mfa_attempts][auth_method] += 1 + user_session['pii_like_key'] = ['mfa_attempts'] + 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 57b3d5b6012..f0e96b486dd 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -21,6 +21,9 @@ def show end def create + if UserSessionContext.confirmation_context?(context) + increment_mfa_selection_attempt_count(:otp) + end result = otp_verification_form.submit post_analytics(result) @@ -42,6 +45,7 @@ def create end reset_otp_session_data + user_session.delete(:mfa_attempts) else handle_invalid_otp(type: 'otp') end @@ -156,6 +160,12 @@ def analytics_properties phone_configuration_id: phone_configuration&.id, in_account_creation_flow: user_session[:in_account_creation_flow] || false, enabled_mfa_methods_count: mfa_context.enabled_mfa_methods_count, + mfa_attempts: user_session[:mfa_attempts], + pii_like_keypaths: [ + [:mfa_attempts, :otp], + [:errors, :personal_key], + [:error_details, :personal_key], + ], } end diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 72e0983d558..96aafe9965a 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -26,7 +26,6 @@ def create result:, auth_method: TwoFactorAuthenticatable::AuthMethod::TOTP, ) - if result.success? handle_remember_device_preference(params[:remember_device]) redirect_to after_sign_in_path_for(current_user) diff --git a/app/controllers/users/piv_cac_authentication_setup_controller.rb b/app/controllers/users/piv_cac_authentication_setup_controller.rb index 9b32434f741..012d5112d76 100644 --- a/app/controllers/users/piv_cac_authentication_setup_controller.rb +++ b/app/controllers/users/piv_cac_authentication_setup_controller.rb @@ -66,11 +66,13 @@ def piv_cac_service_url_with_redirect end def process_piv_cac_setup + increment_mfa_selection_attempt_count(:piv_cac) result = user_piv_cac_form.submit properties = result.to_h.merge(analytics_properties) analytics.multi_factor_auth_setup(**properties) if result.success? process_valid_submission + user_session.delete(:mfa_attempts) else process_invalid_submission end @@ -126,6 +128,7 @@ def analytics_properties { in_account_creation_flow: user_session[:in_account_creation_flow] || false, enabled_mfa_methods_count: mfa_context.enabled_mfa_methods_count, + mfa_attempts: user_session[:mfa_attempts], } end diff --git a/app/controllers/users/totp_setup_controller.rb b/app/controllers/users/totp_setup_controller.rb index fb9524a8be1..a61e4126f87 100644 --- a/app/controllers/users/totp_setup_controller.rb +++ b/app/controllers/users/totp_setup_controller.rb @@ -26,12 +26,13 @@ def new def confirm result = totp_setup_form.submit - + increment_mfa_selection_attempt_count(:totp) properties = result.to_h.merge(analytics_properties) analytics.multi_factor_auth_setup(**properties) if result.success? process_valid_code + user_session.delete(:mfa_attempts) else process_invalid_code end @@ -118,6 +119,7 @@ def analytics_properties { in_account_creation_flow: in_account_creation_flow?, pii_like_keypaths: [[:mfa_method_counts, :phone]], + mfa_attempts: user_session[:mfa_attempts], } end end diff --git a/app/controllers/users/webauthn_setup_controller.rb b/app/controllers/users/webauthn_setup_controller.rb index 6a80cfda56b..753bcff0c61 100644 --- a/app/controllers/users/webauthn_setup_controller.rb +++ b/app/controllers/users/webauthn_setup_controller.rb @@ -51,6 +51,7 @@ def new end flash_error(result.errors) unless result.success? + increment_mfa_selection_attempt_count(:webauthn) end def confirm @@ -74,6 +75,7 @@ def confirm if result.success? process_valid_webauthn(form) + user_session.delete(:mfa_attempts) else flash.now[:error] = result.first_error_message render :new @@ -141,6 +143,7 @@ def process_valid_webauthn(form) def analytics_properties { in_account_creation_flow: user_session[:in_account_creation_flow] || false, + mfa_attempts: user_session[:mfa_attempts], } end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index d410f516c4e..3cf1c999126 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4945,6 +4945,7 @@ def logout_initiated( # @param [Boolean] new_device Whether the user is authenticating from a new device # @param [String] multi_factor_auth_method Authentication method used # @param [String] multi_factor_auth_method_created_at When the authentication method was created + # @param [Hash] mfa_attempts number of MFA setup attempts # @param [Integer] auth_app_configuration_id Database ID of authentication app configuration # @param [Integer] piv_cac_configuration_id Database ID of PIV/CAC configuration # @param [String] piv_cac_configuration_dn_uuid PIV/CAC X509 distinguished name UUID @@ -4968,6 +4969,7 @@ def multi_factor_auth( errors: nil, error_details: nil, context: nil, + mfa_attempts: nil, multi_factor_auth_method_created_at: nil, auth_app_configuration_id: nil, piv_cac_configuration_id: nil, @@ -4991,6 +4993,7 @@ def multi_factor_auth( error_details:, context:, new_device:, + mfa_attempts:, multi_factor_auth_method:, multi_factor_auth_method_created_at:, auth_app_configuration_id:, @@ -5038,10 +5041,12 @@ def multi_factor_auth_added_phone( # @param [Integer] enabled_mfa_methods_count Number of enabled MFA methods on the account # @param [Boolean] in_account_creation_flow whether user is going through creation flow # @param ['piv_cac'] method_name Authentication method added + # @param [Hash] mfa_attempts number of MFA setup attempts def multi_factor_auth_added_piv_cac( enabled_mfa_methods_count:, in_account_creation_flow:, method_name: :piv_cac, + mfa_attempts: nil, **extra ) track_event( @@ -5049,6 +5054,7 @@ def multi_factor_auth_added_piv_cac( method_name:, enabled_mfa_methods_count:, in_account_creation_flow:, + mfa_attempts:, **extra, ) end @@ -5088,6 +5094,7 @@ def multi_factor_auth_enter_backup_code_visit(context:, **extra) end # @param ["authentication", "reauthentication", "confirmation"] context User session context + # @param [Hash] mfa_attempts number of MFA setup attempts # @param [String] multi_factor_auth_method # @param [Boolean] confirmation_for_add_phone # @param [Integer] phone_configuration_id @@ -5107,11 +5114,13 @@ def multi_factor_auth_enter_otp_visit( phone_fingerprint:, in_account_creation_flow:, enabled_mfa_methods_count:, + mfa_attempts: nil, **extra ) track_event( 'Multi-Factor Authentication: enter OTP visited', context:, + mfa_attempts:, multi_factor_auth_method:, confirmation_for_add_phone:, phone_configuration_id:, @@ -5287,6 +5296,7 @@ def multi_factor_auth_phone_setup( # @param [String, nil] key_id PIV/CAC key_id from PKI service # @param [Hash] mfa_method_counts Hash of MFA method with the number of that method on the account # @param [Hash] authenticator_data_flags WebAuthn authenticator data flags + # @param [Hash] mfa_attempts number of MFA setup attempts # @param [String, nil] aaguid AAGUID value of WebAuthn device # @param [String[], nil] unknown_transports Array of unrecognized WebAuthn transports, intended to # be used in case of future specification changes. @@ -5310,6 +5320,7 @@ def multi_factor_auth_setup( key_id: nil, mfa_method_counts: nil, authenticator_data_flags: nil, + mfa_attempts: nil, aaguid: nil, unknown_transports: nil, **extra @@ -5335,6 +5346,7 @@ def multi_factor_auth_setup( key_id:, mfa_method_counts:, authenticator_data_flags:, + mfa_attempts:, aaguid:, unknown_transports:, **extra, @@ -5984,11 +5996,18 @@ def piv_cac_recommended_visited # Tracks when user's piv cac setup # @param [Boolean] in_account_creation_flow Whether user is going through account creation # @param [Integer] enabled_mfa_methods_count Number of enabled MFA methods on the account - def piv_cac_setup_visited(in_account_creation_flow:, enabled_mfa_methods_count: nil, **extra) + # @param [Hash] mfa_attempts number of MFA setup attempts + def piv_cac_setup_visited( + in_account_creation_flow:, + enabled_mfa_methods_count: nil, + mfa_attempts: nil, + **extra + ) track_event( :piv_cac_setup_visited, in_account_creation_flow:, enabled_mfa_methods_count:, + mfa_attempts:, **extra, ) end diff --git a/lib/session_encryptor.rb b/lib/session_encryptor.rb index 57974db862b..2fc4d96bfcd 100644 --- a/lib/session_encryptor.rb +++ b/lib/session_encryptor.rb @@ -173,15 +173,20 @@ def alert_or_raise_if_contains_sensitive_value!(string, hash) def alert_or_raise_if_contains_sensitive_keys!(hash) hash.deep_transform_keys do |key| if SENSITIVE_KEYS.include?(key.to_s) - exception = SensitiveKeyError.new("#{key} unexpectedly appeared in session") - if IdentityConfig.store.session_encryptor_alert_enabled - NewRelic::Agent.notice_error( - exception, custom_params: { - session_structure: hash.deep_transform_values { |_v| '' }, - } - ) - else - raise exception + unless hash['warden.user.user.session']&.dig('pii_like_key'). + present? && hash['warden.user.user.session']['pii_like_key'].any? do |key| + hash['warden.user.user.session'].key?(key) + end + exception = SensitiveKeyError.new("#{key} unexpectedly appeared in session") + if IdentityConfig.store.session_encryptor_alert_enabled + NewRelic::Agent.notice_error( + exception, custom_params: { + session_structure: hash.deep_transform_values { |_v| '' }, + } + ) + else + raise exception + end end end end diff --git a/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb b/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb index f14f9ec9c32..2e283f6e50b 100644 --- a/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb +++ b/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb @@ -36,6 +36,7 @@ multi_factor_auth_method: TwoFactorAuthenticatable::AuthMethod::REMEMBER_DEVICE, enabled_mfa_methods_count: 0, new_device: true, + mfa_attempts: { 'remember_device' => 1 }, ) end @@ -189,6 +190,7 @@ multi_factor_auth_method: TwoFactorAuthenticatable::AuthMethod::SMS, enabled_mfa_methods_count: 1, new_device: true, + mfa_attempts: { 'sms' => 1 }, ) end 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 2b474fdf885..2b8a6e20cbb 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,7 @@ multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), enabled_mfa_methods_count: 1, new_device: true, + mfa_attempts: { 'backup_code' => 1 }, ) expect(subject.user_session[:auth_events]).to eq( @@ -97,6 +98,7 @@ multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), enabled_mfa_methods_count: 1, new_device: true, + mfa_attempts: { 'backup_code' => 1 }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -175,6 +177,7 @@ multi_factor_auth_method: 'backup_code', enabled_mfa_methods_count: 1, new_device: true, + mfa_attempts: { 'backup_code' => 1 }, ) expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') 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 cf02c712fd8..5fca28f9f41 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -152,6 +152,9 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + 'sms' => 1, + }, ) end @@ -234,6 +237,9 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + 'sms' => 1, + }, ) expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end @@ -304,6 +310,9 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + 'sms' => 1, + }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -351,6 +360,9 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + 'sms' => 1, + }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -535,6 +547,9 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: true, + mfa_attempts: { + otp: 1, + }, ) end @@ -604,12 +619,22 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + otp: 1, + }, ) end context 'user enters in valid code after invalid entry' do before do expect(subject.current_user.reload.second_factor_attempts_count).to eq 1 + post( + :create, + params: { + code: '999', + otp_delivery_preference: 'sms', + }, + ) post( :create, params: { @@ -621,6 +646,28 @@ it 'resets second_factor_attempts_count' do expect(subject.current_user.reload.second_factor_attempts_count).to eq 0 end + + it 'tracks an event' do + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication Setup', + success: false, + error_details: { code: { wrong_length: true, incorrect: true } }, + confirmation_for_add_phone: true, + context: 'confirmation', + multi_factor_auth_method: 'sms', + phone_configuration_id: controller.current_user.default_phone_configuration.id, + multi_factor_auth_method_created_at: controller.current_user. + default_phone_configuration.created_at.strftime('%s%L'), + area_code: parsed_phone.area_code, + country_code: parsed_phone.country, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + enabled_mfa_methods_count: 1, + in_account_creation_flow: false, + mfa_attempts: { + otp: 3, + }, + ) + end end end @@ -669,6 +716,9 @@ phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), enabled_mfa_methods_count: 0, in_account_creation_flow: false, + mfa_attempts: { + otp: 1, + }, ) expect(controller).to have_received(:create_user_event).with(:phone_confirmed) 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 d376cc825b9..14971363673 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 @@ -76,6 +76,7 @@ multi_factor_auth_method: 'personal-key', multi_factor_auth_method_created_at:, new_device: true, + mfa_attempts: { 'personal_key' => 1 }, ) expect(@analytics).to have_logged_event( 'Personal key: Alert user about sign in', @@ -218,6 +219,7 @@ multi_factor_auth_method: 'personal-key', multi_factor_auth_method_created_at: personal_key_generated_at.strftime('%s%L'), new_device: true, + mfa_attempts: { 'personal_key' => 1 }, ) expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') 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 d2c04d1ce67..a3ad89d5994 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 @@ -135,6 +135,9 @@ piv_cac_configuration_id: cfg.id, piv_cac_configuration_dn_uuid: cfg.x509_dn_uuid, key_id: 'foo', + mfa_attempts: { + 'piv_cac' => 1, + }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -257,6 +260,9 @@ new_device: true, key_id: 'foo', piv_cac_configuration_dn_uuid: 'bad-uuid', + mfa_attempts: { + 'piv_cac' => 1, + }, ) expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') 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 8d273b1df37..5fa7b9476d8 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -59,6 +59,7 @@ multi_factor_auth_method_created_at: cfg.created_at.strftime('%s%L'), new_device: true, auth_app_configuration_id: controller.current_user.auth_app_configurations.first.id, + mfa_attempts: { 'totp' => 1 }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -177,6 +178,7 @@ enabled_mfa_methods_count: 2, multi_factor_auth_method: 'totp', new_device: true, + mfa_attempts: { 'totp' => 1 }, ) expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') 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 0c8732018ef..d2854bc6576 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -158,6 +158,7 @@ webauthn_configuration_id: webauthn_configuration.id, multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), new_device: true, + mfa_attempts: { 'webauthn' => 1 }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -218,6 +219,7 @@ multi_factor_auth_method_created_at: webauthn_configuration.created_at. strftime('%s%L'), new_device: true, + mfa_attempts: { 'webauthn_platform' => 1 }, ) expect(@analytics).to have_logged_event( 'User marked authenticated', @@ -249,6 +251,7 @@ webauthn_configuration_id: webauthn_configuration.id, multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), new_device: true, + mfa_attempts: { 'webauthn' => 1 }, ) end @@ -315,6 +318,7 @@ second_webauthn_platform_configuration.created_at.strftime('%s%L'), new_device: true, frontend_error: webauthn_error, + mfa_attempts: { 'webauthn_platform' => 1 }, ) end 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 3656a2d72ac..7f6bf4de733 100644 --- a/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb +++ b/spec/controllers/users/piv_cac_authentication_setup_controller_spec.rb @@ -109,8 +109,39 @@ context 'with no additional MFAs chosen on setup' do let(:mfa_selections) { ['piv_cac'] } it 'redirects to suggest 2nd MFA page' do + stub_analytics get :new, params: { token: good_token } expect(response).to redirect_to(auth_method_confirmation_url) + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication Setup', + enabled_mfa_methods_count: 1, + errors: {}, + multi_factor_auth_method: 'piv_cac', + in_account_creation_flow: false, + success: true, + mfa_attempts: { + piv_cac: 1, + }, + ) + end + + it 'logs mfa attempts commensurate to number of attempts' do + stub_analytics + get :new, params: { token: bad_token } + get :new, params: { token: good_token } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication Setup', + enabled_mfa_methods_count: 1, + errors: {}, + multi_factor_auth_method: 'piv_cac', + in_account_creation_flow: false, + success: true, + mfa_attempts: { + piv_cac: 2, + }, + ) end it 'sets the piv/cac session information' do diff --git a/spec/controllers/users/totp_setup_controller_spec.rb b/spec/controllers/users/totp_setup_controller_spec.rb index 0d865fc2e1f..ac6d1dab3cb 100644 --- a/spec/controllers/users/totp_setup_controller_spec.rb +++ b/spec/controllers/users/totp_setup_controller_spec.rb @@ -108,6 +108,9 @@ multi_factor_auth_method: 'totp', enabled_mfa_methods_count: 0, in_account_creation_flow: false, + mfa_attempts: { + totp: 1, + }, ) end end @@ -136,6 +139,42 @@ auth_app_configuration_id: next_auth_app_id, enabled_mfa_methods_count: 2, in_account_creation_flow: false, + mfa_attempts: { + totp: 1, + }, + ) + end + end + + context 'when user presents correct code after submitting an incorrect code' do + before do + user = create(:user, :fully_registered) + secret = ROTP::Base32.random_base32 + stub_sign_in(user) + stub_analytics + + subject.user_session[:new_totp_secret] = 'abcdehij' + + patch :confirm, params: { name: name, code: 123 } + + subject.user_session[:new_totp_secret] = secret + + patch :confirm, params: { name: name, code: generate_totp_code(secret) } + end + + it 'logs correct events' do + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication Setup', + success: true, + errors: {}, + totp_secret_present: true, + multi_factor_auth_method: 'totp', + auth_app_configuration_id: next_auth_app_id, + enabled_mfa_methods_count: 2, + in_account_creation_flow: false, + mfa_attempts: { + totp: 2, + }, ) end end @@ -164,6 +203,9 @@ multi_factor_auth_method: 'totp', enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + 'totp' => 1, + }, ) end end @@ -193,6 +235,9 @@ multi_factor_auth_method: 'totp', enabled_mfa_methods_count: 1, in_account_creation_flow: false, + mfa_attempts: { + totp: 1, + }, ) end end @@ -221,6 +266,9 @@ multi_factor_auth_method: 'totp', enabled_mfa_methods_count: 0, in_account_creation_flow: false, + mfa_attempts: { + totp: 1, + }, ) end end @@ -252,6 +300,9 @@ auth_app_configuration_id: next_auth_app_id, enabled_mfa_methods_count: 1, in_account_creation_flow: true, + mfa_attempts: { + totp: 1, + }, ) end end @@ -271,6 +322,9 @@ auth_app_configuration_id: next_auth_app_id, enabled_mfa_methods_count: 1, in_account_creation_flow: true, + mfa_attempts: { + totp: 1, + }, ) end end @@ -297,6 +351,9 @@ multi_factor_auth_method: 'totp', enabled_mfa_methods_count: 0, in_account_creation_flow: false, + mfa_attempts: { + totp: 1, + }, ) end end diff --git a/spec/controllers/users/webauthn_setup_controller_spec.rb b/spec/controllers/users/webauthn_setup_controller_spec.rb index b801a775bdc..6e1d5ba5729 100644 --- a/spec/controllers/users/webauthn_setup_controller_spec.rb +++ b/spec/controllers/users/webauthn_setup_controller_spec.rb @@ -369,5 +369,61 @@ end end end + + context 'sign in and confirm' do + let(:params) do + { + attestation_object: attestation_object, + client_data_json: setup_client_data_json, + name: 'mykey', + transports: 'usb', + authenticator_data_value: '65', + } + end + + before do + get :new + controller.user_session[:in_account_creation_flow] = true + allow(IdentityConfig.store).to receive(:domain_name).and_return('localhost:3000') + request.host = 'localhost:3000' + controller.user_session[:webauthn_challenge] = webauthn_challenge + end + + let(:mfa_selections) { ['webauthn'] } + + it 'tracks the submission' do + Funnel::Registration::AddMfa.call(user.id, 'webauthn', @analytics) + + patch :confirm, params: params + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication Setup', + enabled_mfa_methods_count: 1, + mfa_method_counts: { + webauthn: 1, + }, + multi_factor_auth_method: 'webauthn', + success: true, + errors: {}, + in_account_creation_flow: true, + authenticator_data_flags: { + up: true, + uv: false, + be: false, + bs: false, + at: true, + ed: false, + }, + mfa_attempts: { + 'webauthn' => 1, + }, + ) + expect(@analytics).to have_logged_event( + :webauthn_setup_submitted, + platform_authenticator: false, + success: true, + ) + end + end end end diff --git a/spec/lib/session_encryptor_spec.rb b/spec/lib/session_encryptor_spec.rb index 048351aa390..a298f034994 100644 --- a/spec/lib/session_encryptor_spec.rb +++ b/spec/lib/session_encryptor_spec.rb @@ -125,6 +125,21 @@ subject.dump(session) end + it 'does not raise for session personal_key key for counting mfa attempts' do + nested_session = { + 'warden.user.user.session': { + mfa_attempts: { personal_key: 1 }, + pii_like_key: ['mfa_attempts'], + }, + } + + expect do + subject.dump(nested_session) + end.not_to raise_error( + SessionEncryptor::SensitiveKeyError, 'personal_key unexpectedly appeared in session' + ) + end + it 'raises if sensitive value is not KMS encrypted' do session = { 'new_key' => Idp::Constants::MOCK_IDV_APPLICANT[:last_name],