From 8afdbe591b96e779d142b84a150172b93ac08103 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Thu, 29 Dec 2022 11:37:09 -0600 Subject: [PATCH 1/8] Replace database phone one-time password rate limiter with Redis changelog: Internal, Rate Limiting, Replace database phone one-time password rate limiter with Redis --- app/services/otp_rate_limiter.rb | 28 ++++++++++++++++++++++---- app/services/throttle.rb | 4 ++++ config/application.yml.default | 2 ++ lib/identity_config.rb | 1 + spec/services/otp_rate_limiter_spec.rb | 18 +++++++++++------ 5 files changed, 43 insertions(+), 10 deletions(-) diff --git a/app/services/otp_rate_limiter.rb b/app/services/otp_rate_limiter.rb index bb62c837b55..f95c65d1f88 100644 --- a/app/services/otp_rate_limiter.rb +++ b/app/services/otp_rate_limiter.rb @@ -6,6 +6,7 @@ def initialize(phone:, user:, phone_confirmed:) end def exceeded_otp_send_limit? + return throttle.throttled? if IdentityConfig.store.redis_throttle_otp_rate_limiter_read_enabled return false if entry_for_current_phone.blank? if rate_limit_period_expired? @@ -26,6 +27,8 @@ def rate_limit_period_expired? def reset_count_and_otp_last_sent_at entry_for_current_phone.update(otp_last_sent_at: Time.zone.now, otp_send_count: 0) + + throttle.reset! end def lock_out_user @@ -35,6 +38,15 @@ def lock_out_user def increment # DO NOT MEMOIZE @entry = OtpRequestsTracker.atomic_increment(entry_for_current_phone.id) + throttle.increment! + end + + def otp_last_sent_at + if IdentityConfig.store.redis_throttle_otp_rate_limiter_read_enabled + throttle.attempted_at + else + entry_for_current_phone.otp_last_sent_at + end end private @@ -47,10 +59,6 @@ def entry_for_current_phone end # rubocop:enable Naming/MemoizedInstanceVariableName - def otp_last_sent_at - entry_for_current_phone.otp_last_sent_at - end - def otp_findtime IdentityConfig.store.otp_delivery_blocklist_findtime.minutes end @@ -58,4 +66,16 @@ def otp_findtime def otp_maxretry_times IdentityConfig.store.otp_delivery_blocklist_maxretry end + + def phone_fingerprint + @phone_fingerprint ||= Pii::Fingerprinter.fingerprint(phone) + end + + def throttle + @throttle ||= Throttle.new(throttle_type: :phone_otp, target: throttle_key) + end + + def throttle_key + "#{phone_fingerprint}:#{phone_confirmed}" + end end diff --git a/app/services/throttle.rb b/app/services/throttle.rb index e4f868061b6..2dd3d221689 100644 --- a/app/services/throttle.rb +++ b/app/services/throttle.rb @@ -53,6 +53,10 @@ class Throttle max_attempts: IdentityConfig.store.inherited_proofing_max_attempts, attempt_window: IdentityConfig.store.inherited_proofing_max_attempt_window_in_minutes, }, + phone_otp: { + max_attempts: IdentityConfig.store.otp_delivery_blocklist_maxretry + 1, + attempt_window: IdentityConfig.store.otp_delivery_blocklist_findtime, + }, }.with_indifferent_access.freeze def initialize(throttle_type:, user: nil, target: nil) diff --git a/config/application.yml.default b/config/application.yml.default index e23b855a73e..f29ee2fa980 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -241,6 +241,7 @@ rails_mailer_previews_enabled: false reauthn_window: 120 recovery_code_length: 4 redis_irs_attempt_api_url: redis://localhost:6379/2 +redis_throttle_otp_rate_limiter_read_enabled: false redis_throttle_url: redis://localhost:6379/1 redis_url: redis://localhost:6379/0 redis_pool_size: 10 @@ -375,6 +376,7 @@ development: rails_mailer_previews_enabled: true rack_timeout_service_timeout_seconds: 9_999_999_999 recurring_jobs_disabled_names: "[]" + redis_throttle_otp_rate_limiter_read_enabled: true s3_report_bucket_prefix: '' s3_report_public_bucket_prefix: '' saml_endpoint_configs: '[{"suffix":"2021","secret_key_passphrase":"trust-but-verify"},{"suffix":"2022","secret_key_passphrase":"trust-but-verify"}]' diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 6d1f4874454..e0f59434b30 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -340,6 +340,7 @@ def self.build_store(config_map) config.add(:recurring_jobs_disabled_names, type: :json) config.add(:redis_irs_attempt_api_url) config.add(:redis_throttle_url) + config.add(:redis_throttle_otp_rate_limiter_read_enabled, type: :boolean) config.add(:redis_url) config.add(:redis_pool_size, type: :integer) config.add(:redis_session_pool_size, type: :integer) diff --git a/spec/services/otp_rate_limiter_spec.rb b/spec/services/otp_rate_limiter_spec.rb index dc1eae240ae..d8f390c6750 100644 --- a/spec/services/otp_rate_limiter_spec.rb +++ b/spec/services/otp_rate_limiter_spec.rb @@ -29,6 +29,16 @@ expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(true) end + it 'is is false after maxretry_times attemps in findtime minutes' do + expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(false) + + IdentityConfig.store.otp_delivery_blocklist_maxretry.times do + otp_rate_limiter.increment + end + + expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(false) + end + it 'tracks verified phones separately. limiting one does not limit the other' do expect(otp_rate_limiter.exceeded_otp_send_limit?).to eq(false) @@ -43,14 +53,10 @@ describe '#increment' do it 'updates otp_last_sent_at' do - tracker = OtpRequestsTracker.find_or_create_with_phone_and_confirmed( - phone, - false, - ) otp_rate_limiter.increment - old_otp_last_sent_at = tracker.reload.otp_last_sent_at + old_otp_last_sent_at = otp_rate_limiter.otp_last_sent_at otp_rate_limiter.increment - new_otp_last_sent_at = tracker.reload.otp_last_sent_at + new_otp_last_sent_at = otp_rate_limiter.otp_last_sent_at expect(new_otp_last_sent_at).to be > old_otp_last_sent_at end From 17eb5a8c0cb52ddacac5e58beaf39dbd5752cad7 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Fri, 30 Dec 2022 10:44:53 -0600 Subject: [PATCH 2/8] Update app/services/otp_rate_limiter.rb Co-authored-by: Zach Margolis --- app/services/otp_rate_limiter.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/otp_rate_limiter.rb b/app/services/otp_rate_limiter.rb index f95c65d1f88..6da3a42afb3 100644 --- a/app/services/otp_rate_limiter.rb +++ b/app/services/otp_rate_limiter.rb @@ -39,6 +39,7 @@ def increment # DO NOT MEMOIZE @entry = OtpRequestsTracker.atomic_increment(entry_for_current_phone.id) throttle.increment! + nil end def otp_last_sent_at From f653c4bdca88a7aab90f35de3811d5ce16254e3c Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Fri, 30 Dec 2022 12:41:52 -0600 Subject: [PATCH 3/8] format phone number --- app/services/otp_rate_limiter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/otp_rate_limiter.rb b/app/services/otp_rate_limiter.rb index 6da3a42afb3..b2bded401a4 100644 --- a/app/services/otp_rate_limiter.rb +++ b/app/services/otp_rate_limiter.rb @@ -69,7 +69,7 @@ def otp_maxretry_times end def phone_fingerprint - @phone_fingerprint ||= Pii::Fingerprinter.fingerprint(phone) + @phone_fingerprint ||= Pii::Fingerprinter.fingerprint(PhoneFormatter.format(phone)) end def throttle From 8567645e454686e707af8974e7a57f26c6f66617 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Fri, 30 Dec 2022 12:43:52 -0600 Subject: [PATCH 4/8] use redis throttle in test --- config/application.yml.default | 1 + 1 file changed, 1 insertion(+) diff --git a/config/application.yml.default b/config/application.yml.default index f29ee2fa980..045c581db35 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -537,6 +537,7 @@ test: piv_cac_verify_token_secret: 3ac13bfa23e22adae321194c083e783faf89469f6f85dcc0802b27475c94b5c3891b5657bd87d0c1ad65de459166440512f2311018db90d57b15d8ab6660748f poll_rate_for_verify_in_seconds: 0 recurring_jobs_disabled_names: '["disabled job"]' + redis_throttle_otp_rate_limiter_read_enabled: true reg_confirmed_email_max_attempts: 3 reg_unconfirmed_email_max_attempts: 4 reg_unconfirmed_email_window_in_minutes: 70 From efdbe1ab2f8a7d593a9694f78493777ffb860de0 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 10 Jan 2023 12:41:36 -0600 Subject: [PATCH 5/8] allow dynamic throttle config when not in production --- app/services/throttle.rb | 128 +++++++++++++++++++++------------------ 1 file changed, 70 insertions(+), 58 deletions(-) diff --git a/app/services/throttle.rb b/app/services/throttle.rb index 2dd3d221689..151a5a77d10 100644 --- a/app/services/throttle.rb +++ b/app/services/throttle.rb @@ -4,67 +4,12 @@ class Throttle attr_reader :throttle_type - THROTTLE_CONFIG = { - idv_doc_auth: { - max_attempts: IdentityConfig.store.doc_auth_max_attempts, - attempt_window: IdentityConfig.store.doc_auth_attempt_window_in_minutes, - }, - reg_unconfirmed_email: { - max_attempts: IdentityConfig.store.reg_unconfirmed_email_max_attempts, - attempt_window: IdentityConfig.store.reg_unconfirmed_email_window_in_minutes, - }, - reg_confirmed_email: { - max_attempts: IdentityConfig.store.reg_confirmed_email_max_attempts, - attempt_window: IdentityConfig.store.reg_confirmed_email_window_in_minutes, - }, - reset_password_email: { - max_attempts: IdentityConfig.store.reset_password_email_max_attempts, - attempt_window: IdentityConfig.store.reset_password_email_window_in_minutes, - }, - idv_resolution: { - max_attempts: IdentityConfig.store.idv_max_attempts, - attempt_window: IdentityConfig.store.idv_attempt_window_in_hours * 60, - }, - idv_send_link: { - max_attempts: IdentityConfig.store.idv_send_link_max_attempts, - attempt_window: IdentityConfig.store.idv_send_link_attempt_window_in_minutes, - }, - verify_personal_key: { - max_attempts: IdentityConfig.store.verify_personal_key_max_attempts, - attempt_window: IdentityConfig.store.verify_personal_key_attempt_window_in_minutes, - }, - verify_gpo_key: { - max_attempts: IdentityConfig.store.verify_gpo_key_max_attempts, - attempt_window: IdentityConfig.store.verify_gpo_key_attempt_window_in_minutes, - }, - proof_ssn: { - max_attempts: IdentityConfig.store.proof_ssn_max_attempts, - attempt_window: IdentityConfig.store.proof_ssn_max_attempt_window_in_minutes, - }, - proof_address: { - 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, - }, - inherited_proofing: { - max_attempts: IdentityConfig.store.inherited_proofing_max_attempts, - attempt_window: IdentityConfig.store.inherited_proofing_max_attempt_window_in_minutes, - }, - phone_otp: { - max_attempts: IdentityConfig.store.otp_delivery_blocklist_maxretry + 1, - attempt_window: IdentityConfig.store.otp_delivery_blocklist_findtime, - }, - }.with_indifferent_access.freeze - def initialize(throttle_type:, user: nil, target: nil) @throttle_type = throttle_type @user = user @target = target - unless THROTTLE_CONFIG.key?(throttle_type) + unless Throttle.throttle_config.key?(throttle_type) raise ArgumentError, 'throttle_type is not valid' end @@ -213,10 +158,77 @@ def key end def self.attempt_window_in_minutes(throttle_type) - THROTTLE_CONFIG.dig(throttle_type, :attempt_window) + throttle_config.dig(throttle_type, :attempt_window) end def self.max_attempts(throttle_type) - THROTTLE_CONFIG.dig(throttle_type, :max_attempts) + throttle_config.dig(throttle_type, :max_attempts) + end + + def self.throttle_config + if Rails.env.production? + CACHED_THROTTLE_CONFIG + else + load_throttle_config + end end + + def self.load_throttle_config + { + idv_doc_auth: { + max_attempts: IdentityConfig.store.doc_auth_max_attempts, + attempt_window: IdentityConfig.store.doc_auth_attempt_window_in_minutes, + }, + reg_unconfirmed_email: { + max_attempts: IdentityConfig.store.reg_unconfirmed_email_max_attempts, + attempt_window: IdentityConfig.store.reg_unconfirmed_email_window_in_minutes, + }, + reg_confirmed_email: { + max_attempts: IdentityConfig.store.reg_confirmed_email_max_attempts, + attempt_window: IdentityConfig.store.reg_confirmed_email_window_in_minutes, + }, + reset_password_email: { + max_attempts: IdentityConfig.store.reset_password_email_max_attempts, + attempt_window: IdentityConfig.store.reset_password_email_window_in_minutes, + }, + idv_resolution: { + max_attempts: IdentityConfig.store.idv_max_attempts, + attempt_window: IdentityConfig.store.idv_attempt_window_in_hours * 60, + }, + idv_send_link: { + max_attempts: IdentityConfig.store.idv_send_link_max_attempts, + attempt_window: IdentityConfig.store.idv_send_link_attempt_window_in_minutes, + }, + verify_personal_key: { + max_attempts: IdentityConfig.store.verify_personal_key_max_attempts, + attempt_window: IdentityConfig.store.verify_personal_key_attempt_window_in_minutes, + }, + verify_gpo_key: { + max_attempts: IdentityConfig.store.verify_gpo_key_max_attempts, + attempt_window: IdentityConfig.store.verify_gpo_key_attempt_window_in_minutes, + }, + proof_ssn: { + max_attempts: IdentityConfig.store.proof_ssn_max_attempts, + attempt_window: IdentityConfig.store.proof_ssn_max_attempt_window_in_minutes, + }, + proof_address: { + 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, + }, + inherited_proofing: { + max_attempts: IdentityConfig.store.inherited_proofing_max_attempts, + attempt_window: IdentityConfig.store.inherited_proofing_max_attempt_window_in_minutes, + }, + phone_otp: { + max_attempts: IdentityConfig.store.otp_delivery_blocklist_maxretry + 1, + attempt_window: IdentityConfig.store.otp_delivery_blocklist_findtime, + }, + } + end + + CACHED_THROTTLE_CONFIG = self.load_throttle_config end From c25eca20d6f92dbfb3bb5ef03ad95a669947885c Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Tue, 10 Jan 2023 13:10:18 -0600 Subject: [PATCH 6/8] fix specs --- app/services/throttle.rb | 4 ++-- spec/features/event_disavowal_spec.rb | 2 +- spec/features/idv/threatmetrix_pending_spec.rb | 4 +--- .../idv/steps/in_person/verify_step_spec.rb | 12 +++--------- spec/services/idv/steps/verify_step_spec.rb | 12 +++--------- spec/services/throttle_spec.rb | 18 +++++++----------- 6 files changed, 17 insertions(+), 35 deletions(-) diff --git a/app/services/throttle.rb b/app/services/throttle.rb index 151a5a77d10..41a73b4e29c 100644 --- a/app/services/throttle.rb +++ b/app/services/throttle.rb @@ -227,8 +227,8 @@ def self.load_throttle_config max_attempts: IdentityConfig.store.otp_delivery_blocklist_maxretry + 1, attempt_window: IdentityConfig.store.otp_delivery_blocklist_findtime, }, - } + }.with_indifferent_access end - CACHED_THROTTLE_CONFIG = self.load_throttle_config + CACHED_THROTTLE_CONFIG = self.load_throttle_config.with_indifferent_access.freeze end diff --git a/spec/features/event_disavowal_spec.rb b/spec/features/event_disavowal_spec.rb index bc713c647a9..eecbeadf2f3 100644 --- a/spec/features/event_disavowal_spec.rb +++ b/spec/features/event_disavowal_spec.rb @@ -19,10 +19,10 @@ end scenario 'disavowing a new device sign in' do + allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(3) signin(user.email, user.password) Capybara.reset_session! visit root_path - OtpRequestsTracker.destroy_all # Prevent OTP rate limit from preventing sign in signin(user.email, user.password) disavow_last_action_and_reset_password diff --git a/spec/features/idv/threatmetrix_pending_spec.rb b/spec/features/idv/threatmetrix_pending_spec.rb index d59fa5c1d13..0201fd47ffd 100644 --- a/spec/features/idv/threatmetrix_pending_spec.rb +++ b/spec/features/idv/threatmetrix_pending_spec.rb @@ -9,6 +9,7 @@ end scenario 'users pending threatmetrix see sad face screen and cannot perform idv' do + allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(300) user = create(:user, :signed_up) start_idv_from_sp @@ -26,7 +27,6 @@ # User unable to sign into OIDC with IdV set_new_browser_session - OtpRequestsTracker.destroy_all start_idv_from_sp(:oidc) sign_in_live_with_2fa(user) @@ -35,7 +35,6 @@ # User unable to sign into SAML with IdV set_new_browser_session - OtpRequestsTracker.destroy_all start_idv_from_sp(:saml) sign_in_live_with_2fa(user) @@ -44,7 +43,6 @@ # User able to sign for IAL1 set_new_browser_session - OtpRequestsTracker.destroy_all visit_idp_from_sp_with_ial1(:oidc) sign_in_live_with_2fa(user) click_agree_and_continue diff --git a/spec/services/idv/steps/in_person/verify_step_spec.rb b/spec/services/idv/steps/in_person/verify_step_spec.rb index a6efd623436..994d8302d03 100644 --- a/spec/services/idv/steps/in_person/verify_step_spec.rb +++ b/spec/services/idv/steps/in_person/verify_step_spec.rb @@ -110,15 +110,9 @@ def build_step(controller) end before do - stub_const( - 'Throttle::THROTTLE_CONFIG', - { - proof_ssn: { - max_attempts: 2, - attempt_window: 10, - }, - }.with_indifferent_access, - ) + allow(IdentityConfig.store).to receive(:proof_ssn_max_attempts).and_return(2) + allow(IdentityConfig.store).to receive(:proof_ssn_max_attempt_window_in_minutes). + and_return(10) end def redirect(step) diff --git a/spec/services/idv/steps/verify_step_spec.rb b/spec/services/idv/steps/verify_step_spec.rb index febd9f52289..e21c263ba3a 100644 --- a/spec/services/idv/steps/verify_step_spec.rb +++ b/spec/services/idv/steps/verify_step_spec.rb @@ -110,15 +110,9 @@ def build_step(controller) end before do - stub_const( - 'Throttle::THROTTLE_CONFIG', - { - proof_ssn: { - max_attempts: 2, - attempt_window: 10, - }, - }.with_indifferent_access, - ) + allow(IdentityConfig.store).to receive(:proof_ssn_max_attempts).and_return(2) + allow(IdentityConfig.store).to receive(:proof_ssn_max_attempt_window_in_minutes). + and_return(10) end def redirect(step) diff --git a/spec/services/throttle_spec.rb b/spec/services/throttle_spec.rb index e45965d0f13..ac1f01364bb 100644 --- a/spec/services/throttle_spec.rb +++ b/spec/services/throttle_spec.rb @@ -4,14 +4,10 @@ let(:throttle_type) { :idv_doc_auth } let(:max_attempts) { 3 } let(:attempt_window) { 10 } - - before do - stub_const( - 'Throttle::THROTTLE_CONFIG', - { - throttle_type => { max_attempts: max_attempts, attempt_window: attempt_window }, - }.with_indifferent_access.freeze, - ) + before(:each) do + allow(IdentityConfig.store).to receive(:doc_auth_max_attempts).and_return(max_attempts) + allow(IdentityConfig.store).to receive(:doc_auth_attempt_window_in_minutes). + and_return(attempt_window) end describe '.new' do @@ -83,8 +79,8 @@ describe '#throttled?' do let(:throttle_type) { :idv_doc_auth } - let(:max_attempts) { IdentityConfig.store.doc_auth_max_attempts } - let(:attempt_window_in_minutes) { IdentityConfig.store.doc_auth_attempt_window_in_minutes } + let(:max_attempts) { Throttle.max_attempts(throttle_type) } + let(:attempt_window_in_minutes) { Throttle.attempt_window_in_minutes(throttle_type) } subject(:throttle) { Throttle.new(target: '1', throttle_type: throttle_type) } @@ -110,7 +106,7 @@ throttle.increment! end - travel(attempt_window_in_minutes.minutes) do + travel(attempt_window_in_minutes.minutes + 1) do expect(throttle.throttled?).to eq(false) end end From 60a5965eda2741502d0616bf28f2dd337706f02a Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Wed, 11 Jan 2023 16:09:39 -0600 Subject: [PATCH 7/8] testing --- app/services/otp_rate_limiter.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/services/otp_rate_limiter.rb b/app/services/otp_rate_limiter.rb index b2bded401a4..863519bb3da 100644 --- a/app/services/otp_rate_limiter.rb +++ b/app/services/otp_rate_limiter.rb @@ -6,9 +6,6 @@ def initialize(phone:, user:, phone_confirmed:) end def exceeded_otp_send_limit? - return throttle.throttled? if IdentityConfig.store.redis_throttle_otp_rate_limiter_read_enabled - return false if entry_for_current_phone.blank? - if rate_limit_period_expired? reset_count_and_otp_last_sent_at return false @@ -18,6 +15,8 @@ def exceeded_otp_send_limit? end def max_requests_reached? + return throttle.throttled? if IdentityConfig.store.redis_throttle_otp_rate_limiter_read_enabled + entry_for_current_phone.otp_send_count > otp_maxretry_times end From f6028b37244d4d4985a4edb61c96674b180055c4 Mon Sep 17 00:00:00 2001 From: Mitchell Henke Date: Fri, 13 Jan 2023 18:30:57 -0600 Subject: [PATCH 8/8] fix spec --- app/services/otp_rate_limiter.rb | 1 + spec/features/idv/phone_otp_rate_limiting_spec.rb | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/services/otp_rate_limiter.rb b/app/services/otp_rate_limiter.rb index 863519bb3da..4afde203a54 100644 --- a/app/services/otp_rate_limiter.rb +++ b/app/services/otp_rate_limiter.rb @@ -21,6 +21,7 @@ def max_requests_reached? end def rate_limit_period_expired? + return throttle.expired? if IdentityConfig.store.redis_throttle_otp_rate_limiter_read_enabled otp_last_sent_at.present? && (otp_last_sent_at + otp_findtime) < Time.zone.now end diff --git a/spec/features/idv/phone_otp_rate_limiting_spec.rb b/spec/features/idv/phone_otp_rate_limiting_spec.rb index f2e2928da53..30ccda4c15e 100644 --- a/spec/features/idv/phone_otp_rate_limiting_spec.rb +++ b/spec/features/idv/phone_otp_rate_limiting_spec.rb @@ -6,20 +6,14 @@ let(:user) { user_with_2fa } describe 'otp sends' do - let(:max_attempts) { IdentityConfig.store.otp_delivery_blocklist_maxretry + 1 } - it 'rate limits resends from the otp verification step' do start_idv_from_sp complete_idv_steps_before_phone_otp_verification_step(user) - (max_attempts - 1).times do + (Throttle.max_attempts(:phone_otp) - 1).times do click_on t('links.two_factor_authentication.send_another_code') end - expect_max_otp_request_rate_limiting - end - - def expect_max_otp_request_rate_limiting expect(page).to have_content t('titles.account_locked') expect(page).to have_content t( 'two_factor_authentication.max_otp_requests_reached', @@ -72,6 +66,11 @@ def expect_rate_limit_circumvention_to_be_disallowed(user) def expect_rate_limit_to_expire(user) retry_minutes = IdentityConfig.store.lockout_period_in_minutes + 1 travel_to(retry_minutes.minutes.from_now) do + # This is not good and we can likely drop it once we have upgraded to Redis 7 and switched + # Throttle to use EXPIRETIME rather than TTL + allow_any_instance_of(Throttle).to receive(:attempted_at).and_return( + retry_minutes.minutes.ago, + ) start_idv_from_sp complete_idv_steps_before_phone_otp_verification_step(user)