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-14282 | Pass phone number to Socure proofer #11272

Merged
merged 9 commits into from
Sep 26, 2024
11 changes: 11 additions & 0 deletions app/controllers/idv/verify_info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,21 @@ def analytics_arguments
}.merge(ab_test_analytics_buckets)
end

# This likely belongs somewhere else; we'll figure that out later
def best_effort_phone
if idv_session.phone_for_mobile_flow
{ source: :hybrid_handoff, phone: idv_session.phone_for_mobile_flow }
elsif current_user.default_phone_configuration
{ source: :mfa, phone: current_user.default_phone_configuration.formatted_phone }
end
end

def pii
idv_session.pii_from_doc.to_h.merge(
ssn: idv_session.ssn,
consent_given_at: idv_session.idv_consent_given_at,
# This is only for Socure shadow mode and may not be entirely reliable, which is OK:
best_effort_phone_number_for_socure: best_effort_phone,
**idv_session.updated_user_address.to_h,
).with_indifferent_access
end
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/resolution_proofing_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def perform(
user = User.find_by(id: user_id)
current_sp = ServiceProvider.find_by(issuer: service_provider_issuer)

applicant_pii = decrypted_args[:applicant_pii]
applicant_pii = decrypted_args[:applicant_pii].except(:best_effort_phone_number_for_socure)
applicant_pii[:uuid_prefix] = current_sp&.app_id
applicant_pii[:uuid] = user.uuid

Expand Down
6 changes: 6 additions & 0 deletions app/jobs/socure_shadow_mode_proofing_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def perform(
analytics.idv_socure_shadow_mode_proofing_result(
resolution_result: format_proofing_result_for_logs(proofing_result),
socure_result: socure_result.to_h,
phone_source: applicant[:phone_source],
user_id: user.uuid,
pii_like_keypaths: [
[:errors, :ssn],
Expand Down Expand Up @@ -91,6 +92,10 @@ def build_applicant(
)

applicant_pii = decrypted_arguments[:applicant_pii]
if applicant_pii[:phone].nil? && applicant_pii[:best_effort_phone_number_for_socure]
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that, in actuality, nothing is passing :phone into the Socure proofer today, so this conditional isn't strictly necessary. But it feels cleaner, and IMHO makes clearer what we're doing. (Plus it frees me from needing nil-safe operators on applicant_pii[:best_effort_phone_number_for_socure] and keeps phone and phone_source together.)

applicant_pii[:phone] = applicant_pii[:best_effort_phone_number_for_socure][:phone]
applicant_pii[:phone_source] = applicant_pii[:best_effort_phone_number_for_socure][:source]
end

{
**applicant_pii.slice(
Expand All @@ -102,6 +107,7 @@ def build_applicant(
:state,
:zipcode,
:phone,
:phone_source, # This shouldn't actually go to Socure
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
:dob,
:ssn,
:consent_given_at,
Expand Down
3 changes: 3 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4591,14 +4591,17 @@ def idv_session_error_visited(
# Logs a Socure KYC result alongside a resolution result for later comparison.
# @param [Hash] socure_result Result from Socure KYC API call
# @param [Hash] resolution_result Result from resolution proofing
# @param [String,nil] phone_source Where the phone number came from
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
def idv_socure_shadow_mode_proofing_result(
socure_result:,
resolution_result:,
phone_source:,
**extra
)
track_event(
:idv_socure_shadow_mode_proofing_result,
resolution_result: resolution_result.to_h,
phone_source:,
socure_result: socure_result.to_h,
**extra,
)
Expand Down
3 changes: 2 additions & 1 deletion app/services/proofing/socure/id_plus/proofer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def initialize(config)
# @param [Hash] applicant
# @return [Proofing::Resolution::Result]
def proof(applicant)
input = Input.new(applicant)
# does phone_source just disappear here?
input = Input.new(applicant.except(:phone_source))
Copy link
Member Author

Choose a reason for hiding this comment

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

And phone_source is just along for the ride into logs.


request = Request.new(config:, input:)

Expand Down
69 changes: 53 additions & 16 deletions spec/jobs/socure_shadow_mode_proofing_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -349,22 +349,59 @@
job.build_applicant(encrypted_arguments:, user_email:)
end

it 'builds an applicant structure that looks right' do
expect(build_applicant).to eql(
{
first_name: 'FAKEY',
last_name: 'MCFAKERSON',
address1: '1 FAKE RD',
address2: nil,
city: 'GREAT FALLS',
state: 'MT',
zipcode: '59010-1234',
phone: '12025551212',
dob: '1938-10-06',
ssn: '900-66-1234',
email: user.email,
},
)
let(:expected_attributes) do
{
first_name: 'FAKEY',
last_name: 'MCFAKERSON',
address1: '1 FAKE RD',
address2: nil,
city: 'GREAT FALLS',
state: 'MT',
zipcode: '59010-1234',
dob: '1938-10-06',
ssn: '900-66-1234',
email: user.email,
}
end

context 'when the user has a phone directly passed in' do
# existing test
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
it 'builds an applicant structure that looks right' do
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
expect(build_applicant).to eql(
expected_attributes.merge(phone: '12025551212'),
)
end
end

context 'when the user only has an MFA phone' do
let(:applicant_pii_no_phone) do
Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN.merge(
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
best_effort_phone_number_for_socure: {
source: :hybrid_handoff,
phone: '12025556789',
},
)
end

let(:encrypted_arguments) do
Encryption::Encryptors::BackgroundProofingArgEncryptor.new.encrypt(
JSON.generate({ applicant_pii: applicant_pii_no_phone }),
)
end

# it shoehorns that in
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
it 'builds an applicant structure that looks right' do
expect(build_applicant).to eql(
expected_attributes.merge(
phone: '12025556789',
phone_source: 'hybrid_handoff',
),
)
end
end

context 'when no phone is available for the user' do
# it just doesn't send a phone at all
n1zyy marked this conversation as resolved.
Show resolved Hide resolved
end
end

Expand Down