From efde5aca2f5a2b846900397a6729462763147ad1 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Thu, 18 Aug 2022 10:55:25 -0400 Subject: [PATCH 1/9] Added attempt events when user is locked out - sms changelog: Internal, Attempts API, Track additional events --- .../two_factor_authenticatable_methods.rb | 14 +++++++++++--- .../otp_verification_controller.rb | 2 +- .../otp_verification_controller_spec.rb | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 9c70b576be4..99666ee02a2 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -17,11 +17,19 @@ def authenticate_user authenticate_user!(force: true) end - def handle_second_factor_locked_user(type) + def handle_second_factor_locked_user(type:, context: nil, phone: nil) analytics.multi_factor_auth_max_attempts event = PushNotification::MfaLimitAccountLockedEvent.new(user: current_user) PushNotification::HttpPush.deliver(event) handle_max_attempts(type + '_login_attempts') + + if !context.nil? || !phone.nil? + if UserSessionContext.authentication_context?(context) + irs_attempts_api_tracker.mfa_verify_phone_otp_rate_limited(phone_number: phone) + elsif UserSessionContext.confirmation_context?(context) + irs_attempts_api_tracker.mfa_enroll_phone_otp_rate_limited(phone_number: phone) + end + end end def handle_too_many_otp_sends @@ -108,13 +116,13 @@ def two_factor_authentication_method # 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} - def handle_invalid_otp(type: 'otp') + def handle_invalid_otp(type: 'otp', context: nil, phone: nil) update_invalid_user flash.now[:error] = invalid_otp_error(type) if decorated_user.locked_out? - handle_second_factor_locked_user(type) + handle_second_factor_locked_user(type: type, context: context, phone: phone) else render_show_after_invalid end diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index d794b94ef13..81bd46435c9 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -22,7 +22,7 @@ def create if result.success? handle_valid_otp else - handle_invalid_otp + handle_invalid_otp(context: context, phone: phone) end 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 a33a207b234..f3f1fb1a0c3 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -179,6 +179,9 @@ expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). with({ reauthentication: false, success: false }) + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_phone_otp_rate_limited). + with({ phone_number: '+1 202-555-1212' }) + post :create, params: { code: '12345', otp_delivery_preference: 'sms' } @@ -461,6 +464,21 @@ expect(@analytics).to have_received(:track_event). with('Multi-Factor Authentication Setup', properties) end + + context 'user has exceeded the maximum number of attempts' do + it 'tracks the attempt event' do + allow_any_instance_of(User).to receive(:max_login_attempts?).and_return(true) + sign_in_before_2fa + + stub_attempts_tracker + expect(@irs_attempts_api_tracker).to receive(:mfa_enroll_phone_otp_rate_limited). + with(phone_number: '+1 202-555-1212') + + post :create, params: + { code: '12345', + otp_delivery_preference: 'sms' } + end + end end context 'user does not include a code parameter' do From d305bc06cc62624b2261ce56416902d7980d4d9a Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Thu, 18 Aug 2022 13:20:21 -0400 Subject: [PATCH 2/9] Updated code to complete lg7201 --- .../two_factor_authenticatable_methods.rb | 14 +++++---- .../otp_verification_controller.rb | 2 +- .../personal_key_verification_controller.rb | 2 +- .../piv_cac_verification_controller.rb | 2 +- .../totp_verification_controller.rb | 2 +- .../max_attempts_reached_presenter.rb | 4 +++ .../irs_attempts_api/tracker_events.rb | 30 +++++++++++++++++++ .../otp_verification_controller_spec.rb | 16 +++++----- ...rsonal_key_verification_controller_spec.rb | 5 ++++ .../piv_cac_verification_controller_spec.rb | 3 ++ .../totp_verification_controller_spec.rb | 4 +++ 11 files changed, 65 insertions(+), 19 deletions(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 99666ee02a2..254fcc71bef 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -17,17 +17,17 @@ def authenticate_user authenticate_user!(force: true) end - def handle_second_factor_locked_user(type:, context: nil, phone: nil) + def handle_second_factor_locked_user(context: nil, type:) analytics.multi_factor_auth_max_attempts event = PushNotification::MfaLimitAccountLockedEvent.new(user: current_user) PushNotification::HttpPush.deliver(event) handle_max_attempts(type + '_login_attempts') - if !context.nil? || !phone.nil? + if context if UserSessionContext.authentication_context?(context) - irs_attempts_api_tracker.mfa_verify_phone_otp_rate_limited(phone_number: phone) + irs_attempts_api_tracker.mfa_verify_rate_limited(type: type) elsif UserSessionContext.confirmation_context?(context) - irs_attempts_api_tracker.mfa_enroll_phone_otp_rate_limited(phone_number: phone) + irs_attempts_api_tracker.mfa_enroll_rate_limited(type: type) end end end @@ -116,13 +116,13 @@ def two_factor_authentication_method # 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} - def handle_invalid_otp(type: 'otp', context: nil, phone: nil) + def handle_invalid_otp(context: nil, type: 'otp') update_invalid_user flash.now[:error] = invalid_otp_error(type) if decorated_user.locked_out? - handle_second_factor_locked_user(type: type, context: context, phone: phone) + handle_second_factor_locked_user(context: context, type: type) else render_show_after_invalid end @@ -132,6 +132,8 @@ def invalid_otp_error(type) case type when 'otp' t('two_factor_authentication.invalid_otp') + when 'totp' + t('two_factor_authentication.invalid_otp') when 'personal_key' t('two_factor_authentication.invalid_personal_key') when 'piv_cac' diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index 81bd46435c9..cd7d16cf329 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -22,7 +22,7 @@ def create if result.success? handle_valid_otp else - handle_invalid_otp(context: context, phone: phone) + handle_invalid_otp(context: context, type: "otp") 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 6ec0fe51e4f..3f8dc40d29d 100644 --- a/app/controllers/two_factor_authentication/personal_key_verification_controller.rb +++ b/app/controllers/two_factor_authentication/personal_key_verification_controller.rb @@ -40,7 +40,7 @@ def handle_result(result) generate_new_personal_key_for_verified_users_otherwise_retire_the_key_and_ensure_two_mfa handle_valid_otp else - handle_invalid_otp(type: 'personal_key') + handle_invalid_otp(context: context, type: 'personal_key') 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 5177c656e35..e36aae6a8b7 100644 --- a/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb +++ b/app/controllers/two_factor_authentication/piv_cac_verification_controller.rb @@ -57,7 +57,7 @@ def handle_valid_piv_cac def handle_invalid_piv_cac clear_piv_cac_information - handle_invalid_otp(type: 'piv_cac') + handle_invalid_otp(context: context, type: 'piv_cac') end # This overrides the method in TwoFactorAuthenticatable so that we diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 4b2dae910d3..63c02b5c948 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -24,7 +24,7 @@ def create if result.success? handle_valid_otp else - handle_invalid_otp + handle_invalid_otp(context: context, type: "totp") end end diff --git a/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb b/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb index 718de9e35b3..e6e06fef4fe 100644 --- a/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb +++ b/app/presenters/two_factor_auth_code/max_attempts_reached_presenter.rb @@ -19,6 +19,10 @@ def locked_reason t('two_factor_authentication.max_otp_login_attempts_reached') when 'otp_requests' t('two_factor_authentication.max_otp_requests_reached') + when 'totp_login_attempts' + t('two_factor_authentication.max_otp_login_attempts_reached') + when 'totp_requests' + t('two_factor_authentication.max_otp_requests_reached') when 'personal_key_login_attempts' t('two_factor_authentication.max_personal_key_login_attempts_reached') when 'piv_cac_login_attempts' diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index 9a3726fdb30..8ba43267842 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -96,6 +96,16 @@ def mfa_enroll_piv_cac( failure_reason: failure_reason, ) end + + # @param [String] phone_number the user's phone_number used for multi-factor authentication + # The user has exceeded the rate limit for SMS OTP during enrollment + # and account has been locked + def mfa_enroll_rate_limited(type:) + track_event( + :mfa_enroll_rate_limited, + type: type, + ) + end # Tracks when the user has attempted to enroll the TOTP MFA method to their account # @param [Boolean] success @@ -159,6 +169,26 @@ def mfa_login_phone_otp_submitted(reauthentication:, success:) # Tracks when the user has attempted to log in with the piv cac MFA method to their account # @param [String] subject_dn + # @param [String] phone_number - the user's phone_number used for multi-factor authentication + # The user has exceeded the rate limit for SMS OTP during verification + # and account has been locked + def mfa_verify_rate_limited(type) + track_event( + :mfa_verify_rate_limited, + type: type, + ) + end + + # Tracks when the user has attempted to verify via the TOTP MFA method to access their account + # @param [Boolean] success + def mfa_verify_totp(success:) + track_event( + :mfa_verify_totp, + success: success, + ) + end + + # Tracks when user has attempted to verify via the WebAuthn-Platform MFA method to their account # @param [Boolean] success # @param [Hash>] failure_reason def mfa_login_piv_cac( 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 f3f1fb1a0c3..58d124cc23e 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -179,8 +179,8 @@ expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). with({ reauthentication: false, success: false }) - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_phone_otp_rate_limited). - with({ phone_number: '+1 202-555-1212' }) + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + with(type: 'otp') post :create, params: { code: '12345', @@ -383,7 +383,7 @@ controller.user_session[:phone_id] = phone_id expect(@irs_attempts_api_tracker).to receive(:mfa_enroll_phone_otp_submitted). - with({ success: true }) + with(success: true) post( :create, @@ -413,7 +413,7 @@ context 'user enters an invalid code' do before do expect(@irs_attempts_api_tracker).to receive(:mfa_enroll_phone_otp_submitted). - with({ success: false }) + with(success: false) post( :create, @@ -471,12 +471,10 @@ sign_in_before_2fa stub_attempts_tracker - expect(@irs_attempts_api_tracker).to receive(:mfa_enroll_phone_otp_rate_limited). - with(phone_number: '+1 202-555-1212') + expect(@irs_attempts_api_tracker).to receive(:mfa_enroll_rate_limited). + with(type: 'otp') - post :create, params: - { code: '12345', - otp_delivery_preference: 'sms' } + post :create, params: { code: '12345', otp_delivery_preference: 'sms' } end end end 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 f1fa846d7a4..075951c1da3 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 @@ -152,11 +152,16 @@ } 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(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + with(type: 'personal_key') + expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) 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 d73e05c37a0..2db60aace88 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 @@ -192,6 +192,9 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: enter PIV CAC visited', attributes) + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited) + .with(type: 'piv_cac') + submit_attributes = { success: false, errors: { type: 'user.piv_cac_mismatch' }, 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 78d6a68b3e9..37871ef70ce 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -97,6 +97,10 @@ with('Multi-Factor Authentication: max attempts reached') expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: false) + + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited) + .with(type: 'totp') + expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) From 2a0211d4dca0f505442575f93f51182af3f9ab35 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Thu, 18 Aug 2022 13:21:46 -0400 Subject: [PATCH 3/9] linter fixes --- .../concerns/two_factor_authenticatable_methods.rb | 2 +- .../two_factor_authentication/otp_verification_controller.rb | 2 +- .../two_factor_authentication/totp_verification_controller.rb | 2 +- .../otp_verification_controller_spec.rb | 2 +- .../piv_cac_verification_controller_spec.rb | 4 ++-- .../totp_verification_controller_spec.rb | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 254fcc71bef..a6221c95e35 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -17,7 +17,7 @@ def authenticate_user authenticate_user!(force: true) end - def handle_second_factor_locked_user(context: nil, type:) + def handle_second_factor_locked_user(type:, context: nil) analytics.multi_factor_auth_max_attempts event = PushNotification::MfaLimitAccountLockedEvent.new(user: current_user) PushNotification::HttpPush.deliver(event) diff --git a/app/controllers/two_factor_authentication/otp_verification_controller.rb b/app/controllers/two_factor_authentication/otp_verification_controller.rb index cd7d16cf329..3b662280d53 100644 --- a/app/controllers/two_factor_authentication/otp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/otp_verification_controller.rb @@ -22,7 +22,7 @@ def create if result.success? handle_valid_otp else - handle_invalid_otp(context: context, type: "otp") + handle_invalid_otp(context: context, type: 'otp') end end diff --git a/app/controllers/two_factor_authentication/totp_verification_controller.rb b/app/controllers/two_factor_authentication/totp_verification_controller.rb index 63c02b5c948..7a50fd5e33c 100644 --- a/app/controllers/two_factor_authentication/totp_verification_controller.rb +++ b/app/controllers/two_factor_authentication/totp_verification_controller.rb @@ -24,7 +24,7 @@ def create if result.success? handle_valid_otp else - handle_invalid_otp(context: context, type: "totp") + handle_invalid_otp(context: context, type: 'totp') end 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 58d124cc23e..6f0aeb1cb5d 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -238,7 +238,7 @@ 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 }) + with(reauthentication: false, success: true) post :create, params: { code: subject.current_user.reload.direct_otp, 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 2db60aace88..ed08b2fe137 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 @@ -192,8 +192,8 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: enter PIV CAC visited', attributes) - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited) - .with(type: 'piv_cac') + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + with(type: 'piv_cac') submit_attributes = { success: false, 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 37871ef70ce..61cfdc64908 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -98,8 +98,8 @@ expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: false) - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited) - .with(type: 'totp') + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + with(type: 'totp') expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) From 573ce035988a10c27968b6fd455a0156fbba36f8 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Thu, 18 Aug 2022 13:26:27 -0400 Subject: [PATCH 4/9] updated event descriptions --- app/services/irs_attempts_api/tracker_events.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index 8ba43267842..b7090192d66 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -97,8 +97,8 @@ def mfa_enroll_piv_cac( ) end - # @param [String] phone_number the user's phone_number used for multi-factor authentication - # The user has exceeded the rate limit for SMS OTP during enrollment + # @param [String] the type of multi-factor authentication used + # The user has exceeded the rate limit during enrollment # and account has been locked def mfa_enroll_rate_limited(type:) track_event( @@ -167,10 +167,8 @@ def mfa_login_phone_otp_submitted(reauthentication:, success:) ) end - # Tracks when the user has attempted to log in with the piv cac MFA method to their account - # @param [String] subject_dn - # @param [String] phone_number - the user's phone_number used for multi-factor authentication - # The user has exceeded the rate limit for SMS OTP during verification + # @param [String] the type of multi-factor authentication used + # The user has exceeded the rate limit during verification # and account has been locked def mfa_verify_rate_limited(type) track_event( From b359731ffe0cfba968a619ad2c47e7377e5c4a1f Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Thu, 18 Aug 2022 14:11:14 -0400 Subject: [PATCH 5/9] updated backup_code to support new events --- .../backup_code_verification_controller.rb | 2 +- app/services/irs_attempts_api/tracker_events.rb | 6 +++--- .../backup_code_verification_controller_spec.rb | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) 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 883b4ef474d..9ec76045cea 100644 --- a/app/controllers/two_factor_authentication/backup_code_verification_controller.rb +++ b/app/controllers/two_factor_authentication/backup_code_verification_controller.rb @@ -53,7 +53,7 @@ def handle_invalid_backup_code flash.now[:error] = t('two_factor_authentication.invalid_backup_code') if decorated_user.locked_out? - handle_second_factor_locked_user('backup_code') + handle_second_factor_locked_user(context: context, type: 'backup_code') else render_show_after_invalid end diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index b7090192d66..6e6920e76ec 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -97,7 +97,7 @@ def mfa_enroll_piv_cac( ) end - # @param [String] the type of multi-factor authentication used + # @param [String] type - the type of multi-factor authentication used # The user has exceeded the rate limit during enrollment # and account has been locked def mfa_enroll_rate_limited(type:) @@ -167,10 +167,10 @@ def mfa_login_phone_otp_submitted(reauthentication:, success:) ) end - # @param [String] the type of multi-factor authentication used + # @param [String] type - the type of multi-factor authentication used # The user has exceeded the rate limit during verification # and account has been locked - def mfa_verify_rate_limited(type) + def mfa_verify_rate_limited(type:) track_event( :mfa_verify_rate_limited, type: type, 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 bf0c728934d..fbda233bb81 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 @@ -141,6 +141,10 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: max attempts reached') + + expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + with(type: 'backup_code') + expect(PushNotification::HttpPush).to receive(:deliver). with(PushNotification::MfaLimitAccountLockedEvent.new(user: subject.current_user)) From 32739076f1d7cda9725ff1f9eb41bcae4c5485b3 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Thu, 18 Aug 2022 14:12:28 -0400 Subject: [PATCH 6/9] removed default parameter baed on feedback --- app/controllers/concerns/two_factor_authenticatable_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index a6221c95e35..77cde7176c5 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -116,7 +116,7 @@ def two_factor_authentication_method # 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} - def handle_invalid_otp(context: nil, type: 'otp') + def handle_invalid_otp(context: nil, type:) update_invalid_user flash.now[:error] = invalid_otp_error(type) From 7f695b6f1bee2eaf134295ba77abe4d0fdcea6c2 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Fri, 19 Aug 2022 12:10:19 -0400 Subject: [PATCH 7/9] lint fix --- app/controllers/concerns/two_factor_authenticatable_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 77cde7176c5..264260b0995 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -116,7 +116,7 @@ def two_factor_authentication_method # 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} - def handle_invalid_otp(context: nil, type:) + def handle_invalid_otp(type:, context: nil) update_invalid_user flash.now[:error] = invalid_otp_error(type) From ad561de71e77872e8f3a0b6e79b86ffdeadb8207 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava Date: Fri, 19 Aug 2022 12:30:42 -0400 Subject: [PATCH 8/9] Resolved Merge conflicts --- .../two_factor_authenticatable_methods.rb | 2 +- .../irs_attempts_api/tracker_events.rb | 31 +++++++------------ ...ackup_code_verification_controller_spec.rb | 2 +- .../otp_verification_controller_spec.rb | 2 +- ...rsonal_key_verification_controller_spec.rb | 2 +- .../piv_cac_verification_controller_spec.rb | 2 +- .../totp_verification_controller_spec.rb | 2 +- 7 files changed, 17 insertions(+), 26 deletions(-) diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 264260b0995..e56a650379a 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -25,7 +25,7 @@ def handle_second_factor_locked_user(type:, context: nil) if context if UserSessionContext.authentication_context?(context) - irs_attempts_api_tracker.mfa_verify_rate_limited(type: type) + irs_attempts_api_tracker.mfa_login_rate_limited(type: type) elsif UserSessionContext.confirmation_context?(context) irs_attempts_api_tracker.mfa_enroll_rate_limited(type: type) end diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index 6e6920e76ec..f256efe38fb 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -96,7 +96,7 @@ def mfa_enroll_piv_cac( failure_reason: failure_reason, ) end - + # @param [String] type - the type of multi-factor authentication used # The user has exceeded the rate limit during enrollment # and account has been locked @@ -167,25 +167,6 @@ def mfa_login_phone_otp_submitted(reauthentication:, success:) ) end - # @param [String] type - the type of multi-factor authentication used - # The user has exceeded the rate limit during verification - # and account has been locked - def mfa_verify_rate_limited(type:) - track_event( - :mfa_verify_rate_limited, - type: type, - ) - end - - # Tracks when the user has attempted to verify via the TOTP MFA method to access their account - # @param [Boolean] success - def mfa_verify_totp(success:) - track_event( - :mfa_verify_totp, - success: success, - ) - end - # Tracks when user has attempted to verify via the WebAuthn-Platform MFA method to their account # @param [Boolean] success # @param [Hash>] failure_reason @@ -202,6 +183,16 @@ def mfa_login_piv_cac( ) end + # @param [String] type - the type of multi-factor authentication used + # The user has exceeded the rate limit during verification + # and account has been locked + def mfa_login_rate_limited(type:) + track_event( + :mfa_login_rate_limited, + type: type, + ) + end + # Tracks when the user has attempted to log in with the TOTP MFA method to access their account # @param [Boolean] success def mfa_login_totp(success:) 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 fbda233bb81..673f1867c2c 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 @@ -142,7 +142,7 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: max attempts reached') - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(type: 'backup_code') expect(PushNotification::HttpPush).to receive(:deliver). 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 6f0aeb1cb5d..4906718d01d 100644 --- a/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/otp_verification_controller_spec.rb @@ -179,7 +179,7 @@ expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_submitted). with({ reauthentication: false, success: false }) - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(type: 'otp') post :create, params: 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 075951c1da3..8e81fa14877 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 @@ -159,7 +159,7 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: max attempts reached') - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(type: 'personal_key') expect(PushNotification::HttpPush).to receive(:deliver). 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 ed08b2fe137..a0cb3ccbf6a 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 @@ -192,7 +192,7 @@ expect(@analytics).to receive(:track_event). with('Multi-Factor Authentication: enter PIV CAC visited', attributes) - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(type: 'piv_cac') submit_attributes = { 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 61cfdc64908..4765db3cde2 100644 --- a/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/totp_verification_controller_spec.rb @@ -98,7 +98,7 @@ expect(@irs_attempts_api_tracker).to receive(:track_event). with(:mfa_login_totp, success: false) - expect(@irs_attempts_api_tracker).to receive(:mfa_verify_rate_limited). + expect(@irs_attempts_api_tracker).to receive(:mfa_login_rate_limited). with(type: 'totp') expect(PushNotification::HttpPush).to receive(:deliver). From 19096b295b49020a66a5565c12ca7f904f6a3e90 Mon Sep 17 00:00:00 2001 From: Rwolfe-Nava <87499456+Rwolfe-Nava@users.noreply.github.com> Date: Fri, 19 Aug 2022 12:49:30 -0400 Subject: [PATCH 9/9] Fixed login_piv_cac description regression --- app/services/irs_attempts_api/tracker_events.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index f256efe38fb..3cdf52c8883 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -167,8 +167,9 @@ def mfa_login_phone_otp_submitted(reauthentication:, success:) ) end - # Tracks when user has attempted to verify via the WebAuthn-Platform MFA method to their account + # Tracks when the user has attempted to log in with the piv cac MFA method to their account # @param [Boolean] success + # @param [String] subject_dn # @param [Hash>] failure_reason def mfa_login_piv_cac( success:,