diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index e56a650379a..25cd415cf70 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -32,9 +32,23 @@ def handle_second_factor_locked_user(type:, context: nil) end end - def handle_too_many_otp_sends + def handle_too_many_otp_sends(phone: nil, context: nil) analytics.multi_factor_auth_max_sends handle_max_attempts('otp_requests') + + if context && phone + if UserSessionContext.authentication_context?(context) + irs_attempts_api_tracker.mfa_login_phone_otp_sent_rate_limited( + phone_number: phone, + success: true, + ) + elsif UserSessionContext.confirmation_context?(context) + irs_attempts_api_tracker.mfa_enroll_phone_otp_sent_rate_limited( + phone_number: phone, + success: true, + ) + end + end end def handle_max_attempts(type) diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 93983579a8d..b380d7767e3 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -172,9 +172,19 @@ def reauthn_param def handle_valid_otp_params(method, default = nil) otp_rate_limiter.reset_count_and_otp_last_sent_at if decorated_user.no_longer_locked_out? - return handle_too_many_otp_sends if exceeded_otp_send_limit? + if exceeded_otp_send_limit? + return handle_too_many_otp_sends( + phone: parsed_phone.e164, + context: context, + ) + end otp_rate_limiter.increment - return handle_too_many_otp_sends if exceeded_otp_send_limit? + if exceeded_otp_send_limit? + return handle_too_many_otp_sends( + phone: parsed_phone.e164, + context: context, + ) + end return handle_too_many_confirmation_sends if exceeded_phone_confirmation_limit? @telephony_result = send_user_otp(method) diff --git a/app/services/irs_attempts_api/tracker_events.rb b/app/services/irs_attempts_api/tracker_events.rb index 950e5c9bfe0..0d85ab490e1 100644 --- a/app/services/irs_attempts_api/tracker_events.rb +++ b/app/services/irs_attempts_api/tracker_events.rb @@ -128,6 +128,17 @@ def mfa_enroll_phone_otp_submitted(success:) ) end + # @param [String] phone_number - The user's phone number used for multi-factor authentication + # @param [Boolean] success - True if the user was locked out + # The user has exceeded the rate limit for SMS OTP sends. + def mfa_enroll_phone_otp_sent_rate_limited(phone_number:, success:) + track_event( + :mfa_enroll_phone_otp_sent_rate_limited, + phone_number: phone_number, + success: success, + ) + end + # Tracks when the user has attempted to enroll the piv cac MFA method to their account # @param [String] subject_dn # @param [Boolean] success @@ -204,6 +215,17 @@ def mfa_login_phone_otp_sent(reauthentication:, phone_number:, success:) ) end + # @param [String] phone_number - The user's phone number used for multi-factor authentication + # @param [Boolean] success - True if the user was locked out + # The user has exceeded the rate limit for SMS OTP sends. + def mfa_login_phone_otp_sent_rate_limited(phone_number:, success:) + track_event( + :mfa_login_phone_otp_sent_rate_limited, + phone_number: phone_number, + success: success, + ) + end + # @param [Boolean] success - True if the sms otp submitted matched what was sent # During a login attempt, the user, having previously been sent an OTP code via SMS # has entered an OTP code. diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index 4cd745abb16..9f00f406697 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -360,6 +360,13 @@ def index it 'marks the user as locked out after too many attempts' do expect(@user.second_factor_locked_at).to be_nil + allow(OtpRateLimiter).to receive(:exceeded_otp_send_limit?). + and_return(true) + + stub_attempts_tracker + expect(@irs_attempts_api_tracker).to receive(:mfa_login_phone_otp_sent_rate_limited). + with(phone_number: '+12025551212', success: true) + freeze_time do (IdentityConfig.store.otp_delivery_blocklist_maxretry + 1).times do get :send_code, params: { @@ -556,6 +563,34 @@ def index end end + it 'marks the user as locked out after too many attempts on sign up' do + sign_in_before_2fa(@user) + subject.user_session[:context] = 'confirmation' + subject.user_session[:unconfirmed_phone] = '+1 (202) 555-1213' + + expect(@user.second_factor_locked_at).to be_nil + + allow(OtpRateLimiter).to receive(:exceeded_otp_send_limit?). + and_return(true) + + stub_attempts_tracker + expect(@irs_attempts_api_tracker).to receive(:mfa_enroll_phone_otp_sent_rate_limited). + with(phone_number: '+12025551213', success: true) + + freeze_time do + (IdentityConfig.store.otp_delivery_blocklist_maxretry + 1).times do + get :send_code, params: { + otp_delivery_selection_form: { + otp_delivery_preference: 'sms', + otp_make_default_number: nil, + }, + } + end + + expect(@user.reload.second_factor_locked_at).to eq Time.zone.now + end + end + it 'rate limits confirmation OTPs when adding number to existing account' do stub_sign_in(@user) subject.user_session[:context] = 'confirmation'