LG-13706: Socure KYC Proofer by matthinz · Pull Request #11093 · 18F/identity-idp

matthinz · 2024-08-15T22:55:45Z

🎫 Ticket

Link to the relevant ticket:
LG-13706

🛠 Summary of changes

This PR adds an implementation of a Proofer for Socure's ID+ service (specifically the KYC module). It does not actually wire the proofer in anywhere--that will come in a future update.

📜 Testing Plan

You can test this manually!

DO NOT USE REAL PII

Log into the Socure dashboard and grab an Id+ API key in the "Sandbox" environment
Make it available in your shell: export SOCURE_IDPLUS_API_KEY=your-secret-api-key
Run this super-advanced test script in the Rails console:

config = Proofing::Socure::IdPlus::Config.new(api_key: ENV['SOCURE_IDPLUS_API_KEY'], base_url: 'https://sandbox.socure.us')

proofer = Proofing::Socure::IdPlus::Proofer.new(config)

result = proofer.proof(
  first_name: 'Fakey',
  last_name: 'McFakerson',
  address1: '1234 Fake St.',
  city: 'Fakeville',
  state: 'CA',
  zipcode: '90210',
  phone: '360-555-1234',
  email: 'testy@example.org',
  dob: '1999-12-31',
  ssn: '999-22-3344',
)

- Basic Proofer implementation for Socure KYC. - Request / Response classes, error handling, etc. changelog: Upcoming Features, Identity verification, Implement proofer for Socure KYC

app/services/proofing/socure/id_plus/proofer.rb

app/services/proofing/socure/id_plus/request.rb

spec/services/proofing/socure/id_plus/input_spec.rb

spec/services/proofing/socure/id_plus/request_spec.rb

zachmargolis · 2024-08-15T23:09:19Z

app/services/proofing/socure/id_plus/response.rb

+        def kyc_field_validations
+          @field_validations ||= kyc('fieldValidations').
+            each_with_object({}) do |(field, valid), obj|
+              obj[field.to_sym] = valid.round == 1


would we ever want this logic to change per-field? or are we set with anything above 0.5 is a pass

They will literally only send either 0.99 or 0.01, I just figured .round would avoid any weirdness with == and floating points

- Lint fixes - Move temporary consent timestamp back a little bit

n1zyy · 2024-08-19T19:22:20Z

app/services/proofing/socure/id_plus/proofer.rb

+        VERIFIED_ATTRIBUTE_MAP = {
+          address: %i[streetAddress city state zip].freeze,
+          first_name: :firstName,
+          last_name: :surName,


This is offtopic and unimportant, but is camelcase surName our pattern or Socure's? My brain keeps either reading it as surfName, or as two words (since camelCase) and wondering what a sur name is, like it's a foreign word. (I mean, it is, but who knows what a "south name" is?)

This is Socure's field naming. My thinking is that request/response class speak in terms of Socure's API and their naming, and the proofer class translates that to our conventions / case style

n1zyy · 2024-08-19T19:28:31Z

app/services/proofing/socure/id_plus/request.rb

+            dob: input.dob&.to_date&.to_s,
+
+            userConsent: true,
+            consentTimestamp: 5.minutes.ago.iso8601,


This seems weird? Do they actually require a timestamp here?

They do, and it needs to be recent enough or they will return an error

n1zyy · 2024-08-19T19:31:18Z

app/services/proofing/socure/id_plus/request.rb

+
+            # > The country or jurisdiction from where the transaction originates,
+            # > specified in ISO-2 country codes format
+            countryOfOrigin: 'US',


Idle curiosity: does the transaction "originate" from our servers, or the end user? I.e., we know CBP has a lot of users from Mexico; do we need to try to make this dynamic, or is it where we are?

This is where we are--on Socure's end they are using it for compliance stuff (like, "does GDPR apply to this transaction?")

n1zyy · 2024-08-19T20:00:20Z

spec/services/proofing/socure/id_plus/request_spec.rb

+            # XXX: This should be set to the time the user submitted agreement,
+            #      which we are not currently tracking. The "5.minutes.ago" is
+            #      because Socure will reject times "in the future", so we avoid
+            #      our clocks being out of sync with theirs.


Unimportant but IMHO this comment makes more sense in the code. I wondered why it was set this way.

There are a couple of tickets in the epic to follow up on this:

https://cm-jira.usa.gov/browse/LG-14196

https://cm-jira.usa.gov/browse/LG-14260

spec/services/proofing/socure/id_plus/request_spec.rb

n1zyy · 2024-08-19T20:04:07Z

spec/services/proofing/socure/id_plus/request_spec.rb

+        stub_request(:post, 'https://example.org/api/3.0/EmailAuthScore').
+          to_return(
+            status: 500,
+            body: 'It works!',


Suggested change

body: 'It works!',

body: 'It does not work!',

(Only half-joking?)

This is my attempt at humor--Apache will display an "It works!" page by default, meaning that Apache is working, but this page is almost never what you actually want.

Oh, ha! I am all for Apache jokes. I somehow missed this one, but I am 200 OK with this now.

…-kyc-proofer

"it does the thing" is not good enough

n1zyy

This looks reasonable to me. I was able to test it in the console successfully.

Definitely out of scope, if we ever even do it, but with @errors={:reason_codes=>["I919"]} I can't help but think it'd be neat to map those to descriptions. But it's possible these are proprietary and/or there's not really a need to have them in the code? Just a musing for another time.

matthinz · 2024-08-21T18:19:48Z

This looks reasonable to me. I was able to test it in the console successfully.

Definitely out of scope, if we ever even do it, but with @errors={:reason_codes=>["I919"]} I can't help but think it'd be neat to map those to descriptions. But it's possible these are proprietary and/or there's not really a need to have them in the code? Just a musing for another time.

I think there is an API endpoint we can hit to get the descriptions, but they don't change too much over time, so we could possibly have a static mapping

Socure KYC Proofer

f561963

- Basic Proofer implementation for Socure KYC. - Request / Response classes, error handling, etc. changelog: Upcoming Features, Identity verification, Implement proofer for Socure KYC

matthinz assigned n1zyy and jmhooper Aug 15, 2024

zachmargolis reviewed Aug 15, 2024

View reviewed changes

matthinz unassigned n1zyy and jmhooper Aug 16, 2024

matthinz requested a review from a team August 16, 2024 00:10

matthinz added 7 commits August 15, 2024 17:11

Updates after manual integration test

44fd914

- Lint fixes - Move temporary consent timestamp back a little bit

In ruby we just say @return

bd9e8b4

Array.wrap -> Array

dce1b7c

Clean up accessors with .try

61315d3

Remove pointless begin / end

332d626

Use named subject in request spec

15d27c4

Remove empty lines

7d2ee0f

n1zyy reviewed Aug 19, 2024

View reviewed changes

matthinz added 2 commits August 20, 2024 10:01

Merge remote-tracking branch 'origin/main' into matthinz/13706-socure…

e4bf6ef

…-kyc-proofer

Improve test names

06f4bf6

"it does the thing" is not good enough

n1zyy approved these changes Aug 21, 2024

View reviewed changes

matthinz merged commit e863fe9 into main Aug 21, 2024

matthinz deleted the matthinz/13706-socure-kyc-proofer branch August 21, 2024 18:19

solipet mentioned this pull request Aug 22, 2024

Deploy RC 408 to Production #11136

Merged

matthinz mentioned this pull request Aug 22, 2024

LG-13963: 👤 Socure shadow mode background job #11139

Merged

Conversation

matthinz commented Aug 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Summary of changes

📜 Testing Plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

n1zyy left a comment

Choose a reason for hiding this comment

Uh oh!

matthinz commented Aug 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matthinz commented Aug 15, 2024 •

edited

Loading