From 96bd67b220e0c8e5ac579f6abae3bceb3cbcc33a Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Mon, 16 Sep 2024 15:24:03 -0700 Subject: [PATCH 1/3] Require threatmetrix_session_id be present in idv_session for Verify Info screen If the user does not have a session ID, redirect them back to the SSN step so that they get one. changelog: Internal, Identity verification, Prevent errors during verify info step due to missing session id. --- .../concerns/idv/verify_info_concern.rb | 7 ++++ app/controllers/idv/verify_info_controller.rb | 4 ++- app/services/idv/session.rb | 4 +++ .../idv/enter_password_controller_spec.rb | 1 + .../idv/otp_verification_controller_spec.rb | 1 + .../idv/phone_errors_controller_spec.rb | 1 + .../idv/verify_info_controller_spec.rb | 32 +++++++++++++++++-- spec/support/flow_policy_helper.rb | 1 + 8 files changed, 47 insertions(+), 4 deletions(-) diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index 24248b2ea94..0328617717d 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -6,6 +6,13 @@ module VerifyInfoConcern STEP_NAME = 'verify_info' + class_methods do + def threatmetrix_session_id_present_or_not_required?(idv_session:) + return true unless FeatureManagement.proofing_device_profiling_decisioning_enabled? + idv_session.threatmetrix_session_id.present? + end + end + def shared_update return if idv_session.verify_info_step_document_capture_session_uuid analytics.idv_doc_auth_verify_submitted(**analytics_arguments) diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 16d0a51d01f..a4dbad851f6 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -45,7 +45,9 @@ def self.step_info controller: self, next_steps: [:phone, :request_letter], preconditions: ->(idv_session:, user:) do - idv_session.ssn && idv_session.remote_document_capture_complete? + idv_session.remote_document_capture_complete? && + idv_session.ssn_step_complete? && + threatmetrix_session_id_present_or_not_required?(idv_session:) end, undo_step: ->(idv_session:, user:) do idv_session.resolution_successful = nil diff --git a/app/services/idv/session.rb b/app/services/idv/session.rb index bb0a114ff75..923f5d2e9a8 100644 --- a/app/services/idv/session.rb +++ b/app/services/idv/session.rb @@ -240,6 +240,10 @@ def ipp_state_id_complete? user_session['idv/in_person'][:pii_from_user].has_key?(:identity_doc_address1) end + def ssn_step_complete? + ssn.present? + end + def verify_info_step_complete? resolution_successful end diff --git a/spec/controllers/idv/enter_password_controller_spec.rb b/spec/controllers/idv/enter_password_controller_spec.rb index 97c6e7f4bc3..7c8554f6f6d 100644 --- a/spec/controllers/idv/enter_password_controller_spec.rb +++ b/spec/controllers/idv/enter_password_controller_spec.rb @@ -27,6 +27,7 @@ subject.idv_session.flow_path = 'standard' subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT) subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE[:ssn] + subject.idv_session.threatmetrix_session_id = 'random-session-id' subject.idv_session.resolution_successful = true subject.idv_session.applicant = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE subject.idv_session.resolution_successful = true diff --git a/spec/controllers/idv/otp_verification_controller_spec.rb b/spec/controllers/idv/otp_verification_controller_spec.rb index ddd3833539a..0ec9a8cef26 100644 --- a/spec/controllers/idv/otp_verification_controller_spec.rb +++ b/spec/controllers/idv/otp_verification_controller_spec.rb @@ -28,6 +28,7 @@ subject.idv_session.idv_consent_given_at = Time.zone.now subject.idv_session.flow_path = 'standard' subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT) + subject.idv_session.threatmetrix_session_id = 'random-session-id' subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE[:ssn] subject.idv_session.resolution_successful = true subject.idv_session.applicant[:phone] = phone diff --git a/spec/controllers/idv/phone_errors_controller_spec.rb b/spec/controllers/idv/phone_errors_controller_spec.rb index 85f45b5df22..5be8c12dac0 100644 --- a/spec/controllers/idv/phone_errors_controller_spec.rb +++ b/spec/controllers/idv/phone_errors_controller_spec.rb @@ -17,6 +17,7 @@ subject.idv_session.idv_consent_given_at = Time.zone.now subject.idv_session.flow_path = 'standard' subject.idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT) + subject.idv_session.threatmetrix_session_id = 'random-session-id' subject.idv_session.ssn = '123-45-6789' subject.idv_session.applicant = Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE subject.idv_session.resolution_successful = true diff --git a/spec/controllers/idv/verify_info_controller_spec.rb b/spec/controllers/idv/verify_info_controller_spec.rb index 1be8cdbe1d0..2e96dd270aa 100644 --- a/spec/controllers/idv/verify_info_controller_spec.rb +++ b/spec/controllers/idv/verify_info_controller_spec.rb @@ -143,7 +143,7 @@ context 'when proofing_device_profiling is enabled' do let(:threatmetrix_client_id) { 'threatmetrix_client' } - + let(:review_status) { 'pass' } let(:idv_result) do { context: { @@ -178,9 +178,18 @@ allow(IdentityConfig.store).to receive(:proofing_device_profiling).and_return(:enabled) end - context 'when threatmetrix response is Pass' do - let(:review_status) { 'pass' } + context 'when idv_session is missing threatmetrix_session_id' do + before do + controller.idv_session.threatmetrix_session_id = nil + get :show + end + it 'redirects back to the SSN step' do + expect(response).to redirect_to(idv_ssn_url) + end + end + + context 'when threatmetrix response is Pass' do it 'sets the review status in the idv session' do get :show expect(controller.idv_session.threatmetrix_review_status).to eq('pass') @@ -238,6 +247,23 @@ end end + context 'when proofing_device_profiling is disabled' do + before do + allow(IdentityConfig.store).to receive(:proofing_device_profiling).and_return(:disabled) + end + + context 'when idv_session is missing threatmetrix_session_id' do + before do + controller.idv_session.threatmetrix_session_id = nil + get :show + end + + it 'does not redirect back to the SSN step' do + expect(response).not_to redirect_to(idv_ssn_url) + end + end + end + context 'for an aamva request' do before do allow(controller).to receive(:load_async_state).and_return(async_state) diff --git a/spec/support/flow_policy_helper.rb b/spec/support/flow_policy_helper.rb index ecda95a489e..b87810148c3 100644 --- a/spec/support/flow_policy_helper.rb +++ b/spec/support/flow_policy_helper.rb @@ -25,6 +25,7 @@ def stub_step(key:, idv_session:) idv_session.pii_from_doc = Pii::StateId.new(**Idp::Constants::MOCK_IDV_APPLICANT) when :ssn idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN[:ssn] + idv_session.threatmetrix_session_id = 'a-random-session-id' when :ipp_ssn idv_session.send(:user_session)['idv/in_person'] = { pii_from_user: Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID.dup, From 7ed8d3034be2b0ad32509524d868821e4d9bfdba Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Tue, 17 Sep 2024 15:37:09 -0700 Subject: [PATCH 2/3] Update ThreatMetrix session ID generation logic - If the user is updating their SSN but their is no session id present, generate a new one --- .../idv/steps/threat_metrix_step_helper.rb | 13 +++++- spec/controllers/idv/ssn_controller_spec.rb | 40 +++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/app/services/idv/steps/threat_metrix_step_helper.rb b/app/services/idv/steps/threat_metrix_step_helper.rb index cb4bbab8eea..a17d4581368 100644 --- a/app/services/idv/steps/threat_metrix_step_helper.rb +++ b/app/services/idv/steps/threat_metrix_step_helper.rb @@ -14,10 +14,21 @@ def threatmetrix_view_variables(updating_ssn) end def generate_threatmetrix_session_id(updating_ssn) - idv_session.threatmetrix_session_id = SecureRandom.uuid if !updating_ssn + if should_generate_new_threatmetrix_session_id?(updating_ssn) + idv_session.threatmetrix_session_id = SecureRandom.uuid + end + idv_session.threatmetrix_session_id end + def should_generate_new_threatmetrix_session_id?(updating_ssn) + if updating_ssn + idv_session.threatmetrix_session_id.blank? + else + true + end + end + # @return [Array] def threatmetrix_javascript_urls(session_id) sources = if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled diff --git a/spec/controllers/idv/ssn_controller_spec.rb b/spec/controllers/idv/ssn_controller_spec.rb index d4e32ca1373..3c85bd46b61 100644 --- a/spec/controllers/idv/ssn_controller_spec.rb +++ b/spec/controllers/idv/ssn_controller_spec.rb @@ -76,9 +76,43 @@ expect { get :show }.to change { subject.idv_session.threatmetrix_session_id }.from(nil) end - it 'does not change threatmetrix_session_id when updating ssn' do - subject.idv_session.ssn = ssn - expect { get :show }.not_to change { subject.idv_session.threatmetrix_session_id } + context 'when updating ssn' do + let(:threatmetrix_session_id) { 'original-session-id' } + + before do + subject.idv_session.ssn = ssn + subject.idv_session.threatmetrix_session_id = threatmetrix_session_id + end + it 'does not change threatmetrix_session_id' do + expect { get :show }.not_to change { subject.idv_session.threatmetrix_session_id } + end + + context 'but there is no threatmetrix_session_id in the session' do + let(:threatmetrix_session_id) { nil } + + it 'sets a threatmetrix_session_id' do + expect { get :show }.to change { subject.idv_session.threatmetrix_session_id } + end + end + end + + context 'proofing_device_profiling disabled' do + before do + allow(IdentityConfig.store).to receive(:proofing_device_profiling).and_return(:disabled) + end + + it 'still add a threatmetrix session id to idv_session' do + expect { get :show }.to change { subject.idv_session.threatmetrix_session_id }.from(nil) + end + + context 'when idv_session has a threatmetrix_session_id' do + before do + subject.idv_session.threatmetrix_session_id = 'fake-session-id' + end + it 'changes the threatmetrix_session_id' do + expect { get :show }.to change { subject.idv_session.threatmetrix_session_id } + end + end end context 'with an ssn in idv_session' do From 927e791eb45e7a89596a4df891dcc93cdec6a9d2 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 18 Sep 2024 13:47:24 -0700 Subject: [PATCH 3/3] Add an analytics event when missing tmx session id --- .../concerns/idv/verify_info_concern.rb | 5 +++++ app/controllers/idv/verify_info_controller.rb | 1 + app/services/analytics_events.rb | 5 +++++ .../idv/verify_info_controller_spec.rb | 21 ++++++++++++++++++- 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index 0328617717d..993438e6103 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -43,6 +43,11 @@ def shared_update return true end + def log_event_for_missing_threatmetrix_session_id + return if self.class.threatmetrix_session_id_present_or_not_required?(idv_session:) + analytics.idv_verify_info_missing_threatmetrix_session_id if idv_session.ssn_step_complete? + end + private def ipp_enrollment_in_progress? diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index a4dbad851f6..9496890cf04 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -9,6 +9,7 @@ class VerifyInfoController < ApplicationController include Steps::ThreatMetrixStepHelper before_action :confirm_not_rate_limited_after_doc_auth, except: [:show] + before_action :log_event_for_missing_threatmetrix_session_id before_action :confirm_step_allowed def show diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index da60ab14e2c..0b4c0b77299 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -4758,6 +4758,11 @@ def idv_verify_in_person_troubleshooting_option_clicked( ) end + # The user ended up at the "Verify info" screen without a Threatmetrix session id. + def idv_verify_info_missing_threatmetrix_session_id(**extra) + track_event(:idv_verify_info_missing_threatmetrix_session_id, **extra) + end + # @param [Boolean] acuant_sdk_upgrade_a_b_testing_enabled # @param [String] acuant_version # @param ["hybrid","standard"] flow_path Document capture user flow diff --git a/spec/controllers/idv/verify_info_controller_spec.rb b/spec/controllers/idv/verify_info_controller_spec.rb index 2e96dd270aa..7779892c0ea 100644 --- a/spec/controllers/idv/verify_info_controller_spec.rb +++ b/spec/controllers/idv/verify_info_controller_spec.rb @@ -181,12 +181,31 @@ context 'when idv_session is missing threatmetrix_session_id' do before do controller.idv_session.threatmetrix_session_id = nil - get :show end it 'redirects back to the SSN step' do + get :show expect(response).to redirect_to(idv_ssn_url) end + + it 'logs an idv_verify_info_missing_threatmetrix_session_id event' do + get :show + expect(@analytics).to have_logged_event( + :idv_verify_info_missing_threatmetrix_session_id, + ) + end + + context 'when ssn is not present in idv_session' do + before do + controller.idv_session.ssn = nil + end + it 'does not log an idv_verify_info_missing_threatmetrix_session_id event' do + get :show + expect(@analytics).not_to have_logged_event( + :idv_verify_info_missing_threatmetrix_session_id, + ) + end + end end context 'when threatmetrix response is Pass' do