From e2f0c5e45c462cfa20f2568a32cc116342c082dd Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 11 Jan 2023 13:00:50 -0800 Subject: [PATCH 01/36] Add test for Continue from verify_info screen Co-authored-by: Jonathan Hooper --- config/routes.rb | 1 + .../idv/doc_auth/verify_info_step_spec.rb | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 0fd0d0be542..3651e9c99b3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -310,6 +310,7 @@ get '/otp_delivery_method' => 'otp_delivery_method#new' put '/otp_delivery_method' => 'otp_delivery_method#create' get '/verify_info' => 'verify_info#show' + put '/verify_info' => 'verify_info#update' get '/phone' => 'phone#new' put '/phone' => 'phone#create' get '/phone/errors/warning' => 'phone_errors#warning' diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 7c05e118bfc..6e34014cb5f 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -3,12 +3,17 @@ feature 'doc auth verify_info step', :js do include IdvStepHelper include DocAuthHelper - include DocCaptureHelper + + let(:fake_analytics) { FakeAnalytics.new } + let(:fake_attempts_tracker) { IrsAttemptsApiTrackingHelper::FakeAttemptsTracker.new } context 'with verify_info_controller enabled' do before do allow(IdentityConfig.store).to receive(:doc_auth_verify_info_controller_enabled). and_return(true) + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics) + allow_any_instance_of(ApplicationController).to receive(:irs_attempts_api_tracker). + and_return(fake_attempts_tracker) sign_in_and_2fa_user complete_doc_auth_steps_before_verify_step end @@ -55,5 +60,35 @@ check t('forms.ssn.show') expect(page).to have_text('900-45-6789') end + + it 'proceeds to the next page upon confirmation' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: true, + failure_reason: nil, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '900-66-1234', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(page).to have_current_path(idv_phone_path) + expect(page).to have_content(t('doc_auth.forms.doc_success')) + user = User.first + expect(user.proofing_component.resolution_check).to eq(Idp::Constants::Vendors::LEXIS_NEXIS) + expect(user.proofing_component.source_check).to eq(Idp::Constants::Vendors::AAMVA) + expect(DocAuthLog.find_by(user_id: user.id).aamva).to eq(true) + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth optional verify_wait submitted', + hash_including(address_edited: false), + ) + end end end From 9d7bb48b5e2bafc70eac0cf716c4a3a445018dd7 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 12 Jan 2023 08:20:07 -0800 Subject: [PATCH 02/36] Add scaffolding to remove verify_wait from flow state machine Co-authored-by: Jonathan Hooper --- app/controllers/idv/verify_info_controller.rb | 59 +++++++++++++++++++ app/services/idv/session.rb | 1 + app/views/idv/verify_info/show.html.erb | 18 ++---- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index bba3356a78f..ce15826d7a0 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -1,5 +1,7 @@ module Idv class VerifyInfoController < ApplicationController + include IdvSession + before_action :render_404_if_verify_info_controller_disabled before_action :confirm_two_factor_authenticated before_action :confirm_ssn_step_complete @@ -9,6 +11,45 @@ def show analytics.idv_doc_auth_verify_visited(**analytics_arguments) end + def update + return if idv_session.idv_verify_info_step_document_capture_session_uuid + + pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id + + throttle = Throttle.new( + target: Pii::Fingerprinter.fingerprint(pii[:ssn]), + throttle_type: :proof_ssn, + ) + + if throttle.throttled_else_increment? + analytics.throttler_rate_limit_triggered( + throttle_type: :proof_ssn, + step_name: 'verify_info', + ) + redirect_to idv_session_errors_ssn_failure_url + return + end + + document_capture_session = DocumentCaptureSession.create( + user_id: current_user.id, + issuer: sp_session[:issuer], + ) + document_capture_session.requested_at = Time.zone.now + + idv_session.idv_verify_info_step_document_capture_session_uuid = document_capture_session.uuid + + Idv::Agent.new(pii).proof_resolution( + document_capture_session, + should_proof_state_id: should_use_aamva?(pii), + trace_id: amzn_trace_id, + user_id: current_user.id, + threatmetrix_session_id: flow_session[:threatmetrix_session_id], + request_ip: request.remote_ip, + ) + + redirect_to idv_verify_info_url + end + private def render_404_if_verify_info_controller_disabled @@ -74,5 +115,23 @@ def current_flow_step_counts def increment_step_counts current_flow_step_counts['verify'] += 1 end + + # copied from verify_base_step + def should_use_aamva?(pii) + aamva_state?(pii) && !aamva_disallowed_for_service_provider? + end + + def aamva_state?(pii) + IdentityConfig.store.aamva_supported_jurisdictions.include?( + pii['state_id_jurisdiction'], + ) + end + + def aamva_disallowed_for_service_provider? + return false if sp_session.nil? + banlist = IdentityConfig.store.aamva_sp_banlist_issuers + banlist.include?(sp_session[:issuer]) + end + end end diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index 506b5a675cf..a3f671047c4 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -4,6 +4,7 @@ class Session address_verification_mechanism applicant go_back_path + idv_verify_info_step_document_capture_session_uuid idv_phone_step_document_capture_session_uuid vendor_phone_confirmation user_phone_confirmation diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 793609d647d..85d6ab37d6d 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -90,25 +90,15 @@ locals:
- <%= render SpinnerButtonComponent.new( + <%= render ButtonComponent.new( action: ->(**tag_options, &block) do - button_to(url_for, **tag_options, &block) + button_to(idv_verify_info_url, **tag_options, &block) end, big: true, wide: true, - action_message: t('idv.messages.verifying'), method: :put, - form: { - class: 'button_to', - data: { - form_steps_wait: '', - error_message: t('idv.failure.exceptions.internal_error'), - alert_target: '#form-steps-wait-alert', - wait_step_path: idv_doc_auth_step_url(step: :verify_wait), - poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, - }, - }, - ).with_content(t('forms.buttons.continue')) %> + ).with_content(t('forms.buttons.continue')) + %>
From 25a5151d3fae815fdd79ec3ceb1b4b4559cdc11c Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 12 Jan 2023 16:13:12 -0800 Subject: [PATCH 03/36] Add more methods to make verify_info#update work Co-authored-by: Eric Gade Co-authored-by: Jonathan Hooper --- app/controllers/idv/verify_info_controller.rb | 264 +++++++++++++++++- app/services/analytics_events.rb | 4 + app/services/idv/session.rb | 3 +- app/views/idv/verify_info/show.html.erb | 25 ++ 4 files changed, 288 insertions(+), 8 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index ce15826d7a0..bf759346094 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -9,18 +9,17 @@ class VerifyInfoController < ApplicationController def show increment_step_counts analytics.idv_doc_auth_verify_visited(**analytics_arguments) + + redirect_to failure_url(:fail) and return if throttle.throttled? + + process_async_state(load_async_state) end def update - return if idv_session.idv_verify_info_step_document_capture_session_uuid + return if idv_session.verify_info_step_document_capture_session_uuid pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id - throttle = Throttle.new( - target: Pii::Fingerprinter.fingerprint(pii[:ssn]), - throttle_type: :proof_ssn, - ) - if throttle.throttled_else_increment? analytics.throttler_rate_limit_triggered( throttle_type: :proof_ssn, @@ -36,7 +35,9 @@ def update ) document_capture_session.requested_at = Time.zone.now - idv_session.idv_verify_info_step_document_capture_session_uuid = document_capture_session.uuid + idv_session.verify_info_step_document_capture_session_uuid = document_capture_session.uuid + idv_session.vendor_phone_confirmation = false + idv_session.user_phone_confirmation = false Idv::Agent.new(pii).proof_resolution( document_capture_session, @@ -100,6 +101,10 @@ def pii @pii = flow_session[:pii_from_doc] if flow_session end + def delete_pii + flow_session.delete(:pii_from_user) + end + # copied from address_controller def confirm_ssn_step_complete return if pii.present? && pii[:ssn].present? @@ -133,5 +138,250 @@ def aamva_disallowed_for_service_provider? banlist.include?(sp_session[:issuer]) end + def throttle + @throttle ||= Throttle.new( + target: Pii::Fingerprinter.fingerprint(pii[:ssn]), + throttle_type: :proof_ssn, + ) + end + + def idv_failure(result) + throttle.increment! if result.extra.dig(:proofing_results, :exception).blank? + if throttle.throttled? + idv_failure_log_throttled + redirect_to throttled_url + elsif result.extra.dig(:proofing_results, :exception).present? + idv_failure_log_error + redirect_to exception_url + else + idv_failure_log_warning + redirect_to warning_url + end + end + + def idv_failure_log_throttled + irs_attempts_api_tracker.idv_verification_rate_limited + analytics.throttler_rate_limit_triggered( + throttle_type: :idv_resolution, + step_name: self.class.name, + ) + end + + def idv_failure_log_error + analytics.idv_doc_auth_exception_visited( + step_name: self.class.name, + remaining_attempts: throttle.remaining_count, + ) + end + + def idv_failure_log_warning + analytics.idv_doc_auth_warning_visited( + step_name: self.class.name, + remaining_attempts: throttle.remaining_count, + ) + end + + def throttled_url + idv_session_errors_failure_url + end + + def exception_url + idv_session_errors_exception_url + end + + def warning_url + idv_session_errors_warning_url + end + + # copied from verify_base_step. May want reconciliation with phone_step + def process_async_state(current_async_state) + if current_async_state.none? + idv_session.resolution_info_verified = false + render :show + elsif current_async_state.in_progress? + render :wait + elsif current_async_state.missing? + analytics.idv_proofing_resolution_result_missing + flash.now[:error] = I18n.t('idv.failure.timeout') + render :show + + delete_async + idv_session.resolution_info_verified = false + + log_idv_verification_submitted_event( + success: false, + failure_reason: { idv_verification: [:timeout] }, + ) + elsif current_async_state.done? + async_state_done(current_async_state) + end + end + + def async_state_done(current_async_state) + add_proofing_costs(current_async_state.result) + form_response = idv_result_to_form_response( + result: current_async_state.result, + state: pii[:state], + state_id_jurisdiction: pii[:state_id_jurisdiction], + state_id_number: pii[:state_id_number], + # todo: add other edited fields? + extra: { + address_edited: !!flow_session['address_edited'], + pii_like_keypaths: [[:errors, :ssn], [:response_body, :first_name]], + }, + ) + log_idv_verification_submitted_event( + success: form_response.success?, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(form_response), + ) + + if form_response.success? + response = check_ssn + form_response = form_response.merge(response) + end + summarize_result_and_throttle_failures(form_response) + delete_async + + if form_response.success? + idv_session.resolution_info_verified = true + redirect_to idv_phone_url + else + idv_session.resolution_info_verified = false + end + + analytics.idv_doc_auth_verify_proofing_results(**form_response.to_h) + end + + def summarize_result_and_throttle_failures(summary_result) + if summary_result.success? + add_proofing_components + summary_result + else + idv_failure(summary_result) + end + end + + def add_proofing_components + ProofingComponent.create_or_find_by(user: current_user).update( + resolution_check: Idp::Constants::Vendors::LEXIS_NEXIS, + source_check: Idp::Constants::Vendors::AAMVA, + ) + end + + def load_async_state + dcs_uuid = idv_session.verify_info_step_document_capture_session_uuid + dcs = DocumentCaptureSession.find_by(uuid: dcs_uuid) + return ProofingSessionAsyncResult.none if dcs_uuid.nil? + return ProofingSessionAsyncResult.missing if dcs.nil? + + proofing_job_result = dcs.load_proofing_result + return ProofingSessionAsyncResult.missing if proofing_job_result.nil? + + proofing_job_result + end + + def delete_async + idv_session.verify_info_step_document_capture_session_uuid = nil + end + + def idv_result_to_form_response( + result:, + state: nil, + state_id_jurisdiction: nil, + state_id_number: nil, + extra: {} + ) + state_id = result.dig(:context, :stages, :state_id) + if state_id + state_id[:state] = state if state + state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction + state_id[:state_id_number] = redact(state_id_number) if state_id_number + end + FormResponse.new( + success: result[:success], + errors: result[:errors], + extra: extra.merge(proofing_results: result[:extra]), + ) + end + + def log_idv_verification_submitted_event(success: false, failure_reason: nil) + pii_from_doc = pii || {} + irs_attempts_api_tracker.idv_verification_submitted( + success: success, + document_state: pii_from_doc[:state], + document_number: pii_from_doc[:state_id_number], + document_issued: pii_from_doc[:state_id_issued], + document_expiration: pii_from_doc[:state_id_expiration], + first_name: pii_from_doc[:first_name], + last_name: pii_from_doc[:last_name], + date_of_birth: pii_from_doc[:dob], + address: pii_from_doc[:address1], + ssn: pii_from_doc[:ssn], + failure_reason: failure_reason, + ) + end + + def redact(text) + text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') + end + + def check_ssn + result = Idv::SsnForm.new(current_user).submit(ssn: pii[:ssn]) + + if result.success? + save_legacy_state + delete_pii + end + + result + end + + def save_legacy_state + skip_legacy_steps + idv_session.applicant = pii + idv_session.applicant['uuid'] = current_user.uuid + end + + def skip_legacy_steps + idv_session.profile_confirmation = true + idv_session.vendor_phone_confirmation = false + idv_session.user_phone_confirmation = false + idv_session.address_verification_mechanism = 'phone' + idv_session.resolution_successful = 'phone' + end + + def add_proofing_costs(results) + results[:context][:stages].each do |stage, hash| + if stage == :resolution + # transaction_id comes from ConversationId + add_cost(:lexis_nexis_resolution, transaction_id: hash[:transaction_id]) + elsif stage == :state_id + next if hash[:vendor_name] == 'UnsupportedJurisdiction' + process_aamva(hash[:transaction_id]) + elsif stage == :threatmetrix + # transaction_id comes from request_id + tmx_id = hash[:transaction_id] + add_cost(:threatmetrix, transaction_id: tmx_id) if tmx_id + end + end + end + + def process_aamva(transaction_id) + # transaction_id comes from TransactionLocatorId + add_cost(:aamva, transaction_id: transaction_id) + track_aamva + end + + def track_aamva + return unless IdentityConfig.store.state_tracking_enabled + doc_auth_log = DocAuthLog.find_by(user_id: current_user.id) + return unless doc_auth_log + doc_auth_log.aamva = true + doc_auth_log.save! + end + + def add_cost(token, transaction_id: nil) + Db::SpCost::AddSpCost.call(current_sp, 2, token, transaction_id: transaction_id) + end end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 8e24980d28e..db0b93f4d39 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -930,6 +930,10 @@ def idv_doc_auth_verify_visited(**extra) track_event('IdV: doc auth verify visited', **extra) end + def idv_doc_auth_verify_proofing_results(**extra) + track_event('IdV: doc auth verify proofing results', **extra) + end + # @identity.idp.previous_event_name IdV: in person proofing verify_wait visited def idv_doc_auth_verify_wait_step_visited(**extra) track_event('IdV: doc auth verify_wait visited', **extra) diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index a3f671047c4..e0a3e2d8eaa 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -4,7 +4,8 @@ class Session address_verification_mechanism applicant go_back_path - idv_verify_info_step_document_capture_session_uuid + verify_info_step_document_capture_session_uuid + resolution_info_verified idv_phone_step_document_capture_session_uuid vendor_phone_confirmation user_phone_confirmation diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 85d6ab37d6d..cadec175ba8 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -99,6 +99,31 @@ locals: method: :put, ).with_content(t('forms.buttons.continue')) %> + <%# <%= button_to( t('forms.buttons.continue'), + idv_verify_info_url, + method: :put, + class: 'usa-button usa-button--big usa-button--wide', + ) + %> + <%# <%= render SpinnerButtonComponent.new( + action: ->(**tag_options, &block) do + button_to(url_for, **tag_options, &block) + end, + big: true, + wide: true, + action_message: t('idv.messages.verifying'), + method: :put, + form: { + class: 'button_to', + data: { + form_steps_wait: '', + error_message: t('idv.failure.exceptions.internal_error'), + alert_target: '#form-steps-wait-alert', + wait_step_path: idv_doc_auth_step_url(step: :verify_wait), + poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, + }, + }, + ).with_content(t('forms.buttons.continue')) %> From 7085cd87b4cb4acdfbf4589d1ae4b1102aa86825 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 10:24:20 -0800 Subject: [PATCH 04/36] Update test to go to next step, now passes --- spec/features/idv/doc_auth/verify_info_step_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 6e34014cb5f..809496d289e 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -81,12 +81,12 @@ expect(page).to have_current_path(idv_phone_path) expect(page).to have_content(t('doc_auth.forms.doc_success')) - user = User.first + user = User.last expect(user.proofing_component.resolution_check).to eq(Idp::Constants::Vendors::LEXIS_NEXIS) expect(user.proofing_component.source_check).to eq(Idp::Constants::Vendors::AAMVA) expect(DocAuthLog.find_by(user_id: user.id).aamva).to eq(true) expect(fake_analytics).to have_logged_event( - 'IdV: doc auth optional verify_wait submitted', + 'IdV: doc auth verify proofing results', hash_including(address_edited: false), ) end From 60f511742632d946c5e1413f45b69d9692adcc2f Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 13:23:25 -0800 Subject: [PATCH 05/36] New specs and Idv::Session field change Replace resolution_info_verified with already existing resolution_info_verified Co-authored-by: Eric Gade --- app/controllers/idv/verify_info_controller.rb | 8 +- app/services/idv/session.rb | 1 - .../idv/doc_auth/verify_info_step_spec.rb | 225 ++++++++++++++++++ 3 files changed, 229 insertions(+), 5 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index bf759346094..3b550b92cc7 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -196,7 +196,7 @@ def warning_url # copied from verify_base_step. May want reconciliation with phone_step def process_async_state(current_async_state) if current_async_state.none? - idv_session.resolution_info_verified = false + idv_session.resolution_successful = false render :show elsif current_async_state.in_progress? render :wait @@ -206,7 +206,7 @@ def process_async_state(current_async_state) render :show delete_async - idv_session.resolution_info_verified = false + idv_session.resolution_successful = false log_idv_verification_submitted_event( success: false, @@ -243,10 +243,10 @@ def async_state_done(current_async_state) delete_async if form_response.success? - idv_session.resolution_info_verified = true + idv_session.resolution_successful = true redirect_to idv_phone_url else - idv_session.resolution_info_verified = false + idv_session.resolution_successful = false end analytics.idv_doc_auth_verify_proofing_results(**form_response.to_h) diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index e0a3e2d8eaa..d0c3ed03759 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -5,7 +5,6 @@ class Session applicant go_back_path verify_info_step_document_capture_session_uuid - resolution_info_verified idv_phone_step_document_capture_session_uuid vendor_phone_confirmation user_phone_confirmation diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 809496d289e..eac2e86a713 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -90,5 +90,230 @@ hash_including(address_edited: false), ) end + + it 'does not proceed to the next page if resolution fails' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: { ssn: ['Unverified SSN.'] }, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '123-45-6666', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_ssn_step + fill_out_ssn_form_with_ssn_that_fails_resolution + click_idv_continue + click_idv_continue + + expect(page).to have_current_path(idv_session_errors_warning_path) + click_on t('idv.failure.button.warning') + + expect(page).to have_current_path(idv_doc_auth_verify_step) + end + + it 'does not proceed to the next page if resolution raises an exception' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: nil, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '000-00-0000', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_ssn_step + fill_out_ssn_form_with_ssn_that_raises_exception + click_idv_continue + click_idv_continue + + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth exception visited', + step_name: 'Idv::Steps::VerifyWaitStepShow', + remaining_attempts: 5, + ) + expect(page).to have_current_path(idv_session_errors_exception_path) + + click_on t('idv.failure.button.warning') + + expect(page).to have_current_path(idv_doc_auth_verify_step) + end + + it 'throttles resolution and continues when it expires' do + expect(fake_attempts_tracker).to receive(:idv_verification_rate_limited) + sign_in_and_2fa_user + complete_doc_auth_steps_before_ssn_step + fill_out_ssn_form_with_ssn_that_fails_resolution + click_idv_continue + (max_attempts - 1).times do + click_idv_continue + expect(page).to have_current_path(idv_session_errors_warning_path) + visit idv_doc_auth_verify_step + end + click_idv_continue + expect(page).to have_current_path(idv_session_errors_failure_path) + expect(fake_analytics).to have_logged_event( + 'Throttler Rate Limit Triggered', + throttle_type: :idv_resolution, + step_name: 'Idv::Steps::VerifyWaitStepShow', + ) + travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(page).to have_current_path(idv_phone_path) + end + end + + context 'when the user lives in an AAMVA supported state' do + it 'performs a resolution and state ID check' do + allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( + [Idp::Constants::MOCK_IDV_APPLICANT[:state_id_jurisdiction]], + ) + user = create(:user, :signed_up) + expect_any_instance_of(Idv::Agent). + to receive(:proof_resolution). + with( + anything, + should_proof_state_id: true, + trace_id: anything, + threatmetrix_session_id: anything, + user_id: user.id, + request_ip: kind_of(String), + ). + and_call_original + + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(DocAuthLog.find_by(user_id: user.id).aamva).not_to be_nil + end + end + + context 'when the user does not live in an AAMVA supported state' do + it 'does not perform the state ID check' do + allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( + IdentityConfig.store.aamva_supported_jurisdictions - + [Idp::Constants::MOCK_IDV_APPLICANT[:state_id_jurisdiction]], + ) + user = create(:user, :signed_up) + expect_any_instance_of(Idv::Agent). + to receive(:proof_resolution). + with( + anything, + should_proof_state_id: false, + trace_id: anything, + threatmetrix_session_id: anything, + user_id: user.id, + request_ip: kind_of(String), + ). + and_call_original + + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(DocAuthLog.find_by(user_id: user.id).aamva).to be_nil + end + end + + context 'when the SP is in the AAMVA banlist' do + it 'does not perform the state ID check' do + allow(IdentityConfig.store).to receive(:aamva_sp_banlist_issuers). + and_return('["urn:gov:gsa:openidconnect:sp:server"]') + user = create(:user, :signed_up) + expect_any_instance_of(Idv::Agent). + to receive(:proof_resolution). + with( + anything, + should_proof_state_id: false, + trace_id: anything, + threatmetrix_session_id: anything, + user_id: user.id, + request_ip: kind_of(String), + ). + and_call_original + + visit_idp_from_sp_with_ial1(:oidc) + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(DocAuthLog.find_by(user_id: user.id).aamva).to be_nil + end + end + + context 'async missing' do + it 'allows resubmitting form' do + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(fake_analytics).to have_logged_event('Proofing Resolution Result Missing') + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + click_idv_continue + expect(page).to have_current_path(idv_phone_path) + end + + it 'tracks attempts tracker event with failure reason' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: { idv_verification: [:timeout] }, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '900-66-1234', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + end + end + + context 'async timed out' do + it 'allows resubmitting form' do + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + click_idv_continue + expect(page).to have_current_path(idv_phone_path) + end + end end end From d3dfc1affba8531fffeaf10db29de38afcb85061 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 15:13:57 -0800 Subject: [PATCH 06/36] Small ssn_step cleanup --- app/services/idv/steps/ssn_step.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/idv/steps/ssn_step.rb b/app/services/idv/steps/ssn_step.rb index 4e5e74a383a..f1b462f77ca 100644 --- a/app/services/idv/steps/ssn_step.rb +++ b/app/services/idv/steps/ssn_step.rb @@ -17,7 +17,6 @@ def call return invalid_state_response if invalid_state? flow_session[:pii_from_doc][:ssn] = ssn - add_verify_info_variables @flow.irs_attempts_api_tracker.idv_ssn_submitted( ssn: ssn, @@ -31,10 +30,6 @@ def call # rubocop:enable Style/IfUnlessModifier end - def add_verify_info_variables - flow_session[:flow_path] = @flow.flow_path - end - def extra_view_variables { updating_ssn: updating_ssn, @@ -68,6 +63,7 @@ def updating_ssn def exit_flow_state_machine mark_step_complete(:verify) mark_step_complete(:verify_wait) + flow_session[:flow_path] = @flow.flow_path redirect_to idv_verify_info_url end end From f0ed81f5c1dd6a2250bbe4d7a8f22612a9cf09ae Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 16:26:07 -0800 Subject: [PATCH 07/36] throttle and continue spec passes --- spec/features/idv/doc_auth/verify_info_step_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index eac2e86a713..59fe309dd8e 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -6,6 +6,7 @@ let(:fake_analytics) { FakeAnalytics.new } let(:fake_attempts_tracker) { IrsAttemptsApiTrackingHelper::FakeAttemptsTracker.new } + let(:max_attempts) { Throttle.max_attempts(:idv_resolution) } context 'with verify_info_controller enabled' do before do @@ -165,7 +166,7 @@ expect(fake_analytics).to have_logged_event( 'Throttler Rate Limit Triggered', throttle_type: :idv_resolution, - step_name: 'Idv::Steps::VerifyWaitStepShow', + step_name: 'Idv::VerifyInfoController', ) travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do sign_in_and_2fa_user From 64254dc0cf3f44bb39c575f72f2e5ac4d786235d Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 17 Jan 2023 13:01:22 -0800 Subject: [PATCH 08/36] Separate kinds of throttles Look for proofing results in :stages, :resolution rather than :extra Co-authored-by: Eric Gade --- app/controllers/idv/verify_info_controller.rb | 32 +++++++++++++------ .../idv/doc_auth/verify_info_step_spec.rb | 3 +- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 3b550b92cc7..e94a4061247 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -10,7 +10,7 @@ def show increment_step_counts analytics.idv_doc_auth_verify_visited(**analytics_arguments) - redirect_to failure_url(:fail) and return if throttle.throttled? + redirect_to failure_url(:fail) and return if any_throttled? process_async_state(load_async_state) end @@ -20,7 +20,7 @@ def update pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id - if throttle.throttled_else_increment? + if ssn_throttle.throttled_else_increment? analytics.throttler_rate_limit_triggered( throttle_type: :proof_ssn, step_name: 'verify_info', @@ -138,19 +138,30 @@ def aamva_disallowed_for_service_provider? banlist.include?(sp_session[:issuer]) end - def throttle - @throttle ||= Throttle.new( + def resolution_throttle + @resolution_throttle ||= Throttle.new( + user: current_user, + throttle_type: :idv_resolution, + ) + end + + def ssn_throttle + @ssn_throttle ||= Throttle.new( target: Pii::Fingerprinter.fingerprint(pii[:ssn]), throttle_type: :proof_ssn, ) end + def any_throttled? + ssn_throttle.throttled? || resolution_throttle.throttled? + end + def idv_failure(result) - throttle.increment! if result.extra.dig(:proofing_results, :exception).blank? - if throttle.throttled? + resolution_throttle.increment! if result.extra.dig(:proofing_results, :stages, :resolution, :exception).blank? + if resolution_throttle.throttled? idv_failure_log_throttled redirect_to throttled_url - elsif result.extra.dig(:proofing_results, :exception).present? + elsif result.extra.dig(:proofing_results, :stages, :resolution, :exception).present? idv_failure_log_error redirect_to exception_url else @@ -170,14 +181,14 @@ def idv_failure_log_throttled def idv_failure_log_error analytics.idv_doc_auth_exception_visited( step_name: self.class.name, - remaining_attempts: throttle.remaining_count, + remaining_attempts: resolution_throttle.remaining_count, ) end def idv_failure_log_warning analytics.idv_doc_auth_warning_visited( step_name: self.class.name, - remaining_attempts: throttle.remaining_count, + remaining_attempts: resolution_throttle.remaining_count, ) end @@ -297,10 +308,11 @@ def idv_result_to_form_response( state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction state_id[:state_id_number] = redact(state_id_number) if state_id_number end + FormResponse.new( success: result[:success], errors: result[:errors], - extra: extra.merge(proofing_results: result[:extra]), + extra: extra.merge(proofing_results: result[:context]), ) end diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 59fe309dd8e..292a424a972 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -135,12 +135,13 @@ sign_in_and_2fa_user complete_doc_auth_steps_before_ssn_step fill_out_ssn_form_with_ssn_that_raises_exception + click_idv_continue click_idv_continue expect(fake_analytics).to have_logged_event( 'IdV: doc auth exception visited', - step_name: 'Idv::Steps::VerifyWaitStepShow', + step_name: 'Idv::VerifyInfoController', remaining_attempts: 5, ) expect(page).to have_current_path(idv_session_errors_exception_path) From 0d8334dfa3b75c84c9dcd1f464d5456db3d04d56 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 18 Jan 2023 14:47:45 -0800 Subject: [PATCH 09/36] Verify_info feature tests passing Redirect back to verify_info if needed from review controller before action Proofing results exception found deeper in data structure on failure Co-authored-by: Eric Gade Co-authored-by: Alex Bradley --- app/controllers/idv/review_controller.rb | 11 +++++++++++ app/controllers/idv/verify_info_controller.rb | 8 ++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index 6857fa40b10..8506b956ddf 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -1,6 +1,7 @@ module Idv class ReviewController < ApplicationController before_action :personal_key_confirmed + before_action :confirm_verify_info_complete include IdvStepConcern include StepIndicatorConcern @@ -124,6 +125,16 @@ def password params.fetch(:user, {})[:password].presence end + def confirm_verify_info_complete + # rubocop:disable Style/IfUnlessModifier + if IdentityConfig.store.doc_auth_verify_info_controller_enabled + if !idv_session.resolution_successful + redirect_to idv_verify_info_url + end + end + # rubocop:enable Style/IfUnlessModifier + end + def personal_key_confirmed return unless current_user return unless current_user.active_profile.present? && need_personal_key_confirmation? diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index e94a4061247..029a17a1f91 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -157,11 +157,15 @@ def any_throttled? end def idv_failure(result) - resolution_throttle.increment! if result.extra.dig(:proofing_results, :stages, :resolution, :exception).blank? + proofing_results_exception = result.extra.dig( + :proofing_results, :stages, :resolution, + :exception + ) + resolution_throttle.increment! if proofing_results_exception.blank? if resolution_throttle.throttled? idv_failure_log_throttled redirect_to throttled_url - elsif result.extra.dig(:proofing_results, :stages, :resolution, :exception).present? + elsif proofing_results_exception.present? idv_failure_log_error redirect_to exception_url else From 3a2c085d3310bc6a6be60b2f8f29acd7fe3037d0 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 08:33:32 -0800 Subject: [PATCH 10/36] Remove line disabling lint and rearrange. --- app/controllers/idv/review_controller.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index 8506b956ddf..200e6758708 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -126,13 +126,10 @@ def password end def confirm_verify_info_complete - # rubocop:disable Style/IfUnlessModifier - if IdentityConfig.store.doc_auth_verify_info_controller_enabled - if !idv_session.resolution_successful - redirect_to idv_verify_info_url - end + if IdentityConfig.store.doc_auth_verify_info_controller_enabled && + !idv_session.resolution_successful + redirect_to idv_verify_info_url end - # rubocop:enable Style/IfUnlessModifier end def personal_key_confirmed From 41a6339aed4420c711c28ed2bbb425a62f7cc5ac Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:20:55 -0800 Subject: [PATCH 11/36] Create Concern for redact method per PR feedback. This is now available for a future PR to use in PhoneController and VerifyBaseStep. Co-authored-by: Alex Bradley Co-authored-by: Eric Gade --- app/controllers/concerns/string_redacter.rb | 7 +++++++ app/controllers/idv/verify_info_controller.rb | 7 ++----- 2 files changed, 9 insertions(+), 5 deletions(-) create mode 100644 app/controllers/concerns/string_redacter.rb diff --git a/app/controllers/concerns/string_redacter.rb b/app/controllers/concerns/string_redacter.rb new file mode 100644 index 00000000000..8896ed9da8d --- /dev/null +++ b/app/controllers/concerns/string_redacter.rb @@ -0,0 +1,7 @@ +module StringRedacter + extend ActiveSupport::Concern + + def redact_alphanumeric(text) + text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') + end +end diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 029a17a1f91..585605b1138 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -1,5 +1,6 @@ module Idv class VerifyInfoController < ApplicationController + include StringRedacter include IdvSession before_action :render_404_if_verify_info_controller_disabled @@ -310,7 +311,7 @@ def idv_result_to_form_response( if state_id state_id[:state] = state if state state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction - state_id[:state_id_number] = redact(state_id_number) if state_id_number + state_id[:state_id_number] = redact_alphanumeric(state_id_number) if state_id_number end FormResponse.new( @@ -337,10 +338,6 @@ def log_idv_verification_submitted_event(success: false, failure_reason: nil) ) end - def redact(text) - text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') - end - def check_ssn result = Idv::SsnForm.new(current_user).submit(ssn: pii[:ssn]) From 98c7232a6df555e940c3bea7e75e3cc9e4e4d9f6 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:28:56 -0800 Subject: [PATCH 12/36] Clean up comments and lint for verify_info show.html.erb --- app/views/idv/verify_info/show.html.erb | 27 +------------------------ 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index cadec175ba8..1928e2db62d 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -97,33 +97,8 @@ locals: big: true, wide: true, method: :put, - ).with_content(t('forms.buttons.continue')) + ).with_content(t('forms.buttons.continue')) %> - <%# <%= button_to( t('forms.buttons.continue'), - idv_verify_info_url, - method: :put, - class: 'usa-button usa-button--big usa-button--wide', - ) - %> - <%# <%= render SpinnerButtonComponent.new( - action: ->(**tag_options, &block) do - button_to(url_for, **tag_options, &block) - end, - big: true, - wide: true, - action_message: t('idv.messages.verifying'), - method: :put, - form: { - class: 'button_to', - data: { - form_steps_wait: '', - error_message: t('idv.failure.exceptions.internal_error'), - alert_target: '#form-steps-wait-alert', - wait_step_path: idv_doc_auth_step_url(step: :verify_wait), - poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, - }, - }, - ).with_content(t('forms.buttons.continue')) %> From fcd3ebec3ba098899aa43dce2246b764f5db04e7 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:33:03 -0800 Subject: [PATCH 13/36] Add changelog changelog: Internal improvements, Flow State Machine, Add update method to new VerifyInfoController From a37b79516cd84c8b11491d25ca416e3698b10342 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 11 Jan 2023 13:00:50 -0800 Subject: [PATCH 14/36] Add test for Continue from verify_info screen Co-authored-by: Jonathan Hooper --- config/routes.rb | 1 + .../idv/doc_auth/verify_info_step_spec.rb | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 005f2e3426d..04943c1284c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -308,6 +308,7 @@ get '/otp_delivery_method' => 'otp_delivery_method#new' put '/otp_delivery_method' => 'otp_delivery_method#create' get '/verify_info' => 'verify_info#show' + put '/verify_info' => 'verify_info#update' get '/phone' => 'phone#new' put '/phone' => 'phone#create' get '/phone/errors/warning' => 'phone_errors#warning' diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 7c05e118bfc..6e34014cb5f 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -3,12 +3,17 @@ feature 'doc auth verify_info step', :js do include IdvStepHelper include DocAuthHelper - include DocCaptureHelper + + let(:fake_analytics) { FakeAnalytics.new } + let(:fake_attempts_tracker) { IrsAttemptsApiTrackingHelper::FakeAttemptsTracker.new } context 'with verify_info_controller enabled' do before do allow(IdentityConfig.store).to receive(:doc_auth_verify_info_controller_enabled). and_return(true) + allow_any_instance_of(ApplicationController).to receive(:analytics).and_return(fake_analytics) + allow_any_instance_of(ApplicationController).to receive(:irs_attempts_api_tracker). + and_return(fake_attempts_tracker) sign_in_and_2fa_user complete_doc_auth_steps_before_verify_step end @@ -55,5 +60,35 @@ check t('forms.ssn.show') expect(page).to have_text('900-45-6789') end + + it 'proceeds to the next page upon confirmation' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: true, + failure_reason: nil, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '900-66-1234', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(page).to have_current_path(idv_phone_path) + expect(page).to have_content(t('doc_auth.forms.doc_success')) + user = User.first + expect(user.proofing_component.resolution_check).to eq(Idp::Constants::Vendors::LEXIS_NEXIS) + expect(user.proofing_component.source_check).to eq(Idp::Constants::Vendors::AAMVA) + expect(DocAuthLog.find_by(user_id: user.id).aamva).to eq(true) + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth optional verify_wait submitted', + hash_including(address_edited: false), + ) + end end end From b4ea51dad3d088c928095887a88bde8904624790 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 12 Jan 2023 08:20:07 -0800 Subject: [PATCH 15/36] Add scaffolding to remove verify_wait from flow state machine Co-authored-by: Jonathan Hooper --- app/controllers/idv/verify_info_controller.rb | 59 +++++++++++++++++++ app/services/idv/session.rb | 1 + app/views/idv/verify_info/show.html.erb | 18 ++---- 3 files changed, 64 insertions(+), 14 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index bba3356a78f..ce15826d7a0 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -1,5 +1,7 @@ module Idv class VerifyInfoController < ApplicationController + include IdvSession + before_action :render_404_if_verify_info_controller_disabled before_action :confirm_two_factor_authenticated before_action :confirm_ssn_step_complete @@ -9,6 +11,45 @@ def show analytics.idv_doc_auth_verify_visited(**analytics_arguments) end + def update + return if idv_session.idv_verify_info_step_document_capture_session_uuid + + pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id + + throttle = Throttle.new( + target: Pii::Fingerprinter.fingerprint(pii[:ssn]), + throttle_type: :proof_ssn, + ) + + if throttle.throttled_else_increment? + analytics.throttler_rate_limit_triggered( + throttle_type: :proof_ssn, + step_name: 'verify_info', + ) + redirect_to idv_session_errors_ssn_failure_url + return + end + + document_capture_session = DocumentCaptureSession.create( + user_id: current_user.id, + issuer: sp_session[:issuer], + ) + document_capture_session.requested_at = Time.zone.now + + idv_session.idv_verify_info_step_document_capture_session_uuid = document_capture_session.uuid + + Idv::Agent.new(pii).proof_resolution( + document_capture_session, + should_proof_state_id: should_use_aamva?(pii), + trace_id: amzn_trace_id, + user_id: current_user.id, + threatmetrix_session_id: flow_session[:threatmetrix_session_id], + request_ip: request.remote_ip, + ) + + redirect_to idv_verify_info_url + end + private def render_404_if_verify_info_controller_disabled @@ -74,5 +115,23 @@ def current_flow_step_counts def increment_step_counts current_flow_step_counts['verify'] += 1 end + + # copied from verify_base_step + def should_use_aamva?(pii) + aamva_state?(pii) && !aamva_disallowed_for_service_provider? + end + + def aamva_state?(pii) + IdentityConfig.store.aamva_supported_jurisdictions.include?( + pii['state_id_jurisdiction'], + ) + end + + def aamva_disallowed_for_service_provider? + return false if sp_session.nil? + banlist = IdentityConfig.store.aamva_sp_banlist_issuers + banlist.include?(sp_session[:issuer]) + end + end end diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index 4e5f9a396ff..73c433184b3 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -4,6 +4,7 @@ class Session address_verification_mechanism applicant go_back_path + idv_verify_info_step_document_capture_session_uuid idv_phone_step_document_capture_session_uuid vendor_phone_confirmation user_phone_confirmation diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 793609d647d..85d6ab37d6d 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -90,25 +90,15 @@ locals:
- <%= render SpinnerButtonComponent.new( + <%= render ButtonComponent.new( action: ->(**tag_options, &block) do - button_to(url_for, **tag_options, &block) + button_to(idv_verify_info_url, **tag_options, &block) end, big: true, wide: true, - action_message: t('idv.messages.verifying'), method: :put, - form: { - class: 'button_to', - data: { - form_steps_wait: '', - error_message: t('idv.failure.exceptions.internal_error'), - alert_target: '#form-steps-wait-alert', - wait_step_path: idv_doc_auth_step_url(step: :verify_wait), - poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, - }, - }, - ).with_content(t('forms.buttons.continue')) %> + ).with_content(t('forms.buttons.continue')) + %>
From 1d1f1d2bd8a839b4dbcdb0622beff808191e2778 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 12 Jan 2023 16:13:12 -0800 Subject: [PATCH 16/36] Add more methods to make verify_info#update work Co-authored-by: Eric Gade Co-authored-by: Jonathan Hooper --- app/controllers/idv/verify_info_controller.rb | 264 +++++++++++++++++- app/services/analytics_events.rb | 4 + app/services/idv/session.rb | 3 +- app/views/idv/verify_info/show.html.erb | 25 ++ 4 files changed, 288 insertions(+), 8 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index ce15826d7a0..bf759346094 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -9,18 +9,17 @@ class VerifyInfoController < ApplicationController def show increment_step_counts analytics.idv_doc_auth_verify_visited(**analytics_arguments) + + redirect_to failure_url(:fail) and return if throttle.throttled? + + process_async_state(load_async_state) end def update - return if idv_session.idv_verify_info_step_document_capture_session_uuid + return if idv_session.verify_info_step_document_capture_session_uuid pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id - throttle = Throttle.new( - target: Pii::Fingerprinter.fingerprint(pii[:ssn]), - throttle_type: :proof_ssn, - ) - if throttle.throttled_else_increment? analytics.throttler_rate_limit_triggered( throttle_type: :proof_ssn, @@ -36,7 +35,9 @@ def update ) document_capture_session.requested_at = Time.zone.now - idv_session.idv_verify_info_step_document_capture_session_uuid = document_capture_session.uuid + idv_session.verify_info_step_document_capture_session_uuid = document_capture_session.uuid + idv_session.vendor_phone_confirmation = false + idv_session.user_phone_confirmation = false Idv::Agent.new(pii).proof_resolution( document_capture_session, @@ -100,6 +101,10 @@ def pii @pii = flow_session[:pii_from_doc] if flow_session end + def delete_pii + flow_session.delete(:pii_from_user) + end + # copied from address_controller def confirm_ssn_step_complete return if pii.present? && pii[:ssn].present? @@ -133,5 +138,250 @@ def aamva_disallowed_for_service_provider? banlist.include?(sp_session[:issuer]) end + def throttle + @throttle ||= Throttle.new( + target: Pii::Fingerprinter.fingerprint(pii[:ssn]), + throttle_type: :proof_ssn, + ) + end + + def idv_failure(result) + throttle.increment! if result.extra.dig(:proofing_results, :exception).blank? + if throttle.throttled? + idv_failure_log_throttled + redirect_to throttled_url + elsif result.extra.dig(:proofing_results, :exception).present? + idv_failure_log_error + redirect_to exception_url + else + idv_failure_log_warning + redirect_to warning_url + end + end + + def idv_failure_log_throttled + irs_attempts_api_tracker.idv_verification_rate_limited + analytics.throttler_rate_limit_triggered( + throttle_type: :idv_resolution, + step_name: self.class.name, + ) + end + + def idv_failure_log_error + analytics.idv_doc_auth_exception_visited( + step_name: self.class.name, + remaining_attempts: throttle.remaining_count, + ) + end + + def idv_failure_log_warning + analytics.idv_doc_auth_warning_visited( + step_name: self.class.name, + remaining_attempts: throttle.remaining_count, + ) + end + + def throttled_url + idv_session_errors_failure_url + end + + def exception_url + idv_session_errors_exception_url + end + + def warning_url + idv_session_errors_warning_url + end + + # copied from verify_base_step. May want reconciliation with phone_step + def process_async_state(current_async_state) + if current_async_state.none? + idv_session.resolution_info_verified = false + render :show + elsif current_async_state.in_progress? + render :wait + elsif current_async_state.missing? + analytics.idv_proofing_resolution_result_missing + flash.now[:error] = I18n.t('idv.failure.timeout') + render :show + + delete_async + idv_session.resolution_info_verified = false + + log_idv_verification_submitted_event( + success: false, + failure_reason: { idv_verification: [:timeout] }, + ) + elsif current_async_state.done? + async_state_done(current_async_state) + end + end + + def async_state_done(current_async_state) + add_proofing_costs(current_async_state.result) + form_response = idv_result_to_form_response( + result: current_async_state.result, + state: pii[:state], + state_id_jurisdiction: pii[:state_id_jurisdiction], + state_id_number: pii[:state_id_number], + # todo: add other edited fields? + extra: { + address_edited: !!flow_session['address_edited'], + pii_like_keypaths: [[:errors, :ssn], [:response_body, :first_name]], + }, + ) + log_idv_verification_submitted_event( + success: form_response.success?, + failure_reason: irs_attempts_api_tracker.parse_failure_reason(form_response), + ) + + if form_response.success? + response = check_ssn + form_response = form_response.merge(response) + end + summarize_result_and_throttle_failures(form_response) + delete_async + + if form_response.success? + idv_session.resolution_info_verified = true + redirect_to idv_phone_url + else + idv_session.resolution_info_verified = false + end + + analytics.idv_doc_auth_verify_proofing_results(**form_response.to_h) + end + + def summarize_result_and_throttle_failures(summary_result) + if summary_result.success? + add_proofing_components + summary_result + else + idv_failure(summary_result) + end + end + + def add_proofing_components + ProofingComponent.create_or_find_by(user: current_user).update( + resolution_check: Idp::Constants::Vendors::LEXIS_NEXIS, + source_check: Idp::Constants::Vendors::AAMVA, + ) + end + + def load_async_state + dcs_uuid = idv_session.verify_info_step_document_capture_session_uuid + dcs = DocumentCaptureSession.find_by(uuid: dcs_uuid) + return ProofingSessionAsyncResult.none if dcs_uuid.nil? + return ProofingSessionAsyncResult.missing if dcs.nil? + + proofing_job_result = dcs.load_proofing_result + return ProofingSessionAsyncResult.missing if proofing_job_result.nil? + + proofing_job_result + end + + def delete_async + idv_session.verify_info_step_document_capture_session_uuid = nil + end + + def idv_result_to_form_response( + result:, + state: nil, + state_id_jurisdiction: nil, + state_id_number: nil, + extra: {} + ) + state_id = result.dig(:context, :stages, :state_id) + if state_id + state_id[:state] = state if state + state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction + state_id[:state_id_number] = redact(state_id_number) if state_id_number + end + FormResponse.new( + success: result[:success], + errors: result[:errors], + extra: extra.merge(proofing_results: result[:extra]), + ) + end + + def log_idv_verification_submitted_event(success: false, failure_reason: nil) + pii_from_doc = pii || {} + irs_attempts_api_tracker.idv_verification_submitted( + success: success, + document_state: pii_from_doc[:state], + document_number: pii_from_doc[:state_id_number], + document_issued: pii_from_doc[:state_id_issued], + document_expiration: pii_from_doc[:state_id_expiration], + first_name: pii_from_doc[:first_name], + last_name: pii_from_doc[:last_name], + date_of_birth: pii_from_doc[:dob], + address: pii_from_doc[:address1], + ssn: pii_from_doc[:ssn], + failure_reason: failure_reason, + ) + end + + def redact(text) + text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') + end + + def check_ssn + result = Idv::SsnForm.new(current_user).submit(ssn: pii[:ssn]) + + if result.success? + save_legacy_state + delete_pii + end + + result + end + + def save_legacy_state + skip_legacy_steps + idv_session.applicant = pii + idv_session.applicant['uuid'] = current_user.uuid + end + + def skip_legacy_steps + idv_session.profile_confirmation = true + idv_session.vendor_phone_confirmation = false + idv_session.user_phone_confirmation = false + idv_session.address_verification_mechanism = 'phone' + idv_session.resolution_successful = 'phone' + end + + def add_proofing_costs(results) + results[:context][:stages].each do |stage, hash| + if stage == :resolution + # transaction_id comes from ConversationId + add_cost(:lexis_nexis_resolution, transaction_id: hash[:transaction_id]) + elsif stage == :state_id + next if hash[:vendor_name] == 'UnsupportedJurisdiction' + process_aamva(hash[:transaction_id]) + elsif stage == :threatmetrix + # transaction_id comes from request_id + tmx_id = hash[:transaction_id] + add_cost(:threatmetrix, transaction_id: tmx_id) if tmx_id + end + end + end + + def process_aamva(transaction_id) + # transaction_id comes from TransactionLocatorId + add_cost(:aamva, transaction_id: transaction_id) + track_aamva + end + + def track_aamva + return unless IdentityConfig.store.state_tracking_enabled + doc_auth_log = DocAuthLog.find_by(user_id: current_user.id) + return unless doc_auth_log + doc_auth_log.aamva = true + doc_auth_log.save! + end + + def add_cost(token, transaction_id: nil) + Db::SpCost::AddSpCost.call(current_sp, 2, token, transaction_id: transaction_id) + end end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index bf1b5d114d6..9dc13a15d25 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -930,6 +930,10 @@ def idv_doc_auth_verify_visited(**extra) track_event('IdV: doc auth verify visited', **extra) end + def idv_doc_auth_verify_proofing_results(**extra) + track_event('IdV: doc auth verify proofing results', **extra) + end + # @identity.idp.previous_event_name IdV: in person proofing verify_wait visited def idv_doc_auth_verify_wait_step_visited(**extra) track_event('IdV: doc auth verify_wait visited', **extra) diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index 73c433184b3..d5e5fa5a2e9 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -4,7 +4,8 @@ class Session address_verification_mechanism applicant go_back_path - idv_verify_info_step_document_capture_session_uuid + verify_info_step_document_capture_session_uuid + resolution_info_verified idv_phone_step_document_capture_session_uuid vendor_phone_confirmation user_phone_confirmation diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 85d6ab37d6d..cadec175ba8 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -99,6 +99,31 @@ locals: method: :put, ).with_content(t('forms.buttons.continue')) %> + <%# <%= button_to( t('forms.buttons.continue'), + idv_verify_info_url, + method: :put, + class: 'usa-button usa-button--big usa-button--wide', + ) + %> + <%# <%= render SpinnerButtonComponent.new( + action: ->(**tag_options, &block) do + button_to(url_for, **tag_options, &block) + end, + big: true, + wide: true, + action_message: t('idv.messages.verifying'), + method: :put, + form: { + class: 'button_to', + data: { + form_steps_wait: '', + error_message: t('idv.failure.exceptions.internal_error'), + alert_target: '#form-steps-wait-alert', + wait_step_path: idv_doc_auth_step_url(step: :verify_wait), + poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, + }, + }, + ).with_content(t('forms.buttons.continue')) %> From e41beaf819e481523a4307dac27986b5054abf23 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 10:24:20 -0800 Subject: [PATCH 17/36] Update test to go to next step, now passes --- spec/features/idv/doc_auth/verify_info_step_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 6e34014cb5f..809496d289e 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -81,12 +81,12 @@ expect(page).to have_current_path(idv_phone_path) expect(page).to have_content(t('doc_auth.forms.doc_success')) - user = User.first + user = User.last expect(user.proofing_component.resolution_check).to eq(Idp::Constants::Vendors::LEXIS_NEXIS) expect(user.proofing_component.source_check).to eq(Idp::Constants::Vendors::AAMVA) expect(DocAuthLog.find_by(user_id: user.id).aamva).to eq(true) expect(fake_analytics).to have_logged_event( - 'IdV: doc auth optional verify_wait submitted', + 'IdV: doc auth verify proofing results', hash_including(address_edited: false), ) end From 56584955dad55346430f6b424165d56a918d5dfa Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 13:23:25 -0800 Subject: [PATCH 18/36] New specs and Idv::Session field change Replace resolution_info_verified with already existing resolution_info_verified Co-authored-by: Eric Gade --- app/controllers/idv/verify_info_controller.rb | 8 +- app/services/idv/session.rb | 1 - .../idv/doc_auth/verify_info_step_spec.rb | 225 ++++++++++++++++++ 3 files changed, 229 insertions(+), 5 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index bf759346094..3b550b92cc7 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -196,7 +196,7 @@ def warning_url # copied from verify_base_step. May want reconciliation with phone_step def process_async_state(current_async_state) if current_async_state.none? - idv_session.resolution_info_verified = false + idv_session.resolution_successful = false render :show elsif current_async_state.in_progress? render :wait @@ -206,7 +206,7 @@ def process_async_state(current_async_state) render :show delete_async - idv_session.resolution_info_verified = false + idv_session.resolution_successful = false log_idv_verification_submitted_event( success: false, @@ -243,10 +243,10 @@ def async_state_done(current_async_state) delete_async if form_response.success? - idv_session.resolution_info_verified = true + idv_session.resolution_successful = true redirect_to idv_phone_url else - idv_session.resolution_info_verified = false + idv_session.resolution_successful = false end analytics.idv_doc_auth_verify_proofing_results(**form_response.to_h) diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index d5e5fa5a2e9..3f7234bf895 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -5,7 +5,6 @@ class Session applicant go_back_path verify_info_step_document_capture_session_uuid - resolution_info_verified idv_phone_step_document_capture_session_uuid vendor_phone_confirmation user_phone_confirmation diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 809496d289e..eac2e86a713 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -90,5 +90,230 @@ hash_including(address_edited: false), ) end + + it 'does not proceed to the next page if resolution fails' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: { ssn: ['Unverified SSN.'] }, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '123-45-6666', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_ssn_step + fill_out_ssn_form_with_ssn_that_fails_resolution + click_idv_continue + click_idv_continue + + expect(page).to have_current_path(idv_session_errors_warning_path) + click_on t('idv.failure.button.warning') + + expect(page).to have_current_path(idv_doc_auth_verify_step) + end + + it 'does not proceed to the next page if resolution raises an exception' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: nil, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '000-00-0000', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_ssn_step + fill_out_ssn_form_with_ssn_that_raises_exception + click_idv_continue + click_idv_continue + + expect(fake_analytics).to have_logged_event( + 'IdV: doc auth exception visited', + step_name: 'Idv::Steps::VerifyWaitStepShow', + remaining_attempts: 5, + ) + expect(page).to have_current_path(idv_session_errors_exception_path) + + click_on t('idv.failure.button.warning') + + expect(page).to have_current_path(idv_doc_auth_verify_step) + end + + it 'throttles resolution and continues when it expires' do + expect(fake_attempts_tracker).to receive(:idv_verification_rate_limited) + sign_in_and_2fa_user + complete_doc_auth_steps_before_ssn_step + fill_out_ssn_form_with_ssn_that_fails_resolution + click_idv_continue + (max_attempts - 1).times do + click_idv_continue + expect(page).to have_current_path(idv_session_errors_warning_path) + visit idv_doc_auth_verify_step + end + click_idv_continue + expect(page).to have_current_path(idv_session_errors_failure_path) + expect(fake_analytics).to have_logged_event( + 'Throttler Rate Limit Triggered', + throttle_type: :idv_resolution, + step_name: 'Idv::Steps::VerifyWaitStepShow', + ) + travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(page).to have_current_path(idv_phone_path) + end + end + + context 'when the user lives in an AAMVA supported state' do + it 'performs a resolution and state ID check' do + allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( + [Idp::Constants::MOCK_IDV_APPLICANT[:state_id_jurisdiction]], + ) + user = create(:user, :signed_up) + expect_any_instance_of(Idv::Agent). + to receive(:proof_resolution). + with( + anything, + should_proof_state_id: true, + trace_id: anything, + threatmetrix_session_id: anything, + user_id: user.id, + request_ip: kind_of(String), + ). + and_call_original + + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(DocAuthLog.find_by(user_id: user.id).aamva).not_to be_nil + end + end + + context 'when the user does not live in an AAMVA supported state' do + it 'does not perform the state ID check' do + allow(IdentityConfig.store).to receive(:aamva_supported_jurisdictions).and_return( + IdentityConfig.store.aamva_supported_jurisdictions - + [Idp::Constants::MOCK_IDV_APPLICANT[:state_id_jurisdiction]], + ) + user = create(:user, :signed_up) + expect_any_instance_of(Idv::Agent). + to receive(:proof_resolution). + with( + anything, + should_proof_state_id: false, + trace_id: anything, + threatmetrix_session_id: anything, + user_id: user.id, + request_ip: kind_of(String), + ). + and_call_original + + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(DocAuthLog.find_by(user_id: user.id).aamva).to be_nil + end + end + + context 'when the SP is in the AAMVA banlist' do + it 'does not perform the state ID check' do + allow(IdentityConfig.store).to receive(:aamva_sp_banlist_issuers). + and_return('["urn:gov:gsa:openidconnect:sp:server"]') + user = create(:user, :signed_up) + expect_any_instance_of(Idv::Agent). + to receive(:proof_resolution). + with( + anything, + should_proof_state_id: false, + trace_id: anything, + threatmetrix_session_id: anything, + user_id: user.id, + request_ip: kind_of(String), + ). + and_call_original + + visit_idp_from_sp_with_ial1(:oidc) + sign_in_and_2fa_user(user) + complete_doc_auth_steps_before_verify_step + click_idv_continue + + expect(DocAuthLog.find_by(user_id: user.id).aamva).to be_nil + end + end + + context 'async missing' do + it 'allows resubmitting form' do + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(fake_analytics).to have_logged_event('Proofing Resolution Result Missing') + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + click_idv_continue + expect(page).to have_current_path(idv_phone_path) + end + + it 'tracks attempts tracker event with failure reason' do + expect(fake_attempts_tracker).to receive(:idv_verification_submitted).with( + success: false, + failure_reason: { idv_verification: [:timeout] }, + document_state: 'MT', + document_number: '1111111111111', + document_issued: '2019-12-31', + document_expiration: '2099-12-31', + first_name: 'FAKEY', + last_name: 'MCFAKERSON', + date_of_birth: '1938-10-06', + address: '1 FAKE RD', + ssn: '900-66-1234', + ) + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + end + end + + context 'async timed out' do + it 'allows resubmitting form' do + sign_in_and_2fa_user + complete_doc_auth_steps_before_verify_step + + allow(DocumentCaptureSession).to receive(:find_by). + and_return(nil) + + click_idv_continue + expect(page).to have_content(t('idv.failure.timeout')) + expect(page).to have_current_path(idv_doc_auth_verify_step) + allow(DocumentCaptureSession).to receive(:find_by).and_call_original + click_idv_continue + expect(page).to have_current_path(idv_phone_path) + end + end end end From cd038038348209ed5067a7a48edccbaba009ebd6 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 15:13:57 -0800 Subject: [PATCH 19/36] Small ssn_step cleanup --- app/services/idv/steps/ssn_step.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/services/idv/steps/ssn_step.rb b/app/services/idv/steps/ssn_step.rb index 4e5e74a383a..f1b462f77ca 100644 --- a/app/services/idv/steps/ssn_step.rb +++ b/app/services/idv/steps/ssn_step.rb @@ -17,7 +17,6 @@ def call return invalid_state_response if invalid_state? flow_session[:pii_from_doc][:ssn] = ssn - add_verify_info_variables @flow.irs_attempts_api_tracker.idv_ssn_submitted( ssn: ssn, @@ -31,10 +30,6 @@ def call # rubocop:enable Style/IfUnlessModifier end - def add_verify_info_variables - flow_session[:flow_path] = @flow.flow_path - end - def extra_view_variables { updating_ssn: updating_ssn, @@ -68,6 +63,7 @@ def updating_ssn def exit_flow_state_machine mark_step_complete(:verify) mark_step_complete(:verify_wait) + flow_session[:flow_path] = @flow.flow_path redirect_to idv_verify_info_url end end From 0d97dabd9e303413c6ff4ba6931b9e2df6171ec8 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 16:26:07 -0800 Subject: [PATCH 20/36] throttle and continue spec passes --- spec/features/idv/doc_auth/verify_info_step_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index eac2e86a713..59fe309dd8e 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -6,6 +6,7 @@ let(:fake_analytics) { FakeAnalytics.new } let(:fake_attempts_tracker) { IrsAttemptsApiTrackingHelper::FakeAttemptsTracker.new } + let(:max_attempts) { Throttle.max_attempts(:idv_resolution) } context 'with verify_info_controller enabled' do before do @@ -165,7 +166,7 @@ expect(fake_analytics).to have_logged_event( 'Throttler Rate Limit Triggered', throttle_type: :idv_resolution, - step_name: 'Idv::Steps::VerifyWaitStepShow', + step_name: 'Idv::VerifyInfoController', ) travel_to(IdentityConfig.store.idv_attempt_window_in_hours.hours.from_now + 1) do sign_in_and_2fa_user From f3c3d86fe486d4cf2a5e5ed8cc5b6530c5718f30 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 17 Jan 2023 13:01:22 -0800 Subject: [PATCH 21/36] Separate kinds of throttles Look for proofing results in :stages, :resolution rather than :extra Co-authored-by: Eric Gade --- app/controllers/idv/verify_info_controller.rb | 32 +++++++++++++------ .../idv/doc_auth/verify_info_step_spec.rb | 3 +- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 3b550b92cc7..e94a4061247 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -10,7 +10,7 @@ def show increment_step_counts analytics.idv_doc_auth_verify_visited(**analytics_arguments) - redirect_to failure_url(:fail) and return if throttle.throttled? + redirect_to failure_url(:fail) and return if any_throttled? process_async_state(load_async_state) end @@ -20,7 +20,7 @@ def update pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id - if throttle.throttled_else_increment? + if ssn_throttle.throttled_else_increment? analytics.throttler_rate_limit_triggered( throttle_type: :proof_ssn, step_name: 'verify_info', @@ -138,19 +138,30 @@ def aamva_disallowed_for_service_provider? banlist.include?(sp_session[:issuer]) end - def throttle - @throttle ||= Throttle.new( + def resolution_throttle + @resolution_throttle ||= Throttle.new( + user: current_user, + throttle_type: :idv_resolution, + ) + end + + def ssn_throttle + @ssn_throttle ||= Throttle.new( target: Pii::Fingerprinter.fingerprint(pii[:ssn]), throttle_type: :proof_ssn, ) end + def any_throttled? + ssn_throttle.throttled? || resolution_throttle.throttled? + end + def idv_failure(result) - throttle.increment! if result.extra.dig(:proofing_results, :exception).blank? - if throttle.throttled? + resolution_throttle.increment! if result.extra.dig(:proofing_results, :stages, :resolution, :exception).blank? + if resolution_throttle.throttled? idv_failure_log_throttled redirect_to throttled_url - elsif result.extra.dig(:proofing_results, :exception).present? + elsif result.extra.dig(:proofing_results, :stages, :resolution, :exception).present? idv_failure_log_error redirect_to exception_url else @@ -170,14 +181,14 @@ def idv_failure_log_throttled def idv_failure_log_error analytics.idv_doc_auth_exception_visited( step_name: self.class.name, - remaining_attempts: throttle.remaining_count, + remaining_attempts: resolution_throttle.remaining_count, ) end def idv_failure_log_warning analytics.idv_doc_auth_warning_visited( step_name: self.class.name, - remaining_attempts: throttle.remaining_count, + remaining_attempts: resolution_throttle.remaining_count, ) end @@ -297,10 +308,11 @@ def idv_result_to_form_response( state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction state_id[:state_id_number] = redact(state_id_number) if state_id_number end + FormResponse.new( success: result[:success], errors: result[:errors], - extra: extra.merge(proofing_results: result[:extra]), + extra: extra.merge(proofing_results: result[:context]), ) end diff --git a/spec/features/idv/doc_auth/verify_info_step_spec.rb b/spec/features/idv/doc_auth/verify_info_step_spec.rb index 59fe309dd8e..292a424a972 100644 --- a/spec/features/idv/doc_auth/verify_info_step_spec.rb +++ b/spec/features/idv/doc_auth/verify_info_step_spec.rb @@ -135,12 +135,13 @@ sign_in_and_2fa_user complete_doc_auth_steps_before_ssn_step fill_out_ssn_form_with_ssn_that_raises_exception + click_idv_continue click_idv_continue expect(fake_analytics).to have_logged_event( 'IdV: doc auth exception visited', - step_name: 'Idv::Steps::VerifyWaitStepShow', + step_name: 'Idv::VerifyInfoController', remaining_attempts: 5, ) expect(page).to have_current_path(idv_session_errors_exception_path) From 383b95f3e5ad99e2f28b14710ae16a6a1c6cf389 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 18 Jan 2023 14:47:45 -0800 Subject: [PATCH 22/36] Verify_info feature tests passing Redirect back to verify_info if needed from review controller before action Proofing results exception found deeper in data structure on failure Co-authored-by: Eric Gade Co-authored-by: Alex Bradley --- app/controllers/idv/review_controller.rb | 11 +++++++++++ app/controllers/idv/verify_info_controller.rb | 8 ++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index 6857fa40b10..8506b956ddf 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -1,6 +1,7 @@ module Idv class ReviewController < ApplicationController before_action :personal_key_confirmed + before_action :confirm_verify_info_complete include IdvStepConcern include StepIndicatorConcern @@ -124,6 +125,16 @@ def password params.fetch(:user, {})[:password].presence end + def confirm_verify_info_complete + # rubocop:disable Style/IfUnlessModifier + if IdentityConfig.store.doc_auth_verify_info_controller_enabled + if !idv_session.resolution_successful + redirect_to idv_verify_info_url + end + end + # rubocop:enable Style/IfUnlessModifier + end + def personal_key_confirmed return unless current_user return unless current_user.active_profile.present? && need_personal_key_confirmation? diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index e94a4061247..029a17a1f91 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -157,11 +157,15 @@ def any_throttled? end def idv_failure(result) - resolution_throttle.increment! if result.extra.dig(:proofing_results, :stages, :resolution, :exception).blank? + proofing_results_exception = result.extra.dig( + :proofing_results, :stages, :resolution, + :exception + ) + resolution_throttle.increment! if proofing_results_exception.blank? if resolution_throttle.throttled? idv_failure_log_throttled redirect_to throttled_url - elsif result.extra.dig(:proofing_results, :stages, :resolution, :exception).present? + elsif proofing_results_exception.present? idv_failure_log_error redirect_to exception_url else From 63ce60b70782b77340eadd75367ba3637af2ed95 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 08:33:32 -0800 Subject: [PATCH 23/36] Remove line disabling lint and rearrange. --- app/controllers/idv/review_controller.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/controllers/idv/review_controller.rb b/app/controllers/idv/review_controller.rb index 8506b956ddf..200e6758708 100644 --- a/app/controllers/idv/review_controller.rb +++ b/app/controllers/idv/review_controller.rb @@ -126,13 +126,10 @@ def password end def confirm_verify_info_complete - # rubocop:disable Style/IfUnlessModifier - if IdentityConfig.store.doc_auth_verify_info_controller_enabled - if !idv_session.resolution_successful - redirect_to idv_verify_info_url - end + if IdentityConfig.store.doc_auth_verify_info_controller_enabled && + !idv_session.resolution_successful + redirect_to idv_verify_info_url end - # rubocop:enable Style/IfUnlessModifier end def personal_key_confirmed From 8a9d0c01ce6c26224d9256122bf0713801a87ee2 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:20:55 -0800 Subject: [PATCH 24/36] Create Concern for redact method per PR feedback. This is now available for a future PR to use in PhoneController and VerifyBaseStep. Co-authored-by: Alex Bradley Co-authored-by: Eric Gade --- app/controllers/concerns/string_redacter.rb | 7 +++++++ app/controllers/idv/verify_info_controller.rb | 7 ++----- 2 files changed, 9 insertions(+), 5 deletions(-) create mode 100644 app/controllers/concerns/string_redacter.rb diff --git a/app/controllers/concerns/string_redacter.rb b/app/controllers/concerns/string_redacter.rb new file mode 100644 index 00000000000..8896ed9da8d --- /dev/null +++ b/app/controllers/concerns/string_redacter.rb @@ -0,0 +1,7 @@ +module StringRedacter + extend ActiveSupport::Concern + + def redact_alphanumeric(text) + text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') + end +end diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 029a17a1f91..585605b1138 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -1,5 +1,6 @@ module Idv class VerifyInfoController < ApplicationController + include StringRedacter include IdvSession before_action :render_404_if_verify_info_controller_disabled @@ -310,7 +311,7 @@ def idv_result_to_form_response( if state_id state_id[:state] = state if state state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction - state_id[:state_id_number] = redact(state_id_number) if state_id_number + state_id[:state_id_number] = redact_alphanumeric(state_id_number) if state_id_number end FormResponse.new( @@ -337,10 +338,6 @@ def log_idv_verification_submitted_event(success: false, failure_reason: nil) ) end - def redact(text) - text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') - end - def check_ssn result = Idv::SsnForm.new(current_user).submit(ssn: pii[:ssn]) From 4c4068e1734ac6d3f8bcf3a07b54e2cf65692634 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:28:56 -0800 Subject: [PATCH 25/36] Clean up comments and lint for verify_info show.html.erb --- app/views/idv/verify_info/show.html.erb | 27 +------------------------ 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index cadec175ba8..1928e2db62d 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -97,33 +97,8 @@ locals: big: true, wide: true, method: :put, - ).with_content(t('forms.buttons.continue')) + ).with_content(t('forms.buttons.continue')) %> - <%# <%= button_to( t('forms.buttons.continue'), - idv_verify_info_url, - method: :put, - class: 'usa-button usa-button--big usa-button--wide', - ) - %> - <%# <%= render SpinnerButtonComponent.new( - action: ->(**tag_options, &block) do - button_to(url_for, **tag_options, &block) - end, - big: true, - wide: true, - action_message: t('idv.messages.verifying'), - method: :put, - form: { - class: 'button_to', - data: { - form_steps_wait: '', - error_message: t('idv.failure.exceptions.internal_error'), - alert_target: '#form-steps-wait-alert', - wait_step_path: idv_doc_auth_step_url(step: :verify_wait), - poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, - }, - }, - ).with_content(t('forms.buttons.continue')) %> From 4efe550078f014d0e8040b0bb16ae95ffbc0a050 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Fri, 20 Jan 2023 16:16:14 -0500 Subject: [PATCH 26/36] fix an issue where exceptions were not appearing in the proofing result as expected --- app/controllers/idv/verify_info_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 585605b1138..87afd2c4305 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -158,10 +158,8 @@ def any_throttled? end def idv_failure(result) - proofing_results_exception = result.extra.dig( - :proofing_results, :stages, :resolution, - :exception - ) + proofing_results_exception = result.extra.dig(:proofing_results, :exception) + resolution_throttle.increment! if proofing_results_exception.blank? if resolution_throttle.throttled? idv_failure_log_throttled @@ -317,7 +315,7 @@ def idv_result_to_form_response( FormResponse.new( success: result[:success], errors: result[:errors], - extra: extra.merge(proofing_results: result[:context]), + extra: extra.merge(proofing_results: result.except(:errors, :success)), ) end From 6c8669d283cbe8f6ebe58220f186611bf201efb6 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 20 Jan 2023 13:52:43 -0800 Subject: [PATCH 27/36] Add changelog changelog: Internal, Flow State Machine, Add update action for new VerifyInfoController From 9e3bc332d3734401c4e93417a90a40aa0b11d79d Mon Sep 17 00:00:00 2001 From: eric-gade Date: Thu, 19 Jan 2023 17:46:04 -0500 Subject: [PATCH 28/36] Add spinner button to verify_info_controller#show Also adding the wait template Co-authored-by: Sonia Connolly Co-authored-by: Alex Bradley [skip changelog] --- app/views/idv/verify_info/show.html.erb | 18 ++++++++++++++---- app/views/idv/verify_info/wait.html.erb | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 app/views/idv/verify_info/wait.html.erb diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 1928e2db62d..9f207246269 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -90,15 +90,25 @@ locals:
- <%= render ButtonComponent.new( + <%= render SpinnerButtonComponent.new( action: ->(**tag_options, &block) do - button_to(idv_verify_info_url, **tag_options, &block) + button_to(idv_verify_info_path, **tag_options, &block) end, big: true, wide: true, + action_message: t('idv.messages.verifying'), method: :put, - ).with_content(t('forms.buttons.continue')) - %> + form: { + class: 'button_to', + data: { + form_steps_wait: '', + error_message: t('idv.failure.exceptions.internal_error'), + alert_target: '#form-steps-wait-alert', + wait_step_path: idv_verify_info_url, + poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, + }, + }, + ).with_content(t('forms.buttons.continue')) %>
diff --git a/app/views/idv/verify_info/wait.html.erb b/app/views/idv/verify_info/wait.html.erb new file mode 100644 index 00000000000..5cbd1bb081b --- /dev/null +++ b/app/views/idv/verify_info/wait.html.erb @@ -0,0 +1,4 @@ +<%= content_for(:meta_refresh) { '15' } %> +<% title t('titles.doc_auth.verify') %> + +<%= render PageHeadingComponent.new.with_content(t('doc_auth.info.interstitial_eta')) %> From 1f24aeba98818e80dfb351500f2f3e6030e9b332 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Fri, 20 Jan 2023 12:52:05 -0500 Subject: [PATCH 29/36] Show error alerts when verify info is throttled --- app/controllers/idv/verify_info_controller.rb | 2 +- app/views/idv/verify_info/show.html.erb | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 87afd2c4305..6ff24c5c103 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -11,7 +11,7 @@ def show increment_step_counts analytics.idv_doc_auth_verify_visited(**analytics_arguments) - redirect_to failure_url(:fail) and return if any_throttled? + redirect_to throttled_url and return if any_throttled? process_async_state(load_async_state) end diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 9f207246269..70c24401e0a 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -11,6 +11,10 @@ locals: ) %> <% end %> +
+ +
+ <% title t('titles.idv.verify_info') %> <%= render PageHeadingComponent.new.with_content(t('headings.verify')) %> @@ -104,7 +108,7 @@ locals: form_steps_wait: '', error_message: t('idv.failure.exceptions.internal_error'), alert_target: '#form-steps-wait-alert', - wait_step_path: idv_verify_info_url, + wait_step_path: idv_verify_info_path, poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, }, }, From 254b239f93fe539f2eabeab5ba6a2e330f70eeef Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 23 Jan 2023 11:12:06 -0800 Subject: [PATCH 30/36] Use StringRedacter instead of redact method [skip changelog] --- app/services/idv/steps/verify_base_step.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/services/idv/steps/verify_base_step.rb b/app/services/idv/steps/verify_base_step.rb index 9425e82ad52..0400afc330f 100644 --- a/app/services/idv/steps/verify_base_step.rb +++ b/app/services/idv/steps/verify_base_step.rb @@ -1,6 +1,8 @@ module Idv module Steps class VerifyBaseStep < DocAuthBaseStep + include StringRedacter + private def summarize_result_and_throttle_failures(summary_result) @@ -296,7 +298,7 @@ def idv_result_to_form_response( if state_id state_id[:state] = state if state state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction - state_id[:state_id_number] = redact(state_id_number) if state_id_number + state_id[:state_id_number] = redact_alphanumeric(state_id_number) if state_id_number end FormResponse.new( success: idv_success(result), @@ -305,10 +307,6 @@ def idv_result_to_form_response( ) end - def redact(text) - text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') - end - def log_idv_verification_submitted_event(success: false, failure_reason: nil) pii_from_doc = pii || {} @flow.irs_attempts_api_tracker.idv_verification_submitted( From ac053c82eed28231f7a6a2bb8708cf925a4b9f26 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 23 Jan 2023 11:13:21 -0800 Subject: [PATCH 31/36] Use StringRedacter instead of redact method Move test from NewPhoneForm to new StringRedacter spec --- app/forms/new_phone_form.rb | 7 ++----- spec/controllers/concerns/string_redacter_spec.rb | 15 +++++++++++++++ spec/forms/new_phone_form_spec.rb | 6 ------ 3 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 spec/controllers/concerns/string_redacter_spec.rb diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index 3b60bc1021e..a46cf9a99bb 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -2,6 +2,7 @@ class NewPhoneForm include ActiveModel::Model include FormPhoneValidator include OtpDeliveryPreferenceValidator + include StringRedacter BLOCKED_PHONE_TYPES = [ :premium_rate, @@ -58,7 +59,7 @@ def phone_info @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) rescue Aws::Pinpoint::Errors::BadRequestException errors.add(:phone, :improbable_phone, type: :improbable_phone) - @redacted_phone = redact(phone) + @redacted_phone = redact_alphanumeric(phone) @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) end @@ -139,10 +140,6 @@ def ingest_submitted_params(params) self.otp_make_default_number = true if default_prefs end - def redact(phone) - phone.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') - end - def confirmed_phone? false end diff --git a/spec/controllers/concerns/string_redacter_spec.rb b/spec/controllers/concerns/string_redacter_spec.rb new file mode 100644 index 00000000000..477339caec6 --- /dev/null +++ b/spec/controllers/concerns/string_redacter_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +describe 'StringRedacter' do + module Idv + class RedactTestController < ApplicationController + include StringRedacter + end + end + + describe '#redact_alphanumeric' do + it 'leaves in punctuation and spaces, but removes letters and numbers' do + expect(Idv::RedactTestController.new.redact_alphanumeric('+11 (555) DEF-1234')).to eq('+## (###) XXX-####') + end + end +end \ No newline at end of file diff --git a/spec/forms/new_phone_form_spec.rb b/spec/forms/new_phone_form_spec.rb index 9a1a9a561f1..01105092a8c 100644 --- a/spec/forms/new_phone_form_spec.rb +++ b/spec/forms/new_phone_form_spec.rb @@ -382,10 +382,4 @@ end end end - - describe '#redact' do - it 'leaves in punctuation and spaces, but removes letters and numbers' do - expect(form.send(:redact, '+11 (555) DEF-1234')).to eq('+## (###) XXX-####') - end - end end From 4c5a72ae25cbea943c942a67db1e7555e9e6bcb5 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 24 Jan 2023 10:39:20 -0800 Subject: [PATCH 32/36] Lint --- app/views/idv/verify_info/show.html.erb | 30 +++++++++---------- .../concerns/string_redacter_spec.rb | 13 ++++---- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/app/views/idv/verify_info/show.html.erb b/app/views/idv/verify_info/show.html.erb index 70c24401e0a..aa438ea28fa 100644 --- a/app/views/idv/verify_info/show.html.erb +++ b/app/views/idv/verify_info/show.html.erb @@ -95,24 +95,24 @@ locals:
<%= render SpinnerButtonComponent.new( - action: ->(**tag_options, &block) do - button_to(idv_verify_info_path, **tag_options, &block) - end, - big: true, - wide: true, - action_message: t('idv.messages.verifying'), - method: :put, - form: { + action: ->(**tag_options, &block) do + button_to(idv_verify_info_path, **tag_options, &block) + end, + big: true, + wide: true, + action_message: t('idv.messages.verifying'), + method: :put, + form: { class: 'button_to', data: { - form_steps_wait: '', - error_message: t('idv.failure.exceptions.internal_error'), - alert_target: '#form-steps-wait-alert', - wait_step_path: idv_verify_info_path, - poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, + form_steps_wait: '', + error_message: t('idv.failure.exceptions.internal_error'), + alert_target: '#form-steps-wait-alert', + wait_step_path: idv_verify_info_path, + poll_interval_ms: IdentityConfig.store.poll_rate_for_verify_in_seconds * 1000, }, - }, - ).with_content(t('forms.buttons.continue')) %> + }, + ).with_content(t('forms.buttons.continue')) %>
diff --git a/spec/controllers/concerns/string_redacter_spec.rb b/spec/controllers/concerns/string_redacter_spec.rb index 477339caec6..a49f57e1aba 100644 --- a/spec/controllers/concerns/string_redacter_spec.rb +++ b/spec/controllers/concerns/string_redacter_spec.rb @@ -1,15 +1,16 @@ require 'rails_helper' describe 'StringRedacter' do - module Idv - class RedactTestController < ApplicationController - include StringRedacter - end + module Idv + class RedactTestController < ApplicationController + include StringRedacter end + end describe '#redact_alphanumeric' do it 'leaves in punctuation and spaces, but removes letters and numbers' do - expect(Idv::RedactTestController.new.redact_alphanumeric('+11 (555) DEF-1234')).to eq('+## (###) XXX-####') + expect(Idv::RedactTestController.new.redact_alphanumeric('+11 (555) DEF-1234')). + to eq('+## (###) XXX-####') end end -end \ No newline at end of file +end From 70c79e6673d90e1cd56d459ea4375535e62039aa Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 24 Jan 2023 11:42:52 -0800 Subject: [PATCH 33/36] Make redact_alphanumeric a static method per PR feedback --- app/controllers/concerns/string_redacter.rb | 2 +- app/controllers/idv/verify_info_controller.rb | 5 ++++- app/forms/new_phone_form.rb | 2 +- spec/controllers/concerns/string_redacter_spec.rb | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/concerns/string_redacter.rb b/app/controllers/concerns/string_redacter.rb index 8896ed9da8d..128d6955844 100644 --- a/app/controllers/concerns/string_redacter.rb +++ b/app/controllers/concerns/string_redacter.rb @@ -1,5 +1,5 @@ module StringRedacter - extend ActiveSupport::Concern + module_function def redact_alphanumeric(text) text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 6ff24c5c103..682584c5a66 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -309,7 +309,10 @@ def idv_result_to_form_response( if state_id state_id[:state] = state if state state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction - state_id[:state_id_number] = redact_alphanumeric(state_id_number) if state_id_number + if state_id_number + state_id[:state_id_number] = + StringRedacter.redact_alphanumeric(state_id_number) + end end FormResponse.new( diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index a46cf9a99bb..6202d717818 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -59,7 +59,7 @@ def phone_info @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) rescue Aws::Pinpoint::Errors::BadRequestException errors.add(:phone, :improbable_phone, type: :improbable_phone) - @redacted_phone = redact_alphanumeric(phone) + @redacted_phone = StringRedacter.redact_alphanumeric(phone) @phone_info = Telephony::PhoneNumberInfo.new(type: :unknown) end diff --git a/spec/controllers/concerns/string_redacter_spec.rb b/spec/controllers/concerns/string_redacter_spec.rb index a49f57e1aba..6a583b53db7 100644 --- a/spec/controllers/concerns/string_redacter_spec.rb +++ b/spec/controllers/concerns/string_redacter_spec.rb @@ -9,7 +9,7 @@ class RedactTestController < ApplicationController describe '#redact_alphanumeric' do it 'leaves in punctuation and spaces, but removes letters and numbers' do - expect(Idv::RedactTestController.new.redact_alphanumeric('+11 (555) DEF-1234')). + expect(StringRedacter.redact_alphanumeric('+11 (555) DEF-1234')). to eq('+## (###) XXX-####') end end From 7f3a351beea39fdb2ea30e40f49c2d5ffee606c8 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 24 Jan 2023 11:47:34 -0800 Subject: [PATCH 34/36] move string_redacter under services --- app/{controllers/concerns => services}/string_redacter.rb | 0 spec/{controllers/concerns => services}/string_redacter_spec.rb | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename app/{controllers/concerns => services}/string_redacter.rb (100%) rename spec/{controllers/concerns => services}/string_redacter_spec.rb (100%) diff --git a/app/controllers/concerns/string_redacter.rb b/app/services/string_redacter.rb similarity index 100% rename from app/controllers/concerns/string_redacter.rb rename to app/services/string_redacter.rb diff --git a/spec/controllers/concerns/string_redacter_spec.rb b/spec/services/string_redacter_spec.rb similarity index 100% rename from spec/controllers/concerns/string_redacter_spec.rb rename to spec/services/string_redacter_spec.rb From 9b5458fd7a592927fbed3a69f399e08ebc2c14fd Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 24 Jan 2023 13:19:05 -0800 Subject: [PATCH 35/36] Remove unnecessary controller from spec --- spec/services/string_redacter_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/services/string_redacter_spec.rb b/spec/services/string_redacter_spec.rb index 6a583b53db7..14574db27f9 100644 --- a/spec/services/string_redacter_spec.rb +++ b/spec/services/string_redacter_spec.rb @@ -1,12 +1,6 @@ require 'rails_helper' describe 'StringRedacter' do - module Idv - class RedactTestController < ApplicationController - include StringRedacter - end - end - describe '#redact_alphanumeric' do it 'leaves in punctuation and spaces, but removes letters and numbers' do expect(StringRedacter.redact_alphanumeric('+11 (555) DEF-1234')). From 3bda211fc0bc595c9717fdb6182ec3e9c8b5ab79 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 24 Jan 2023 13:29:36 -0800 Subject: [PATCH 36/36] remove unneeded includes Co-authored-by: Zach Margolis --- app/controllers/idv/verify_info_controller.rb | 1 - app/forms/new_phone_form.rb | 1 - app/services/idv/steps/verify_base_step.rb | 7 ++++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 682584c5a66..ebd2031c0bf 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -1,6 +1,5 @@ module Idv class VerifyInfoController < ApplicationController - include StringRedacter include IdvSession before_action :render_404_if_verify_info_controller_disabled diff --git a/app/forms/new_phone_form.rb b/app/forms/new_phone_form.rb index 6202d717818..e259e031022 100644 --- a/app/forms/new_phone_form.rb +++ b/app/forms/new_phone_form.rb @@ -2,7 +2,6 @@ class NewPhoneForm include ActiveModel::Model include FormPhoneValidator include OtpDeliveryPreferenceValidator - include StringRedacter BLOCKED_PHONE_TYPES = [ :premium_rate, diff --git a/app/services/idv/steps/verify_base_step.rb b/app/services/idv/steps/verify_base_step.rb index 0400afc330f..ea04569951e 100644 --- a/app/services/idv/steps/verify_base_step.rb +++ b/app/services/idv/steps/verify_base_step.rb @@ -1,8 +1,6 @@ module Idv module Steps class VerifyBaseStep < DocAuthBaseStep - include StringRedacter - private def summarize_result_and_throttle_failures(summary_result) @@ -298,7 +296,10 @@ def idv_result_to_form_response( if state_id state_id[:state] = state if state state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction - state_id[:state_id_number] = redact_alphanumeric(state_id_number) if state_id_number + if state_id_number + state_id[:state_id_number] = + StringRedacter.redact_alphanumeric(state_id_number) + end end FormResponse.new( success: idv_success(result),