Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-13761: ProofingComponents to store real vendor used to verify document PII #11499

Merged
merged 2 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions app/controllers/concerns/idv/verify_info_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ def async_state_done(current_async_state)

if form_response.success?
save_threatmetrix_status(form_response)
save_source_check_vendor(form_response)
Comment on lines 234 to +235
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A/N: I feel like these two methods could be merged into an handle_success_updates_for_idv_session method, but leaving as is for now.

move_applicant_to_idv_session
idv_session.mark_verify_info_step_complete!

Expand All @@ -251,21 +252,38 @@ def save_threatmetrix_status(form_response)
idv_session.threatmetrix_review_status = review_status
end

def save_source_check_vendor(form_response)
vendor = form_response.extra.dig(
:proofing_results,
:context,
:stages,
:state_id,
:vendor_name,
)
lmgeorge marked this conversation as resolved.
Show resolved Hide resolved
idv_session.source_check_vendor = vendor
end

def summarize_result_and_rate_limit(summary_result)
proofing_results_exception = summary_result.extra.dig(:proofing_results, :exception)
resolution_rate_limiter.increment! if proofing_results_exception.blank?

if summary_result.success?
add_proofing_components
add_proofing_components(summary_result)
else
idv_failure(summary_result)
end
end

def add_proofing_components
def add_proofing_components(summary_result)
ProofingComponent.create_or_find_by(user: current_user).update(
resolution_check: Idp::Constants::Vendors::LEXIS_NEXIS,
source_check: Idp::Constants::Vendors::AAMVA,
source_check: summary_result.extra.dig(
:proofing_results,
:context,
:stages,
:state_id,
:vendor_name,
),
)
end

Expand Down
1 change: 1 addition & 0 deletions app/controllers/idv/in_person/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def self.step_info
idv_session.resolution_successful = nil
idv_session.verify_info_step_document_capture_session_uuid = nil
idv_session.threatmetrix_review_status = nil
idv_session.source_check_vendor = nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any specs for these undo_step things that need to be updated? Or do we just test this as a side effect of other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, we test undo state primarily as a side effect and only test defaults for individual classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I was waiting for the pipeline to complete to verify that assumption, so taking a look now)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, seems like undo step validation is primarily tested through feature specs.

idv_session.applicant = nil
end,
)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/idv/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def self.step_info
end,
undo_step: ->(idv_session:, user:) do
idv_session.resolution_successful = nil
idv_session.source_check_vendor = nil
idv_session.address_edited = nil
idv_session.verify_info_step_document_capture_session_uuid = nil
idv_session.threatmetrix_review_status = nil
Expand Down
3 changes: 1 addition & 2 deletions app/services/idv/proofing_components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def document_type
end

def source_check
Idp::Constants::Vendors::AAMVA if idv_session.verify_info_step_complete?
idv_session.source_check_vendor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to tweak to account for the 50/50 state:

Suggested change
idv_session.source_check_vendor
idv_session.source_check_vendor || (Idp::Constants::Vendors::AAMVA if idv_session.verify_info_step_complete?)

(you can probably come up with a better / more ruby-ish way to do ☝️ )

Basically, if the key is not in the session, we need to fall back to the old (bad) way

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

end

def resolution_check
Expand Down Expand Up @@ -67,7 +67,6 @@ def to_h
address_check:,
threatmetrix:,
threatmetrix_review_status:,

}.compact
end

Expand Down
41 changes: 41 additions & 0 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,46 @@
# frozen_string_literal: true

module Idv
# @attr address_edited [Boolean, nil]
# @attr address_verification_mechanism [String, nil]
# @attr applicant [Struct, nil]
# @attr document_capture_session_uuid [String, nil]
# @attr flow_path [String, nil]
# @attr go_back_path [String, nil]
# @attr gpo_code_verified [Boolean, nil]
# @attr had_barcode_attention_error [Boolean, nil]
# @attr had_barcode_read_failure [Boolean, nil]
# @attr idv_consent_given [Boolean, nil]
# @attr idv_consent_given_at [String, nil]
# @attr idv_phone_step_document_capture_session_uuid [String, nil]
# @attr mail_only_warning_shown [Boolean, nil]
# @attr opted_in_to_in_person_proofing [Boolean, nil]
# @attr personal_key [String, nil]
# @attr personal_key_acknowledged [Boolean, nil]
# @attr phone_for_mobile_flow [String, nil]
# @attr previous_phone_step_params [Array]
# @attr previous_ssn [String, nil]
# @attr profile_id [String, nil]
# @attr proofing_started_at [String, nil]
# @attr redo_document_capture [Boolean, nil]
# @attr resolution_successful [Boolean, nil]
# @attr selfie_check_performed [Boolean, nil]
# @attr selfie_check_required [Boolean, nil]
# @attr skip_doc_auth [Boolean, nil]
# @attr skip_doc_auth_from_handoff [Boolean, nil]
# @attr skip_doc_auth_from_how_to_verify [Boolean, nil]
# @attr skip_hybrid_handoff [Boolean, nil]
# @attr source_check_vendor [String, nil]
# @attr ssn [String, nil]
# @attr threatmetrix_review_status [String, nil]
# @attr threatmetrix_session_id [String, nil]
# @attr user_phone_confirmation [Boolean, nil]
# @attr vendor_phone_confirmation [Boolean, nil]
# @attr verify_info_step_document_capture_session_uuid [String, nil]
# @attr welcome_visited [Boolean, nil]
# @attr_reader current_user [User]
# @attr_reader gpo_otp [String, nil]
# @attr_reader service_provider [ServiceProvider]
lmgeorge marked this conversation as resolved.
Show resolved Hide resolved
class Session
VALID_SESSION_ATTRIBUTES = %i[
address_edited
Expand All @@ -25,6 +65,7 @@ class Session
profile_id
proofing_started_at
redo_document_capture
source_check_vendor
resolution_successful
selfie_check_performed
selfie_check_required
Expand Down
2 changes: 1 addition & 1 deletion app/services/proofing/resolution/plugins/aamva_plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def out_of_aamva_jurisdiction_result
errors: {},
exception: nil,
success: true,
vendor_name: 'UnsupportedJurisdiction',
vendor_name: Idp::Constants::Vendors::AAMVA_UNSUPPORTED_JURISDICTION,
)
end

Expand Down
3 changes: 3 additions & 0 deletions lib/idp/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ module Vendors
MOCK = 'mock'
USPS = 'usps'
AAMVA = 'aamva'
AAMVA_UNSUPPORTED_JURISDICTION = 'UnsupportedJurisdiction'
STATE_ID_MOCK = 'StateIdMock'
SOURCE_CHECK = [AAMVA, AAMVA_UNSUPPORTED_JURISDICTION, STATE_ID_MOCK].freeze
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Possible bikeshedding) Is there a clearer name for this? I.e., SOCURE and LEXIS_NEXIS are sources that we check; can we make it obvious why they don't belong in this array? STATE_ID_SOURCE_CHECK?

Suggesting only in case you agree or have a better idea; I don't feel strongly at all here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was on the fence about what to call this, but I wanted the relationship between this and ProofingComponents#source_check to be clear and deeply coupled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back and forth about asking for state_id_source_check, but the idea here is: you have given us a document of some kind. we then verify it with the issuing source. right now that is a drivers license + AAMVA. In the near future, hopefully, that will be other document types + other verifiers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, source_check is probably good

end

# US State and Territory codes are
Expand Down
17 changes: 11 additions & 6 deletions spec/controllers/idv/verify_info_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -396,18 +396,14 @@
end

context 'for an aamva request' do
before do
allow(controller).to receive(:load_async_state).and_return(async_state)
end

let(:document_capture_session) do
DocumentCaptureSession.create(user:)
end

let(:success) { true }
let(:errors) { {} }
let(:exception) { nil }
let(:vendor_name) { :aamva }
let(:vendor_name) { 'aamva_placeholder' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will not approve without emoji vendor name


let(:async_state) do
# Here we're trying to match the store to redis -> read from redis flow this data travels
Expand Down Expand Up @@ -436,6 +432,10 @@
document_capture_session.load_proofing_result
end

before do
allow(controller).to receive(:load_async_state).and_return(async_state)
end

context 'when aamva processes the request normally' do
it 'redirect to phone confirmation url' do
put :show
Expand All @@ -458,7 +458,12 @@

event = @analytics.events['IdV: doc auth verify proofing results'].first
state_id = event.dig(:proofing_results, :context, :stages, :state_id)
expect(state_id).to match(a_hash_including(state_id_type: 'drivers_license'))
expect(state_id).to match(
hash_including(
state_id_type: 'drivers_license',
vendor_name: 'aamva_placeholder',
),
)
end
end

Expand Down
29 changes: 16 additions & 13 deletions spec/features/idv/analytics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
{
document_check: 'mock',
document_type: 'state_id',
source_check: 'aamva',
source_check: 'StateIdMock',
resolution_check: 'lexis_nexis',
threatmetrix: threatmetrix,
threatmetrix_review_status: 'pass',
Expand Down Expand Up @@ -590,54 +590,54 @@
},
'IdV: phone confirmation form' => {
success: true, errors: {}, phone_type: :mobile, types: [:fixed_or_mobile], carrier: 'Test Mobile Carrier', country_code: 'US', area_code: '202', otp_delivery_preference: 'sms',
proofing_components: { document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'aamva' }
proofing_components: { document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'StateIdMock' }
},
'IdV: phone confirmation vendor' => {
success: true, errors: {}, vendor: { exception: nil, vendor_name: 'AddressMock', transaction_id: 'address-mock-transaction-id-123', timed_out: false, reference: '' }, new_phone_added: false, hybrid_handoff_phone_used: false, area_code: '202', country_code: 'US', phone_fingerprint: anything,
proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'aamva' }
proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'StateIdMock' }
},
'IdV: phone confirmation otp sent' => {
success: true, otp_delivery_preference: :sms, country_code: 'US', area_code: '202', adapter: :test, errors: {}, phone_fingerprint: anything, rate_limit_exceeded: false, telephony_response: anything,
proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'aamva' }
proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'StateIdMock' }
},
'IdV: phone confirmation otp visited' => {
proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'aamva' },
proofing_components: { address_check: 'lexis_nexis_address', document_check: 'usps', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', source_check: 'StateIdMock' },
},
'IdV: phone confirmation otp submitted' => {
success: true, code_expired: false, code_matches: true, otp_delivery_preference: :sms, second_factor_attempts_count: 0, errors: {},
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
},
:idv_enter_password_visited => {
address_verification_method: 'phone',
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' },
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' },
},
:idv_enter_password_submitted => {
success: true, fraud_review_pending: false, fraud_rejection: false, gpo_verification_pending: false, in_person_verification_pending: true, proofing_workflow_time_in_seconds: 0.0,
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
},
'IdV: final resolution' => {
success: true, fraud_review_pending: false, fraud_rejection: false, gpo_verification_pending: false, in_person_verification_pending: true, proofing_workflow_time_in_seconds: 0.0,
# NOTE: pending_profile_idv_level should be set here, a nil value is cached for current_user.pending_profile.
profile_history: match_array(kind_of(Idv::ProfileLogging)),
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
},
'IdV: personal key visited' => {
in_person_verification_pending: true,
address_verification_method: 'phone',
encrypted_profiles_missing: false, pending_profile_idv_level: idv_level,
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
},
'IdV: personal key acknowledgment toggled' => {
checked: true, pending_profile_idv_level: idv_level,
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
},
'IdV: personal key submitted' => {
address_verification_method: 'phone', fraud_review_pending: false, fraud_rejection: false, in_person_verification_pending: true, pending_profile_idv_level: idv_level,
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' }
},
'IdV: in person ready to verify visited' => {
pending_profile_idv_level: idv_level,
proofing_components: { document_check: 'usps', source_check: 'aamva', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' },
proofing_components: { document_check: 'usps', source_check: 'StateIdMock', resolution_check: 'lexis_nexis', threatmetrix: threatmetrix, threatmetrix_review_status: 'pass', address_check: 'lexis_nexis_address' },
},
'IdV: user clicked what to bring link on ready to verify page' => {},
'IdV: user clicked sp link on ready to verify page' => {},
Expand Down Expand Up @@ -974,6 +974,7 @@
end
end
end

context 'Hybrid flow' do
context 'facial comparison not required - Happy path' do
before do
Expand Down Expand Up @@ -1038,6 +1039,7 @@
end
end
end

context 'facial comparison required - Happy path' do
before do
allow_any_instance_of(DocAuth::Response).to receive(:selfie_status).and_return(:success)
Expand Down Expand Up @@ -1097,6 +1099,7 @@
end
end
end

context 'facial comparison not required - Happy path' do
before do
allow(Telephony).to receive(:send_doc_auth_link).and_wrap_original do |impl, config|
Expand Down
4 changes: 3 additions & 1 deletion spec/features/idv/end_to_end_idv_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ def validate_verify_info_page
def validate_verify_info_submit(user)
expect(page).to have_content(t('doc_auth.forms.doc_success'))
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(user.proofing_component.source_check).to satisfy do |v|
Idp::Constants::Vendors::SOURCE_CHECK.include?(v)
end
end

def validate_phone_page
Expand Down
1 change: 1 addition & 0 deletions spec/features/idv/proofing_components_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
proofing_components = user.active_profile.proofing_components
expect(proofing_components['document_check']).to eq('mock')
expect(proofing_components['document_type']).to eq('state_id')
expect(proofing_components['source_check']).to eq('StateIdMock')
end
end
end
Expand Down
Loading