diff --git a/app/controllers/users/two_factor_authentication_controller.rb b/app/controllers/users/two_factor_authentication_controller.rb index 3acc3784cfe..1835679e98f 100644 --- a/app/controllers/users/two_factor_authentication_controller.rb +++ b/app/controllers/users/two_factor_authentication_controller.rb @@ -1,6 +1,7 @@ module Users class TwoFactorAuthenticationController < ApplicationController include TwoFactorAuthenticatable + include ActionView::Helpers::DateHelper before_action :check_remember_device_preference before_action :redirect_to_vendor_outage_if_phone_only, only: [:show] @@ -164,6 +165,7 @@ def handle_valid_otp_params(method, default = nil) return handle_too_many_otp_sends if exceeded_otp_send_limit? otp_rate_limiter.increment return handle_too_many_otp_sends if exceeded_otp_send_limit? + return handle_too_many_confirmation_sends if exceeded_phone_confirmation_limit? @telephony_result = send_user_otp(method) handle_telephony_result(method: method, default: default) @@ -196,6 +198,18 @@ def exceeded_otp_send_limit? return otp_rate_limiter.lock_out_user if otp_rate_limiter.exceeded_otp_send_limit? end + def phone_confirmation_throttle + @phone_confirmation_throttle ||= Throttle.for( + user: current_user, + throttle_type: :phone_confirmation, + ) + end + + def exceeded_phone_confirmation_limit? + return false unless UserSessionContext.confirmation_context?(context) + phone_confirmation_throttle.throttled_else_increment? + end + def send_user_otp(method) if PhoneNumberOptOut.find_with_phone(phone_to_deliver_to) return Telephony::Response.new( @@ -274,5 +288,21 @@ def webauthn_params params[:platform] = current_user.webauthn_configurations.platform_authenticators.present? params end + + def handle_too_many_confirmation_sends + flash[:error] = t( + 'errors.messages.phone_confirmation_throttled', + timeout: distance_of_time_in_words( + Time.zone.now, + [phone_confirmation_throttle.expires_at, Time.zone.now].compact.max, + except: :seconds, + ), + ) + if user_fully_authenticated? + redirect_to account_url + else + redirect_to login_two_factor_options_url + end + end end end diff --git a/app/models/throttle.rb b/app/models/throttle.rb index bdfd4c65a02..de8752ee339 100644 --- a/app/models/throttle.rb +++ b/app/models/throttle.rb @@ -14,6 +14,7 @@ class Throttle < ApplicationRecord verify_gpo_key: 8, proof_ssn: 9, proof_address: 10, + phone_confirmation: 11, } THROTTLE_CONFIG = { @@ -57,6 +58,10 @@ class Throttle < ApplicationRecord max_attempts: IdentityConfig.store.proof_address_max_attempts, attempt_window: IdentityConfig.store.proof_address_max_attempt_window_in_minutes, }, + phone_confirmation: { + max_attempts: IdentityConfig.store.phone_confirmation_max_attempts, + attempt_window: IdentityConfig.store.phone_confirmation_max_attempt_window_in_minutes, + }, }.with_indifferent_access.freeze # Either target or user must be supplied diff --git a/config/application.yml.default b/config/application.yml.default index eeb41301a3b..0e3d4dc1ea6 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -153,6 +153,8 @@ participate_in_dap: false partner_api_bucket_prefix: '' password_max_attempts: 3 personal_key_retired: true +phone_confirmation_max_attempts: 20 +phone_confirmation_max_attempt_window_in_minutes: 1_440 phone_setups_per_ip_limit: 25 phone_setups_per_ip_period: 300 phone_setups_per_ip_track_only_mode: false @@ -428,6 +430,8 @@ test: otps_per_ip_period: 10 otps_per_ip_track_only_mode: false password_pepper: f22d4b2cafac9066fe2f4416f5b7a32c + phone_confirmation_max_attempts: 5 + phone_confirmation_max_attempt_window_in_minutes: 10 phone_setups_per_ip_limit: 3 phone_setups_per_ip_period: 10 phone_setups_per_ip_track_only_mode: false diff --git a/config/locales/errors/en.yml b/config/locales/errors/en.yml index dae6915cd09..049bee7e87a 100644 --- a/config/locales/errors/en.yml +++ b/config/locales/errors/en.yml @@ -71,6 +71,7 @@ en: otp_failed: Your security code failed to send. password_incorrect: Incorrect password personal_key_incorrect: Incorrect personal key + phone_confirmation_throttled: You tried too many times, please try again in %{timeout}. phone_country_constraint_usa: Must be a U.S. phone number phone_duplicate: This account is already using the phone number you entered as an authenticator. Please use a different phone number. diff --git a/config/locales/errors/es.yml b/config/locales/errors/es.yml index 6863f472eb9..0674e94c139 100644 --- a/config/locales/errors/es.yml +++ b/config/locales/errors/es.yml @@ -74,6 +74,7 @@ es: otp_failed: Se produjo un error al enviar el código de seguridad. password_incorrect: La contraseña es incorrecta personal_key_incorrect: La clave personal es incorrecta + phone_confirmation_throttled: Lo intentaste muchas veces, vuelve a intentarlo en %{timeout}. phone_country_constraint_usa: Debe ser un número telefónico de los Estados Unidos phone_duplicate: Esta cuenta ya está utilizando el número de teléfono que ingresó como autenticador. Por favor, use un número de teléfono diff --git a/config/locales/errors/fr.yml b/config/locales/errors/fr.yml index 140d9600638..a173700b69a 100644 --- a/config/locales/errors/fr.yml +++ b/config/locales/errors/fr.yml @@ -81,6 +81,7 @@ fr: otp_failed: Échec de l’envoi de votre code de sécurité. password_incorrect: Mot de passe incorrect personal_key_incorrect: Clé personnelle incorrecte + phone_confirmation_throttled: Vous avez essayé plusieurs fois, essayez à nouveau dans %{timeout}. phone_country_constraint_usa: Dois être un numéro de téléphone américain phone_duplicate: Ce compte utilise déjà le numéro de téléphone que vous avez entré en tant qu’authentificateur. Veuillez utiliser un numéro de diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 7432b6b5b16..e6476e6f6f9 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -235,6 +235,8 @@ def self.build_store(config_map) config.add(:password_max_attempts, type: :integer) config.add(:password_pepper, type: :string) config.add(:personal_key_retired, type: :boolean) + config.add(:phone_confirmation_max_attempts, type: :integer) + config.add(:phone_confirmation_max_attempt_window_in_minutes, type: :integer) config.add(:phone_setups_per_ip_limit, type: :integer) config.add(:phone_setups_per_ip_period, type: :integer) config.add(:pii_lock_timeout_in_minutes, type: :integer) diff --git a/spec/controllers/users/two_factor_authentication_controller_spec.rb b/spec/controllers/users/two_factor_authentication_controller_spec.rb index f997a8b31a9..b32900f5838 100644 --- a/spec/controllers/users/two_factor_authentication_controller_spec.rb +++ b/spec/controllers/users/two_factor_authentication_controller_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe Users::TwoFactorAuthenticationController do + include ActionView::Helpers::DateHelper + describe 'before_actions' do it 'includes the appropriate before_actions' do expect(subject).to have_actions( @@ -444,13 +446,13 @@ def index before do @user = create(:user) @unconfirmed_phone = '+1 (202) 555-1213' + end + it 'sends OTP inline when confirming phone' do sign_in_before_2fa(@user) subject.user_session[:context] = 'confirmation' subject.user_session[:unconfirmed_phone] = @unconfirmed_phone - end - it 'sends OTP inline when confirming phone' do allow(Telephony).to receive(:send_confirmation_otp).and_call_original get :send_code, params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } } @@ -465,7 +467,62 @@ def index ) end + it 'rate limits confirmation OTPs on sign up' do + 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' + get :send_code, + params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } } + end + + timeout = distance_of_time_in_words( + Throttle.attempt_window_in_minutes(:phone_confirmation).minutes, + ) + + expect(flash[:error]).to eq( + I18n.t( + 'errors.messages.phone_confirmation_throttled', + timeout: timeout, + ), + ) + expect(response).to redirect_to login_two_factor_options_url + end + end + + it 'rate limits confirmation OTPs when adding number to existing account' do + stub_sign_in(@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' + get :send_code, + params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } } + end + + timeout = distance_of_time_in_words( + Throttle.attempt_window_in_minutes(:phone_confirmation).minutes, + ) + + expect(flash[:error]).to eq( + I18n.t( + 'errors.messages.phone_confirmation_throttled', + timeout: timeout, + ), + ) + expect(response).to redirect_to account_url + end + end + it 'flashes an sms error when the telephony gem responds with an sms error' do + sign_in_before_2fa(@user) + subject.user_session[:context] = 'confirmation' + subject.user_session[:unconfirmed_phone] = @unconfirmed_phone subject.user_session[:unconfirmed_phone] = '+1 (225) 555-1000' get :send_code, params: { otp_delivery_selection_form: { otp_delivery_preference: 'sms' } } diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 2cd28c44d61..17d038c7b29 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -3,6 +3,7 @@ feature 'Sign Up' do include SamlAuthHelper include DocAuthHelper + include ActionView::Helpers::DateHelper context 'confirmation token error message does not persist on success' do scenario 'with blank token' do @@ -107,6 +108,32 @@ expect(page).to have_content(I18n.t('telephony.error.friendly_message.generic')) end + scenario 'rate limits sign-up phone confirmation attempts' do + allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(999) + + sign_up_and_set_password + + freeze_time do + (IdentityConfig.store.phone_confirmation_max_attempts + 1).times do + visit phone_setup_path + fill_in 'new_phone_form_phone', with: '2025551313' + click_send_security_code + end + + timeout = distance_of_time_in_words( + Throttle.attempt_window_in_minutes(:phone_confirmation).minutes, + ) + + expect(current_path).to eq(login_two_factor_options_path) + expect(page).to have_content( + I18n.t( + 'errors.messages.phone_confirmation_throttled', + timeout: timeout, + ), + ) + end + end + context 'with js', js: true do before do page.driver.browser.execute_cdp(