From 6c0463a2107417a7b37f56a802ee63b5c70bac69 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Tue, 25 Jun 2024 14:23:31 -0400 Subject: [PATCH 1/2] Remove the resend functionality from the `RequestLetterController` In #10864 a new controller was introduced to handle resends. In #10865 the controller started being used. Once both of those changes are fully deployed this change can be merged to remove the resend funcitonality from the `RequestLetterController`. This change will make the `RequestLetterController` behave like the rest of the IdV-step controllers. It will no longer be accessible outside of the IdV workflow. [skip changelog] --- .../idv/by_mail/request_letter_controller.rb | 59 +---- .../idv/by_mail/request_letter_presenter.rb | 59 ----- app/services/analytics_events.rb | 3 - .../idv/by_mail/request_letter/index.html.erb | 110 ++++------ .../by_mail/request_letter_controller_spec.rb | 207 +++--------------- spec/features/idv/analytics_spec.rb | 6 +- .../idv/steps/request_letter_step_spec.rb | 149 ------------- .../idv/steps/resend_letter_step_spec.rb | 153 +++++++++++++ .../by_mail/request_letter_presenter_spec.rb | 91 -------- .../request_letter/index.html.erb_spec.rb | 56 +---- 10 files changed, 244 insertions(+), 649 deletions(-) delete mode 100644 app/presenters/idv/by_mail/request_letter_presenter.rb create mode 100644 spec/features/idv/steps/resend_letter_step_spec.rb delete mode 100644 spec/presenters/idv/by_mail/request_letter_presenter_spec.rb diff --git a/app/controllers/idv/by_mail/request_letter_controller.rb b/app/controllers/idv/by_mail/request_letter_controller.rb index 26335c07528..b071093cf9c 100644 --- a/app/controllers/idv/by_mail/request_letter_controller.rb +++ b/app/controllers/idv/by_mail/request_letter_controller.rb @@ -5,39 +5,25 @@ module ByMail class RequestLetterController < ApplicationController include Idv::AvailabilityConcern include IdvStepConcern - skip_before_action :confirm_no_pending_gpo_profile include Idv::StepIndicatorConcern include VerifyByMailConcern before_action :confirm_mail_not_rate_limited before_action :confirm_step_allowed - before_action :confirm_profile_not_too_old def index @applicant = idv_session.applicant - @presenter = RequestLetterPresenter.new(current_user, url_options) Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer). call(:usps_address, :view, true) - analytics.idv_request_letter_visited( - letter_already_sent: @presenter.resend_requested?, - ) + analytics.idv_request_letter_visited end def create clear_future_steps! update_tracking idv_session.address_verification_mechanism = :gpo - - if resend_requested? && pii_locked? - redirect_to capture_password_url - elsif resend_requested? - resend_letter - flash[:success] = t('idv.messages.gpo.another_letter_on_the_way') - redirect_to idv_letter_enqueued_url - else - redirect_to idv_enter_password_url - end + redirect_to idv_enter_password_url end def self.step_info @@ -55,59 +41,20 @@ def self.step_info private - def confirm_profile_not_too_old - redirect_to idv_path if gpo_verify_by_mail_policy.profile_too_old? - end - def update_tracking Funnel::DocAuth::RegisterStep.new(current_user.id, current_sp&.issuer). call(:usps_letter_sent, :update, true) - log_letter_requested_analytics(resend: resend_requested?) + log_letter_requested_analytics(resend: false) create_user_event(:gpo_mail_sent, current_user) ProofingComponent.find_or_create_by(user: current_user).update(address_check: 'gpo_letter') end - def resend_requested? - current_user.gpo_verification_pending_profile? - end - def confirm_mail_not_rate_limited redirect_to idv_enter_password_url if gpo_verify_by_mail_policy.rate_limited? end - def resend_letter - log_letter_enqueued_analytics(resend: true) - confirmation_maker = confirmation_maker_perform - send_reminder - return unless FeatureManagement.reveal_gpo_code? - session[:last_gpo_confirmation_code] = confirmation_maker.otp - end - - def confirmation_maker_perform - confirmation_maker = GpoConfirmationMaker.new( - pii: pii, - service_provider: current_sp, - profile: current_user.pending_profile, - ) - confirmation_maker.perform - confirmation_maker - end - - def pii - Pii::Cacher.new(current_user, user_session). - fetch(current_user.gpo_verification_pending_profile.id) - end - - def send_reminder - current_user.send_email_to_all_addresses(:verify_by_mail_letter_requested) - end - - def pii_locked? - !Pii::Cacher.new(current_user, user_session).exists_in_session? - end - def step_indicator_steps if in_person_proofing? Idv::Flows::InPersonFlow::STEP_INDICATOR_STEPS_GPO diff --git a/app/presenters/idv/by_mail/request_letter_presenter.rb b/app/presenters/idv/by_mail/request_letter_presenter.rb deleted file mode 100644 index 349e7554619..00000000000 --- a/app/presenters/idv/by_mail/request_letter_presenter.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -module Idv - module ByMail - class RequestLetterPresenter - include Rails.application.routes.url_helpers - - attr_reader :current_user, :url_options - - def initialize(current_user, url_options) - @current_user = current_user - @url_options = url_options - end - - def title - resend_requested? ? - I18n.t('idv.gpo.request_another_letter.title') : - I18n.t('idv.titles.mail.verify') - end - - def button - resend_requested? ? - I18n.t('idv.gpo.request_another_letter.button') : - I18n.t('idv.buttons.mail.send') - end - - def fallback_back_path - return idv_verify_info_path if OutageStatus.new.any_phone_vendor_outage? - user_needs_address_otp_verification? ? idv_verify_by_mail_enter_code_path : idv_phone_path - end - - def resend_requested? - current_user.gpo_verification_pending_profile? - end - - def back_or_cancel_partial - if FeatureManagement.idv_by_mail_only? - 'idv/doc_auth/cancel' - else - 'idv/shared/back' - end - end - - def back_or_cancel_parameters - if FeatureManagement.idv_by_mail_only? - { step: 'gpo' } - else - { fallback_path: fallback_back_path } - end - end - - private - - def user_needs_address_otp_verification? - current_user.pending_profile? - end - end - end -end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 699e56d39c0..df5769b556c 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -3261,16 +3261,13 @@ def idv_proofing_resolution_result_missing( ) end - # @param [Boolean] letter_already_sent # GPO "request letter" page visited # @identity.idp.previous_event_name IdV: USPS address visited def idv_request_letter_visited( - letter_already_sent:, **extra ) track_event( 'IdV: request letter visited', - letter_already_sent: letter_already_sent, **extra, ) end diff --git a/app/views/idv/by_mail/request_letter/index.html.erb b/app/views/idv/by_mail/request_letter/index.html.erb index 99a2f06e42b..88d8bdfff71 100644 --- a/app/views/idv/by_mail/request_letter/index.html.erb +++ b/app/views/idv/by_mail/request_letter/index.html.erb @@ -9,77 +9,59 @@ ) %> <% end %> -<% if @presenter.resend_requested? %> - <%= render PageHeadingComponent.new.with_content(@presenter.title) %> -

- <%= t('idv.gpo.request_another_letter.instructions_html') %> -

-

- <%= new_tab_link_to( - t('idv.gpo.request_another_letter.learn_more_link'), - help_center_redirect_url( - category: 'verify-your-identity', - article: 'verify-your-address-by-mail', - flow: :idv, - step: :gpo_send_letter, - ), - ) %> -

-<% else %> - <%= render AlertComponent.new( - type: :info, - message: t('idv.messages.gpo.info_alert'), - class: 'margin-bottom-4', - ) %> - <%= render PageHeadingComponent.new.with_content(@presenter.title) %> -

- <%= t('idv.messages.gpo.timeframe_html') %> -
- <%= new_tab_link_to( - t('idv.messages.gpo.learn_more_verify_by_mail'), - help_center_redirect_url( - category: 'verify-your-identity', - article: 'verify-your-address-by-mail', - flow: :idv, - step: :gpo_send_letter, - ), - ) - %> -

-

- <%= t('idv.messages.gpo.address_on_file') %> -

+<%= render AlertComponent.new( + type: :info, + message: t('idv.messages.gpo.info_alert'), + class: 'margin-bottom-4', + ) %> -

- <%= @applicant[:address1] %> -
- <% if @applicant[:address2] %> - <%= @applicant[:address2] %> -
- <% end %> - <%= @applicant[:city] %>, <%= @applicant[:state] %> <%= @applicant[:zipcode] %> -
-

+<%= render PageHeadingComponent.new.with_content(t('idv.titles.mail.verify')) %> -

- <%= start_over_link_html = link_to( - t('idv.messages.gpo.start_over_link_text'), - idv_confirm_start_over_before_letter_path, - ) - t( - 'idv.messages.gpo.start_over_html', - start_over_link_html: start_over_link_html, - ) - %> -

+

+ <%= t('idv.messages.gpo.timeframe_html') %> +
+ <%= new_tab_link_to( + t('idv.messages.gpo.learn_more_verify_by_mail'), + help_center_redirect_url( + category: 'verify-your-identity', + article: 'verify-your-address-by-mail', + flow: :idv, + step: :gpo_send_letter, + ), + ) + %> +

+

+ <%= t('idv.messages.gpo.address_on_file') %> +

-<% end %> +

+ <%= render 'shared/address', + address: @applicant.slice(:address1, :address2, :city, :state, :zipcode) + %> +

+ +

+ <%= start_over_link_html = link_to( + t('idv.messages.gpo.start_over_link_text'), + idv_confirm_start_over_before_letter_path, + ) + t( + 'idv.messages.gpo.start_over_html', + start_over_link_html: start_over_link_html, + ) + %> +

- <%= button_to @presenter.button, + <%= button_to t('idv.buttons.mail.send'), idv_request_letter_path, method: 'put', class: 'usa-button usa-button--big usa-button--wide' %>
-<%= render @presenter.back_or_cancel_partial, @presenter.back_or_cancel_parameters %> +<% if FeatureManagement.idv_by_mail_only? %> + <%= render 'idv/doc_auth/cancel', step: 'gpo' %> +<% else %> + <%= render 'idv/shared/back', fallback_path: idv_phone_path %> +<% end %> diff --git a/spec/controllers/idv/by_mail/request_letter_controller_spec.rb b/spec/controllers/idv/by_mail/request_letter_controller_spec.rb index ad404cba873..bcee2c46ed0 100644 --- a/spec/controllers/idv/by_mail/request_letter_controller_spec.rb +++ b/spec/controllers/idv/by_mail/request_letter_controller_spec.rb @@ -26,7 +26,6 @@ :confirm_two_factor_authenticated, :confirm_idv_needed, :confirm_mail_not_rate_limited, - :confirm_profile_not_too_old, ) end @@ -44,10 +43,7 @@ get :index expect(response).to have_http_status(200) - expect(@analytics).to have_logged_event( - 'IdV: request letter visited', - letter_already_sent: false, - ) + expect(@analytics).to have_logged_event('IdV: request letter visited') end it 'updates the doc auth log for the user for the usps_address_view event' do @@ -67,190 +63,51 @@ expect(response).to redirect_to idv_enter_password_path end + end - it 'allows a user to request another letter' do - allow(controller.gpo_verify_by_mail_policy).to receive(:rate_limited?).and_return(false) - get :index - - expect(response).to be_ok + describe '#create' do + before do + stub_verify_steps_one_and_two(user) end - context 'with letter already sent' do - before do - allow_any_instance_of(Idv::ByMail::RequestLetterPresenter). - to receive(:resend_requested?).and_return(true) - end + it 'invalidates future steps' do + expect(subject).to receive(:clear_future_steps!) - it 'logs visited event' do - get :index - - expect(@analytics).to have_logged_event( - 'IdV: request letter visited', - letter_already_sent: true, - ) - end + put :create 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 'sets session to :gpo and redirects' do + expect(subject.idv_session.address_verification_mechanism).to be_nil - it 'renders ok' do - get :index - expect(response).to be_ok - end + put :create - 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 + expect(response).to redirect_to idv_enter_password_path + expect(subject.idv_session.address_verification_mechanism).to eq :gpo end - end - - describe '#create' do - context 'first time through the idv process' do - before do - stub_verify_steps_one_and_two(user) - end - - it 'invalidates future steps' do - expect(subject).to receive(:clear_future_steps!) - - put :create - end - - it 'sets session to :gpo and redirects' do - expect(subject.idv_session.address_verification_mechanism).to be_nil - - put :create - - expect(response).to redirect_to idv_enter_password_path - expect(subject.idv_session.address_verification_mechanism).to eq :gpo - end - - it 'logs USPS address letter requested analytics event with phone step attempts' do - RateLimiter.new(user: user, rate_limit_type: :proof_address).increment! - put :create - expect(@analytics).to have_logged_event( - 'IdV: USPS address letter requested', - hash_including( - resend: false, - phone_step_attempts: 1, - first_letter_requested_at: nil, - hours_since_first_letter: 0, - **ab_test_args, - ), - ) - end + it 'logs USPS address letter requested analytics event with phone step attempts' do + RateLimiter.new(user: user, rate_limit_type: :proof_address).increment! + put :create - it 'updates the doc auth log for the user for the usps_letter_sent event' do - unstub_analytics - doc_auth_log = DocAuthLog.create(user_id: user.id) - - expect { put :create }.to( - change { doc_auth_log.reload.usps_letter_sent_submit_count }.from(0).to(1), - ) - end + expect(@analytics).to have_logged_event( + 'IdV: USPS address letter requested', + hash_including( + resend: false, + phone_step_attempts: 1, + first_letter_requested_at: nil, + hours_since_first_letter: 0, + **ab_test_args, + ), + ) end - context 'resending a letter' do - let(:has_pending_profile) { true } - let(:pending_profile) { create(:profile, :with_pii, :verify_by_mail_pending) } - - before do - stub_sign_in(user) - stub_user_with_pending_profile(user) - allow(user).to receive(:gpo_verification_pending_profile?).and_return(true) - subject.idv_session.welcome_visited = true - subject.idv_session.idv_consent_given = true - subject.idv_session.flow_path = 'standard' - subject.idv_session.resolution_successful = true - subject.idv_session.applicant = Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN - end - - it 'calls the GpoConfirmationMaker to send another letter and redirects' do - expect_resend_letter_to_send_letter_and_redirect(otp: false) - end - - it 'calls GpoConfirmationMaker to send another letter with reveal_gpo_code on' do - allow(FeatureManagement).to receive(:reveal_gpo_code?).and_return(true) - expect_resend_letter_to_send_letter_and_redirect(otp: true) - end - - it 'logs USPS address letter analytics events with phone step attempts', :freeze_time do - RateLimiter.new(user: user, rate_limit_type: :proof_address).increment! - expect_resend_letter_to_send_letter_and_redirect(otp: false) - - expect(@analytics).to have_logged_event( - 'IdV: USPS address letter requested', - hash_including( - resend: true, - phone_step_attempts: 1, - first_letter_requested_at: pending_profile.gpo_verification_pending_at, - hours_since_first_letter: 24, - **ab_test_args, - ), - ) - - expect(@analytics).to have_logged_event( - 'IdV: USPS address letter enqueued', - hash_including( - resend: true, - first_letter_requested_at: pending_profile.gpo_verification_pending_at, - hours_since_first_letter: 24, - enqueued_at: Time.zone.now, - phone_step_attempts: 1, - proofing_components: nil, - **ab_test_args, - ), - ) - end - - it 'redirects to capture password if pii is locked' do - pii_cacher = instance_double(Pii::Cacher) - allow(pii_cacher).to receive(:fetch).and_return(nil) - allow(pii_cacher).to receive(:exists_in_session?).and_return(false) - allow(Pii::Cacher).to receive(:new).and_return(pii_cacher) - - put :create + it 'updates the doc auth log for the user for the usps_letter_sent event' do + unstub_analytics + doc_auth_log = DocAuthLog.create(user_id: user.id) - expect(response).to redirect_to capture_password_path - end + expect { put :create }.to( + change { doc_auth_log.reload.usps_letter_sent_submit_count }.from(0).to(1), + ) end end - - def expect_resend_letter_to_send_letter_and_redirect(otp:) - pii = pending_profile.decrypt_pii(user.password).to_h - pii_cacher = instance_double(Pii::Cacher) - allow(pii_cacher).to receive(:fetch).with(pending_profile.id).and_return(pii) - allow(pii_cacher).to receive(:exists_in_session?).and_return(true) - allow(Pii::Cacher).to receive(:new).and_return(pii_cacher) - - service_provider = create(:service_provider, issuer: '123abc') - session[:sp] = { issuer: service_provider.issuer, vtr: ['C1'] } - - gpo_confirmation_maker = instance_double(GpoConfirmationMaker) - allow(GpoConfirmationMaker).to receive(:new). - with(pii: pii, service_provider: service_provider, profile: pending_profile). - and_return(gpo_confirmation_maker) - - expect(gpo_confirmation_maker).to receive(:perform) - expect(gpo_confirmation_maker).to receive(:otp) if otp - expect { put :create }.to change { ActionMailer::Base.deliveries.count }.by(1) - expect(response).to redirect_to idv_letter_enqueued_path - end end diff --git a/spec/features/idv/analytics_spec.rb b/spec/features/idv/analytics_spec.rb index 2da550bb912..1550cb883a9 100644 --- a/spec/features/idv/analytics_spec.rb +++ b/spec/features/idv/analytics_spec.rb @@ -460,9 +460,7 @@ active_profile_idv_level: nil, pending_profile_idv_level: nil, proofing_components: base_proofing_components }, - 'IdV: request letter visited' => { - letter_already_sent: false, - }, + 'IdV: request letter visited' => {}, :idv_enter_password_visited => { address_verification_method: 'gpo', acuant_sdk_upgrade_ab_test_bucket: :default, skip_hybrid_handoff: nil, active_profile_idv_level: nil, pending_profile_idv_level: nil, @@ -675,7 +673,7 @@ 'IdV: doc auth image upload form submitted' => { success: true, errors: {}, error_details: nil, submit_attempts: 1, remaining_submit_attempts: 3, user_id: user.uuid, flow_path: 'standard', front_image_fingerprint: an_instance_of(String), back_image_fingerprint: an_instance_of(String), selfie_image_fingerprint: an_instance_of(String), liveness_checking_required: boolean }, - 'IdV: doc auth image upload vendor submitted' => hash_including(success: true, flow_path: 'standard', attention_with_barcode: false, doc_auth_result: 'Passed', liveness_checking_required: boolean), + 'IdV: doc auth image upload vendor submitted' => hash_including(success: true, flow_path: 'standard', attention_with_barcode: false, doc_auth_result: 'Passed'), 'IdV: doc auth image upload vendor pii validation' => { success: true, errors: {}, error_details: nil, user_id: user.uuid, submit_attempts: 1, remaining_submit_attempts: 3, flow_path: 'standard', attention_with_barcode: false, front_image_fingerprint: an_instance_of(String), back_image_fingerprint: an_instance_of(String), selfie_image_fingerprint: an_instance_of(String), liveness_checking_required: boolean, classification_info: {}, id_issued_status: 'present', id_expiration_status: 'present' }, diff --git a/spec/features/idv/steps/request_letter_step_spec.rb b/spec/features/idv/steps/request_letter_step_spec.rb index 6c32f38a1de..1c144b7473e 100644 --- a/spec/features/idv/steps/request_letter_step_spec.rb +++ b/spec/features/idv/steps/request_letter_step_spec.rb @@ -4,19 +4,6 @@ include IdvStepHelper include OidcAuthHelper - let(:minimum_wait_for_letter) { 24 } - let(:days_passed) { max_days_before_resend_disabled + 1 } - let(:max_days_before_resend_disabled) { 30 } - - before do - allow(IdentityConfig.store).to receive(:minimum_wait_before_another_usps_letter_in_hours). - and_return(minimum_wait_for_letter) - allow(IdentityConfig.store).to receive(:gpo_max_profile_age_to_send_letter_in_days). - and_return(max_days_before_resend_disabled) - allow(IdentityConfig.store).to receive(:second_mfa_reminder_account_age_in_days). - and_return(days_passed + 1) - end - it 'visits and completes the enter password step when the user chooses verify by letter', :js do start_idv_from_sp complete_idv_steps_before_gpo_step @@ -29,142 +16,6 @@ expect(page).to have_content(t('idv.messages.gpo.letter_on_the_way')) end - context 'the user has sent a letter but not verified an OTP' do - let(:user) { user_with_2fa } - - before do - # Without this, the check for GPO expiration leaves an expired - # OTP rate limiter laying around. - allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(3) - end - - it 'if not rate limited, allow user to resend letter & redirect to letter enqueued step', :js do - complete_idv_by_mail_and_sign_out - - # rate-limited because too little time has passed - sign_in_live_with_2fa(user) - confirm_rate_limited - sign_out - - # still rate-limited because too little time has passed - travel_to((minimum_wait_for_letter - 1).hours.from_now) do - sign_in_live_with_2fa(user) - confirm_rate_limited - sign_out - end - - # will be rate-limted after expiration - travel_to(days_passed.days.from_now) do - sign_in_live_with_2fa(user) - confirm_rate_limited - sign_out - # Clear MFA SMS message from the future to allow re-logging in with test helper - Telephony::Test::Message.clear_messages - end - - # can re-request after the waiting period - travel_to((minimum_wait_for_letter + 1).hours.from_now) do - sign_in_live_with_2fa(user) - click_on t('idv.messages.gpo.resend') - - # Confirm that we show the correct content on - # the GPO page for users requesting re-send - expect(page).to have_content(t('idv.gpo.request_another_letter.title')) - expect(page).to have_content( - strip_tags(t('idv.gpo.request_another_letter.instructions_html')), - ) - expect(page).to have_content(t('idv.gpo.request_another_letter.button')) - expect(page).to_not have_content(t('idv.messages.gpo.info_alert')) - - # Ensure user can go back from this page - click_doc_auth_back_link - expect(page).to have_content(t('idv.gpo.title')) - expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) - expect_user_to_be_unverified(user) - click_on t('idv.messages.gpo.resend') - - # And then actually ask for a resend - expect { click_on t('idv.gpo.request_another_letter.button') }. - to change { GpoConfirmation.count }.from(1).to(2) - expect_user_to_be_unverified(user) - expect(page).to have_content(t('idv.messages.gpo.another_letter_on_the_way')) - expect(page).to have_content(t('idv.titles.come_back_later')) - expect(page).to have_current_path(idv_letter_enqueued_path) - - # Confirm that user cannot visit other IdV pages while unverified - visit idv_agreement_path - expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) - visit idv_ssn_url - expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) - visit idv_verify_info_url - expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) - - # complete verification: end to end gpo test - sign_out - sign_in_live_with_2fa(user) - - complete_gpo_verification(user) - expect(user.identity_verified?).to be(true) - expect(page).to_not have_content(t('account.index.verification.reactivate_button')) - end - end - - context 'logged in with PIV/CAC and no password' do - it 'does not 500' do - create(:profile, :with_pii, user: user, gpo_verification_pending_at: 1.day.ago) - create(:gpo_confirmation_code, profile: user.pending_profile) - create(:piv_cac_configuration, user: user, x509_dn_uuid: 'helloworld', name: 'My PIV Card') - - signin_with_piv(user) - fill_in t('account.index.password'), with: user.password - click_button t('forms.buttons.submit.default') - - complete_gpo_verification(user) - - expect(user.identity_verified?).to be(true) - - expect(page).to_not have_content(t('account.index.verification.reactivate_button')) - end - end - - def complete_idv_by_mail_and_sign_out - start_idv_from_sp - complete_idv_steps_before_gpo_step(user) - click_on t('idv.buttons.mail.send') - fill_in 'Password', with: user_password - click_continue - sign_out - end - - def expect_user_to_be_unverified(user) - 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 - expect(profile.gpo_verification_pending?).to eq true - end - - def sign_out - visit sign_out_url - end - - def confirm_rate_limited - expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) - expect(page).not_to have_link( - t('idv.messages.gpo.resend'), - ) - # does not allow the user to go to the resend page manually - visit idv_request_letter_path - - expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) - expect(page).not_to have_link( - t('idv.messages.gpo.resend'), - ) - end - end - context 'GPO verified user has reset their password and needs to re-verify with GPO again', :js do let(:user) { user_verified_with_gpo } diff --git a/spec/features/idv/steps/resend_letter_step_spec.rb b/spec/features/idv/steps/resend_letter_step_spec.rb new file mode 100644 index 00000000000..ab10aae5d6c --- /dev/null +++ b/spec/features/idv/steps/resend_letter_step_spec.rb @@ -0,0 +1,153 @@ +require 'rails_helper' + +RSpec.feature 'idv resend letter step', allowed_extra_analytics: [:*] do + include IdvStepHelper + include OidcAuthHelper + + let(:minimum_wait_for_letter) { 24 } + let(:days_passed) { max_days_before_resend_disabled + 1 } + let(:max_days_before_resend_disabled) { 30 } + + before do + allow(IdentityConfig.store).to receive(:minimum_wait_before_another_usps_letter_in_hours). + and_return(minimum_wait_for_letter) + allow(IdentityConfig.store).to receive(:gpo_max_profile_age_to_send_letter_in_days). + and_return(max_days_before_resend_disabled) + allow(IdentityConfig.store).to receive(:second_mfa_reminder_account_age_in_days). + and_return(days_passed + 1) + end + + let(:user) { user_with_2fa } + + before do + # Without this, the check for GPO expiration leaves an expired + # OTP rate limiter laying around. + allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(3) + end + + it 'if not rate limited, allow user to resend letter & redirect to letter enqueued step', :js do + complete_idv_by_mail_and_sign_out + + # rate-limited because too little time has passed + sign_in_live_with_2fa(user) + confirm_rate_limited + sign_out + + # still rate-limited because too little time has passed + travel_to((minimum_wait_for_letter - 1).hours.from_now) do + sign_in_live_with_2fa(user) + confirm_rate_limited + sign_out + end + + # will be rate-limted after expiration + travel_to(days_passed.days.from_now) do + sign_in_live_with_2fa(user) + confirm_rate_limited + sign_out + # Clear MFA SMS message from the future to allow re-logging in with test helper + Telephony::Test::Message.clear_messages + end + + # can re-request after the waiting period + travel_to((minimum_wait_for_letter + 1).hours.from_now) do + sign_in_live_with_2fa(user) + click_on t('idv.messages.gpo.resend') + + # Confirm that we show the correct content on + # the GPO page for users requesting re-send + expect(page).to have_content(t('idv.gpo.request_another_letter.title')) + expect(page).to have_content( + strip_tags(t('idv.gpo.request_another_letter.instructions_html')), + ) + expect(page).to have_content(t('idv.gpo.request_another_letter.button')) + expect(page).to_not have_content(t('idv.messages.gpo.info_alert')) + + # Ensure user can go back from this page + click_doc_auth_back_link + expect(page).to have_content(t('idv.gpo.title')) + expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) + expect_user_to_be_unverified(user) + click_on t('idv.messages.gpo.resend') + + # And then actually ask for a resend + expect { click_on t('idv.gpo.request_another_letter.button') }. + to change { GpoConfirmation.count }.from(1).to(2) + expect_user_to_be_unverified(user) + expect(page).to have_content(t('idv.messages.gpo.another_letter_on_the_way')) + expect(page).to have_content(t('idv.titles.come_back_later')) + expect(page).to have_current_path(idv_letter_enqueued_path) + + # Confirm that user cannot visit other IdV pages while unverified + visit idv_agreement_path + expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) + visit idv_ssn_url + expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) + visit idv_verify_info_url + expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) + + # complete verification: end to end gpo test + sign_out + sign_in_live_with_2fa(user) + + complete_gpo_verification(user) + expect(user.identity_verified?).to be(true) + expect(page).to_not have_content(t('account.index.verification.reactivate_button')) + end + end + + context 'logged in with PIV/CAC and no password' do + it 'does not 500' do + create(:profile, :with_pii, user: user, gpo_verification_pending_at: 1.day.ago) + create(:gpo_confirmation_code, profile: user.pending_profile) + create(:piv_cac_configuration, user: user, x509_dn_uuid: 'helloworld', name: 'My PIV Card') + + signin_with_piv(user) + fill_in t('account.index.password'), with: user.password + click_button t('forms.buttons.submit.default') + + complete_gpo_verification(user) + + expect(user.identity_verified?).to be(true) + + expect(page).to_not have_content(t('account.index.verification.reactivate_button')) + end + end + + def complete_idv_by_mail_and_sign_out + start_idv_from_sp + complete_idv_steps_before_gpo_step(user) + click_on t('idv.buttons.mail.send') + fill_in 'Password', with: user_password + click_continue + sign_out + end + + def expect_user_to_be_unverified(user) + 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 + expect(profile.gpo_verification_pending?).to eq true + end + + def sign_out + visit sign_out_url + end + + def confirm_rate_limited + expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) + expect(page).not_to have_link( + t('idv.messages.gpo.resend'), + ) + # does not allow the user to go to the resend page manually + visit idv_request_letter_path + + expect(page).to have_current_path(idv_verify_by_mail_enter_code_path) + expect(page).not_to have_link( + t('idv.messages.gpo.resend'), + ) + end +end diff --git a/spec/presenters/idv/by_mail/request_letter_presenter_spec.rb b/spec/presenters/idv/by_mail/request_letter_presenter_spec.rb deleted file mode 100644 index 629b02c76d0..00000000000 --- a/spec/presenters/idv/by_mail/request_letter_presenter_spec.rb +++ /dev/null @@ -1,91 +0,0 @@ -require 'rails_helper' - -RSpec.describe Idv::ByMail::RequestLetterPresenter do - include Rails.application.routes.url_helpers - - let(:user) { create(:user) } - - subject(:decorator) do - described_class.new(user, {}) - end - - describe '#title' do - context 'a letter has not been sent' do - it 'provides text to send' do - expect(subject.title).to eq( - I18n.t('idv.titles.mail.verify'), - ) - end - end - - context 'a letter has been sent' do - before do - allow(user).to receive(:gpo_verification_pending_profile).and_return(true) - end - it 'provides text to resend' do - create_letter_send_event - - expect(subject.title).to eq( - I18n.t('idv.gpo.request_another_letter.title'), - ) - end - end - - context 'the user has verified with GPO before, but is re-proofing' do - let(:user) { user_verified_with_gpo } - it 'provides text to send' do - create_letter_send_event - expect(subject.title).to eq( - I18n.t('idv.titles.mail.verify'), - ) - end - end - end - - describe '#button' do - let(:user) { create(:user) } - context 'a letter has not been sent' do - it 'provides text to send' do - expect(subject.button).to eq( - I18n.t('idv.buttons.mail.send'), - ) - end - end - - context 'a letter has been sent' do - before do - allow(user).to receive(:gpo_verification_pending_profile).and_return(true) - end - it 'provides text to resend' do - create_letter_send_event - expect(subject.button).to eq( - I18n.t('idv.gpo.request_another_letter.button'), - ) - end - end - end - - describe '#fallback_back_path' do - context 'when the user has a pending profile' do - it 'returns the verify account path' do - create(:profile, user: user, gpo_verification_pending_at: 1.day.ago) - expect(subject.fallback_back_path).to eq(idv_verify_by_mail_enter_code_path) - end - end - - context 'when the user does not have a pending profile' do - it 'returns the idv phone path' do - expect(subject.fallback_back_path).to eq(idv_phone_path) - end - end - end - - def create_letter_send_event - device = create(:device, user: user) - create(:event, user: user, device: device, event_type: :gpo_mail_sent) - end - - def user_verified_with_gpo - create(:user, :proofed_with_gpo) - end -end diff --git a/spec/views/idv/by_mail/request_letter/index.html.erb_spec.rb b/spec/views/idv/by_mail/request_letter/index.html.erb_spec.rb index 6c581ad775c..b01459d71a9 100644 --- a/spec/views/idv/by_mail/request_letter/index.html.erb_spec.rb +++ b/spec/views/idv/by_mail/request_letter/index.html.erb_spec.rb @@ -1,14 +1,10 @@ require 'rails_helper' RSpec.describe 'idv/by_mail/request_letter/index.html.erb' do - let(:resend_requested) { false } - let(:user_needs_address_otp_verification) { false } + let(:user) { build(:user) } let(:go_back_path) { nil } let(:step_indicator_steps) { Idv::StepIndicatorConcern::STEP_INDICATOR_STEPS } - let(:presenter) do - user = build_stubbed(:user, :fully_registered) - Idv::ByMail::RequestLetterPresenter.new(user, {}) - end + let(:idv_by_mail_only) { false } let(:address1) { 'applicant address 1' } let(:address2) { nil } @@ -17,14 +13,11 @@ let(:zipcode) { 'applicant zipcode' } before do + allow(view).to receive(:current_user).and_return(user) allow(view).to receive(:go_back_path).and_return(go_back_path) allow(view).to receive(:step_indicator_steps).and_return(step_indicator_steps) + allow(FeatureManagement).to receive(:idv_by_mail_only?).and_return(idv_by_mail_only) - allow(presenter).to receive(:resend_requested?).and_return(resend_requested) - allow(presenter).to receive(:user_needs_address_otp_verification?). - and_return(user_needs_address_otp_verification) - - @presenter = presenter @applicant = { address1: 'applicant address 1', city: 'applicant city', @@ -67,44 +60,11 @@ end end - context 'letter already sent' do - let(:resend_requested) { true } - - it 'has the right title' do - expect(rendered).to have_css('h1', text: t('idv.gpo.request_another_letter.title')) - end - - it 'has the right body' do - expect(rendered).to have_text( - strip_tags(t('idv.gpo.request_another_letter.instructions_html')), - ) - end - - it 'includes link to help' do - expect(rendered).to have_link( - t('idv.gpo.request_another_letter.learn_more_link'), - href: help_center_redirect_url( - category: 'verify-your-identity', - article: 'verify-your-address-by-mail', - flow: :idv, - step: :gpo_send_letter, - ), - ) - end - - it 'does not include troubleshooting options' do - expect(rendered).not_to have_css('.troubleshooting-options') - end - end - - context 'user needs address otp verification' do - let(:user_needs_address_otp_verification) { true } + context 'idv_by_mail_only is enabled' do + let(:idv_by_mail_only) { true } - it 'renders fallback link to return to verify path' do - expect(rendered).to have_link( - '‹ ' + t('forms.buttons.back'), - href: idv_verify_by_mail_enter_code_path, - ) + it 'returns a cancel link' do + expect(rendered).to have_link(t('links.cancel'), href: idv_cancel_path(step: 'gpo')) end end end From 6559adc8c22619ef00a3d1ed84f4d7f08880b50d Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Fri, 28 Jun 2024 13:45:23 -0400 Subject: [PATCH 2/2] remove erb assignment --- app/views/idv/by_mail/request_letter/index.html.erb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/views/idv/by_mail/request_letter/index.html.erb b/app/views/idv/by_mail/request_letter/index.html.erb index 88d8bdfff71..730c829806c 100644 --- a/app/views/idv/by_mail/request_letter/index.html.erb +++ b/app/views/idv/by_mail/request_letter/index.html.erb @@ -42,13 +42,12 @@

- <%= start_over_link_html = link_to( - t('idv.messages.gpo.start_over_link_text'), - idv_confirm_start_over_before_letter_path, - ) - t( + <%= t( 'idv.messages.gpo.start_over_html', - start_over_link_html: start_over_link_html, + start_over_link_html: link_to( + t('idv.messages.gpo.start_over_link_text'), + idv_confirm_start_over_before_letter_path, + ), ) %>