-
Notifications
You must be signed in to change notification settings - Fork 166
LG-7400 Add update functionality to VerifyInfoController outside Flow State Machine #7661
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
Changes from all commits
e2f0c5e
9d7bb48
25a5151
7085cd8
60f5117
d3dfc1a
f0ed81f
64254dc
0d8334d
3a2c085
41a6339
98c7232
fcd3ebe
a37b795
b4ea51d
1d1f1d2
e41beaf
5658495
cd03803
0d97dab
f3c3d86
383b95f
63ce60b
8a9d0c0
4c4068e
4efe550
02c29ac
6c8669d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| module StringRedacter | ||
| extend ActiveSupport::Concern | ||
|
|
||
| def redact_alphanumeric(text) | ||
| text.gsub(/[a-z]/i, 'X').gsub(/\d/i, '#') | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,55 @@ | ||||||||||||||
| module Idv | ||||||||||||||
| class VerifyInfoController < ApplicationController | ||||||||||||||
| include StringRedacter | ||||||||||||||
| include IdvSession | ||||||||||||||
|
|
||||||||||||||
| before_action :render_404_if_verify_info_controller_disabled | ||||||||||||||
| before_action :confirm_two_factor_authenticated | ||||||||||||||
| before_action :confirm_ssn_step_complete | ||||||||||||||
|
|
||||||||||||||
| def show | ||||||||||||||
| increment_step_counts | ||||||||||||||
| analytics.idv_doc_auth_verify_visited(**analytics_arguments) | ||||||||||||||
|
|
||||||||||||||
| redirect_to failure_url(:fail) and return if any_throttled? | ||||||||||||||
|
|
||||||||||||||
| process_async_state(load_async_state) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def update | ||||||||||||||
| return if idv_session.verify_info_step_document_capture_session_uuid | ||||||||||||||
|
|
||||||||||||||
| pii[:uuid_prefix] = ServiceProvider.find_by(issuer: sp_session[:issuer])&.app_id | ||||||||||||||
|
|
||||||||||||||
| if ssn_throttle.throttled_else_increment? | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the SSN rate limiting in effect here, but I do not see the resolution rate limiting in effect. I only see that in the new method. Is there something I am missing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two different throttles as you noted. In the old code they are both referred to as "throttle." We decided to resolve the confusion. Verify_base_step was only checking the ssn throttle on enqueue_job (see second link) which is our update method. Do you want us to add resolution throttle here? See identity-idp/app/services/idv/steps/verify_base_step.rb Lines 72 to 74 in cc482cb
identity-idp/app/services/idv/steps/verify_base_step.rb Lines 175 to 177 in cc482cb
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Chatted with the team; we're going to address this in the PR for LG-7401 |
||||||||||||||
| analytics.throttler_rate_limit_triggered( | ||||||||||||||
| throttle_type: :proof_ssn, | ||||||||||||||
| step_name: 'verify_info', | ||||||||||||||
| ) | ||||||||||||||
| redirect_to idv_session_errors_ssn_failure_url | ||||||||||||||
| return | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| document_capture_session = DocumentCaptureSession.create( | ||||||||||||||
| user_id: current_user.id, | ||||||||||||||
| issuer: sp_session[:issuer], | ||||||||||||||
| ) | ||||||||||||||
| document_capture_session.requested_at = Time.zone.now | ||||||||||||||
|
|
||||||||||||||
| idv_session.verify_info_step_document_capture_session_uuid = document_capture_session.uuid | ||||||||||||||
| idv_session.vendor_phone_confirmation = false | ||||||||||||||
| idv_session.user_phone_confirmation = false | ||||||||||||||
|
|
||||||||||||||
| Idv::Agent.new(pii).proof_resolution( | ||||||||||||||
| document_capture_session, | ||||||||||||||
| should_proof_state_id: should_use_aamva?(pii), | ||||||||||||||
| trace_id: amzn_trace_id, | ||||||||||||||
| user_id: current_user.id, | ||||||||||||||
| threatmetrix_session_id: flow_session[:threatmetrix_session_id], | ||||||||||||||
| request_ip: request.remote_ip, | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| redirect_to idv_verify_info_url | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| private | ||||||||||||||
|
|
@@ -59,6 +102,10 @@ def pii | |||||||||||||
| @pii = flow_session[:pii_from_doc] if flow_session | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def delete_pii | ||||||||||||||
| flow_session.delete(:pii_from_user) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| # copied from address_controller | ||||||||||||||
| def confirm_ssn_step_complete | ||||||||||||||
| return if pii.present? && pii[:ssn].present? | ||||||||||||||
|
|
@@ -74,5 +121,278 @@ def current_flow_step_counts | |||||||||||||
| def increment_step_counts | ||||||||||||||
| current_flow_step_counts['verify'] += 1 | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| # copied from verify_base_step | ||||||||||||||
| def should_use_aamva?(pii) | ||||||||||||||
| aamva_state?(pii) && !aamva_disallowed_for_service_provider? | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def aamva_state?(pii) | ||||||||||||||
| IdentityConfig.store.aamva_supported_jurisdictions.include?( | ||||||||||||||
| pii['state_id_jurisdiction'], | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def aamva_disallowed_for_service_provider? | ||||||||||||||
| return false if sp_session.nil? | ||||||||||||||
| banlist = IdentityConfig.store.aamva_sp_banlist_issuers | ||||||||||||||
| banlist.include?(sp_session[:issuer]) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def resolution_throttle | ||||||||||||||
| @resolution_throttle ||= Throttle.new( | ||||||||||||||
| user: current_user, | ||||||||||||||
| throttle_type: :idv_resolution, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def ssn_throttle | ||||||||||||||
| @ssn_throttle ||= Throttle.new( | ||||||||||||||
| target: Pii::Fingerprinter.fingerprint(pii[:ssn]), | ||||||||||||||
| throttle_type: :proof_ssn, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def any_throttled? | ||||||||||||||
| ssn_throttle.throttled? || resolution_throttle.throttled? | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def idv_failure(result) | ||||||||||||||
| proofing_results_exception = result.extra.dig(:proofing_results, :exception) | ||||||||||||||
|
|
||||||||||||||
| resolution_throttle.increment! if proofing_results_exception.blank? | ||||||||||||||
| if resolution_throttle.throttled? | ||||||||||||||
| idv_failure_log_throttled | ||||||||||||||
| redirect_to throttled_url | ||||||||||||||
| elsif proofing_results_exception.present? | ||||||||||||||
| idv_failure_log_error | ||||||||||||||
| redirect_to exception_url | ||||||||||||||
| else | ||||||||||||||
| idv_failure_log_warning | ||||||||||||||
| redirect_to warning_url | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def idv_failure_log_throttled | ||||||||||||||
| irs_attempts_api_tracker.idv_verification_rate_limited | ||||||||||||||
| analytics.throttler_rate_limit_triggered( | ||||||||||||||
| throttle_type: :idv_resolution, | ||||||||||||||
| step_name: self.class.name, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def idv_failure_log_error | ||||||||||||||
| analytics.idv_doc_auth_exception_visited( | ||||||||||||||
| step_name: self.class.name, | ||||||||||||||
| remaining_attempts: resolution_throttle.remaining_count, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def idv_failure_log_warning | ||||||||||||||
| analytics.idv_doc_auth_warning_visited( | ||||||||||||||
| step_name: self.class.name, | ||||||||||||||
| remaining_attempts: resolution_throttle.remaining_count, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def throttled_url | ||||||||||||||
| idv_session_errors_failure_url | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def exception_url | ||||||||||||||
| idv_session_errors_exception_url | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def warning_url | ||||||||||||||
| idv_session_errors_warning_url | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| # copied from verify_base_step. May want reconciliation with phone_step | ||||||||||||||
| def process_async_state(current_async_state) | ||||||||||||||
| if current_async_state.none? | ||||||||||||||
| idv_session.resolution_successful = false | ||||||||||||||
| render :show | ||||||||||||||
| elsif current_async_state.in_progress? | ||||||||||||||
| render :wait | ||||||||||||||
| elsif current_async_state.missing? | ||||||||||||||
| analytics.idv_proofing_resolution_result_missing | ||||||||||||||
| flash.now[:error] = I18n.t('idv.failure.timeout') | ||||||||||||||
| render :show | ||||||||||||||
|
|
||||||||||||||
| delete_async | ||||||||||||||
| idv_session.resolution_successful = false | ||||||||||||||
|
|
||||||||||||||
| log_idv_verification_submitted_event( | ||||||||||||||
| success: false, | ||||||||||||||
| failure_reason: { idv_verification: [:timeout] }, | ||||||||||||||
| ) | ||||||||||||||
| elsif current_async_state.done? | ||||||||||||||
| async_state_done(current_async_state) | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def async_state_done(current_async_state) | ||||||||||||||
| add_proofing_costs(current_async_state.result) | ||||||||||||||
| form_response = idv_result_to_form_response( | ||||||||||||||
| result: current_async_state.result, | ||||||||||||||
| state: pii[:state], | ||||||||||||||
| state_id_jurisdiction: pii[:state_id_jurisdiction], | ||||||||||||||
| state_id_number: pii[:state_id_number], | ||||||||||||||
| # todo: add other edited fields? | ||||||||||||||
| extra: { | ||||||||||||||
| address_edited: !!flow_session['address_edited'], | ||||||||||||||
| pii_like_keypaths: [[:errors, :ssn], [:response_body, :first_name]], | ||||||||||||||
| }, | ||||||||||||||
| ) | ||||||||||||||
| log_idv_verification_submitted_event( | ||||||||||||||
| success: form_response.success?, | ||||||||||||||
| failure_reason: irs_attempts_api_tracker.parse_failure_reason(form_response), | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| if form_response.success? | ||||||||||||||
| response = check_ssn | ||||||||||||||
| form_response = form_response.merge(response) | ||||||||||||||
| end | ||||||||||||||
| summarize_result_and_throttle_failures(form_response) | ||||||||||||||
| delete_async | ||||||||||||||
|
|
||||||||||||||
| if form_response.success? | ||||||||||||||
| idv_session.resolution_successful = true | ||||||||||||||
| redirect_to idv_phone_url | ||||||||||||||
| else | ||||||||||||||
| idv_session.resolution_successful = false | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| analytics.idv_doc_auth_verify_proofing_results(**form_response.to_h) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def summarize_result_and_throttle_failures(summary_result) | ||||||||||||||
| if summary_result.success? | ||||||||||||||
| add_proofing_components | ||||||||||||||
| summary_result | ||||||||||||||
| else | ||||||||||||||
| idv_failure(summary_result) | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def add_proofing_components | ||||||||||||||
| ProofingComponent.create_or_find_by(user: current_user).update( | ||||||||||||||
| resolution_check: Idp::Constants::Vendors::LEXIS_NEXIS, | ||||||||||||||
| source_check: Idp::Constants::Vendors::AAMVA, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def load_async_state | ||||||||||||||
| dcs_uuid = idv_session.verify_info_step_document_capture_session_uuid | ||||||||||||||
| dcs = DocumentCaptureSession.find_by(uuid: dcs_uuid) | ||||||||||||||
| return ProofingSessionAsyncResult.none if dcs_uuid.nil? | ||||||||||||||
| return ProofingSessionAsyncResult.missing if dcs.nil? | ||||||||||||||
|
|
||||||||||||||
| proofing_job_result = dcs.load_proofing_result | ||||||||||||||
| return ProofingSessionAsyncResult.missing if proofing_job_result.nil? | ||||||||||||||
|
|
||||||||||||||
| proofing_job_result | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def delete_async | ||||||||||||||
| idv_session.verify_info_step_document_capture_session_uuid = nil | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def idv_result_to_form_response( | ||||||||||||||
| result:, | ||||||||||||||
| state: nil, | ||||||||||||||
| state_id_jurisdiction: nil, | ||||||||||||||
| state_id_number: nil, | ||||||||||||||
| extra: {} | ||||||||||||||
| ) | ||||||||||||||
| state_id = result.dig(:context, :stages, :state_id) | ||||||||||||||
| if state_id | ||||||||||||||
| state_id[:state] = state if state | ||||||||||||||
| state_id[:state_id_jurisdiction] = state_id_jurisdiction if state_id_jurisdiction | ||||||||||||||
| state_id[:state_id_number] = redact_alphanumeric(state_id_number) if state_id_number | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| FormResponse.new( | ||||||||||||||
| success: result[:success], | ||||||||||||||
| errors: result[:errors], | ||||||||||||||
| extra: extra.merge(proofing_results: result.except(:errors, :success)), | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def log_idv_verification_submitted_event(success: false, failure_reason: nil) | ||||||||||||||
| pii_from_doc = pii || {} | ||||||||||||||
| irs_attempts_api_tracker.idv_verification_submitted( | ||||||||||||||
| success: success, | ||||||||||||||
| document_state: pii_from_doc[:state], | ||||||||||||||
| document_number: pii_from_doc[:state_id_number], | ||||||||||||||
| document_issued: pii_from_doc[:state_id_issued], | ||||||||||||||
| document_expiration: pii_from_doc[:state_id_expiration], | ||||||||||||||
| first_name: pii_from_doc[:first_name], | ||||||||||||||
| last_name: pii_from_doc[:last_name], | ||||||||||||||
| date_of_birth: pii_from_doc[:dob], | ||||||||||||||
| address: pii_from_doc[:address1], | ||||||||||||||
| ssn: pii_from_doc[:ssn], | ||||||||||||||
| failure_reason: failure_reason, | ||||||||||||||
| ) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def check_ssn | ||||||||||||||
| result = Idv::SsnForm.new(current_user).submit(ssn: pii[:ssn]) | ||||||||||||||
|
|
||||||||||||||
| if result.success? | ||||||||||||||
| save_legacy_state | ||||||||||||||
| delete_pii | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| result | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def save_legacy_state | ||||||||||||||
| skip_legacy_steps | ||||||||||||||
| idv_session.applicant = pii | ||||||||||||||
| idv_session.applicant['uuid'] = current_user.uuid | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def skip_legacy_steps | ||||||||||||||
| idv_session.profile_confirmation = true | ||||||||||||||
| idv_session.vendor_phone_confirmation = false | ||||||||||||||
| idv_session.user_phone_confirmation = false | ||||||||||||||
| idv_session.address_verification_mechanism = 'phone' | ||||||||||||||
| idv_session.resolution_successful = 'phone' | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def add_proofing_costs(results) | ||||||||||||||
| results[:context][:stages].each do |stage, hash| | ||||||||||||||
| if stage == :resolution | ||||||||||||||
| # transaction_id comes from ConversationId | ||||||||||||||
| add_cost(:lexis_nexis_resolution, transaction_id: hash[:transaction_id]) | ||||||||||||||
| elsif stage == :state_id | ||||||||||||||
| next if hash[:vendor_name] == 'UnsupportedJurisdiction' | ||||||||||||||
| process_aamva(hash[:transaction_id]) | ||||||||||||||
| elsif stage == :threatmetrix | ||||||||||||||
| # transaction_id comes from request_id | ||||||||||||||
| tmx_id = hash[:transaction_id] | ||||||||||||||
| add_cost(:threatmetrix, transaction_id: tmx_id) if tmx_id | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def process_aamva(transaction_id) | ||||||||||||||
| # transaction_id comes from TransactionLocatorId | ||||||||||||||
| add_cost(:aamva, transaction_id: transaction_id) | ||||||||||||||
| track_aamva | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def track_aamva | ||||||||||||||
| return unless IdentityConfig.store.state_tracking_enabled | ||||||||||||||
| doc_auth_log = DocAuthLog.find_by(user_id: current_user.id) | ||||||||||||||
| return unless doc_auth_log | ||||||||||||||
| doc_auth_log.aamva = true | ||||||||||||||
| doc_auth_log.save! | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| def add_cost(token, transaction_id: nil) | ||||||||||||||
| Db::SpCost::AddSpCost.call(current_sp, 2, token, transaction_id: transaction_id) | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
| end | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking at
#confirm_idv_steps_completewhich is also a before action and has a call to#idv_profile_complete?. Wdyt of wrapping this logic into that method? That way a user can confirm in either the FSM or the new controller and still pass the before actions here.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly it doesn't get to that before action, but makes a redirect loop between review and verify_info somewhere before that. I couldn't figure out where, but putting this before action right at the top fixed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paired on this with the team. We figured out that the problem is a little complicated. The process looks like this:
confirm_idv_session_startedbefore action which checks for an applicant. Since resolution did not succeed there is no applicant. This causes it to redirect back to the doc auth FSMWith all that in mind we are all convinced the before action is probably the best route. We will have to figure out how to handle the redirects back to the flow state machine or our new code when we handle the deployment ticket (LG-7402).