From 66aa5f44855177fcd405516c44082a40585c287a Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Thu, 24 Oct 2024 15:45:36 -0700 Subject: [PATCH 1/5] Refactor ThreatMetrix out into a plugin For now, just take the proof_with_threatmetrix_if_needed method + deps and move into a different file. [skip changelog] --- .../plugins/threat_metrix_plugin.rb | 78 +++++++++++ .../resolution/progressive_proofer.rb | 84 ++--------- .../plugins/threatmetrix_plugin_spec.rb | 110 +++++++++++++++ .../resolution/progressive_proofer_spec.rb | 132 ++++++++---------- 4 files changed, 259 insertions(+), 145 deletions(-) create mode 100644 app/services/proofing/resolution/plugins/threat_metrix_plugin.rb create mode 100644 spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb diff --git a/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb b/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb new file mode 100644 index 00000000000..fae8c5372e9 --- /dev/null +++ b/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module Proofing + module Resolution + module Plugins + class ThreatMetrixPlugin + def call( + applicant_pii:, + current_sp:, + request_ip:, + threatmetrix_session_id:, + timer:, + user_email: + ) + unless FeatureManagement.proofing_device_profiling_collecting_enabled? + return threatmetrix_disabled_result + end + + # The API call will fail without a session ID, so do not attempt to make + # it to avoid leaking data when not required. + return threatmetrix_id_missing_result if threatmetrix_session_id.blank? + return threatmetrix_pii_missing_result if applicant_pii.blank? + + ddp_pii = applicant_pii.dup + ddp_pii[:threatmetrix_session_id] = threatmetrix_session_id + ddp_pii[:email] = user_email + ddp_pii[:request_ip] = request_ip + + timer.time('threatmetrix') do + proofer.proof(ddp_pii) + end.tap do |result| + Db::SpCost::AddSpCost.call( + current_sp, :threatmetrix, + transaction_id: result.transaction_id + ) + end + end + + def proofer + @proofer ||= + if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled + Proofing::Mock::DdpMockClient.new + else + Proofing::LexisNexis::Ddp::Proofer.new( + api_key: IdentityConfig.store.lexisnexis_threatmetrix_api_key, + org_id: IdentityConfig.store.lexisnexis_threatmetrix_org_id, + base_url: IdentityConfig.store.lexisnexis_threatmetrix_base_url, + ) + end + end + + def threatmetrix_disabled_result + Proofing::DdpResult.new( + success: true, + client: 'tmx_disabled', + review_status: 'pass', + ) + end + + def threatmetrix_pii_missing_result + Proofing::DdpResult.new( + success: false, + client: 'tmx_pii_missing', + review_status: 'reject', + ) + end + + def threatmetrix_id_missing_result + Proofing::DdpResult.new( + success: false, + client: 'tmx_session_id_missing', + review_status: 'reject', + ) + end + end + end + end +end diff --git a/app/services/proofing/resolution/progressive_proofer.rb b/app/services/proofing/resolution/progressive_proofer.rb index edb8a6f07f4..5639d89f450 100644 --- a/app/services/proofing/resolution/progressive_proofer.rb +++ b/app/services/proofing/resolution/progressive_proofer.rb @@ -8,12 +8,12 @@ module Resolution # 2. The user has only provided one address for their residential and identity document # address or separate residential and identity document addresses class ProgressiveProofer - attr_reader :applicant_pii, - :request_ip, - :threatmetrix_session_id, - :timer, - :user_email, - :current_sp + attr_reader :applicant_pii, :timer, :current_sp + attr_reader :threatmetrix_plugin + + def initialize + @threatmetrix_plugin = Plugins::ThreatMetrixPlugin.new + end # @param [Hash] applicant_pii keys are symbols and values are strings, confidential user info # @param [Boolean] ipp_enrollment_in_progress flag that indicates if user will have @@ -33,14 +33,19 @@ def proof( current_sp: ) @applicant_pii = applicant_pii.except(:best_effort_phone_number_for_socure) - @request_ip = request_ip - @threatmetrix_session_id = threatmetrix_session_id @timer = timer - @user_email = user_email @ipp_enrollment_in_progress = ipp_enrollment_in_progress @current_sp = current_sp - @device_profiling_result = proof_with_threatmetrix_if_needed + device_profiling_result = threatmetrix_plugin.call( + applicant_pii:, + current_sp:, + threatmetrix_session_id:, + request_ip:, + timer:, + user_email:, + ) + @residential_instant_verify_result = proof_residential_address_if_needed @instant_verify_result = proof_id_address_with_lexis_nexis_if_needed @state_id_result = proof_id_with_aamva_if_needed @@ -64,28 +69,6 @@ def proof( :instant_verify_result, :state_id_result - def proof_with_threatmetrix_if_needed - unless FeatureManagement.proofing_device_profiling_collecting_enabled? - return threatmetrix_disabled_result - end - - # The API call will fail without a session ID, so do not attempt to make - # it to avoid leaking data when not required. - return threatmetrix_id_missing_result if threatmetrix_session_id.blank? - return threatmetrix_pii_missing_result if applicant_pii.blank? - - ddp_pii = applicant_pii.dup - ddp_pii[:threatmetrix_session_id] = threatmetrix_session_id - ddp_pii[:email] = user_email - ddp_pii[:request_ip] = request_ip - - timer.time('threatmetrix') do - lexisnexis_ddp_proofer.proof(ddp_pii) - end.tap do |result| - add_sp_cost(:threatmetrix, result.transaction_id) - end - end - def proof_residential_address_if_needed return residential_address_unnecessary_result unless ipp_enrollment_in_progress? @@ -173,30 +156,6 @@ def ipp_enrollment_in_progress? @ipp_enrollment_in_progress end - def threatmetrix_disabled_result - Proofing::DdpResult.new( - success: true, - client: 'tmx_disabled', - review_status: 'pass', - ) - end - - def threatmetrix_pii_missing_result - Proofing::DdpResult.new( - success: false, - client: 'tmx_pii_missing', - review_status: 'reject', - ) - end - - def threatmetrix_id_missing_result - Proofing::DdpResult.new( - success: false, - client: 'tmx_session_id_missing', - review_status: 'reject', - ) - end - def out_of_aamva_jurisdiction_result Proofing::StateIdResult.new( errors: {}, @@ -206,19 +165,6 @@ def out_of_aamva_jurisdiction_result ) end - def lexisnexis_ddp_proofer - @lexisnexis_ddp_proofer ||= - if IdentityConfig.store.lexisnexis_threatmetrix_mock_enabled - Proofing::Mock::DdpMockClient.new - else - Proofing::LexisNexis::Ddp::Proofer.new( - api_key: IdentityConfig.store.lexisnexis_threatmetrix_api_key, - org_id: IdentityConfig.store.lexisnexis_threatmetrix_org_id, - base_url: IdentityConfig.store.lexisnexis_threatmetrix_base_url, - ) - end - end - def resolution_proofer @resolution_proofer ||= if IdentityConfig.store.proofer_mock_fallback diff --git a/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb b/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb new file mode 100644 index 00000000000..10a88e862f0 --- /dev/null +++ b/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb @@ -0,0 +1,110 @@ +require 'rails_helper' + +RSpec.describe Proofing::Resolution::Plugins::ThreatMetrixPlugin do + let(:applicant_pii) { Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN } + let(:current_sp) { build(:service_provider) } + let(:proofer_result) do + instance_double(Proofing::DdpResult, success?: true, transaction_id: 'ddp-123') + end + let(:request_ip) { Faker::Internet.ip_v4_address } + let(:threatmetrix_session_id) { 'cool-session-id' } + let(:user_email) { Faker::Internet.email } + + subject(:plugin) do + described_class.new + end + + before do + allow(IdentityConfig.store).to receive(:lexisnexis_threatmetrix_mock_enabled). + and_return(false) + allow(plugin.proofer).to receive(:proof).and_return(proofer_result) + end + + describe '#call' do + subject(:call) do + plugin.call( + applicant_pii:, + current_sp:, + request_ip:, + threatmetrix_session_id:, + timer: JobHelpers::Timer.new, + user_email:, + ) + end + + context 'ThreatMetrix is enabled' do + before do + allow(FeatureManagement).to receive(:proofing_device_profiling_collecting_enabled?). + and_return(true) + end + + it 'calls the ThreatMetrix proofer' do + call + expect(plugin.proofer).to have_received(:proof) + end + + it 'creates a ThreatMetrix associated cost' do + expect { call }. + to change { + SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer).count + }.to eql(1) + end + + context 'session id is missing' do + let(:threatmetrix_session_id) { nil } + + it 'does not call the ThreatMetrix proofer' do + expect(plugin.proofer).not_to receive(:proof) + call + end + + it 'returns a failed result' do + call.tap do |result| + expect(result.success).to be(false) + expect(result.client).to eq('tmx_session_id_missing') + expect(result.review_status).to eq('reject') + end + end + end + + context 'pii is missing' do + let(:applicant_pii) { {} } + + it 'does not call the ThreatMetrix proofer' do + expect(plugin.proofer).not_to receive(:proof) + call + end + + it 'returns a failed result' do + call.tap do |result| + expect(result.success).to be(false) + expect(result.client).to eq('tmx_pii_missing') + expect(result.review_status).to eq('reject') + end + end + end + end + + context 'ThreatMetrix is disabled' do + before do + allow(FeatureManagement).to receive(:proofing_device_profiling_collecting_enabled?). + and_return(false) + end + + it 'returns a disabled result' do + call.tap do |result| + expect(result.success).to be(true) + expect(result.client).to eq('tmx_disabled') + expect(result.review_status).to eq('pass') + end + end + + it 'does not create a ThreatMetrix associated cost' do + expect { call }. + not_to change { + SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer).count + } + end + end + end +end diff --git a/spec/services/proofing/resolution/progressive_proofer_spec.rb b/spec/services/proofing/resolution/progressive_proofer_spec.rb index 17fcf87abb3..8390e66966d 100644 --- a/spec/services/proofing/resolution/progressive_proofer_spec.rb +++ b/spec/services/proofing/resolution/progressive_proofer_spec.rb @@ -3,7 +3,9 @@ RSpec.describe Proofing::Resolution::ProgressiveProofer do let(:applicant_pii) { Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN } let(:ipp_enrollment_in_progress) { false } + let(:request_ip) { Faker::Internet.ip_v4_address } let(:threatmetrix_session_id) { SecureRandom.uuid } + let(:user_email) { Faker::Internet.email } let(:current_sp) { build(:service_provider) } let(:instant_verify_proofing_success) { true } @@ -32,9 +34,17 @@ end let(:aamva_proofer) { instance_double(Proofing::Aamva::Proofer, proof: aamva_proofer_result) } + let(:threatmetrix_plugin) do + Proofing::Resolution::Plugins::ThreatMetrixPlugin.new + end + let(:threatmetrix_proofer_result) do - instance_double(Proofing::DdpResult, success?: true, transaction_id: 'ddp-123') + Proofing::DdpResult.new( + success: true, + transaction_id: 'ddp-123', + ) end + let(:threatmetrix_proofer) do instance_double( Proofing::LexisNexis::Ddp::Proofer, @@ -101,6 +111,9 @@ def block_real_instant_verify_requests end before do + allow(progressive_proofer).to receive(:threatmetrix_plugin).and_return(threatmetrix_plugin) + allow(threatmetrix_plugin).to receive(:proofer).and_return(threatmetrix_proofer) + allow(progressive_proofer).to receive(:resolution_proofer).and_return(instant_verify_proofer) allow(progressive_proofer).to receive(:lexisnexis_ddp_proofer).and_return(threatmetrix_proofer) allow(progressive_proofer).to receive(:state_id_proofer).and_return(aamva_proofer) @@ -108,6 +121,12 @@ def block_real_instant_verify_requests block_real_instant_verify_requests end + it 'assigns threatmetrix_plugin' do + expect(described_class.new.threatmetrix_plugin).to be_a( + Proofing::Resolution::Plugins::ThreatMetrixPlugin, + ) + end + describe '#proof' do before do allow(IdentityConfig.store).to receive(:proofer_mock_fallback).and_return(false) @@ -115,90 +134,51 @@ def block_real_instant_verify_requests subject(:proof) do progressive_proofer.proof( - applicant_pii: applicant_pii, - ipp_enrollment_in_progress: ipp_enrollment_in_progress, - request_ip: Faker::Internet.ip_v4_address, - threatmetrix_session_id: threatmetrix_session_id, + applicant_pii:, + ipp_enrollment_in_progress:, + request_ip:, + threatmetrix_session_id:, timer: JobHelpers::Timer.new, - user_email: Faker::Internet.email, - current_sp: current_sp, + user_email:, + current_sp:, ) end - context 'remote proofing' do - it 'returns a ResultAdjudicator' do - expect(proof).to be_an_instance_of(Proofing::Resolution::ResultAdjudicator) - expect(proof.same_address_as_id).to eq(nil) + context 'remote unsupervised proofing' do + it 'calls ThreatMetrixPlugin' do + expect(threatmetrix_plugin).to receive(:call).with( + applicant_pii:, + current_sp:, + request_ip:, + threatmetrix_session_id:, + timer: an_instance_of(JobHelpers::Timer), + user_email:, + ) + proof end + end - context 'ThreatMetrix is enabled' do - before do - enable_threatmetrix - allow(IdentityConfig.store).to receive(:lexisnexis_threatmetrix_mock_enabled). - and_return(false) - - proof - end - - it 'makes a request to the ThreatMetrix proofer' do - expect(threatmetrix_proofer).to have_received(:proof) - end - - it 'creates a ThreatMetrix associated cost' do - threatmetrix_sp_costs = SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer) - expect(threatmetrix_sp_costs.count).to eq(1) - end - - context 'session id is missing' do - let(:threatmetrix_session_id) { nil } - - it 'does not make a request to the ThreatMetrix proofer' do - expect(threatmetrix_proofer).not_to have_received(:proof) - end - - it 'returns a failed result' do - device_profiling_result = proof.device_profiling_result - - expect(device_profiling_result.success).to be(false) - expect(device_profiling_result.client).to eq('tmx_session_id_missing') - expect(device_profiling_result.review_status).to eq('reject') - end - end - - context 'pii is missing' do - let(:applicant_pii) { {} } - - it 'does not make a request to the ThreatMetrix proofer' do - expect(threatmetrix_proofer).not_to have_received(:proof) - end - - it 'returns a failed result' do - device_profiling_result = proof.device_profiling_result + context 'in-person proofing' do + let(:ipp_enrollment_in_progress) { true } + let(:applicant_pii) { Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID } - expect(device_profiling_result.success).to be(false) - expect(device_profiling_result.client).to eq('tmx_pii_missing') - expect(device_profiling_result.review_status).to eq('reject') - end - end + it 'calls ThreatMetrixPlugin' do + expect(threatmetrix_plugin).to receive(:call).with( + applicant_pii:, + current_sp:, + request_ip:, + threatmetrix_session_id:, + timer: an_instance_of(JobHelpers::Timer), + user_email:, + ) + proof end + end - context 'ThreatMetrix is disabled' do - before do - disable_threatmetrix - end - - it 'returns a disabled result' do - device_profiling_result = proof.device_profiling_result - - expect(device_profiling_result.success).to be(true) - expect(device_profiling_result.client).to eq('tmx_disabled') - expect(device_profiling_result.review_status).to eq('pass') - end - - it 'does not create a ThreatMetrix associated cost' do - threatmetrix_sp_costs = SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer) - expect(threatmetrix_sp_costs.count).to eq(0) - end + context 'remote proofing' do + it 'returns a ResultAdjudicator' do + expect(proof).to be_an_instance_of(Proofing::Resolution::ResultAdjudicator) + expect(proof.same_address_as_id).to eq(nil) end context 'AAMVA raises an exception' do From 68571460d59c6466b1f9ca088348a189072ba0c2 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Tue, 29 Oct 2024 14:41:14 -0700 Subject: [PATCH 2/5] Remove unused methods from spec --- .../proofing/resolution/progressive_proofer_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/services/proofing/resolution/progressive_proofer_spec.rb b/spec/services/proofing/resolution/progressive_proofer_spec.rb index 8390e66966d..eb4b3d38e53 100644 --- a/spec/services/proofing/resolution/progressive_proofer_spec.rb +++ b/spec/services/proofing/resolution/progressive_proofer_spec.rb @@ -96,16 +96,6 @@ instance_double(Proofing::Resolution::Result, success?: true, errors: nil) end - def enable_threatmetrix - allow(FeatureManagement).to receive(:proofing_device_profiling_collecting_enabled?). - and_return(true) - end - - def disable_threatmetrix - allow(FeatureManagement).to receive(:proofing_device_profiling_collecting_enabled?). - and_return(false) - end - def block_real_instant_verify_requests allow(Proofing::LexisNexis::InstantVerify::VerificationRequest).to receive(:new) end From 7d96842d5bdbaf23cf220db6223e86eb19b50857 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 30 Oct 2024 12:27:16 -0700 Subject: [PATCH 3/5] Update app/services/proofing/resolution/plugins/threat_metrix_plugin.rb Co-authored-by: Zach Margolis --- .../proofing/resolution/plugins/threat_metrix_plugin.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb b/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb index fae8c5372e9..03fa63dbeab 100644 --- a/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb +++ b/app/services/proofing/resolution/plugins/threat_metrix_plugin.rb @@ -21,10 +21,11 @@ def call( return threatmetrix_id_missing_result if threatmetrix_session_id.blank? return threatmetrix_pii_missing_result if applicant_pii.blank? - ddp_pii = applicant_pii.dup - ddp_pii[:threatmetrix_session_id] = threatmetrix_session_id - ddp_pii[:email] = user_email - ddp_pii[:request_ip] = request_ip + ddp_pii = applicant_pii.merge( + threatmetrix_session_id: threatmetrix_session_id, + email: user_email, + request_id: request_ip, + ) timer.time('threatmetrix') do proofer.proof(ddp_pii) From 083f8f566c02e46acb77be9e87d2fda382f6db40 Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 30 Oct 2024 12:29:07 -0700 Subject: [PATCH 4/5] Add sp_cost_count method to TMX plugin spec Allows much cleaner (1-line) checking of whether the plugin modifies cost counts --- .../resolution/plugins/threatmetrix_plugin_spec.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb b/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb index 10a88e862f0..f59639e4a47 100644 --- a/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb +++ b/spec/services/proofing/resolution/plugins/threatmetrix_plugin_spec.rb @@ -21,6 +21,10 @@ end describe '#call' do + def sp_cost_count + SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer).count + end + subject(:call) do plugin.call( applicant_pii:, @@ -44,10 +48,7 @@ end it 'creates a ThreatMetrix associated cost' do - expect { call }. - to change { - SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer).count - }.to eql(1) + expect { call }.to change { sp_cost_count }.to(1) end context 'session id is missing' do @@ -100,10 +101,7 @@ end it 'does not create a ThreatMetrix associated cost' do - expect { call }. - not_to change { - SpCost.where(cost_type: :threatmetrix, issuer: current_sp.issuer).count - } + expect { call }.not_to change { sp_cost_count } end end end From 0267f9b567ce2aff882e581eb56b8b5b57f3319b Mon Sep 17 00:00:00 2001 From: Matt Hinz Date: Wed, 30 Oct 2024 14:32:54 -0700 Subject: [PATCH 5/5] Remove stray allow in ProgressiveProofer spec --- spec/services/proofing/resolution/progressive_proofer_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/services/proofing/resolution/progressive_proofer_spec.rb b/spec/services/proofing/resolution/progressive_proofer_spec.rb index eb4b3d38e53..bd108150885 100644 --- a/spec/services/proofing/resolution/progressive_proofer_spec.rb +++ b/spec/services/proofing/resolution/progressive_proofer_spec.rb @@ -105,7 +105,6 @@ def block_real_instant_verify_requests allow(threatmetrix_plugin).to receive(:proofer).and_return(threatmetrix_proofer) allow(progressive_proofer).to receive(:resolution_proofer).and_return(instant_verify_proofer) - allow(progressive_proofer).to receive(:lexisnexis_ddp_proofer).and_return(threatmetrix_proofer) allow(progressive_proofer).to receive(:state_id_proofer).and_return(aamva_proofer) block_real_instant_verify_requests