Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions app/controllers/concerns/idv/verify_info_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,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(pii[:ssn]),
target: Pii::Fingerprinter.fingerprint(ssn),
rate_limit_type: :proof_ssn,
)
end
Expand Down Expand Up @@ -306,11 +307,13 @@ def log_idv_verification_submitted_event(success: false, failure_reason: nil)
end

def check_ssn
Idv::SsnForm.new(current_user).submit(ssn: pii[: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['uuid'] = current_user.uuid
delete_pii
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/concerns/idv_step_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def flow_path
private

def confirm_ssn_step_complete
return if pii.present? && pii[:ssn].present?
return if pii.present? && (idv_session.ssn.present? || pii[:ssn].present?)
redirect_to prev_url
end

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/concerns/rate_limit_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ def idv_attempter_rate_limited?(rate_limit_type)
end

def pii_ssn
return unless defined?(flow_session) && user_session
pii_from_doc_ssn = flow_session[:pii_from_doc]&.[](: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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Initially I thought we needed to make an update to line 70 to include an or statement but whether Remote (pii_from_doc) or IPP (pii_from_user) - if idv_session.ssn was present, line 68 would return it so it would not make it past. Nothing to change - just writing my thoughts.

end
Expand Down
10 changes: 6 additions & 4 deletions app/controllers/idv/in_person/ssn_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ class SsnController < ApplicationController
attr_accessor :error_message

def show
@ssn_form = Idv::SsnFormatForm.new(current_user, flow_session)
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)
Expand All @@ -27,8 +28,8 @@ def show

def update
@error_message = nil

@ssn_form = Idv::SsnFormatForm.new(current_user, flow_session)
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)

Expand All @@ -42,6 +43,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
else
Expand All @@ -68,7 +70,7 @@ def flow_path
end

def confirm_repeat_ssn
return if !pii_from_user[: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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/idv/session_errors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def rate_limited
private

def ssn_from_doc
user_session&.dig('idv/doc_auth', :pii_from_doc, 'ssn')
idv_session&.ssn || user_session&.dig('idv/doc_auth', :pii_from_doc, :ssn)
end

def confirm_two_factor_authenticated_or_user_id_in_session
Expand Down
9 changes: 6 additions & 3 deletions app/controllers/idv/ssn_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ class SsnController < ApplicationController
attr_accessor :error_message

def show
@ssn_form = Idv::SsnFormatForm.new(current_user, flow_session)
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)
Expand All @@ -27,7 +28,8 @@ def show
def update
@error_message = nil

@ssn_form = Idv::SsnFormatForm.new(current_user, flow_session)
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(
Expand All @@ -39,6 +41,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
else
Expand All @@ -50,7 +53,7 @@ def update
private

def confirm_repeat_ssn
return if !pii_from_doc[:ssn]
return if !idv_session.ssn && !pii_from_doc[:ssn]
return if request.referer == idv_verify_info_url

redirect_to idv_verify_info_url
Expand Down
4 changes: 2 additions & 2 deletions app/forms/idv/ssn_format_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ def self.model_name
ActiveModel::Name.new(self, nil, 'doc_auth')
end

def initialize(user, flow_session = {})
def initialize(user, incoming_ssn)
@user = user
@ssn = flow_session.dig(:pii_from_doc, :ssn)
@ssn = incoming_ssn
@updating_ssn = ssn.present?
end

Expand Down
1 change: 1 addition & 0 deletions app/services/idv/session.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class Session
redo_document_capture
resolution_successful
skip_hybrid_handoff
ssn
threatmetrix_review_status
threatmetrix_session_id
user_phone_confirmation
Expand Down
2 changes: 1 addition & 1 deletion app/views/idv/in_person/ssn/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ locals:
<% end %>

<%= simple_form_for(
Idv::SsnFormatForm.new(current_user),
Idv::SsnFormatForm.new(current_user, nil),
url: url_for,
method: :put,
html: { autocomplete: 'off' },
Expand Down
30 changes: 30 additions & 0 deletions spec/controllers/concerns/rate_limit_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,35 @@ def update
expect(response).to redirect_to idv_phone_errors_failure_url
end
end

context 'with proof_ssn rate_limiter (across steps)' do
let(:ssn) { Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN[:ssn] }

before do
RateLimiter.new(
target: Pii::Fingerprinter.fingerprint(ssn),
rate_limit_type: :proof_ssn,
).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
get :show

expect(response).to redirect_to idv_session_errors_ssn_failure_url
end
end
end
end
end
55 changes: 36 additions & 19 deletions spec/controllers/idv/in_person/ssn_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
let(:pii_from_user) { Idp::Constants::MOCK_IDV_APPLICANT_SAME_ADDRESS_AS_ID_WITH_NO_SSN.dup }

let(:flow_session) do
{ 'document_capture_session_uuid' => 'fd14e181-6fb1-4cdc-92e0-ef66dad0df4e',
:pii_from_user => pii_from_user,
:threatmetrix_session_id => 'c90ae7a5-6629-4e77-b97c-f1987c2df7d0',
:flow_path => 'standard' }
{ pii_from_user: pii_from_user,
flow_path: 'standard' }
end

let(:ssn) { Idp::Constants::MOCK_IDV_APPLICANT_WITH_SSN[:ssn] }
Expand Down Expand Up @@ -83,7 +81,7 @@
expect { get :show }.to change { subject.idv_session.threatmetrix_session_id }.from(nil)
end

context 'with an ssn in session' do
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
Expand All @@ -107,6 +105,31 @@
end
end
end

context 'with an ssn in idv_session' do
let(:referer) { idv_in_person_step_url(step: :address) }
before do
subject.idv_session.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
end

describe '#update' do
Expand All @@ -126,16 +149,6 @@
}.merge(ab_test_args)
end

let(:idv_session) do
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This let won't affect idv_session

{
applicant: Idp::Constants::MOCK_IDV_APPLICANT,
resolution_successful: true,
profile_confirmation: true,
vendor_phone_confirmation: true,
user_phone_confirmation: true,
}
end

it 'sends analytics_submitted event' do
put :update, params: params

Expand All @@ -156,14 +169,18 @@
expect(flow_session[:pii_from_user][:ssn]).to eq(ssn)
end

it 'adds ssn to idv_session' do
put :update, params: params

expect(subject.idv_session.ssn).to eq(ssn)
end

it 'invalidates steps after ssn' do
subject.idv_session.applicant = Idp::Constants::MOCK_IDV_APPLICANT

put :update, params: params

expect(subject.idv_session.applicant).to be_blank
expect(subject.idv_session.resolution_successful).to be_blank
expect(subject.idv_session.profile_confirmation).to be_blank
expect(subject.idv_session.vendor_phone_confirmation).to be_blank
expect(subject.idv_session.user_phone_confirmation).to be_blank
Comment on lines -163 to -166
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These attributes are deleted from the spec because they have to be blank because they are set later by the phone step or review step, and ssn step can't be accessed when they are set.

end

it 'redirects to the expected page' do
Expand Down
27 changes: 26 additions & 1 deletion spec/controllers/idv/session_errors_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
allow(idv_session).to receive(:verify_info_step_complete?).
and_return(verify_info_step_complete)
allow(idv_session).to receive(:address_verification_mechanism).and_return(nil)
allow(idv_session).to receive(:ssn).and_return(nil)
allow(controller).to receive(:idv_session).and_return(idv_session)
stub_sign_in(user) if user
stub_analytics
Expand Down Expand Up @@ -284,7 +285,7 @@
rate_limit_type: :proof_ssn,
target: Pii::Fingerprinter.fingerprint(ssn),
).increment_to_limited!
controller.user_session['idv/doc_auth'] = { pii_from_doc: { 'ssn' => ssn } }
controller.user_session['idv/doc_auth'] = { pii_from_doc: { ssn: ssn } }
end

it 'assigns expiration time' do
Expand All @@ -303,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

Expand Down
32 changes: 31 additions & 1 deletion spec/controllers/idv/ssn_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
expect { get :show }.to change { subject.idv_session.threatmetrix_session_id }.from(nil)
end

context 'with an ssn in session' do
context 'with an ssn in flow_session' do
let(:referer) { idv_document_capture_url }
before do
flow_session[:pii_from_doc][:ssn] = ssn
Expand All @@ -115,6 +115,31 @@
end
end

context 'with an ssn in idv_session' do
let(:referer) { idv_document_capture_url }
before do
subject.idv_session.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

it 'overrides Content Security Policies for ThreatMetrix' do
allow(IdentityConfig.store).to receive(:proofing_device_profiling).
and_return(:enabled)
Expand Down Expand Up @@ -159,6 +184,11 @@
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)
end

context 'with a Puerto Rico address' do
it 'redirects to address controller after user enters their SSN' do
flow_session[:pii_from_doc][:state] = 'PR'
Expand Down
Loading