diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index ec8980a3a9f..7f8f4616200 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -186,6 +186,10 @@ def handle_valid_otp_params(otp_delivery_selection_result, method, default = nil context: context, ) end + + if exceeded_short_term_otp_rate_limit? + return handle_too_many_short_term_otp_sends(method: method, default: default) + end return handle_too_many_confirmation_sends if exceeded_phone_confirmation_limit? @telephony_result = send_user_otp(method) @@ -261,6 +265,19 @@ def phone_confirmation_rate_limiter ) end + def short_term_otp_rate_limiter + @short_term_otp_rate_limiter ||= RateLimiter.new( + user: current_user, + rate_limit_type: :short_term_phone_otp, + ) + end + + def exceeded_short_term_otp_rate_limit? + return false unless IdentityConfig.store.short_term_phone_otp_rate_limiter_enabled + short_term_otp_rate_limiter.increment! + short_term_otp_rate_limiter.limited? + end + def exceeded_phone_confirmation_limit? return false unless UserSessionContext.confirmation_context?(context) phone_confirmation_rate_limiter.increment! @@ -344,7 +361,36 @@ def webauthn_params { platform: current_user.webauthn_configurations.platform_authenticators.present? } end + def handle_too_many_short_term_otp_sends(method:, default:) + analytics.rate_limit_reached( + limiter_type: short_term_otp_rate_limiter.rate_limit_type, + country_code: parsed_phone.country, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + context: context, + otp_delivery_preference: method, + ) + + flash[:error] = t( + 'errors.messages.phone_confirmation_limited', + timeout: distance_of_time_in_words( + Time.zone.now, + [short_term_otp_rate_limiter.expires_at, Time.zone.now].compact.max, + ), + ) + + redirect_to login_two_factor_url( + otp_delivery_preference: method, + otp_make_default_number: default, + ) + end + def handle_too_many_confirmation_sends + analytics.rate_limit_reached( + limiter_type: phone_confirmation_rate_limiter.rate_limit_type, + country_code: parsed_phone.country, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + ) + flash[:error] = t( 'errors.messages.phone_confirmation_limited', timeout: distance_of_time_in_words( diff --git a/app/services/rate_limiter.rb b/app/services/rate_limiter.rb index 67f7f06f7f1..b7d3d3145ae 100644 --- a/app/services/rate_limiter.rb +++ b/app/services/rate_limiter.rb @@ -216,6 +216,11 @@ def self.load_rate_limit_config max_attempts: IdentityConfig.store.otp_delivery_blocklist_maxretry + 1, attempt_window: IdentityConfig.store.otp_delivery_blocklist_findtime, }, + short_term_phone_otp: { + max_attempts: IdentityConfig.store.short_term_phone_otp_max_attempts, + attempt_window: IdentityConfig.store. + short_term_phone_otp_max_attempt_window_in_seconds.seconds.in_minutes.to_f, + }, }.with_indifferent_access end diff --git a/config/application.yml.default b/config/application.yml.default index 3edd02d204c..cd9b1139264 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -229,6 +229,9 @@ participate_in_dap: false password_max_attempts: 3 personal_key_retired: true phone_carrier_registration_blocklist_array: '[]' +short_term_phone_otp_max_attempt_window_in_seconds: 10 +short_term_phone_otp_max_attempts: 2 +short_term_phone_otp_rate_limiter_enabled: true phone_confirmation_max_attempts: 20 phone_confirmation_max_attempt_window_in_minutes: 1_440 phone_service_check: true @@ -483,6 +486,7 @@ production: phone_recaptcha_mock_validator: false piv_cac_verify_token_secret: session_encryptor_alert_enabled: true + short_term_phone_otp_rate_limiter_enabled: false redis_throttle_url: redis://redis.login.gov.internal:6379/1 redis_url: redis://redis.login.gov.internal:6379 report_timeout: 1_000_000 @@ -579,6 +583,7 @@ test: scrypt_cost: 800$8$1$ secret_key_base: test_secret_key_base session_encryption_key: 27bad3c25711099429c1afdfd1890910f3b59f5a4faec1c85e945cb8b02b02f261ba501d99cfbb4fab394e0102de6fecf8ffe260f322f610db3e96b2a775c120 + short_term_phone_otp_rate_limiter_enabled: false skip_encryption_allowed_list: '[]' state_tracking_enabled: true team_ada_email: 'ada@example.com' diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 9e72d906ce0..34b57baa453 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -451,6 +451,9 @@ def self.build_store(config_map) config.add(:session_total_duration_timeout_in_minutes, type: :integer) config.add(:show_unsupported_passkey_platform_authentication_setup, type: :boolean) config.add(:show_user_attribute_deprecation_warnings, type: :boolean) + config.add(:short_term_phone_otp_max_attempts, type: :integer) + config.add(:short_term_phone_otp_max_attempt_window_in_seconds, type: :integer) + config.add(:short_term_phone_otp_rate_limiter_enabled, type: :boolean) config.add(:skip_encryption_allowed_list, type: :json) config.add(:sp_handoff_bounce_max_seconds, type: :integer) config.add(:sp_issuer_user_counts_report_configs, type: :json) diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index 4468862f24a..e9a597c6b5c 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -572,13 +572,15 @@ def index end it 'rate limits confirmation OTPs on sign up' do + parsed_phone = Phonelib.parse(@unconfirmed_phone) + stub_analytics sign_in_before_2fa(@user) subject.user_session[:context] = 'confirmation' allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(999) freeze_time do - (IdentityConfig.store.phone_confirmation_max_attempts + 1).times do - subject.user_session[:unconfirmed_phone] = '+1 (202) 555-1213' + IdentityConfig.store.phone_confirmation_max_attempts.times do + subject.user_session[:unconfirmed_phone] = @unconfirmed_phone get :send_code, params: otp_delivery_form_sms end @@ -594,6 +596,52 @@ def index ) expect(response).to redirect_to authentication_methods_setup_url end + expect(@analytics).to have_logged_event( + 'Rate Limit Reached', + country_code: parsed_phone.country, + limiter_type: :phone_confirmation, + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + ) + end + + it 'rate limits between OTPs' do + parsed_phone = Phonelib.parse(@unconfirmed_phone) + stub_analytics + sign_in_before_2fa(@user) + subject.user_session[:context] = 'confirmation' + allow(IdentityConfig.store).to receive(:short_term_phone_otp_rate_limiter_enabled). + and_return(true) + allow(IdentityConfig.store).to receive(:short_term_phone_otp_max_attempts).and_return(2) + allow(IdentityConfig.store).to receive(:short_term_phone_otp_max_attempt_window_in_seconds). + and_return(5) + + freeze_time do + IdentityConfig.store.short_term_phone_otp_max_attempts.times do + subject.user_session[:unconfirmed_phone] = @unconfirmed_phone + get :send_code, params: otp_delivery_form_sms + end + + timeout = distance_of_time_in_words( + RateLimiter.attempt_window_in_minutes(:short_term_phone_otp).minutes, + ) + + expect(flash[:error]).to eq( + I18n.t( + 'errors.messages.phone_confirmation_limited', + timeout: timeout, + ), + ) + expect(response).to redirect_to login_two_factor_url(otp_delivery_preference: 'sms') + end + + expect(@analytics).to have_logged_event( + 'Rate Limit Reached', + context: 'confirmation', + country_code: parsed_phone.country, + limiter_type: :short_term_phone_otp, + otp_delivery_preference: 'sms', + phone_fingerprint: Pii::Fingerprinter.fingerprint(parsed_phone.e164), + ) end it 'marks the user as locked out after too many attempts on sign up' do