From b90191364ddc3afbe1d87f25f70de68cb147fdda Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 11 Jan 2023 13:00:50 -0800 Subject: [PATCH 01/14] 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 60132279937a77d63e423f7a70891d77374483fb Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 12 Jan 2023 08:20:07 -0800 Subject: [PATCH 02/14] 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 e01ec20cd41f02017dcf8849865798d78deb2571 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 12 Jan 2023 16:13:12 -0800 Subject: [PATCH 03/14] 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 c7c03ed1e6736c151440bf319378a4c221a2102b Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 10:24:20 -0800 Subject: [PATCH 04/14] 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 63ec400b1f8dd93c1e6f0cda05083347e46d1f8c Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 13:23:25 -0800 Subject: [PATCH 05/14] New specs and Idv::Session field change Replace resolution_info_verified with already existing resolution_successful 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 71d341bcb26de799be5d2140f072ede4491d4918 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 15:13:57 -0800 Subject: [PATCH 06/14] 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 7e66b232481614513283e2c4bca74493dd0e2d5a Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 13 Jan 2023 16:26:07 -0800 Subject: [PATCH 07/14] 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 f45a72fd27f9ff28fcb54d24d60998d957e735f2 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Tue, 17 Jan 2023 13:01:22 -0800 Subject: [PATCH 08/14] 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 265f15b30e22faa6c11d6f00879d72f4492c6466 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Wed, 18 Jan 2023 14:47:45 -0800 Subject: [PATCH 09/14] 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 7ef94c391be215c781db1240d205e8a4d5d9ad24 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 08:33:32 -0800 Subject: [PATCH 10/14] 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 5917638cf715199b7db8dd5ebf2fde39803fa254 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:20:55 -0800 Subject: [PATCH 11/14] 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 4075fe1caa9339167ea51a4378439a9c3ee16cef Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Thu, 19 Jan 2023 11:28:56 -0800 Subject: [PATCH 12/14] 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 a80a45711692c6793d35a747836d164bc1503369 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Fri, 20 Jan 2023 16:16:14 -0500 Subject: [PATCH 13/14] 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 c85c5c88d392b6416071a6a0ed7fd4d8e4bf623e Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Fri, 20 Jan 2023 13:52:43 -0800 Subject: [PATCH 14/14] Add changelog changelog: Internal, Flow State Machine, Add update action for new VerifyInfoController