From 1225f1b1ad8de81358d9c8cebb7f89cb4d16923d Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 27 Mar 2023 11:09:08 -0700 Subject: [PATCH 1/4] Move confirm_profile_not_already_confirmed to IdvStepConcern In a post-FSM world we want all the logic about redirects between steps to be in IdvStepConcern. Include this method in the pattern, and include IdvStepConcern in SsnController. Also remove redundant calls to `confirm_two_factor_authenticated` before action since it is included by IdvStepConcern. changelog: Internal, Identity Verification, Refactor code that moves between steps --- app/controllers/concerns/idv/step_utilities_concern.rb | 5 ----- app/controllers/concerns/idv_step_concern.rb | 5 +++++ app/controllers/idv/ssn_controller.rb | 2 +- app/controllers/idv/verify_info_controller.rb | 1 - 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/controllers/concerns/idv/step_utilities_concern.rb b/app/controllers/concerns/idv/step_utilities_concern.rb index 626abf69c62..488d180123d 100644 --- a/app/controllers/concerns/idv/step_utilities_concern.rb +++ b/app/controllers/concerns/idv/step_utilities_concern.rb @@ -19,11 +19,6 @@ def confirm_pii_from_doc redirect_to idv_doc_auth_url end - def confirm_profile_not_already_confirmed - return unless idv_session.verify_info_step_complete? - redirect_to idv_review_url - end - # Copied from capture_doc_flow.rb # and from doc_auth_flow.rb def acuant_sdk_ab_test_analytics_args diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 0067dc4c46d..055122a32d2 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -18,6 +18,11 @@ def confirm_verify_info_step_complete end end + def confirm_profile_not_already_confirmed + return unless idv_session.verify_info_step_complete? + redirect_to idv_review_url + end + def confirm_address_step_complete return if idv_session.address_step_complete? diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb index 76ddf64111a..10fc1fe9e03 100644 --- a/app/controllers/idv/ssn_controller.rb +++ b/app/controllers/idv/ssn_controller.rb @@ -1,11 +1,11 @@ module Idv class SsnController < ApplicationController include IdvSession + include IdvStepConcern include StepIndicatorConcern include StepUtilitiesConcern include Steps::ThreatMetrixStepHelper - before_action :confirm_two_factor_authenticated before_action :confirm_profile_not_already_confirmed before_action :confirm_pii_from_doc diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 2601be0f70e..07d04d2b4ee 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -6,7 +6,6 @@ class VerifyInfoController < ApplicationController include VerifyInfoConcern include Steps::ThreatMetrixStepHelper - before_action :confirm_two_factor_authenticated before_action :confirm_ssn_step_complete before_action :confirm_profile_not_already_confirmed From 09464a0829e9c5722a3504fae2bf619951840ef3 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 27 Mar 2023 13:14:44 -0700 Subject: [PATCH 2/4] Rename confirm_profile_not_already_confirmed to confirm_verify_info_step_needed Note that in_person/verify_info_controller has its own implementation which was also renamed. --- app/controllers/concerns/idv_step_concern.rb | 2 +- app/controllers/idv/in_person/verify_info_controller.rb | 4 ++-- app/controllers/idv/ssn_controller.rb | 2 +- app/controllers/idv/verify_info_controller.rb | 2 +- spec/controllers/idv/in_person/verify_info_controller_spec.rb | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 055122a32d2..451dab8189e 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -18,7 +18,7 @@ def confirm_verify_info_step_complete end end - def confirm_profile_not_already_confirmed + def confirm_verify_info_step_needed return unless idv_session.verify_info_step_complete? redirect_to idv_review_url end diff --git a/app/controllers/idv/in_person/verify_info_controller.rb b/app/controllers/idv/in_person/verify_info_controller.rb index 21747e62fb5..3fef4d4b638 100644 --- a/app/controllers/idv/in_person/verify_info_controller.rb +++ b/app/controllers/idv/in_person/verify_info_controller.rb @@ -9,7 +9,7 @@ class VerifyInfoController < ApplicationController before_action :renders_404_if_flag_not_set before_action :confirm_two_factor_authenticated before_action :confirm_ssn_step_complete - before_action :confirm_profile_not_already_confirmed + before_action :confirm_verify_info_step_needed def show @in_person_proofing = true @@ -107,7 +107,7 @@ def confirm_ssn_step_complete redirect_to idv_in_person_url end - def confirm_profile_not_already_confirmed + def confirm_verify_info_step_needed # todo: should this instead be like so? # return unless idv_session.resolution_successful == true return unless idv_session.verify_info_step_complete? diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb index 10fc1fe9e03..462e0a30e47 100644 --- a/app/controllers/idv/ssn_controller.rb +++ b/app/controllers/idv/ssn_controller.rb @@ -6,7 +6,7 @@ class SsnController < ApplicationController include StepUtilitiesConcern include Steps::ThreatMetrixStepHelper - before_action :confirm_profile_not_already_confirmed + before_action :confirm_verify_info_step_needed before_action :confirm_pii_from_doc attr_accessor :error_message diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index 07d04d2b4ee..9ea99d5ae5b 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -7,7 +7,7 @@ class VerifyInfoController < ApplicationController include Steps::ThreatMetrixStepHelper before_action :confirm_ssn_step_complete - before_action :confirm_profile_not_already_confirmed + before_action :confirm_verify_info_step_needed def show @in_person_proofing = false diff --git a/spec/controllers/idv/in_person/verify_info_controller_spec.rb b/spec/controllers/idv/in_person/verify_info_controller_spec.rb index d3a0cf80a5d..83ea59e23d9 100644 --- a/spec/controllers/idv/in_person/verify_info_controller_spec.rb +++ b/spec/controllers/idv/in_person/verify_info_controller_spec.rb @@ -35,7 +35,7 @@ it 'confirms verify step not already complete' do expect(subject).to have_actions( :before, - :confirm_profile_not_already_confirmed, + :confirm_verify_info_step_needed, ) end From b64c134839a650527392d07525a7eaf209a8d125 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 27 Mar 2023 13:49:39 -0700 Subject: [PATCH 3/4] Move confirm_pii_from_doc to IdvStepConcern Note that AddressController has its own slightly different implementation of this method. --- app/controllers/concerns/idv/step_utilities_concern.rb | 8 -------- app/controllers/concerns/idv_step_concern.rb | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/concerns/idv/step_utilities_concern.rb b/app/controllers/concerns/idv/step_utilities_concern.rb index 488d180123d..59e03150f23 100644 --- a/app/controllers/concerns/idv/step_utilities_concern.rb +++ b/app/controllers/concerns/idv/step_utilities_concern.rb @@ -11,14 +11,6 @@ def flow_path flow_session[:flow_path] end - def confirm_pii_from_doc - @pii = flow_session&.[]('pii_from_doc') # hash with indifferent access - return if @pii.present? - - flow_session&.delete('Idv::Steps::DocumentCaptureStep') - redirect_to idv_doc_auth_url - end - # Copied from capture_doc_flow.rb # and from doc_auth_flow.rb def acuant_sdk_ab_test_analytics_args diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 451dab8189e..44661abcb09 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -8,6 +8,14 @@ module IdvStepConcern before_action :confirm_idv_needed end + def confirm_pii_from_doc + @pii = flow_session&.[]('pii_from_doc') # hash with indifferent access + return if @pii.present? + + flow_session&.delete('Idv::Steps::DocumentCaptureStep') + redirect_to idv_doc_auth_url + end + def confirm_verify_info_step_complete return if idv_session.verify_info_step_complete? From 83779d1e3c06169ed835e951589e006231fd1ab0 Mon Sep 17 00:00:00 2001 From: Sonia Connolly Date: Mon, 27 Mar 2023 14:02:57 -0700 Subject: [PATCH 4/4] Rename `confirm_pii_from_doc` to `confirm_document_capture_complete` including in AddressController --- app/controllers/concerns/idv_step_concern.rb | 2 +- app/controllers/idv/address_controller.rb | 4 ++-- app/controllers/idv/ssn_controller.rb | 2 +- spec/controllers/idv/ssn_controller_spec.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 44661abcb09..dc5f3742356 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -8,7 +8,7 @@ module IdvStepConcern before_action :confirm_idv_needed end - def confirm_pii_from_doc + def confirm_document_capture_complete @pii = flow_session&.[]('pii_from_doc') # hash with indifferent access return if @pii.present? diff --git a/app/controllers/idv/address_controller.rb b/app/controllers/idv/address_controller.rb index a52f40c116b..9492de49ae9 100644 --- a/app/controllers/idv/address_controller.rb +++ b/app/controllers/idv/address_controller.rb @@ -3,7 +3,7 @@ class AddressController < ApplicationController include IdvSession before_action :confirm_two_factor_authenticated - before_action :confirm_pii_from_doc + before_action :confirm_document_capture_complete def new analytics.idv_address_visit @@ -24,7 +24,7 @@ def update private - def confirm_pii_from_doc + def confirm_document_capture_complete @pii = user_session.dig('idv/doc_auth', 'pii_from_doc') return if @pii.present? redirect_to idv_doc_auth_url diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb index 462e0a30e47..25945d0373f 100644 --- a/app/controllers/idv/ssn_controller.rb +++ b/app/controllers/idv/ssn_controller.rb @@ -7,7 +7,7 @@ class SsnController < ApplicationController include Steps::ThreatMetrixStepHelper before_action :confirm_verify_info_step_needed - before_action :confirm_pii_from_doc + before_action :confirm_document_capture_complete attr_accessor :error_message diff --git a/spec/controllers/idv/ssn_controller_spec.rb b/spec/controllers/idv/ssn_controller_spec.rb index 19a9a49da96..af91a94147f 100644 --- a/spec/controllers/idv/ssn_controller_spec.rb +++ b/spec/controllers/idv/ssn_controller_spec.rb @@ -31,7 +31,7 @@ it 'checks that the previous step is complete' do expect(subject).to have_actions( :before, - :confirm_pii_from_doc, + :confirm_document_capture_complete, ) end end