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-13963: 👤 Socure shadow mode background job #11139

Merged
merged 20 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
206a468
Add Socure configs to IdentityConfig
matthinz Aug 20, 2024
dc29a66
Add failure_message_when_negated to HaveLoggedEventMatcher
matthinz Aug 20, 2024
8bf0ec4
Add SocureShadowModeProofingJob
matthinz Aug 20, 2024
8ebc93a
Make ResolutionProofingJob schedule Socure KYC call
matthinz Aug 21, 2024
05697ad
Add verified_attributes to resolution result to_h
matthinz Aug 22, 2024
5755551
Add more detail to resolution proofer logging test
matthinz Aug 22, 2024
46d1c97
Don't log TMX response body ong idv_socure_shadow_mode_proofing_result
matthinz Aug 22, 2024
fab11b8
Tweak socure default base URL for dev
matthinz Aug 22, 2024
dbaf5a8
Remove pointless user_id arg to analytics event
matthinz Aug 26, 2024
181ca80
Update app/jobs/socure_shadow_mode_proofing_job.rb
matthinz Aug 26, 2024
c4fbd40
Clarify comment in spec
matthinz Aug 27, 2024
cb276e6
Clarify spec name
matthinz Aug 27, 2024
91b1736
Clarify spec name
matthinz Aug 27, 2024
3130112
Merge remote-tracking branch 'origin/main' into matthinz/13963-socure…
matthinz Aug 27, 2024
4ebb7cd
Merge branch 'main' into matthinz/13963-socure-shadow-mode
matthinz Aug 28, 2024
fd4dcab
Use user.first_email
matthinz Aug 28, 2024
87a34bf
Revert "Use user.first_email"
matthinz Aug 28, 2024
ca499e4
Merge remote-tracking branch 'origin/main' into matthinz/13963-socure…
matthinz Aug 28, 2024
e829c13
Merge branch 'main' into matthinz/13963-socure-shadow-mode
matthinz Aug 30, 2024
6af08a8
Remove pointless service_provider_issuer let
matthinz Aug 30, 2024
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
16 changes: 15 additions & 1 deletion app/jobs/resolution_proofing_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ def perform(
device_profiling_success: callback_log_data&.device_profiling_success,
timing: timer.results,
)

if IdentityConfig.store.idv_socure_shadow_mode_enabled
SocureShadowModeProofingJob.perform_later(
document_capture_session_result_id: document_capture_session.result_id,
encrypted_arguments:,
service_provider_issuer:,
user_email: user_email_for_proofing(user),
user_uuid: user.uuid,
)
Comment on lines +78 to +84
Copy link
Contributor Author

Choose a reason for hiding this comment

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

More to do here around including phone number (which we don't currently pass to ResolutionProofingJob). Captured as LG-14282

Copy link
Contributor

Choose a reason for hiding this comment

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

did we consider enqueuing this job in parallel from inside IDV agent?

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 job depends on the resolution proofing job completing before it can run (since it reads results from the proofing doc capture session). So parallel wouldn't work here

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that so we can compare the results easily in Cloudwatch? I feel like we would be able to accomplish that with a coalesce() | stats ... by properties.user_id or maybe we come up with a job id to join them by that's shared?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, yeah, I like the job id idea, lemme noodle on that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is maybe an argument for using trace_id as a value that can connect the two jobs... It gets passed into ResolutionProofingJob, but then the IdV: doc auth verify proofing results event gets logged by the verify info concern as part of a different request, and so will have a different trace id.

So then I thought maybe create a new socure_shadow_mode_job_id, but that gets a little awkward because we have to store it on the DocumentCaptureSession so that the verify info concern can include it in the payload sent to verify proofing results so that we can then link those events with the socure events.

I don't think it's good enough to use user_id as a key here, since a single user can have multiple submissions.

Which brings me back to the solution I'm using here: schedule the job after the resolution proofing job has run, log payloads for existing + socure in a new event.

The intention here is to run this for a short period of time to gather data, so one advantage to this approach is that the change is mostly contained to the SocureShadowModeProofingJob class, with a little tweak to ResolutionProofingJob.

(I did just create LG-14328 to try addressing some of the awkwardness caused by logging our event from a controller rather than a background job)

Copy link
Contributor

Choose a reason for hiding this comment

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

ok! I was thinking this, but for all the reasons you stated this seems fine as-is too

job_id = SecureRandom.uuid
ResolutionProofingJob.perform_later(job_id:, ...)
SocureShadowModeProofingJob.perform_later(job_id:, ...)

end
end

private
Expand All @@ -89,7 +99,7 @@ def make_vendor_proofing_requests(
)
result = progressive_proofer.proof(
applicant_pii: applicant_pii,
user_email: user.confirmed_email_addresses.first.email,
user_email: user_email_for_proofing(user),
threatmetrix_session_id: threatmetrix_session_id,
request_ip: request_ip,
ipp_enrollment_in_progress: ipp_enrollment_in_progress,
Expand All @@ -109,6 +119,10 @@ def make_vendor_proofing_requests(
)
end

def user_email_for_proofing(user)
user.confirmed_email_addresses.first.email
Copy link
Contributor

Choose a reason for hiding this comment

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

I got distracted wondering if we needed to worry about getting a nil in here somewhere. It looks like there's a lot of prior art for not needing it, though.

That, in turn, led to me wondering if this should actually be implemented on User as, e.g., user.first_email_address or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah user.email is a deprecated method but having user.first_email_address would be very clear

end

def log_threatmetrix_info(threatmetrix_result, user)
logger_info_hash(
name: 'ThreatMetrix',
Expand Down
117 changes: 117 additions & 0 deletions app/jobs/socure_shadow_mode_proofing_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# frozen_string_literal: true

class SocureShadowModeProofingJob < ApplicationJob
include JobHelpers::StaleJobHelper

queue_as :low

discard_on JobHelpers::StaleJobHelper::StaleJobError

# @param [String] document_capture_session_result_id
# @param [String] encrypted_arguments
# @param [String,nil] service_provider_issuer
# @param [String] user_email
# @param [String] user_uuid
def perform(
document_capture_session_result_id:,
encrypted_arguments:,
service_provider_issuer:,
user_email:,
user_uuid:
)
raise_stale_job! if stale_job?(enqueued_at)

user = User.find_by(uuid: user_uuid)
raise "User not found: #{user_uuid}" if !user

analytics = create_analytics(
user:,
service_provider_issuer:,
)

proofing_result = load_proofing_result(document_capture_session_result_id:)
if !proofing_result
analytics.idv_socure_shadow_mode_proofing_result_missing
return
end

applicant = build_applicant(encrypted_arguments:, user_email:)

socure_result = proofer.proof(applicant)

analytics.idv_socure_shadow_mode_proofing_result(
resolution_result: format_proofing_result_for_logs(proofing_result),
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking nit: My read is that this method removes the threatmetrix response body. Would it be worth indicating that in the method name?

socure_result: socure_result.to_h,
user_id: user.uuid,
pii_like_keypaths: [
[:errors, :ssn],
[:resolution_result, :context, :stages, :resolution, :errors, :ssn],
[:resolution_result, :context, :stages, :residential_address, :errors, :ssn],
[:resolution_result, :context, :stages, :threatmetrix, :response_body, :first_name],
[:resolution_result, :context, :stages, :state_id, :state_id_jurisdiction],
],
Comment on lines +46 to +52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are copied from where we log the Verify proofing results event. Socure results don't include PII so we should be good.

)
end

def create_analytics(
user:,
service_provider_issuer:
)
Analytics.new(
user:,
request: nil,
sp: service_provider_issuer,
session: {},
)
end

def format_proofing_result_for_logs(proofing_result)
proofing_result.to_h.tap do |hash|
hash.dig(:context, :stages, :threatmetrix)&.delete(:response_body)
end
end

def load_proofing_result(document_capture_session_result_id:)
DocumentCaptureSession.new(
result_id: document_capture_session_result_id,
).load_proofing_result&.result
end
Comment on lines +74 to +78
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 realized that the proofing result should still be available assuming the SocureShadowModeProofingJob runs soon after the ResolutionProofingJob, so it makes more sense to load the result from there rather than pass it in (which I'd been thinking when I wrote the ticket).

If jobs get delayed and this result is occasionally not available that's not a big deal.


def build_applicant(
encrypted_arguments:,
user_email:
)
decrypted_arguments = JSON.parse(
Encryption::Encryptors::BackgroundProofingArgEncryptor.new.decrypt(encrypted_arguments),
symbolize_names: true,
)

applicant_pii = decrypted_arguments[:applicant_pii]

{
**applicant_pii.slice(
:first_name,
:last_name,
:address1,
:address2,
:city,
:state,
:zipcode,
:phone,
:dob,
:ssn,
),
email: user_email,
}
end

def proofer
@proofer ||= Proofing::Socure::IdPlus::Proofer.new(
Proofing::Socure::IdPlus::Config.new(
api_key: IdentityConfig.store.socure_idplus_api_key,
base_url: IdentityConfig.store.socure_idplus_base_url,
timeout: IdentityConfig.store.socure_idplus_timeout_in_seconds,
),
)
end
end
22 changes: 22 additions & 0 deletions app/services/analytics_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4203,6 +4203,28 @@ def idv_session_error_visited(
)
end

# 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
def idv_socure_shadow_mode_proofing_result(
socure_result:,
resolution_result:,
**extra
)
track_event(
:idv_socure_shadow_mode_proofing_result,
resolution_result: resolution_result.to_h,
socure_result: socure_result.to_h,
**extra,
)
end

# Indicates that no proofing result was found when SocureShadowModeProofingJob
# attempted to look for one.
def idv_socure_shadow_mode_proofing_result_missing(**extra)
track_event(:idv_socure_shadow_mode_proofing_result_missing, **extra)
end

# @param [String] step
# @param [String] location
# @param [Hash,nil] proofing_components User's current proofing components
Expand Down
1 change: 1 addition & 0 deletions app/services/proofing/resolution/result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def to_h
attributes_requiring_additional_verification,
vendor_name: vendor_name,
vendor_workflow: vendor_workflow,
verified_attributes: verified_attributes,
}
end

Expand Down
7 changes: 5 additions & 2 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ idv_max_attempts: 5
idv_min_age_years: 13
idv_send_link_attempt_window_in_minutes: 10
idv_send_link_max_attempts: 5
idv_socure_shadow_mode_enabled: false
idv_sp_required: false
in_person_completion_survey_url: 'https://login.gov'
in_person_doc_auth_button_enabled: true
Expand Down Expand Up @@ -335,6 +336,9 @@ sign_in_user_id_per_ip_attempt_window_exponential_factor: 1.1
sign_in_user_id_per_ip_attempt_window_in_minutes: 720
sign_in_user_id_per_ip_attempt_window_max_minutes: 43_200
sign_in_user_id_per_ip_max_attempts: 50
socure_idplus_api_key: ''
socure_idplus_base_url: ''
socure_idplus_timeout_in_seconds: 5
socure_webhook_secret_key: ''
socure_webhook_secret_key_queue: '[]'
sp_handoff_bounce_max_seconds: 2
Expand Down Expand Up @@ -427,8 +431,7 @@ development:
show_unsupported_passkey_platform_authentication_setup: true
sign_in_recaptcha_score_threshold: 0.3
skip_encryption_allowed_list: '["urn:gov:gsa:SAML:2.0.profiles:sp:sso:localhost"]'
socure_webhook_secret_key: 'secret-key'
socure_webhook_secret_key_queue: '["old-key-one", "old-key-two"]'
socure_idplus_base_url: 'https://sandbox.socure.us'
state_tracking_enabled: true
telephony_adapter: test
use_dashboard_service_providers: true
Expand Down
4 changes: 4 additions & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def self.store
config.add(:idv_min_age_years, type: :integer)
config.add(:idv_send_link_attempt_window_in_minutes, type: :integer)
config.add(:idv_send_link_max_attempts, type: :integer)
config.add(:idv_socure_shadow_mode_enabled, type: :boolean)
config.add(:idv_sp_required, type: :boolean)
config.add(:in_person_completion_survey_url, type: :string)
config.add(:in_person_doc_auth_button_enabled, type: :boolean)
Expand Down Expand Up @@ -359,6 +360,9 @@ def self.store
config.add(:s3_reports_enabled, type: :boolean)
config.add(:saml_endpoint_configs, type: :json, options: { symbolize_names: true })
config.add(:saml_secret_rotation_enabled, type: :boolean)
config.add(:socure_idplus_api_key, type: :string)
config.add(:socure_idplus_base_url, type: :string)
config.add(:socure_idplus_timeout_in_seconds, type: :integer)
config.add(:scrypt_cost, type: :string)
config.add(:second_mfa_reminder_account_age_in_days, type: :integer)
config.add(:second_mfa_reminder_sign_in_count, type: :integer)
Expand Down
9 changes: 6 additions & 3 deletions spec/features/idv/analytics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@
can_pass_with_additional_verification: false,
attributes_requiring_additional_verification: [],
vendor_name: 'ResolutionMock',
vendor_workflow: nil }
vendor_workflow: nil,
verified_attributes: nil }
end

let(:base_proofing_results) do
Expand All @@ -95,7 +96,8 @@
timed_out: false,
transaction_id: '',
vendor_name: 'ResidentialAddressNotRequired',
vendor_workflow: nil },
vendor_workflow: nil,
verified_attributes: nil },
state_id: state_id_resolution,
threatmetrix: threatmetrix_response,
},
Expand Down Expand Up @@ -124,7 +126,8 @@
can_pass_with_additional_verification: false,
attributes_requiring_additional_verification: [],
vendor_name: 'ResolutionMock',
vendor_workflow: nil },
vendor_workflow: nil,
verified_attributes: nil },
state_id: state_id_resolution,
threatmetrix: threatmetrix_response,
},
Expand Down
62 changes: 62 additions & 0 deletions spec/jobs/resolution_proofing_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,68 @@
end
end

context 'socure shadow mode' do
context 'turned on' do
before do
allow(IdentityConfig.store).to receive(:idv_socure_shadow_mode_enabled).and_return(true)
end

it 'schedules a SocureShadowModeProofingJob' do
stub_vendor_requests
expect(SocureShadowModeProofingJob).to receive(:perform_later).with(
user_email: user.email,
user_uuid: user.uuid,
document_capture_session_result_id: document_capture_session.result_id,
encrypted_arguments: satisfy do |ciphertext|
json = JSON.parse(
Encryption::Encryptors::BackgroundProofingArgEncryptor.new.decrypt(ciphertext),
symbolize_names: true,
)
expect(json[:applicant_pii]).to eql(
{
first_name: 'FAKEY',
middle_name: nil,
last_name: 'MCFAKERSON',
address1: '1 FAKE RD',
identity_doc_address1: '1 FAKE RD',
identity_doc_address2: nil,
identity_doc_city: 'GREAT FALLS',
identity_doc_address_state: 'MT',
identity_doc_zipcode: '59010-1234',
issuing_country_code: 'US',
address2: nil,
same_address_as_id: 'true',
city: 'GREAT FALLS',
state: 'MT',
zipcode: '59010-1234',
dob: '1938-10-06',
ssn: '900-66-1234',
state_id_jurisdiction: 'ND',
state_id_expiration: '2099-12-31',
state_id_issued: '2019-12-31',
state_id_number: '1111111111111',
state_id_type: 'drivers_license',
},
)
end,
service_provider_issuer: service_provider.issuer,
)

perform
end
end

context 'turned off' do
it 'does not schedule a SocureShadowModeProofingJob' do
stub_vendor_requests

expect(SocureShadowModeProofingJob).not_to receive(:perform_later)

perform
end
end
end

it 'determines the UUID and UUID prefix and passes it to the downstream proofing vendors' do
uuid_info = {
uuid_prefix: service_provider.app_id,
Expand Down
Loading