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 28befc982e4..2fe431237d2 100644 --- a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb +++ b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb @@ -22,9 +22,7 @@ def show def create @backup_code_form = BackupCodeVerificationForm.new(current_user) result = @backup_code_form.submit(backup_code_params) - analytics.track_mfa_submit_event( - result.to_h.merge(new_device: new_device?), - ) + analytics.multi_factor_auth(**result.to_h.merge(new_device: new_device?)) irs_attempts_api_tracker.mfa_login_backup_code(success: result.success?) handle_result(result) end diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 3bf40093f41..f3ca89a228f 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -136,7 +136,7 @@ def post_analytics(result) properties = result.to_h.merge(analytics_properties, new_device: new_device?) analytics.multi_factor_auth_setup(**properties) if context == 'confirmation' - analytics.track_mfa_submit_event(properties) + analytics.multi_factor_auth(**properties) if UserSessionContext.reauthentication_context?(context) irs_attempts_api_tracker.mfa_login_phone_otp_submitted( 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 85302e11991..af95dd27af2 100644 --- a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb +++ b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb @@ -32,7 +32,10 @@ def track_analytics(result) new_device: new_device?, ) - analytics.track_mfa_submit_event(analytics_hash) + analytics.multi_factor_auth( + **analytics_hash, + pii_like_keypaths: [[:errors, :personal_key], [:error_details, :personal_key]], + ) end def check_personal_key_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 56151472bfa..ccd783d158e 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -30,9 +30,7 @@ def redirect_to_piv_cac_service def process_token result = piv_cac_verification_form.submit - analytics.track_mfa_submit_event( - result.to_h.merge(analytics_properties), - ) + analytics.multi_factor_auth(**result.to_h.merge(analytics_properties)) irs_attempts_api_tracker.mfa_login_piv_cac( success: result.success?, subject_dn: piv_cac_verification_form.x509_dn, diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 111c8a52c1b..28c50c48444 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -21,7 +21,7 @@ def show def create result = TotpVerificationForm.new(current_user, params.require(:code).strip).submit - analytics.track_mfa_submit_event(result.to_h.merge(new_device: new_device?)) + analytics.multi_factor_auth(**result.to_h.merge(new_device: new_device?)) irs_attempts_api_tracker.mfa_login_totp(success: result.success?) if result.success? diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 458ebf650a8..75f8e6255a3 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -18,7 +18,7 @@ def show def confirm result = form.submit - analytics.track_mfa_submit_event( + analytics.multi_factor_auth( **result.to_h, **analytics_properties, multi_factor_auth_method_created_at: diff --git a/app/services/analytics.rb b/app/services/analytics.rb index 4016a43c446..ac56e289d4d 100644 --- a/app/services/analytics.rb +++ b/app/services/analytics.rb @@ -52,13 +52,6 @@ def first_event_this_session? session[:first_event] end - def track_mfa_submit_event(attributes) - multi_factor_auth( - **attributes, - pii_like_keypaths: [[:errors, :personal_key], [:error_details, :personal_key]], - ) - end - def request_attributes attributes = { user_ip: request.remote_ip, diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index cd2b15c6291..a2dcef259d0 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3730,6 +3730,7 @@ def logout_initiated( # @param [Boolean] success Whether authentication was successful # @param [Hash] errors Authentication error reasons, if unsuccessful + # @param [Hash] error_details Details for error that occurred in unsuccessful submission # @param [String] context # @param [Boolean] new_device # @param [String] multi_factor_auth_method @@ -3748,6 +3749,7 @@ def logout_initiated( def multi_factor_auth( success:, errors: nil, + error_details: nil, context: nil, new_device: nil, multi_factor_auth_method: nil, @@ -3767,24 +3769,27 @@ def multi_factor_auth( ) track_event( 'Multi-Factor Authentication', - success: success, - errors: errors, - context: context, - new_device: new_device, - multi_factor_auth_method: multi_factor_auth_method, - multi_factor_auth_method_created_at: multi_factor_auth_method_created_at, - auth_app_configuration_id: auth_app_configuration_id, - piv_cac_configuration_id: piv_cac_configuration_id, - key_id: key_id, - webauthn_configuration_id: webauthn_configuration_id, - confirmation_for_add_phone: confirmation_for_add_phone, - phone_configuration_id: phone_configuration_id, - pii_like_keypaths: pii_like_keypaths, - area_code: area_code, - country_code: country_code, - phone_fingerprint: phone_fingerprint, - frontend_error:, - **extra, + { + success: success, + errors: errors, + error_details: error_details, + context: context, + new_device: new_device, + multi_factor_auth_method: multi_factor_auth_method, + multi_factor_auth_method_created_at: multi_factor_auth_method_created_at, + auth_app_configuration_id: auth_app_configuration_id, + piv_cac_configuration_id: piv_cac_configuration_id, + key_id: key_id, + webauthn_configuration_id: webauthn_configuration_id, + confirmation_for_add_phone: confirmation_for_add_phone, + phone_configuration_id: phone_configuration_id, + pii_like_keypaths: pii_like_keypaths, + area_code: area_code, + country_code: country_code, + phone_fingerprint: phone_fingerprint, + frontend_error:, + **extra, + }.compact, ) 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 a1f1118228b..709dc432a76 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 @@ -27,16 +27,6 @@ sign_in_before_2fa(user) stub_analytics stub_attempts_tracker - analytics_hash = { - success: true, - errors: {}, - multi_factor_auth_method: 'backup_code', - multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), - new_device: true, - } - - expect(@analytics).to receive(:track_mfa_submit_event). - with(analytics_hash) expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_backup_code, success: true) @@ -47,6 +37,15 @@ post :create, params: payload + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: true, + errors: {}, + multi_factor_auth_method: 'backup_code', + multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), + new_device: true, + ) + expect(subject.user_session[:auth_events]).to eq( [ auth_method: TwoFactorAuthenticatable::AuthMethod::BACKUP_CODE, @@ -93,22 +92,23 @@ stub_analytics stub_attempts_tracker - expect(@analytics).to receive(:track_mfa_submit_event). - with({ - success: true, - errors: {}, - multi_factor_auth_method: 'backup_code', - multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), - new_device: true, - }) - expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_backup_code, success: true) - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) - post :create, params: payload + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: true, + errors: {}, + multi_factor_auth_method: 'backup_code', + multi_factor_auth_method_created_at: Time.zone.now.strftime('%s%L'), + new_device: true, + ) + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end end end @@ -122,10 +122,12 @@ stub_analytics stub_sign_in_before_2fa(user) - expect(@analytics).to receive(:track_mfa_submit_event). - with(hash_including(new_device: false)) - post :create, params: payload + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + hash_including(new_device: false), + ) end end @@ -171,26 +173,13 @@ user.second_factor_attempts_count = IdentityConfig.store.login_otp_confirmation_max_attempts - 1 user.save - properties = { - success: false, - errors: {}, - multi_factor_auth_method: 'backup_code', - multi_factor_auth_method_created_at: nil, - new_device: true, - } stub_analytics stub_attempts_tracker - expect(@analytics).to receive(:track_mfa_submit_event). - with(properties) - expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_backup_code, success: false) - expect(@analytics).to receive(:track_event). - with('Multi-Factor Authentication: max attempts reached') - expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'backup_code') @@ -198,6 +187,15 @@ with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) post :create, params: payload + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: false, + errors: {}, + multi_factor_auth_method: 'backup_code', + new_device: true, + ) + expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end it 'records unsuccessful 2fa 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 2cc1a037dfa..0b2e8d4a6ab 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -122,41 +122,43 @@ end describe '#create' do - let(:parsed_phone) { Phonelib.parse(controller.current_user.default_phone_configuration.phone) } + let(:user) { create(:user, :with_phone) } + let(:parsed_phone) { Phonelib.parse(user.default_phone_configuration.phone) } context 'when the user enters an invalid OTP during authentication context' do subject(:response) { post :create, params: { code: '12345', otp_delivery_preference: 'sms' } } before do - sign_in_before_2fa + sign_in_before_2fa(user) controller.user_session[:mfa_selections] = ['sms'] expect(controller.current_user.reload.second_factor_attempts_count).to eq 0 - phone_configuration_created_at = controller.current_user. - default_phone_configuration.created_at - properties = { + stub_analytics + stub_attempts_tracker + + expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). + with({ reauthentication: false, success: false }) + end + + it 'logs analytics' do + response + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', success: false, error_details: { code: { wrong_length: true, incorrect: true } }, confirmation_for_add_phone: false, context: 'authentication', multi_factor_auth_method: 'sms', - multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'), + multi_factor_auth_method_created_at: user.default_phone_configuration.created_at. + strftime('%s%L'), new_device: true, - phone_configuration_id: controller.current_user.default_phone_configuration.id, + phone_configuration_id: user.default_phone_configuration.id, 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, - } - - stub_analytics - stub_attempts_tracker - - expect(@analytics).to receive(:track_mfa_submit_event). - with(properties) - - expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). - with({ reauthentication: false, success: false }) + ) end it 'increments second_factor_attempts_count' do @@ -191,7 +193,7 @@ context 'when the user enters an invalid OTP during reauthentication context' do it 'increments second_factor_attempts_count' do - sign_in_before_2fa + sign_in_before_2fa(user) controller.user_session[:context] = 'reauthentication' post :create, params: { code: '12345', otp_delivery_preference: 'sms' } @@ -201,42 +203,23 @@ end context 'when the user has reached the max number of OTP attempts' do - it 'tracks the event' do - user = create( + let(:user) do + create( :user, :fully_registered, + :with_phone, second_factor_attempts_count: IdentityConfig.store.login_otp_confirmation_max_attempts - 1, ) + end + + it 'tracks the event' do sign_in_before_2fa(user) controller.user_session[:mfa_selections] = ['sms'] - phone_configuration_created_at = controller.current_user. - default_phone_configuration.created_at - - properties = { - success: false, - error_details: { code: { wrong_length: true, incorrect: true } }, - confirmation_for_add_phone: false, - context: 'authentication', - multi_factor_auth_method: 'sms', - multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'), - new_device: true, - phone_configuration_id: controller.current_user.default_phone_configuration.id, - 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, - } stub_analytics stub_attempts_tracker - expect(@analytics).to receive(:track_mfa_submit_event). - with(properties) - - expect(@analytics).to receive(:track_event). - with('Multi-Factor Authentication: max attempts reached') expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: controller.current_user)) @@ -246,15 +229,32 @@ expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'otp') - post :create, params: - { code: '12345', - otp_delivery_preference: 'sms' } + post :create, params: { code: '12345', otp_delivery_preference: 'sms' } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: false, + error_details: { code: { wrong_length: true, incorrect: true } }, + confirmation_for_add_phone: false, + context: 'authentication', + multi_factor_auth_method: 'sms', + multi_factor_auth_method_created_at: user.default_phone_configuration.created_at. + strftime('%s%L'), + new_device: true, + phone_configuration_id: user.default_phone_configuration.id, + 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, + ) + expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end end context 'when the user enters a valid OTP' do before do - sign_in_before_2fa + sign_in_before_2fa(user) subject.user_session[:mfa_selections] = ['sms'] expect(subject.current_user.reload.second_factor_attempts_count).to eq 0 end @@ -279,31 +279,9 @@ end it 'tracks the valid authentication event' do - phone_configuration_created_at = controller.current_user. - default_phone_configuration.created_at - properties = { - success: true, - confirmation_for_add_phone: false, - context: 'authentication', - multi_factor_auth_method: 'sms', - multi_factor_auth_method_created_at: phone_configuration_created_at.strftime('%s%L'), - new_device: true, - phone_configuration_id: controller.current_user.default_phone_configuration.id, - 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, - } - stub_analytics stub_attempts_tracker - expect(@analytics).to receive(:track_mfa_submit_event). - with(properties) - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) - expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). with(reauthentication: false, success: true) @@ -327,6 +305,27 @@ ) expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: true, + confirmation_for_add_phone: false, + context: 'authentication', + multi_factor_auth_method: 'sms', + multi_factor_auth_method_created_at: user.default_phone_configuration.created_at. + strftime('%s%L'), + new_device: true, + phone_configuration_id: user.default_phone_configuration.id, + 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, + ) + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end context 'with existing device' do @@ -337,13 +336,15 @@ it 'tracks new device value' do stub_analytics - expect(@analytics).to receive(:track_mfa_submit_event). - with(hash_including(new_device: false)) - post :create, params: { code: subject.current_user.reload.direct_otp, otp_delivery_preference: 'sms', } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + hash_including(new_device: false), + ) end end @@ -402,7 +403,7 @@ context 'when the user lockout period expires' do before do - sign_in_before_2fa + sign_in_before_2fa(user) lockout_period = IdentityConfig.store.lockout_period_in_minutes.minutes subject.current_user.update( second_factor_locked_at: Time.zone.now - lockout_period - 1.second, 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 2baef2c7369..a244e28ee70 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 @@ -47,29 +47,11 @@ let(:payload) { { personal_key_form: personal_key } } it 'tracks the valid authentication event' do personal_key + multi_factor_auth_method_created_at = user.reload. + encrypted_recovery_code_digest_generated_at.strftime('%s%L') sign_in_before_2fa(user) stub_analytics - analytics_hash = { - success: true, - errors: {}, - multi_factor_auth_method: 'personal-key', - multi_factor_auth_method_created_at: user.reload. - encrypted_recovery_code_digest_generated_at.strftime('%s%L'), - new_device: true, - } - - expect(@analytics).to receive(:track_mfa_submit_event). - with(analytics_hash) - - expect(@analytics).to receive(:track_event).with( - 'Personal key: Alert user about sign in', - hash_including(emails: 1, sms_message_ids: ['fake-message-id']), - ) - - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) - expect(controller).to receive(:handle_valid_verification_for_authentication_context). with(auth_method: TwoFactorAuthenticatable::AuthMethod::PERSONAL_KEY). and_call_original @@ -85,6 +67,23 @@ ) expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: true, + errors: {}, + multi_factor_auth_method: 'personal-key', + multi_factor_auth_method_created_at:, + new_device: true, + ) + expect(@analytics).to have_logged_event( + 'Personal key: Alert user about sign in', + hash_including(emails: 1, sms_message_ids: ['fake-message-id']), + ) + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end context 'with enable_additional_mfa_redirect_for_personal_key_mfa? set to true' do @@ -122,10 +121,12 @@ stub_analytics stub_sign_in_before_2fa(user) - expect(@analytics).to receive(:track_mfa_submit_event). - with(hash_including(new_device: false)) - post :create, params: payload + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + hash_including(new_device: false), + ) end end end @@ -203,20 +204,6 @@ stub_analytics stub_attempts_tracker - properties = { - success: false, - errors: { personal_key: [t('errors.messages.personal_key_incorrect')] }, - error_details: { personal_key: { personal_key_incorrect: true } }, - multi_factor_auth_method: 'personal-key', - multi_factor_auth_method_created_at: personal_key_generated_at.strftime('%s%L'), - new_device: true, - } - - expect(@analytics).to receive(:track_mfa_submit_event). - with(properties) - expect(@analytics).to receive(:track_event). - with('Multi-Factor Authentication: max attempts reached') - expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'personal_key') @@ -224,6 +211,17 @@ with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) post :create, params: payload + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: false, + errors: { personal_key: [t('errors.messages.personal_key_incorrect')] }, + error_details: { personal_key: { personal_key_incorrect: true } }, + multi_factor_auth_method: 'personal-key', + multi_factor_auth_method_created_at: personal_key_generated_at.strftime('%s%L'), + new_device: true, + ) + expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end it 'records unsuccessful 2fa event' 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 e62ea8697cd..4e0a56e92a2 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 @@ -105,17 +105,26 @@ stub_attempts_tracker cfg = controller.current_user.piv_cac_configurations.first - attributes = { + expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( + success: true, + subject_dn: x509_subject, + ) + + expect(controller).to receive(:handle_valid_verification_for_authentication_context). + with(auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC). + and_call_original + + get :show, params: { token: 'good-token' } + + expect(@analytics).to have_logged_event( + :multi_factor_auth_enter_piv_cac, context: 'authentication', multi_factor_auth_method: 'piv_cac', new_device: true, piv_cac_configuration_id: nil, - } - - expect(@analytics).to receive(:track_event). - with(:multi_factor_auth_enter_piv_cac, attributes) - - submit_attributes = { + ) + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', success: true, errors: {}, context: 'authentication', @@ -125,23 +134,11 @@ piv_cac_configuration_id: cfg.id, piv_cac_configuration_dn_uuid: cfg.x509_dn_uuid, key_id: 'foo', - } - expect(@analytics).to receive(:track_mfa_submit_event). - with(submit_attributes) - - expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( - success: true, - subject_dn: x509_subject, ) - - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) - - expect(controller).to receive(:handle_valid_verification_for_authentication_context). - with(auth_method: TwoFactorAuthenticatable::AuthMethod::PIV_CAC). - and_call_original - - get :show, params: { token: 'good-token' } + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end context 'with existing device' do @@ -153,10 +150,12 @@ stub_analytics stub_sign_in_before_2fa(user) - expect(@analytics).to receive(:track_mfa_submit_event). - with(hash_including(new_device: false)) - get :show, params: { token: 'good-token' } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + hash_including(new_device: false), + ) end end end @@ -240,46 +239,39 @@ stub_analytics stub_attempts_tracker - attributes = { - context: 'authentication', - multi_factor_auth_method: 'piv_cac', - new_device: true, - piv_cac_configuration_id: nil, - } - - expect(@analytics).to receive(:track_event). - with(:multi_factor_auth_enter_piv_cac, attributes) - expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(mfa_device_type: 'piv_cac') piv_cac_mismatch = { type: 'user.piv_cac_mismatch' } - submit_attributes = { - success: false, - errors: piv_cac_mismatch, - context: 'authentication', - multi_factor_auth_method: 'piv_cac', - multi_factor_auth_method_created_at: nil, - new_device: true, - key_id: 'foo', - piv_cac_configuration_dn_uuid: 'bad-uuid', - piv_cac_configuration_id: nil, - } - expect(@analytics).to receive(:track_mfa_submit_event). - with(submit_attributes) - expect(@irs_attempts_api_tracker).to receive(:mfa_login_piv_cac).with( success: false, subject_dn: bad_dn, ) - expect(@analytics).to receive(:track_event). - with('Multi-Factor Authentication: max attempts reached') expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) get :show, params: { token: 'bad-token' } + + expect(@analytics).to have_logged_event( + :multi_factor_auth_enter_piv_cac, + context: 'authentication', + multi_factor_auth_method: 'piv_cac', + new_device: true, + piv_cac_configuration_id: nil, + ) + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: false, + errors: piv_cac_mismatch, + context: 'authentication', + multi_factor_auth_method: 'piv_cac', + new_device: true, + key_id: 'foo', + piv_cac_configuration_dn_uuid: 'bad-uuid', + ) + expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end 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 5cc253b1c51..e2ec77add5c 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -45,18 +45,6 @@ it 'tracks the valid authentication event' do cfg = controller.current_user.auth_app_configurations.first - attributes = { - success: true, - errors: {}, - multi_factor_auth_method: 'totp', - 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, - } - expect(@analytics).to receive(:track_mfa_submit_event). - with(attributes) - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: true) expect(controller).to receive(:handle_valid_verification_for_authentication_context). @@ -64,6 +52,20 @@ and_call_original post :create, params: { code: generate_totp_code(@secret) } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: true, + errors: {}, + multi_factor_auth_method: 'totp', + 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, + ) + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end context 'with existing device' do @@ -74,10 +76,12 @@ it 'tracks new device value' do stub_analytics - expect(@analytics).to receive(:track_mfa_submit_event). - with(hash_including(new_device: false)) - post :create, params: { code: generate_totp_code(@secret) } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + hash_including(new_device: false), + ) end end end @@ -88,18 +92,26 @@ app1 = Db::AuthAppConfiguration.create(user, user.generate_totp_secret, nil, 'foo') app2 = Db::AuthAppConfiguration.create(user, user.generate_totp_secret, nil, 'bar') - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa).twice - sign_in_as_user(user) post :create, params: { code: generate_totp_code(app1.otp_secret_key) } expect(response).to redirect_to account_url + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) + @analytics.events.clear + sign_out(user) sign_in_as_user(user) post :create, params: { code: generate_totp_code(app2.otp_secret_key) } expect(response).to redirect_to account_url + + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end end @@ -155,19 +167,6 @@ @secret = user.generate_totp_secret Db::AuthAppConfiguration.create(user, @secret, nil, 'foo') - attributes = { - success: false, - errors: {}, - multi_factor_auth_method: 'totp', - multi_factor_auth_method_created_at: nil, - new_device: true, - auth_app_configuration_id: nil, - } - - expect(@analytics).to receive(:track_mfa_submit_event). - with(attributes) - expect(@analytics).to receive(:track_event). - with('Multi-Factor Authentication: max attempts reached') expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: false) @@ -178,6 +177,15 @@ with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) post :create, params: { code: '12345' } + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + success: false, + errors: {}, + multi_factor_auth_method: 'totp', + new_device: true, + ) + expect(@analytics).to have_logged_event('Multi-Factor Authentication: max attempts reached') end 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 802b619982b..7d1ecf24292 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -133,27 +133,12 @@ ) controller.current_user.webauthn_configurations.first end - let(:result) do - { - context: 'authentication', - multi_factor_auth_method: 'webauthn', - success: true, - webauthn_configuration_id: webauthn_configuration.id, - multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), - new_device: true, - } - end before do allow(WebauthnVerificationForm).to receive(:domain_name).and_return('localhost:3000') end it 'tracks a valid submission' do - expect(@analytics).to receive(:track_mfa_submit_event). - with(result) - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) - expect(@irs_attempts_api_tracker).to receive(:track_event).with( :mfa_login_webauthn_roaming, success: true, @@ -173,6 +158,20 @@ ) expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq false end + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + context: 'authentication', + multi_factor_auth_method: 'webauthn', + success: true, + webauthn_configuration_id: webauthn_configuration.id, + multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), + new_device: true, + ) + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end context 'with existing device' do @@ -183,10 +182,12 @@ it 'tracks new device value' do stub_analytics - expect(@analytics).to receive(:track_mfa_submit_event). - with(hash_including(new_device: false)) - patch :confirm, params: params + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + hash_including(new_device: false), + ) end end @@ -201,14 +202,8 @@ ) controller.current_user.webauthn_configurations.first end - let(:result) { super().merge(multi_factor_auth_method: 'webauthn_platform') } it 'tracks a valid submission' do - expect(@analytics).to receive(:track_mfa_submit_event). - with(result) - expect(@analytics).to receive(:track_event). - with('User marked authenticated', authentication_type: :valid_2fa) - expect(@irs_attempts_api_tracker).to receive(:track_event).with( :mfa_login_webauthn_platform, success: true, @@ -226,6 +221,21 @@ false, ) end + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + context: 'authentication', + multi_factor_auth_method: 'webauthn_platform', + success: true, + webauthn_configuration_id: webauthn_configuration.id, + multi_factor_auth_method_created_at: webauthn_configuration.created_at. + strftime('%s%L'), + new_device: true, + ) + expect(@analytics).to have_logged_event( + 'User marked authenticated', + authentication_type: :valid_2fa, + ) end end end @@ -238,19 +248,20 @@ credential_public_key: credential_public_key, ) webauthn_configuration = controller.current_user.webauthn_configurations.first - result = { context: 'authentication', - multi_factor_auth_method: 'webauthn', - success: false, - error_details: { authenticator_data: { invalid_authenticator_data: true } }, - webauthn_configuration_id: webauthn_configuration.id, - multi_factor_auth_method_created_at: webauthn_configuration.created_at. - strftime('%s%L'), - new_device: true } - expect(@analytics).to receive(:track_mfa_submit_event). - with(result) expect(controller).to receive(:create_user_event).with(:sign_in_unsuccessful_2fa) patch :confirm, params: params + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', + context: 'authentication', + multi_factor_auth_method: 'webauthn', + success: false, + error_details: { authenticator_data: { invalid_authenticator_data: true } }, + webauthn_configuration_id: webauthn_configuration.id, + multi_factor_auth_method_created_at: webauthn_configuration.created_at.strftime('%s%L'), + new_device: true, + ) end context 'webauthn_platform returns an error from frontend API' do @@ -297,7 +308,10 @@ end it 'logs an event with error details' do - expect(@analytics).to receive(:track_mfa_submit_event).with( + patch :confirm, params: params + + expect(@analytics).to have_logged_event( + 'Multi-Factor Authentication', success: false, error_details: { authenticator_data: { blank: true }, @@ -311,11 +325,8 @@ multi_factor_auth_method_created_at: second_webauthn_platform_configuration.created_at.strftime('%s%L'), new_device: true, - webauthn_configuration_id: nil, frontend_error: webauthn_error, ) - - patch :confirm, params: params end end end diff --git a/spec/support/fake_analytics.rb b/spec/support/fake_analytics.rb index efb6adc9e05..63abdd58ca0 100644 --- a/spec/support/fake_analytics.rb +++ b/spec/support/fake_analytics.rb @@ -168,10 +168,6 @@ def track_event(event, attributes = {}) nil end - def track_mfa_submit_event(_attributes) - # no-op - end - def browser_attributes {} end