LG-14725: Identity resolution with Socure by matthinz · Pull Request #11479 · 18F/identity-idp

matthinz · 2024-11-07T22:28:03Z

🎫 Ticket

Link to the relevant ticket:
LG-14725

🛠 Summary of changes

This PR adds the ability to configure the vendor used by the ProgressiveProofer for identity resolution. To do this, it does a couple of things:

Genericizes references to InstantVerify in the ProgressiveProofer (for example, instant_verify_state_id_address_result becomes state_id_address_result--this is relatively noisy, sorry)
Reworks the plugins introduced in ProgressiveProofer refactor 3/N: Instant Verify residential address #11433 and ProgressiveProofer refactor 4/N: InstantVerify state id address #11434 to accept vendor-specific data (Proofer + SP cost token) via their initializers
Adds new configuration options related to identity resolution vendor (see below)
Adds appropriate case proofing_vendor-type code to create the correct proofer based on the configured vendor

New configuration flags

Historically, we've had one flag controlling resolution proofing vendor: proofer_mock_fallback. If true, then mock proofers are used for both identity and address resolution. If false, then LexisNexis InstantVerify is used for identity resolution and PhoneFinder is used for address resolution.

This PR adds three new config flags:

Config	Default value
`idv_resolution_default_vendor`	`mock`
`idv_resolution_alternate_vendor`	`mock`
`idv_resolution_alternate_vendor_percent`	`0`

Either _vendor config can be set to one of the following:

mock
instant_verify
socure

If idv_resolution_alternate_vendor_percent is greater than zero, then that percentage of resolutions will use the alternate vendor. Note that whichever vendor is selected is used for both state ID and residential address transactions.

While this update rolls out, we will, um, fall back to the proofer_mock_fallback config if idv_resolution_default_vendor is set to mock. So in production, if no new configuration is applied, the existing proofer_mock_fallback config will take precedence and instant_verify will be used.

The intention here is to allow us to clean up configurations in staging and production after this code goes live. LG-15010 covers eventually removing the proofer_mock_fallback config entirely.

📜 Testing Plan

First, configure Socure in your application.yml:

socure_idplus_api_key: '<KEY FROM THE SOCURE DASHBOARD>'
socure_idplus_base_url: 'https://sandbox.socure.us'

Next, send all of your resolutions to Socure:

# 100% to socure
idv_resolution_default_vendor: mock
idv_resolution_alternate_vendor: socure
idv_resolution_alternate_vendor_percent: 100

Now, run through IdV. Check your log/events.log file and verify that your most recent IdV: doc auth verify proofing results event uses the vendor socure_kyc.

Now, send all your resolutions to the mock proofer:

# 100% to mock
idv_resolution_default_vendor: mock
idv_resolution_alternate_vendor: socure
idv_resolution_alternate_vendor_percent: 0

Run through IdV and verify that your most recent IdV: doc auth verify proofing results event uses the vendor mock.

Now, remove the three idv_resolution_* configs from your application.yml and disable proofer_mock_fallback (simulating the configuration in prod):

proofer_mock_fallback: false

Go through IdV one more time. This should fail on the "Verify info" step. If you look at your most recent IdV: doc auth verify proofing results, you should see that it attempted to proof you with LexisNexis InstantVerify but got an exception (because you can't connect to IV from your developer laptop).

More generic, now state_address_resolution_result

Go with more generic residential_address_resolution_result

Pull out all the logic into an include-able module so all you have to provide is a proofer + cost token.

Gonna share some code here.

State ID + Residential address variants

Add 3 new configs: - idv_resolution_default_vendor - idv_resolution_alternate_vendor - idv_resolution_alternate_vendor_percent These configs allow randomly using an alternate identity resolution vendor in the ProgressiveProofer. changelog: Internal, Identity verification, Enable using multiple identity resolution vendors

If an exception happens earlier in the job, then document_capture_session can be nil here. Previously this would raise a NoMethodError, obscuring the root cause.

There were some autoloading issues in cases where calling code wants access to the Error class but does not load the Request class first

In practice, applicant will have extra fields like `state_id_jurisdiction`, `state_id_number`, etc.

Remove vendor-specific plugins, and go with general-purpose plugins that can have a proofer and sp_cost_token injected into them.

Temporarily fall back to proofer_mock_fallback Until we have idv_resolution_default_vendor specified in all production environments, fall back to using proofer_mock_fallback.

app/jobs/resolution_proofing_job.rb

matthinz · 2024-11-07T23:13:58Z

app/services/db/sp_cost/add_sp_cost.rb

        acuant_result
        lexis_nexis_resolution
        lexis_nexis_address
+        mock_resolution


We were previously creating SP cost records for mock resolutions as lexis_nexis_resolution. This PR now starts sending mock_resolution in those cases.

matthinz · 2024-11-07T23:14:19Z

app/services/db/sp_cost/add_sp_cost.rb

        lexis_nexis_address
+        mock_resolution
        gpo_letter
+        socure_resolution


Checking with biz ops to make sure this new SP cost type doesn't make anything explode.

Chris had expressed to me a desire to have FA1 and FA3 Socure calls recorded separately, since they are separate products. I think this naming is probably a good precedent if Timnit will be socure_docauth or such.

✅ got the ok that sending a new sp cost type will not break anything

matthinz · 2024-11-07T23:15:28Z

app/services/proofing/resolution/plugins/residential_address_plugin.rb

+          proofer:,
+          sp_cost_token:
+        )
+          @proofer = proofer


Responsibility for creating Proofer instances is moved into ProgressiveProofer, allowing these *AddressPlugin classes to be shared between Socure / InstantVerify.

matthinz · 2024-11-07T23:16:07Z

app/services/proofing/resolution/plugins/state_id_address_plugin.rb

              transaction_id: result.transaction_id,
            )
          end
        end

-        def proofer


Logic moved into ProgressiveProofer

matthinz · 2024-11-07T23:22:48Z

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

-          super(build_message)
-        end
+      class Request
+        class Error < StandardError


There were some autoloading issues with RequestError not being inside Request, so I moved it inside and renamed it to just Error.

n1zyy

I've now made my way through this and it seems good. I haven't done the manual testing proposed so I am going to hold off on leaving an approval, but otherwise I'm on board with this.

n1zyy · 2024-11-08T19:14:00Z

app/services/db/sp_cost/add_sp_cost.rb

        lexis_nexis_address
+        mock_resolution
        gpo_letter
+        socure_resolution


Chris had expressed to me a desire to have FA1 and FA3 Socure calls recorded separately, since they are separate products. I think this naming is probably a good precedent if Timnit will be socure_docauth or such.

n1zyy · 2024-11-13T19:05:39Z

app/services/proofing/resolution/progressive_proofer.rb

+      end
+
+      def sp_cost_token
+        token = PROOFING_VENDOR_SP_COST_TOKENS[proofing_vendor]


This elicited in my mind a funny image of us paying vendors with tokens like an arcade. 🪙👾

lmgeorge

I'm ~~only half-way~~ through this review (wanted to post for the sake of getting some of these comments up), ~~so I'll just post as a comment, but~~ and I do think there are a couple of blocking changes around config validation and a couple of open questions.

lmgeorge · 2024-11-13T20:45:47Z

config/application.yml.default

+idv_resolution_alternate_vendor: mock
+idv_resolution_alternate_vendor_percent: 0
+idv_resolution_default_vendor: mock


(suggestion, non-blocking): consider combining these into a :json type that takes a hash that maps vendor to traffic percentage. It adds additional complexity, but feels easier to discern where the traffic is going at a glance. It would also eliminate the need for the default/alternate keys.

Maybe something like:

idv_resolution_vendor_traffic_balance_config: '{ "instant_verify": 0, "socure": 0, "mock": 100}'

Yeah that's a good suggestion, but in this case I'm match patterns in place elsewhere in our config (not saying those are correct, but I think consistency is good). In practice right now you won't ever use 3 vendors, so this feels like a problem we can figure out how to solve when we get there.

Some of this is also speculative--I'm assuming that our strategy for determining which vendor to send to will be random chance, but it could also be informed by what we learn during the shadow mode pilot.

spec/services/proofing/resolution/plugins/state_id_address_plugin_spec.rb

lmgeorge · 2024-11-13T20:52:17Z

spec/services/proofing/resolution/progressive_proofer_spec.rb

+    end
+
+    context 'when proofing_vendor is another value' do
+      let(:proofing_vendor) { :🦨 }


app/jobs/resolution_proofing_job.rb

lmgeorge · 2024-11-13T21:02:01Z

app/services/proofing/resolution/progressive_proofer.rb

todo: Make sure to update the class doc comment!

lmgeorge · 2024-11-13T21:13:37Z

app/services/proofing/resolution/progressive_proofer.rb

+          end
+
+          if default_vendor.blank?
+            raise InvalidProofingVendorError, 'idv_resolution_default_vendor not configured'


issue: This type of configuration loading/validation should happen during startup automatically. Consider moving additional validation to an initializer.

This is a good call-out. I'll look into how we handle this in intializers

The closest we have to config validation prior art is unused_identity_config_keys.rb, but setting up globally available config is probably best demonstrated by the telephony initializer.

Would using an enum in the definition of the config work?

As an example of an existing one:

config.add( :openid_connect_redirect, type: :string, enum: ['server_side', 'client_side', 'client_side_js'], )

app/services/proofing/resolution/progressive_proofer.rb

lmgeorge · 2024-11-13T21:30:02Z

app/services/proofing/resolution/progressive_proofer.rb

+            return default_vendor
+          end
+
+          if (rand * 100) <= alternate_vendor_percent


suggestion: use rand(1..101), this will always give you an integer between (inclusive) 1 and 100.

I had to double check the docs (link), but it looks like rand(1..101) will include 101. ... excludes the ending value, but .. is inclusive.

lmgeorge · 2024-11-13T22:28:29Z

app/services/proofing/resolution/progressive_proofer.rb

+        when :mock then create_mock_proofer
+        when :socure then create_socure_proofer
+        else
+          raise InvalidProofingVendorError


suggestion: Add a message for the InvalidProofingVendorError like

"Cannot create unknown proofing vendor: '#{proofing_vendor}'"

lmgeorge · 2024-11-13T22:33:30Z

app/services/proofing/resolution/progressive_proofer.rb

+          # Initially we will have production environments configured with
+          # `proofer_mock_fallback: false`
+          # We need to honor that configuration until all environments are
+          # updated to use idv_resolution_default_vendor
+          if !IdentityConfig.store.proofer_mock_fallback
+            if default_vendor == :mock
+              return :instant_verify
+            end
+          end
+


question: does this mean the mock proofer is never used when proofer_mock_fallback is false?

Currently, when proofer_mock_fallback is true (the default), we use mocked versions of instant verify / phone finder proofers. When it is configured to be false, (i.e., in staging + prod), we use the actual proofers

Ah, I see. I had a hard time wrapping my head around this. I don't know if that means we a more detailed doc comment here, but it's probably fine as is since it is temporary?

I may actually pull this bit out into a separate PR as it's genuinely tricky and I don't want it to get lost among everything else

n1zyy · 2024-11-14T20:47:05Z

On the first step, I ran into this:

{"name":"IdV: doc auth verify proofing results","properties":{"event_properties":{"address_edited":false,"address_line2_present":false,"analytics_id":"Doc Auth","errors":{},"flow_path":"standard","proofing_results":{"exception":"the server responded with status 500 (500)","timed_out":false,"threatmetrix_review_status":"pass","context":{"device_profiling_adjudication_reason":"device_profiling_result_pass","resolution_adjudication_reason":"fail_resolution_without_state_id_coverage","should_proof_state_id":true,"stages":{"resolution":{"success":false,"errors":{},"exception":"the server responded with status 500 (500)","timed_out":false,"transaction_id":null,"reference":"","can_pass_with_additional_verification":false,"attributes_requiring_additional_verification":[],"vendor_name":"socure_kyc","vendor_workflow":null,"verified_attributes":null},"residential_address":{"success":true,"errors":{},"exception":null,"timed_out":false,"transaction_id":"","reference":"","can_pass_with_additional_verification":false,"attributes_requiring_additional_verification":[],"vendor_name":"ResidentialAddressNotRequired","vendor_workflow":null,"verified_attributes":null},"state_id":{"success":true,"errors":{},"exception":null,"mva_exception":null,"requested_attributes":{},"timed_out":false,"transaction_id":"","vendor_name":"UnsupportedJurisdiction","verified_attributes":[],"jurisdiction_in_maintenance_window":false,"state":"MT","state_id_jurisdiction":"ND","state_id_type":"drivers_license","state_id_number":"#############"},"threatmetrix":{"client":null,"success":true,"errors":{},"exception":null,"timed_out":false,"transaction_id":"ddp-mock-transaction-id-123","review_status":"pass","account_lex_id":"super-cool-test-lex-id","session_id":"super-cool-test-session-id"}}},"biographical_info":{"state":"MT","identity_doc_address_state":null,"state_id_jurisdiction":"ND","state_id_number":"#############"},"ssn_is_unique":false},"step":"verify","success":false},"new_event":true,"path":"/verify/verify_info","service_provider":null,"session_duration":266.676375,"user_id":"0463fb17-6b57-48ae-9f13-6c9d995cd7cd","locale":"en","user_ip":"127.0.0.1","hostname":"localhost","pid":61250,"trace_id":null,"git_sha":"da2f6025","git_branch":"matthinz/14725-socure-prog-proofer","user_agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:132.0) Gecko/20100101 Firefox/132.0","browser_name":"Firefox","browser_version":"132.0","browser_platform_name":"macOS","browser_platform_version":"10.15","browser_device_name":"Unknown","browser_mobile":false,"browser_bot":false,"ab_tests":{"recaptcha_sign_in":{"bucket":"sign_in_recaptcha"}}},"time":"2024-11-14T20:27:24.676Z","id":"7f97b414-b273-4fbd-b4e9-5237f01fe712","visitor_id":"f839a128-18e8-48f7-b0f3-f79f5a7634a7","visit_id":"41847544-d705-4ed3-a624-5ad21785d983","log_filename":"events.log"}

It does technically fulfill the requirement:

Check your log/events.log file and verify that your most recent IdV: doc auth verify proofing results event uses the vendor socure_kyc.

But the fact that it returns a 500 for unclear reasons is not great.

I don't know if this is a separate issue or related, but I initially had socure_enabled: false. Interestingly, it seems like it still tried to reach them?

Edit: That block was a mess, but the salient bit:

"proofing_results":{"exception":"the server responded with status 500 (500)","timed_out":false

app/jobs/resolution_proofing_job.rb

matthinz · 2024-11-15T19:17:44Z

But the fact that it returns a 500 for unclear reasons is not great.

That is indeed weird, I will take a look

n1zyy

After some (self-induced?) bumps away, I have successfully run through the manual testing steps, so I'm giving this an approval. The 500 error I got talking to Socure is concerning, but I understand that it's not strictly relevant here. If you're confident it's not a new issue introduced here, I'm good.

lmgeorge · 2024-11-15T21:24:52Z

Clearing my previous "request changes" with "approval" knowing that some of this PR will get split up.

matthinz · 2024-11-16T00:48:22Z

I'm going to close this PR and open a new one on Monday--I'd like to clean up the history a bit so it is a little clearer what's going on.

matthinz added 11 commits November 5, 2024 10:02

Rename instant_verify_state_address_result

f94bfd4

More generic, now state_address_resolution_result

Rename instant_verify_residential_address_result

0a6c141

Go with more generic residential_address_resolution_result

Extract StateIdAddressPlugin module

b3ab742

Pull out all the logic into an include-able module so all you have to provide is a proofer + cost token.

Extract ResidentialAddressPlugin module

392fd6b

Gonna share some code here.

Add Socure plugins

ebcf54d

State ID + Residential address variants

Add null safe navigation operator to ResolutionProofingJob

e0356df

If an exception happens earlier in the job, then document_capture_session can be nil here. Previously this would raise a NoMethodError, obscuring the root cause.

RequestError -> Request::Error

7acd1aa

There were some autoloading issues in cases where calling code wants access to the Error class but does not load the Request class first

Don't crash when applicant includes extra fields

20692f7

In practice, applicant will have extra fields like `state_id_jurisdiction`, `state_id_number`, etc.

Inject proofer / sp_cost_token into plugins

d75c74d

Remove vendor-specific plugins, and go with general-purpose plugins that can have a proofer and sp_cost_token injected into them.

Use mock resolution vendor by default

de31563

Temporarily fall back to proofer_mock_fallback Until we have idv_resolution_default_vendor specified in all production environments, fall back to using proofer_mock_fallback.

matthinz commented Nov 7, 2024

View reviewed changes

app/jobs/resolution_proofing_job.rb Show resolved Hide resolved

matthinz commented Nov 7, 2024

View reviewed changes

Make sure we're raising InvalidProofingVendorError properly

da2f602

matthinz commented Nov 7, 2024

View reviewed changes

matthinz requested a review from a team November 7, 2024 23:23

lmgeorge mentioned this pull request Nov 13, 2024

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

Merged

3 tasks

n1zyy reviewed Nov 13, 2024

View reviewed changes

lmgeorge reviewed Nov 13, 2024

View reviewed changes

lmgeorge suggested changes Nov 14, 2024

View reviewed changes

app/jobs/resolution_proofing_job.rb Show resolved Hide resolved

n1zyy approved these changes Nov 15, 2024

View reviewed changes

lmgeorge approved these changes Nov 15, 2024

View reviewed changes

This was referenced Nov 15, 2024

LG-15090: Update Socure config key names #11512

Merged

Add idv_resolution_default_vendor config #11515

Merged

matthinz closed this Nov 16, 2024

matthinz mentioned this pull request Nov 18, 2024

LG-14725: Identity resolution via Socure #11523

Merged

Conversation

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

Uh oh!

🎫 Ticket

🛠 Summary of changes

New configuration flags

📜 Testing Plan

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!

n1zyy left a comment

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!

lmgeorge left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

lmgeorge Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

n1zyy commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

matthinz commented Nov 15, 2024

Uh oh!

n1zyy left a comment

Choose a reason for hiding this comment

Uh oh!

lmgeorge commented Nov 15, 2024

Uh oh!

matthinz commented Nov 7, 2024 •

edited

Loading

lmgeorge left a comment •

edited

Loading

lmgeorge Nov 13, 2024 •

edited

Loading

n1zyy commented Nov 14, 2024 •

edited

Loading