From b4b53fe7ce441399943d5ed79790529b02efe9cf Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Thu, 17 Aug 2023 12:13:11 -0400 Subject: [PATCH 1/4] do not enforce idv rate limiter on review controller changelog: Bug Fixes, Identity Verification, Allow a user rate-limited user to re-enter password on review/Secure your account step" --- app/controllers/idv/review_controller.rb | 1 + spec/features/idv/steps/review_step_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index 149698e2c6d..4dafb3f30f7 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -10,6 +10,7 @@ class ReviewController < ApplicationController before_action :confirm_verify_info_step_complete before_action :confirm_address_step_complete before_action :confirm_current_password, only: [:create] + skip_before_action :confirm_not_rate_limited helper_method :step_indicator_step diff --git a/spec/features/idv/steps/review_step_spec.rb b/spec/features/idv/steps/review_step_spec.rb index 7debb6b1361..897fd1b5641 100644 --- a/spec/features/idv/steps/review_step_spec.rb +++ b/spec/features/idv/steps/review_step_spec.rb @@ -68,5 +68,18 @@ expect(gpo_confirmation_entry[:issuer]).to eq(nil) end end + + context 'when rate limited' do + before do + RateLimiter.new(user: user, rate_limit_type: :proof_address).increment_to_limited! + end + + it 'sends a letter without a reference to the sp' do + fill_in 'Password', with: user_password + click_continue + + expect(current_path).to eq idv_come_back_later_path + end + end end end From eacb0b2bce47cd0a09dfb49272ca89a3c8ac1d83 Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Thu, 17 Aug 2023 13:21:42 -0400 Subject: [PATCH 2/4] refactor test --- spec/features/idv/steps/review_step_spec.rb | 40 +++++++++++++-------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/spec/features/idv/steps/review_step_spec.rb b/spec/features/idv/steps/review_step_spec.rb index 897fd1b5641..dfdb089a34a 100644 --- a/spec/features/idv/steps/review_step_spec.rb +++ b/spec/features/idv/steps/review_step_spec.rb @@ -18,7 +18,7 @@ email_count_before_continue = ActionMailer::Base.deliveries.count expect { click_continue }. - to change { GpoConfirmation.count }.from(0).to(1) + to change { GpoConfirmation.count }.by(1) expect_delivered_email_count(email_count_before_continue + 1) expect(last_email.subject).to eq(t('user_mailer.letter_reminder.subject')) @@ -31,6 +31,31 @@ expect(profile.active?).to eq false end + context 'user is rate-limited' do + before do + RateLimiter.new(user: user, rate_limit_type: :proof_address).increment_to_limited! + end + + it 'sends a letter, creates an unverified profile, and sends an email' do + fill_in 'Password', with: user_password + + email_count_before_continue = ActionMailer::Base.deliveries.count + + expect { click_continue }. + to change { GpoConfirmation.count }.by(1) + + expect_delivered_email_count(email_count_before_continue + 1) + expect(last_email.subject).to eq(t('user_mailer.letter_reminder.subject')) + + expect(user.events.account_verified.size).to be(0) + expect(user.profiles.count).to eq 1 + + profile = user.profiles.first + + expect(profile.active?).to eq false + end + end + it 'sends you to the come_back_later page after review step' do fill_in 'Password', with: user_password click_continue @@ -68,18 +93,5 @@ expect(gpo_confirmation_entry[:issuer]).to eq(nil) end end - - context 'when rate limited' do - before do - RateLimiter.new(user: user, rate_limit_type: :proof_address).increment_to_limited! - end - - it 'sends a letter without a reference to the sp' do - fill_in 'Password', with: user_password - click_continue - - expect(current_path).to eq idv_come_back_later_path - end - end end end From ea02a49a59ae3348a9fb969e24e054067e8f1adf Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Thu, 17 Aug 2023 13:29:59 -0400 Subject: [PATCH 3/4] refactor test --- spec/features/idv/steps/review_step_spec.rb | 53 ++++++++------------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/spec/features/idv/steps/review_step_spec.rb b/spec/features/idv/steps/review_step_spec.rb index dfdb089a34a..c2e1014eec3 100644 --- a/spec/features/idv/steps/review_step_spec.rb +++ b/spec/features/idv/steps/review_step_spec.rb @@ -13,22 +13,7 @@ end it 'sends a letter, creates an unverified profile, and sends an email' do - fill_in 'Password', with: user_password - - email_count_before_continue = ActionMailer::Base.deliveries.count - - expect { click_continue }. - to change { GpoConfirmation.count }.by(1) - - expect_delivered_email_count(email_count_before_continue + 1) - expect(last_email.subject).to eq(t('user_mailer.letter_reminder.subject')) - - expect(user.events.account_verified.size).to be(0) - expect(user.profiles.count).to eq 1 - - profile = user.profiles.first - - expect(profile.active?).to eq false + sends_letter_creates_unverified_profile_sends_email end context 'user is rate-limited' do @@ -37,22 +22,7 @@ end it 'sends a letter, creates an unverified profile, and sends an email' do - fill_in 'Password', with: user_password - - email_count_before_continue = ActionMailer::Base.deliveries.count - - expect { click_continue }. - to change { GpoConfirmation.count }.by(1) - - expect_delivered_email_count(email_count_before_continue + 1) - expect(last_email.subject).to eq(t('user_mailer.letter_reminder.subject')) - - expect(user.events.account_verified.size).to be(0) - expect(user.profiles.count).to eq 1 - - profile = user.profiles.first - - expect(profile.active?).to eq false + sends_letter_creates_unverified_profile_sends_email end end @@ -93,5 +63,24 @@ expect(gpo_confirmation_entry[:issuer]).to eq(nil) end end + + def sends_letter_creates_unverified_profile_sends_email + fill_in 'Password', with: user_password + + email_count_before_continue = ActionMailer::Base.deliveries.count + + expect { click_continue }. + to change { GpoConfirmation.count }.by(1) + + expect_delivered_email_count(email_count_before_continue + 1) + expect(last_email.subject).to eq(t('user_mailer.letter_reminder.subject')) + + expect(user.events.account_verified.size).to be(0) + expect(user.profiles.count).to eq 1 + + profile = user.profiles.first + + expect(profile.active?).to eq false + end end end From ab6168fa327f892377706947853260b068ca50fd Mon Sep 17 00:00:00 2001 From: Amir Reavis-Bey Date: Thu, 17 Aug 2023 14:07:45 -0400 Subject: [PATCH 4/4] verify analytics for letter enqueued for rate limited user --- .../controllers/idv/review_controller_spec.rb | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/controllers/idv/review_controller_spec.rb b/spec/controllers/idv/review_controller_spec.rb index c31ba55b5e0..456541e3e59 100644 --- a/spec/controllers/idv/review_controller_spec.rb +++ b/spec/controllers/idv/review_controller_spec.rb @@ -694,6 +694,26 @@ def show ) end + context 'when user is rate limited' do + it 'logs USPS address letter enqueued event with phone_step_attempts', :freeze_time do + rate_limit_type = :proof_address + rate_limiter = RateLimiter.new(user: user, rate_limit_type: rate_limit_type) + rate_limiter.increment_to_limited! + put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } } + + expect(@analytics).to have_logged_event( + 'IdV: USPS address letter enqueued', + resend: false, + enqueued_at: Time.zone.now, + phone_step_attempts: RateLimiter.max_attempts(rate_limit_type), + first_letter_requested_at: idv_session.profile.gpo_verification_pending_at, + hours_since_first_letter: 0, + proofing_components: nil, + **ab_test_args, + ) + end + end + it 'redirects to come back later page' do put :create, params: { user: { password: ControllerHelper::VALID_PASSWORD } }