Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion app/controllers/concerns/two_factor_authenticatable_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 12 additions & 2 deletions app/controllers/users/two_factor_authentication_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions app/services/irs_attempts_api/tracker_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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'
Expand Down