Skip to content

Add idv_resolution_default_vendor config#11515

Merged
matthinz merged 2 commits intomainfrom
matthinz/config-for-resolution-proofer
Nov 18, 2024
Merged

Add idv_resolution_default_vendor config#11515
matthinz merged 2 commits intomainfrom
matthinz/config-for-resolution-proofer

Conversation

@matthinz
Copy link
Contributor

🎫 Ticket

This PR was extracted from #11479, and is in support of
LG-14725

🛠 Summary of changes

Historically, we have used one config flag, proofer_mock_fallback, to control whether mocked resolution and address Proofers are used during identity verification. This value is set to true (use mocks) by default and false (don't use mocks) in production environments.

We are moving (gradually) toward a world where we have multiple identity resolution vendors, and thus 3 possible config states for identity resolution (Instant Verify, Socure KYC, and Mock). #11479 is about running Instant Verify and Socure side-by-side in production, and requires a different configuration strategy than proofer_mock_fallback: false.

This PR adds a new config setting, idv_resolution_default_vendor (the _default_ is because there will soon be an _alternate_ setting as well). This setting can have one of two values:

  • mock (default)
  • instant_verify

Hopefully it is clear which setting corresponds to which proofer being used.

📜 Testing Plan

The testing process here is to:

  1. Set (or delete) some values in your application.yml
  2. Restart your server
  3. Run through IdV up until you submit the "Verify your information" step
proofer_mock_fallback idv_resolution_default_vendor Expected result
(not present) (not present) "Verify your information" succeeds and uses mock proofer
(not present) instant_verify "Verify your information" fails because it tried to contact Instant Verify
(not present) mock "Verify your information" succeeds and uses mock proofer
false (not present) "Verify your information" fails because it tried to contact Instant Verify
false instant_verify "Verify your information" fails because it tried to contact Instant Verify
false mock IMPORTANT: "Verify your information" fails because it tried to contact Instant Verify (this will be the initial config in production upon deploy)
true (not present) "Verify your information" succeeds and uses mock proofer
true instant_verify IMPORTANT: "Verify your information" fails because it tried to contact Instant Verify (we should not be letting the old setting tell us to use mocks)
true mock "Verify your information" succeeds and uses mock proofer
(not presetn) any other value IDP fails to start because config value is invalid

(You can monitor log/events.log to see the relevant IdV: doc auth verify proofing results events going by.)

Start the process of separating configuration of the resolution proofer mock fallback from the address proofer mock fallback.

For a short period of time, we'll need to continue to support the old method of configuration (until staging and prod configs can be updated).

[skip changelog]
Comment on lines +27 to +56
@proofer ||= begin
# Historically, proofer_mock_fallback has controlled whether we
# use mock implementations of the Resolution and Address proofers
# (true = use mock, false = don't use mock).
# We are transitioning to a place where we will have separate
# configs for both. For the time being, we want to keep support for
# proofer_mock_fallback here. This can be removed after this code
# has been deployed and configs have been updated in all relevant
# environments.

old_config_says_mock = IdentityConfig.store.proofer_mock_fallback
old_config_says_iv = !old_config_says_mock
new_config_says_mock =
IdentityConfig.store.idv_resolution_default_vendor == :mock
new_config_says_iv =
IdentityConfig.store.idv_resolution_default_vendor == :instant_verify

proofer_type =
if new_config_says_mock && old_config_says_iv
# This will be the case immediately after deployment, when
# environment configs have not been updated. We need to
# fall back to the old config here.
:instant_verify
elsif new_config_says_iv
:instant_verify
else
:mock
end

if proofer_type == :mock
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that these blocks (and the added specs) are identical for the two plugins--the intention is that this is temporary code (cleanup PR coming), soon to be further streamlined in another PR.

Comment on lines +179 to +183
config.add(
:idv_resolution_default_vendor,
type: :symbol,
enum: [:instant_verify, :mock],
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This, it turns out, is a much better thing to do than raise errors in application code if the setting is misconfigured

Copy link
Contributor

@n1zyy n1zyy left a comment

Choose a reason for hiding this comment

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

One nit about an ever-so-slightly confusing comment, but overall I really like the way this is organized for maximum clarity.

:instant_verify
else
:mock
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a fan of really expressive code like this for this sort of thing.

I think customarily we'd argue that old_config_says_mock is unneeded as a variable, and the first two cases of this conditional could be combined. But in temporary code handling something a little complicated to reason about? This makes it really easy to understand.

Comment on lines +46 to +48
# This will be the case immediately after deployment, when
# environment configs have not been updated. We need to
# fall back to the old config here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor: the examples in the tests say "the new setting takes precedence," but here the comment says we "fall back to the old config" which suggests it's the other way around. They're not discussing the same thing and the behavior is actually consistent, but if you see an easy way to make the wording consistent, it might further prevent confusion for anyone looking at this first thing in the morning while under-caffeinated. (Hypothetically speaking.)

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 think this is a case of bad spec name. I changed it to creates an Instant Verify proofer because the new setting takes precedence over the old one when the old one is set to its default value. The idea is that, when proofer_mock_fallback is false, that is an indication that it has (probably) not been actively configured. If the new setting is set to a non-default value, we should use that instead.

@matthinz matthinz merged commit 272549b into main Nov 18, 2024
@matthinz matthinz deleted the matthinz/config-for-resolution-proofer branch November 18, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants