From 802fad9d6cc198dd419500979327b567ddf84679 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 14 Sep 2023 13:09:41 -0400 Subject: [PATCH] Revert "LG-10886 remove ssn from flow session (#9182)" This reverts commit e44cfbb4d398baa2c786d36c228be409afb8fb53. --- .../concerns/idv/verify_info_concern.rb | 11 +++--- app/controllers/concerns/idv_step_concern.rb | 2 +- .../concerns/rate_limit_concern.rb | 6 ++-- .../idv/in_person/ssn_controller.rb | 11 +++--- .../idv/in_person/verify_info_controller.rb | 2 +- .../idv/session_errors_controller.rb | 8 +++-- app/controllers/idv/ssn_controller.rb | 9 +++-- app/controllers/idv/verify_info_controller.rb | 2 +- .../concerns/rate_limit_concern_spec.rb | 10 ++++++ .../idv/in_person/ssn_controller_spec.rb | 33 ++++++++++++++++- .../in_person/verify_info_controller_spec.rb | 1 - .../idv/session_errors_controller_spec.rb | 26 +++++++++++++- spec/controllers/idv/ssn_controller_spec.rb | 35 +++++++++++++++++-- .../idv/verify_info_controller_spec.rb | 26 +++++++++++--- 14 files changed, 154 insertions(+), 28 deletions(-) diff --git a/app/controllers/concerns/idv/verify_info_concern.rb b/app/controllers/concerns/idv/verify_info_concern.rb index 9169af3cf40..a4172f60aab 100644 --- a/app/controllers/concerns/idv/verify_info_concern.rb +++ b/app/controllers/concerns/idv/verify_info_concern.rb @@ -25,7 +25,6 @@ def shared_update idv_session.vendor_phone_confirmation = false idv_session.user_phone_confirmation = false - pii[:ssn] = idv_session.ssn # Required for proof_resolution job Idv::Agent.new(pii).proof_resolution( document_capture_session, should_proof_state_id: should_use_aamva?(pii), @@ -70,8 +69,9 @@ def resolution_rate_limiter end def ssn_rate_limiter + ssn = idv_session.ssn || pii[:ssn] @ssn_rate_limiter ||= RateLimiter.new( - target: Pii::Fingerprinter.fingerprint(idv_session.ssn), + target: Pii::Fingerprinter.fingerprint(ssn), rate_limit_type: :proof_ssn, ) end @@ -301,18 +301,19 @@ def log_idv_verification_submitted_event(success: false, failure_reason: nil) last_name: pii_from_doc[:last_name], date_of_birth: pii_from_doc[:dob], address: pii_from_doc[:address1], - ssn: idv_session.ssn, + ssn: idv_session.ssn || pii_from_doc[:ssn], failure_reason: failure_reason, ) end def check_ssn - Idv::SsnForm.new(current_user).submit(ssn: idv_session.ssn) + ssn = idv_session.ssn || pii[:ssn] + Idv::SsnForm.new(current_user).submit(ssn: ssn) end def move_applicant_to_idv_session idv_session.applicant = pii - idv_session.applicant[:ssn] = idv_session.ssn + idv_session.applicant[:ssn] ||= idv_session.ssn idv_session.applicant['uuid'] = current_user.uuid delete_pii end diff --git a/app/controllers/concerns/idv_step_concern.rb b/app/controllers/concerns/idv_step_concern.rb index 157b6d8ab8b..44ce2834e9d 100644 --- a/app/controllers/concerns/idv_step_concern.rb +++ b/app/controllers/concerns/idv_step_concern.rb @@ -52,7 +52,7 @@ def flow_path private def confirm_ssn_step_complete - return if pii.present? && idv_session.ssn.present? + return if pii.present? && (idv_session.ssn.present? || pii[:ssn].present?) redirect_to prev_url end diff --git a/app/controllers/concerns/rate_limit_concern.rb b/app/controllers/concerns/rate_limit_concern.rb index 8ada2c10913..d5b3a9ae4e7 100644 --- a/app/controllers/concerns/rate_limit_concern.rb +++ b/app/controllers/concerns/rate_limit_concern.rb @@ -64,7 +64,9 @@ def idv_attempter_rate_limited?(rate_limit_type) end def pii_ssn - return unless defined?(idv_session) && user_session - idv_session&.ssn + return unless defined?(flow_session) && defined?(idv_session) && user_session + pii_from_doc_ssn = idv_session&.ssn || flow_session[:pii_from_doc]&.[](:ssn) + return pii_from_doc_ssn if pii_from_doc_ssn + flow_session[:pii_from_user]&.[](:ssn) end end diff --git a/app/controllers/idv/in_person/ssn_controller.rb b/app/controllers/idv/in_person/ssn_controller.rb index 6b9dad37590..8a717ba6968 100644 --- a/app/controllers/idv/in_person/ssn_controller.rb +++ b/app/controllers/idv/in_person/ssn_controller.rb @@ -14,7 +14,8 @@ class SsnController < ApplicationController attr_accessor :error_message def show - @ssn_form = Idv::SsnFormatForm.new(current_user, idv_session.ssn) + incoming_ssn = idv_session.ssn || flow_session.dig(:pii_from_user, :ssn) + @ssn_form = Idv::SsnFormatForm.new(current_user, incoming_ssn) analytics.idv_doc_auth_redo_ssn_submitted(**analytics_arguments) if updating_ssn? analytics.idv_doc_auth_ssn_visited(**analytics_arguments) @@ -27,7 +28,8 @@ def show def update @error_message = nil - @ssn_form = Idv::SsnFormatForm.new(current_user, idv_session.ssn) + incoming_ssn = idv_session.ssn || flow_session.dig(:pii_from_user, :ssn) + @ssn_form = Idv::SsnFormatForm.new(current_user, incoming_ssn) ssn = params.require(:doc_auth).permit(:ssn) form_response = @ssn_form.submit(ssn) @@ -40,6 +42,7 @@ def update ) if form_response.success? + flow_session[:pii_from_user][:ssn] = params[:doc_auth][:ssn] idv_session.ssn = params[:doc_auth][:ssn] idv_session.invalidate_steps_after_ssn! redirect_to idv_in_person_verify_info_url @@ -67,7 +70,7 @@ def flow_path end def confirm_repeat_ssn - return if !idv_session.ssn + return if !idv_session.ssn && !pii_from_user[:ssn] return if request.referer == idv_in_person_verify_info_url redirect_to idv_in_person_verify_info_url end @@ -83,7 +86,7 @@ def analytics_arguments end def updating_ssn? - idv_session.ssn.present? + idv_session.ssn.present? || flow_session.dig(:pii_from_user, :ssn).present? end def confirm_in_person_address_step_complete diff --git a/app/controllers/idv/in_person/verify_info_controller.rb b/app/controllers/idv/in_person/verify_info_controller.rb index 6ce2fc38a91..e2853047e3c 100644 --- a/app/controllers/idv/in_person/verify_info_controller.rb +++ b/app/controllers/idv/in_person/verify_info_controller.rb @@ -12,7 +12,7 @@ class VerifyInfoController < ApplicationController def show @step_indicator_steps = step_indicator_steps - @ssn = idv_session.ssn + @ssn = idv_session.ssn || flow_session[:pii_from_user][:ssn] @capture_secondary_id_enabled = capture_secondary_id_enabled analytics.idv_doc_auth_verify_visited(**analytics_arguments) diff --git a/app/controllers/idv/session_errors_controller.rb b/app/controllers/idv/session_errors_controller.rb index d7b9ca201c3..4dd98df5847 100644 --- a/app/controllers/idv/session_errors_controller.rb +++ b/app/controllers/idv/session_errors_controller.rb @@ -39,9 +39,9 @@ def failure def ssn_failure rate_limiter = nil - if idv_session&.ssn + if ssn_from_doc rate_limiter = RateLimiter.new( - target: Pii::Fingerprinter.fingerprint(idv_session.ssn), + target: Pii::Fingerprinter.fingerprint(ssn_from_doc), rate_limit_type: :proof_ssn, ) @expires_at = rate_limiter.expires_at @@ -59,6 +59,10 @@ def rate_limited private + def ssn_from_doc + idv_session&.ssn || user_session&.dig('idv/doc_auth', :pii_from_doc, :ssn) + end + def confirm_two_factor_authenticated_or_user_id_in_session return if session[:doc_capture_user_id].present? diff --git a/app/controllers/idv/ssn_controller.rb b/app/controllers/idv/ssn_controller.rb index f0e64000d91..ab41a5b0df5 100644 --- a/app/controllers/idv/ssn_controller.rb +++ b/app/controllers/idv/ssn_controller.rb @@ -13,7 +13,8 @@ class SsnController < ApplicationController attr_accessor :error_message def show - @ssn_form = Idv::SsnFormatForm.new(current_user, idv_session.ssn) + incoming_ssn = idv_session.ssn || flow_session.dig(:pii_from_doc, :ssn) + @ssn_form = Idv::SsnFormatForm.new(current_user, incoming_ssn) analytics.idv_doc_auth_redo_ssn_submitted(**analytics_arguments) if @ssn_form.updating_ssn? analytics.idv_doc_auth_ssn_visited(**analytics_arguments) @@ -27,7 +28,8 @@ def show def update @error_message = nil - @ssn_form = Idv::SsnFormatForm.new(current_user, idv_session.ssn) + incoming_ssn = idv_session.ssn || flow_session.dig(:pii_from_doc, :ssn) + @ssn_form = Idv::SsnFormatForm.new(current_user, incoming_ssn) form_response = @ssn_form.submit(params.require(:doc_auth).permit(:ssn)) analytics.idv_doc_auth_ssn_submitted( @@ -38,6 +40,7 @@ def update ) if form_response.success? + flow_session[:pii_from_doc][:ssn] = params[:doc_auth][:ssn] idv_session.ssn = params[:doc_auth][:ssn] idv_session.invalidate_steps_after_ssn! redirect_to next_url @@ -50,7 +53,7 @@ def update private def confirm_repeat_ssn - return if !idv_session.ssn + return if !idv_session.ssn && !pii_from_doc[:ssn] return if request.referer == idv_verify_info_url redirect_to idv_verify_info_url diff --git a/app/controllers/idv/verify_info_controller.rb b/app/controllers/idv/verify_info_controller.rb index cd4f7e1c340..833143b4cb7 100644 --- a/app/controllers/idv/verify_info_controller.rb +++ b/app/controllers/idv/verify_info_controller.rb @@ -11,7 +11,7 @@ class VerifyInfoController < ApplicationController def show @step_indicator_steps = step_indicator_steps - @ssn = idv_session.ssn + @ssn = idv_session.ssn || pii_from_doc[:ssn] analytics.idv_doc_auth_verify_visited(**analytics_arguments) Funnel::DocAuth::RegisterStep.new(current_user.id, sp_session[:issuer]). diff --git a/spec/controllers/concerns/rate_limit_concern_spec.rb b/spec/controllers/concerns/rate_limit_concern_spec.rb index f8e3b8b29b7..25bc2ee6482 100644 --- a/spec/controllers/concerns/rate_limit_concern_spec.rb +++ b/spec/controllers/concerns/rate_limit_concern_spec.rb @@ -89,6 +89,16 @@ def update ).increment_to_limited! end + context 'ssn is in flow session' do + it 'redirects to proof_ssn rate limited error page' do + flow_session = { pii_from_doc: { ssn: ssn } } + allow(subject).to receive(:flow_session).and_return(flow_session) + get :show + + expect(response).to redirect_to idv_session_errors_ssn_failure_url + end + end + context 'ssn is in idv_session' do it 'redirects to proof_ssn rate limited error page' do subject.idv_session.ssn = ssn diff --git a/spec/controllers/idv/in_person/ssn_controller_spec.rb b/spec/controllers/idv/in_person/ssn_controller_spec.rb index 1bf95e7713f..a5ad306f80b 100644 --- a/spec/controllers/idv/in_person/ssn_controller_spec.rb +++ b/spec/controllers/idv/in_person/ssn_controller_spec.rb @@ -81,6 +81,31 @@ expect { get :show }.to change { subject.idv_session.threatmetrix_session_id }.from(nil) end + context 'with an ssn in flow_session' do + let(:referer) { idv_in_person_step_url(step: :address) } + before do + flow_session[:pii_from_user][:ssn] = ssn + request.env['HTTP_REFERER'] = referer + end + + context 'referer is not verify_info' do + it 'redirects to verify_info' do + get :show + + expect(response).to redirect_to(idv_in_person_verify_info_url) + end + end + + context 'referer is verify_info' do + let(:referer) { idv_in_person_verify_info_url } + it 'does not redirect' do + get :show + + expect(response).to render_template :show + end + end + end + context 'with an ssn in idv_session' do let(:referer) { idv_in_person_step_url(step: :address) } before do @@ -138,6 +163,12 @@ put :update, params: params end + it 'merges ssn into pii session value' do + put :update, params: params + + expect(flow_session[:pii_from_user][:ssn]).to eq(ssn) + end + it 'adds ssn to idv_session' do put :update, params: params @@ -159,7 +190,7 @@ end it 'does not change threatmetrix_session_id when updating ssn' do - subject.idv_session.ssn = ssn + flow_session[:pii_from_user][:ssn] = ssn put :update, params: params session_id = subject.idv_session.threatmetrix_session_id subject.threatmetrix_view_variables 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 eeaca608221..3cd9a7773dd 100644 --- a/spec/controllers/idv/in_person/verify_info_controller_spec.rb +++ b/spec/controllers/idv/in_person/verify_info_controller_spec.rb @@ -20,7 +20,6 @@ before do allow(subject).to receive(:flow_session).and_return(flow_session) stub_sign_in(user) - subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID[:ssn] allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args) end diff --git a/spec/controllers/idv/session_errors_controller_spec.rb b/spec/controllers/idv/session_errors_controller_spec.rb index 8fd17a30332..82c9da477c2 100644 --- a/spec/controllers/idv/session_errors_controller_spec.rb +++ b/spec/controllers/idv/session_errors_controller_spec.rb @@ -285,7 +285,7 @@ rate_limit_type: :proof_ssn, target: Pii::Fingerprinter.fingerprint(ssn), ).increment_to_limited! - allow(idv_session).to receive(:ssn).and_return(ssn) + controller.user_session['idv/doc_auth'] = { pii_from_doc: { ssn: ssn } } end it 'assigns expiration time' do @@ -304,6 +304,30 @@ ) get action end + + context 'with ssn in idv_session' do + before do + controller.user_session['idv/doc_auth'] = {} + allow(idv_session).to receive(:ssn).and_return(ssn) + end + + it 'assigns expiration time' do + get action + + expect(assigns(:expires_at)).not_to eq(Time.zone.now) + end + + it 'logs an event with attempts remaining' do + expect(@analytics).to receive(:track_event).with( + 'IdV: session error visited', + hash_including( + type: 'ssn_failure', + attempts_remaining: 0, + ), + ) + get action + end + end end end diff --git a/spec/controllers/idv/ssn_controller_spec.rb b/spec/controllers/idv/ssn_controller_spec.rb index d16a33c2bcd..e64e04f9f0c 100644 --- a/spec/controllers/idv/ssn_controller_spec.rb +++ b/spec/controllers/idv/ssn_controller_spec.rb @@ -90,6 +90,31 @@ expect { get :show }.to change { subject.idv_session.threatmetrix_session_id }.from(nil) end + context 'with an ssn in flow_session' do + let(:referer) { idv_document_capture_url } + before do + flow_session[:pii_from_doc][:ssn] = ssn + request.env['HTTP_REFERER'] = referer + end + + context 'referer is not verify_info' do + it 'redirects to verify_info' do + get :show + + expect(response).to redirect_to(idv_verify_info_url) + end + end + + context 'referer is verify_info' do + let(:referer) { idv_verify_info_url } + it 'does not redirect' do + get :show + + expect(response).to render_template :show + end + end + end + context 'with an ssn in idv_session' do let(:referer) { idv_document_capture_url } before do @@ -153,6 +178,12 @@ }.merge(ab_test_args) end + it 'merges ssn into pii session value' do + put :update, params: params + + expect(flow_session[:pii_from_doc][:ssn]).to eq(ssn) + end + it 'updates idv_session.ssn' do expect { put :update, params: params }.to change { subject.idv_session.ssn }. from(nil).to(ssn) @@ -168,7 +199,7 @@ end it 'redirects to the verify info controller if a user is updating their SSN' do - subject.idv_session.ssn = ssn + flow_session[:pii_from_doc][:ssn] = ssn flow_session[:pii_from_doc][:state] = 'PR' put :update, params: params @@ -195,7 +226,7 @@ end it 'does not change threatmetrix_session_id when updating ssn' do - subject.idv_session.ssn = ssn + flow_session[:pii_from_doc][:ssn] = ssn put :update, params: params session_id = subject.idv_session.threatmetrix_session_id subject.threatmetrix_view_variables diff --git a/spec/controllers/idv/verify_info_controller_spec.rb b/spec/controllers/idv/verify_info_controller_spec.rb index 97cd1a4f177..8e52b8a21a1 100644 --- a/spec/controllers/idv/verify_info_controller_spec.rb +++ b/spec/controllers/idv/verify_info_controller_spec.rb @@ -4,7 +4,7 @@ include IdvHelper let(:flow_session) do - { pii_from_doc: Idp::Constants::MOCK_IDV_APPLICANT.dup } + { pii_from_doc: Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN.dup } end let(:user) { create(:user) } @@ -26,7 +26,6 @@ stub_attempts_tracker stub_idv_steps_before_verify_step(user) subject.idv_session.flow_path = 'standard' - subject.idv_session.ssn = Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN[:ssn] subject.user_session['idv/doc_auth'] = flow_session allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args) end @@ -65,7 +64,7 @@ }.merge(ab_test_args) end - it 'renders the show template' do + it 'renders the show template (with ssn in flow_session)' do get :show expect(response).to render_template :show @@ -115,6 +114,7 @@ end it 'redirects to ssn controller when ssn info is missing' do + flow_session[:pii_from_doc][:ssn] = nil subject.idv_session.ssn = nil get :show @@ -122,6 +122,15 @@ expect(response).to redirect_to(idv_ssn_url) end + it 'renders show when ssn is in idv_session' do + subject.idv_session.ssn = flow_session[:pii_from_doc][:ssn] + flow_session[:pii_from_doc][:ssn] = nil + + get :show + + expect(response).to render_template :show + end + context 'when the user is ssn rate limited' do before do RateLimiter.new( @@ -132,7 +141,16 @@ ).increment_to_limited! end - it 'redirects to ssn failure url' do + it 'redirects to ssn failure url with ssn in flow session' do + get :show + + expect(response).to redirect_to idv_session_errors_ssn_failure_url + end + + it 'redirects to ssn failure url with ssn in idv_session' do + subject.idv_session.ssn = flow_session[:pii_from_doc][:ssn] + flow_session[:pii_from_doc][:ssn] = nil + get :show expect(response).to redirect_to idv_session_errors_ssn_failure_url