From 0a1244e45fb39d403a5250b8aaec95d158afd081 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 8 May 2023 16:29:31 -0700 Subject: [PATCH 1/7] Add gpo_max_profile_age_to_send_letter_in_days config --- config/application.yml.default | 1 + lib/identity_config.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/config/application.yml.default b/config/application.yml.default index 3092565687a..7dfc045fa9e 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -116,6 +116,7 @@ good_job_max_threads: 5 good_job_queues: 'default:5;low:1;*' good_job_queue_select_limit: 5_000 gpo_designated_receiver_pii: '{}' +gpo_max_profile_age_to_send_letter_in_days: 30 hide_phone_mfa_signup: false identity_pki_disabled: false identity_pki_local_dev: false diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 14188c0b757..a91cccc9c96 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -193,6 +193,7 @@ def self.build_store(config_map) config.add(:good_job_queues, type: :string) config.add(:good_job_queue_select_limit, type: :integer) config.add(:gpo_designated_receiver_pii, type: :json, options: { symbolize_names: true }) + config.add(:gpo_max_profile_age_to_send_letter_in_days, type: :integer) config.add(:hide_phone_mfa_signup, type: :boolean) config.add(:hmac_fingerprinter_key, type: :string) config.add(:hmac_fingerprinter_key_queue, type: :json) From ae5ca8b1fe4648473aca3e54587096812e424745 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 8 May 2023 16:29:43 -0700 Subject: [PATCH 2/7] Wire up @user_can_request_another_gpo_code in GpoVerifyController --- app/controllers/idv/gpo_verify_controller.rb | 12 +++++++++++- .../idv/gpo_verify_controller_spec.rb | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/app/controllers/idv/gpo_verify_controller.rb b/app/controllers/idv/gpo_verify_controller.rb index 8d20615f032..cec46895774 100644 --- a/app/controllers/idv/gpo_verify_controller.rb +++ b/app/controllers/idv/gpo_verify_controller.rb @@ -10,10 +10,14 @@ class GpoVerifyController < ApplicationController def index analytics.idv_gpo_verification_visited gpo_mail = Idv::GpoMail.new(current_user) - @mail_spammed = gpo_mail.mail_spammed? @gpo_verify_form = GpoVerifyForm.new(user: current_user, pii: pii) @code = session[:last_gpo_confirmation_code] if FeatureManagement.reveal_gpo_code? + @user_can_request_another_gpo_code = + FeatureManagement.gpo_verification_enabled? && + !gpo_mail.mail_spammed? && + !profile_is_too_old + if throttle.throttled? render_throttled else @@ -73,6 +77,12 @@ def prepare_for_personal_key enable_personal_key_generation end + def profile_is_too_old + max_age_in_days = IdentityConfig.store.gpo_max_profile_age_to_send_letter_in_days + min_creation_date = Time.zone.now - max_age_in_days.days + current_user.pending_profile.created_at < min_creation_date + end + def throttle @throttle ||= Throttle.new( user: current_user, diff --git a/spec/controllers/idv/gpo_verify_controller_spec.rb b/spec/controllers/idv/gpo_verify_controller_spec.rb index 11a3253dc97..09c2464ff52 100644 --- a/spec/controllers/idv/gpo_verify_controller_spec.rb +++ b/spec/controllers/idv/gpo_verify_controller_spec.rb @@ -6,16 +6,19 @@ let(:otp) { 'ABC123' } let(:submitted_otp) { otp } let(:user) { create(:user) } + let(:profile_created_at) { Time.zone.now } let(:pending_profile) do create( :profile, :with_pii, user: user, proofing_components: proofing_components, + created_at: profile_created_at, ) end let(:proofing_components) { nil } let(:threatmetrix_enabled) { false } + let(:gpo_enabled) { true } before do stub_analytics @@ -32,6 +35,7 @@ allow(IdentityConfig.store).to receive(:proofing_device_profiling). and_return(threatmetrix_enabled ? :enabled : :disabled) + allow(IdentityConfig.store).to receive(:enable_usps_verification).and_return(gpo_enabled) end describe '#index' do @@ -48,6 +52,11 @@ expect(response).to render_template('idv/gpo_verify/index') end + it 'sets @user_can_request_another_gpo_code to true' do + action + expect(assigns(:user_can_request_another_gpo_code)).to eql(true) + end + it 'shows throttled page is user is throttled' do Throttle.new(throttle_type: :verify_gpo_key, user: user).increment_to_throttled! @@ -55,6 +64,14 @@ expect(response).to render_template(:throttled) end + + context 'but that profile is > 30 days old' do + let(:profile_created_at) { 31.days.ago } + it 'sets @user_can_request_another_gpo_code to false' do + action + expect(assigns(:user_can_request_another_gpo_code)).to eql(false) + end + end end context 'user does not have pending profile' do From 1bda94c2d811568eae6006918682608d7cb4343e Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 8 May 2023 16:38:51 -0700 Subject: [PATCH 3/7] Wire up @user_can_request_another_gpo_code in view --- app/views/idv/gpo_verify/index.html.erb | 2 +- .../idv/gpo_verify/index.html.erb_spec.rb | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 spec/views/idv/gpo_verify/index.html.erb_spec.rb diff --git a/app/views/idv/gpo_verify/index.html.erb b/app/views/idv/gpo_verify/index.html.erb index 3db44090e8f..a0c634edf5e 100644 --- a/app/views/idv/gpo_verify/index.html.erb +++ b/app/views/idv/gpo_verify/index.html.erb @@ -41,7 +41,7 @@ <% end %> -<% if FeatureManagement.gpo_verification_enabled? && !@mail_spammed %> +<% if @user_can_request_another_gpo_code %> <%= link_to t('idv.messages.gpo.resend'), idv_gpo_path, class: 'display-block margin-bottom-2' %> <% end %> diff --git a/spec/views/idv/gpo_verify/index.html.erb_spec.rb b/spec/views/idv/gpo_verify/index.html.erb_spec.rb new file mode 100644 index 00000000000..c60065c63de --- /dev/null +++ b/spec/views/idv/gpo_verify/index.html.erb_spec.rb @@ -0,0 +1,39 @@ +require 'rails_helper' + +describe 'idv/gpo_verify/index.html.erb' do + let(:user) do + create(:user) + end + + let(:pii) do + {} + end + + before do + allow(view).to receive(:step_indicator_steps).and_return({}) + @gpo_verify_form = GpoVerifyForm.new( + user: user, + pii: pii, + otp: '1234', + ) + end + + context 'user is allowed to request another GPO letter' do + before do + @user_can_request_another_gpo_code = true + render + end + it 'includes the send another letter link' do + expect(rendered).to have_link(t('idv.messages.gpo.resend'), href: idv_gpo_path) + end + end + context 'user is NOT allowed to request another GPO letter' do + before do + @user_can_request_another_gpo_code = false + render + end + it 'does not include the send another letter link' do + expect(rendered).not_to have_link(t('idv.messages.gpo.resend'), href: idv_gpo_path) + end + end +end From cf0c092d2c6a5185d87dbd01a469ce66e78ae780 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 8 May 2023 16:53:32 -0700 Subject: [PATCH 4/7] Guard against viewing / submitting GPO form if profile too old --- app/controllers/idv/gpo_controller.rb | 5 ++++ app/controllers/idv/gpo_verify_controller.rb | 8 +----- app/services/idv/gpo_mail.rb | 9 ++++++ spec/controllers/idv/gpo_controller_spec.rb | 29 ++++++++++++++++++++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/app/controllers/idv/gpo_controller.rb b/app/controllers/idv/gpo_controller.rb index 4d243172618..f8772cc7d40 100644 --- a/app/controllers/idv/gpo_controller.rb +++ b/app/controllers/idv/gpo_controller.rb @@ -7,6 +7,7 @@ class GpoController < ApplicationController before_action :confirm_idv_needed before_action :confirm_user_completed_idv_profile_step before_action :confirm_mail_not_spammed + before_action :confirm_profile_not_too_old def index @presenter = GpoPresenter.new(current_user, url_options) @@ -38,6 +39,10 @@ def gpo_mail_service private + def confirm_profile_not_too_old + redirect_to idv_path if gpo_mail_service.profile_too_old? + end + def step_indicator_current_step if resend_requested? :get_a_letter diff --git a/app/controllers/idv/gpo_verify_controller.rb b/app/controllers/idv/gpo_verify_controller.rb index cec46895774..33a1608db9d 100644 --- a/app/controllers/idv/gpo_verify_controller.rb +++ b/app/controllers/idv/gpo_verify_controller.rb @@ -16,7 +16,7 @@ def index @user_can_request_another_gpo_code = FeatureManagement.gpo_verification_enabled? && !gpo_mail.mail_spammed? && - !profile_is_too_old + !gpo_mail.profile_too_old? if throttle.throttled? render_throttled @@ -77,12 +77,6 @@ def prepare_for_personal_key enable_personal_key_generation end - def profile_is_too_old - max_age_in_days = IdentityConfig.store.gpo_max_profile_age_to_send_letter_in_days - min_creation_date = Time.zone.now - max_age_in_days.days - current_user.pending_profile.created_at < min_creation_date - end - def throttle @throttle ||= Throttle.new( user: current_user, diff --git a/app/services/idv/gpo_mail.rb b/app/services/idv/gpo_mail.rb index b088c03f747..cf95f80694a 100644 --- a/app/services/idv/gpo_mail.rb +++ b/app/services/idv/gpo_mail.rb @@ -12,6 +12,15 @@ def mail_spammed? max_events? && updated_within_last_month? end + def profile_too_old? + return false if !current_user.pending_profile + + max_age_in_days = IdentityConfig.store.gpo_max_profile_age_to_send_letter_in_days + min_creation_date = Time.zone.now - max_age_in_days.days + + current_user.pending_profile.created_at < min_creation_date + end + private attr_reader :current_user diff --git a/spec/controllers/idv/gpo_controller_spec.rb b/spec/controllers/idv/gpo_controller_spec.rb index 2cef9dd43ef..4129c41991a 100644 --- a/spec/controllers/idv/gpo_controller_spec.rb +++ b/spec/controllers/idv/gpo_controller_spec.rb @@ -15,6 +15,7 @@ :confirm_two_factor_authenticated, :confirm_idv_needed, :confirm_mail_not_spammed, + :confirm_profile_not_too_old, ) end @@ -95,6 +96,34 @@ expect(assigns(:step_indicator_current_step)).to eq(:get_a_letter) end end + + context 'user has a pending profile' do + let(:profile_created_at) { Time.zone.now } + let(:pending_profile) do + create( + :profile, + :with_pii, + user: user, + created_at: profile_created_at, + ) + end + before do + allow(user).to receive(:pending_profile).and_return(pending_profile) + end + + it 'renders ok' do + get :index + expect(response).to be_ok + end + + context 'but pending profile is too old to send another letter' do + let(:profile_created_at) { Time.zone.now - 31.days } + it 'redirects back to /verify' do + get :index + expect(response).to redirect_to(idv_path) + end + end + end end describe '#create' do From 80e7b27cafdaa1ebfa2c20ca43b4f0f846783cbd Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 8 May 2023 16:54:42 -0700 Subject: [PATCH 5/7] changelog: Internal, GPO, Prevent requesting another GPO code when profile is too old. From 1bd5aa435580725efa366c668e230f89f0011cc9 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 10 May 2023 14:30:59 -0700 Subject: [PATCH 6/7] Add feature tests for GPO resend blocking - Test that the user is not presented the option to resend after too much time - Test that the user can't navigate to the resend page after too much time --- spec/features/idv/steps/gpo_step_spec.rb | 35 +++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/spec/features/idv/steps/gpo_step_spec.rb b/spec/features/idv/steps/gpo_step_spec.rb index 10b6ef2ea59..2fbe52b0862 100644 --- a/spec/features/idv/steps/gpo_step_spec.rb +++ b/spec/features/idv/steps/gpo_step_spec.rb @@ -35,6 +35,35 @@ expect(page).to have_current_path(idv_come_back_later_path) end + context 'too much time has passed' do + let(:days_passed) { 31 } + let(:max_days_before_resend_disabled) { 30 } + + before do + allow(IdentityConfig.store).to receive(:gpo_max_profile_age_to_send_letter_in_days). + and_return(max_days_before_resend_disabled) + end + + it 'does not present the user the option to to resend' do + complete_idv_and_sign_out + travel_to(days_passed.days.from_now) do + sign_in_live_with_2fa(user) + expect(page).to have_current_path(idv_gpo_verify_path) + expect(page).not_to have_css('.usa-button', text: t('idv.buttons.mail.resend')) + end + end + + it 'does not allow the user to go to the resend page manually' do + complete_idv_and_sign_out + travel_to(days_passed.days.from_now) do + sign_in_live_with_2fa(user) + visit idv_gpo_path + expect(page).to have_current_path(idv_gpo_verify_path) + expect(page).not_to have_css('.usa-button', text: t('idv.buttons.mail.resend')) + end + end + end + it 'allows the user to return to gpo otp confirmation' do complete_idv_and_return_to_gpo_step click_doc_auth_back_link @@ -44,7 +73,7 @@ expect_user_to_be_unverified(user) end - def complete_idv_and_return_to_gpo_step + def complete_idv_and_sign_out start_idv_from_sp complete_idv_steps_before_gpo_step(user) click_on t('idv.buttons.mail.send') @@ -53,6 +82,10 @@ def complete_idv_and_return_to_gpo_step visit root_path click_on t('forms.verify_profile.return_to_profile') first(:link, t('links.sign_out')).click + end + + def complete_idv_and_return_to_gpo_step + complete_idv_and_sign_out sign_in_live_with_2fa(user) click_on t('idv.messages.gpo.resend') end From d49a77ebd50ad0a250a8c452da03962c943bda20 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 10 May 2023 16:15:33 -0700 Subject: [PATCH 7/7] Update app/services/idv/gpo_mail.rb Co-authored-by: Zach Margolis --- app/services/idv/gpo_mail.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/idv/gpo_mail.rb b/app/services/idv/gpo_mail.rb index cf95f80694a..333d0b96a9c 100644 --- a/app/services/idv/gpo_mail.rb +++ b/app/services/idv/gpo_mail.rb @@ -15,8 +15,8 @@ def mail_spammed? def profile_too_old? return false if !current_user.pending_profile - max_age_in_days = IdentityConfig.store.gpo_max_profile_age_to_send_letter_in_days - min_creation_date = Time.zone.now - max_age_in_days.days + min_creation_date = IdentityConfig.store. + gpo_max_profile_age_to_send_letter_in_days.days.ago current_user.pending_profile.created_at < min_creation_date end