Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
52bd337
prototype of forcing doc auth with selfie on mobile
amirbey Feb 2, 2024
ddba7e8
raise error before making true id request for liveness + cropping wor…
amirbey Feb 5, 2024
22bf920
comment on where to return DocAuth::ErrorRequest
amirbey Feb 5, 2024
d503f4e
do not return workflow for cropping + liveness
amirbey Feb 5, 2024
4cd65f9
revert workflow method changes
amirbey Feb 13, 2024
7990e31
LG-12306: continue work for flow policy check for selfie.
dawei-nava Feb 16, 2024
dd2f77c
LG-12306: cleanup change.
dawei-nava Feb 20, 2024
b8f1931
LG-12306: test validation.
dawei-nava Feb 20, 2024
79022fa
LG-12306: analytics event test. Mark idv_session selfie_check_reqequi…
dawei-nava Feb 21, 2024
fa1d448
LG-12306: test clean up.
dawei-nava Feb 21, 2024
0c2e432
LG-12306: feature test.
dawei-nava Feb 21, 2024
2723c73
LG-12306: format change cause failure.
dawei-nava Feb 21, 2024
e4b9192
LG-12306: dealing with dynamics of selfie requirement.
dawei-nava Feb 21, 2024
4ae1ddf
LG-12306: fix test.
dawei-nava Feb 21, 2024
07618f5
LG-12306: fix test.
dawei-nava Feb 22, 2024
8a5127b
LG-12306: fix test.
dawei-nava Feb 22, 2024
ea59e2f
LG-12306: format.
dawei-nava Feb 22, 2024
580cea7
LG-12306: add configuration flag to indicate selfie allowed on deskto…
dawei-nava Feb 23, 2024
a1f7142
LG-12306: fix test.
dawei-nava Feb 23, 2024
dd0547b
LG-12306: restore events since we now allow selfie in test mode.
dawei-nava Feb 23, 2024
a2f35d2
LG-12306: set the flag to true in dev and test.
dawei-nava Feb 26, 2024
630ce74
LG-12306: try remove the flag.
dawei-nava Feb 26, 2024
21e1da0
LG-12306: rebase to main.
dawei-nava Feb 27, 2024
e5940cf
LG-12306: clean up.
dawei-nava Feb 27, 2024
f538ae2
LG-12306: clean up.
dawei-nava Feb 27, 2024
703ba27
LG-12306: clean up.
dawei-nava Feb 27, 2024
d746033
LG-12306: clean up.
dawei-nava Feb 27, 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
2 changes: 2 additions & 0 deletions app/controllers/concerns/idv_step_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ def flow_policy
end
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.

I wonder if I'm missing a setup step, but when I check out this branch and try running through the testing steps, I did not see the expected result.

Specifically, on desktop, I was not re-directed to the hybrid handoff page. Instead I was able to continue to the upload page.

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.

@night-jellyfish , you have to set doc_auth_selfie_desktop_test_mode: false

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.

Ohh okay. I missed that note somehow. Thanks! I can try again later today if you'd like (quite a few meetings this am) but don't need to block it.


def confirm_step_allowed
Copy link
Copy Markdown
Contributor Author

@dawei-nava dawei-nava Feb 21, 2024

Choose a reason for hiding this comment

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

Maybe it worth considering to inject sp_ession/decoreate_sp_session or even the controller itself into step preconditions, since we may need more and more context information to make decisions.

# set it everytime, since user may switch SP
idv_session.selfie_check_required = decorated_sp_session.selfie_required?
Comment on lines 111 to 112
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.

👍🏿

return if flow_policy.controller_allowed?(controller: self.class)

redirect_to url_for_latest_step
Expand Down
10 changes: 9 additions & 1 deletion app/controllers/idv/document_capture_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ def self.step_info
key: :document_capture,
controller: self,
next_steps: [:ssn, :ipp_ssn], # :ipp_state_id
preconditions: ->(idv_session:, user:) { idv_session.flow_path == 'standard' },
preconditions: ->(idv_session:, user:) {
idv_session.flow_path == 'standard' && (
# mobile
idv_session.skip_hybrid_handoff ||
!idv_session.selfie_check_required || # desktop but selfie not required
idv_session.desktop_selfie_test_mode_enabled?
)
},
undo_step: ->(idv_session:, user:) do
idv_session.pii_from_doc = nil
idv_session.invalidate_in_person_pii_from_user!
Expand All @@ -85,6 +92,7 @@ def analytics_arguments
irs_reproofing: irs_reproofing?,
redo_document_capture: idv_session.redo_document_capture,
skip_hybrid_handoff: idv_session.skip_hybrid_handoff,
selfie_check_required: idv_session.selfie_check_required,
}.merge(ab_test_analytics_buckets)
end

Expand Down
1 change: 1 addition & 0 deletions app/controllers/idv/hybrid_handoff_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ def analytics_arguments
irs_reproofing: irs_reproofing?,
redo_document_capture: params[:redo] ? true : nil,
skip_hybrid_handoff: idv_session.skip_hybrid_handoff,
selfie_check_required: idv_session.selfie_check_required,
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.

👍🏿

}.merge(ab_test_analytics_buckets)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def analytics_arguments
step: 'document_capture',
analytics_id: 'Doc Auth',
irs_reproofing: irs_reproofing?,
selfie_check_required: decorated_sp_session.selfie_required?,
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.

👍🏿

}.merge(
ab_test_analytics_buckets,
)
Expand Down
1 change: 1 addition & 0 deletions app/services/doc_auth/lexis_nexis/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def initialize(config:, user_uuid: nil, uuid_prefix: nil)
end

def fetch
# return DocAuth::Respose with DocAuth:Error if workflow invalid
http_response = send_http_request
return handle_invalid_response(http_response) unless http_response.success?

Expand Down
5 changes: 5 additions & 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
selfie_check_performed
selfie_check_required
skip_doc_auth
skip_hybrid_handoff
ssn
Expand Down Expand Up @@ -249,6 +250,10 @@ def skip_hybrid_handoff?
!!session[:skip_hybrid_handoff]
end

def desktop_selfie_test_mode_enabled?
IdentityConfig.store.doc_auth_selfie_desktop_test_mode
end

private

attr_accessor :user_session
Expand Down
3 changes: 3 additions & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ doc_auth_max_attempts: 5
doc_auth_max_capture_attempts_before_native_camera: 3
doc_auth_max_submission_attempts_before_native_camera: 3
doc_auth_selfie_capture_enabled: false
doc_auth_selfie_desktop_test_mode: false
doc_auth_sdk_capture_orientation: '{"horizontal": 100, "vertical": 0}'
doc_auth_supported_country_codes: '["US", "GU", "VI", "AS", "MP", "PR", "USA" ,"GUM", "VIR", "ASM", "MNP", "PRI"]'
doc_capture_request_valid_for_minutes: 15
Expand Down Expand Up @@ -385,6 +386,7 @@ development:
database_worker_jobs_password: ''
doc_auth_exit_question_section_enabled: false
doc_auth_selfie_capture_enabled: false
doc_auth_selfie_desktop_test_mode: true
doc_auth_vendor: 'mock'
doc_auth_vendor_randomize: false
doc_auth_vendor_randomize_percent: 0
Expand Down Expand Up @@ -527,6 +529,7 @@ test:
database_worker_jobs_password: ''
doc_auth_max_attempts: 4
doc_auth_selfie_capture_enabled: false
doc_auth_selfie_desktop_test_mode: true
doc_auth_vendor: 'mock'
doc_auth_vendor_randomize: false
doc_auth_vendor_randomize_percent: 0
Expand Down
1 change: 1 addition & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def self.build_store(config_map)
config.add(:doc_auth_max_submission_attempts_before_native_camera, type: :integer)
config.add(:doc_auth_s3_request_timeout, type: :integer)
config.add(:doc_auth_selfie_capture_enabled, type: :boolean)
config.add(:doc_auth_selfie_desktop_test_mode, type: :boolean)
config.add(:doc_auth_sdk_capture_orientation, type: :json, options: { symbolize_names: true })
config.add(:doc_auth_supported_country_codes, type: :json)
config.add(:doc_auth_vendor, type: :string)
Expand Down
86 changes: 71 additions & 15 deletions spec/controllers/idv/document_capture_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,52 @@
{ sample_bucket1: :sample_value1, sample_bucket2: :sample_value2 }
end

# selfie related test flags
let(:doc_auth_selfie_capture_enabled) { false }
let(:sp_selfie_enabled) { false }
let(:flow_path) { 'standard' }

before do
stub_sign_in(user)
stub_up_to(:hybrid_handoff, idv_session: subject.idv_session)
stub_analytics
subject.idv_session.document_capture_session_uuid = document_capture_session_uuid

allow(controller.decorated_sp_session).to receive(:selfie_required?).
and_return(doc_auth_selfie_capture_enabled && sp_selfie_enabled)
subject.idv_session.flow_path = flow_path
allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args)
end

describe '#step_info' do
it 'returns a valid StepInfo object' do
expect(Idv::DocumentCaptureController.step_info).to be_valid
end
context 'when selfie feature is enabled system wide' do
let(:doc_auth_selfie_capture_enabled) { true }
describe 'with sp selfie disabled' do
let(:sp_selfie_enabled) { false }
it 'does not satisfy precondition' do
expect(Idv::DocumentCaptureController.step_info.preconditions.is_a?(Proc))
expect(subject).to receive(:render).
with(:show, locals: an_instance_of(Hash)).and_call_original
get :show
expect(response).to render_template :show
end
end
describe 'with sp selfie enabled' do
let(:sp_selfie_enabled) { true }
before do
allow(IdentityConfig.store).to receive(:doc_auth_selfie_desktop_test_mode).
and_return(false)
end
it 'does satisfy precondition' do
expect(Idv::DocumentCaptureController.step_info.preconditions.is_a?(Proc))
expect(subject).not_to receive(:render).with(:show, locals: an_instance_of(Hash))
get :show
expect(response).to redirect_to(idv_hybrid_handoff_path)
end
end
end
end

describe 'before_actions' do
Expand Down Expand Up @@ -61,6 +94,7 @@
skip_hybrid_handoff: nil,
irs_reproofing: false,
step: 'document_capture',
selfie_check_required: sp_selfie_enabled && doc_auth_selfie_capture_enabled,
}.merge(ab_test_args)
end

Expand All @@ -79,23 +113,44 @@
end

context 'when a selfie is requested' do
let(:doc_auth_selfie_capture_enabled) { true }
let(:sp_selfie_enabled) { true }
let(:desktop_selfie_enabled) { false }
before do
allow(subject).to receive(:decorated_sp_session).
and_return(double('decorated_session', { selfie_required?: true, sp_name: 'sp' }))
allow(IdentityConfig.store).to receive(:doc_auth_selfie_desktop_test_mode).
and_return(desktop_selfie_enabled)
end
describe 'when desktop selfie disabled' do
let(:desktop_selfie_enabled) { false }
it 'redirect back to handoff page' do
expect(subject).not_to receive(:render).with(
:show,
locals: hash_including(
document_capture_session_uuid: document_capture_session_uuid,
doc_auth_selfie_capture: true,
),
).and_call_original

get :show

expect(response).to redirect_to(idv_hybrid_handoff_path)
end
end

it 'renders the show template with selfie' do
expect(subject).to receive(:render).with(
:show,
locals: hash_including(
document_capture_session_uuid: document_capture_session_uuid,
doc_auth_selfie_capture: true,
),
).and_call_original

get :show

expect(response).to render_template :show
describe 'when desktop selfie enabled' do
let(:desktop_selfie_enabled) { true }
it 'allows capture' do
expect(subject).to receive(:render).with(
:show,
locals: hash_including(
document_capture_session_uuid: document_capture_session_uuid,
doc_auth_selfie_capture: true,
),
).and_call_original

get :show
expect(response).to render_template :show
end
end
end

Expand Down Expand Up @@ -208,6 +263,7 @@
skip_hybrid_handoff: nil,
irs_reproofing: false,
step: 'document_capture',
selfie_check_required: sp_selfie_enabled && doc_auth_selfie_capture_enabled,
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.

👍🏿

}.merge(ab_test_args)
end
let(:result) { { success: true, errors: {} } }
Expand Down
45 changes: 41 additions & 4 deletions spec/controllers/idv/hybrid_handoff_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,26 @@
end
let(:in_person_proofing) { false }
let(:ipp_opt_in_enabled) { false }
let(:doc_auth_selfie_capture_enabled) { false }
let(:sp_selfie_enabled) { false }

before do
allow(controller).to receive(:current_sp).
and_return(service_provider)
stub_sign_in(user)
stub_up_to(:agreement, idv_session: subject.idv_session)
stub_analytics
stub_attempts_tracker
allow(subject).to receive(:ab_test_analytics_buckets).and_return(ab_test_args)
allow(subject.idv_session).to receive(:service_provider).and_return(service_provider)
allow(subject.decorated_sp_session).to receive(:selfie_required?).
and_return(sp_selfie_enabled && doc_auth_selfie_capture_enabled)
allow(IdentityConfig.store).to receive(:in_person_proofing_enabled) { in_person_proofing }
allow(IdentityConfig.store).to receive(:in_person_proofing_opt_in_enabled) {
ipp_opt_in_enabled
}
allow(IdentityConfig.store).to receive(:doc_auth_selfie_capture_enabled).
and_return(doc_auth_selfie_capture_enabled)
end

describe '#step_info' do
Expand Down Expand Up @@ -58,6 +66,7 @@
redo_document_capture: nil,
skip_hybrid_handoff: nil,
irs_reproofing: false,
selfie_check_required: sp_selfie_enabled && doc_auth_selfie_capture_enabled,
}.merge(ab_test_args)
end

Expand Down Expand Up @@ -200,6 +209,8 @@

context 'opt in selection is nil' do
before do
allow(IdentityConfig.store).to receive(:doc_auth_selfie_desktop_test_mode).
Comment thread
dawei-nava marked this conversation as resolved.
and_return(false)
subject.idv_session.skip_doc_auth = nil
end

Expand All @@ -221,6 +232,8 @@

context 'opted in to ipp flow' do
before do
allow(IdentityConfig.store).to receive(:doc_auth_selfie_desktop_test_mode).
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.

@dawei-nava - do we still need this?

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.

@amirbey removed.

and_return(false)
subject.idv_session.skip_doc_auth = true
end

Expand All @@ -244,6 +257,28 @@
end
end
end

context 'with selfie enabled system wide' do
let(:doc_auth_selfie_capture_enabled) { true }
describe 'when selfie is enabled for sp' do
let(:sp_selfie_enabled) { true }
it 'pass on correct flags and states and logs correct info' do
get :show
expect(response).to render_template :show
expect(@analytics).to have_logged_event(analytics_name, analytics_args)
expect(subject.idv_session.selfie_check_required).to eq(true)
end
end
describe 'when selfie is disabled for sp' do
let(:sp_selfie_enabled) { false }
it 'pass on correct flags and states and logs correct info' do
get :show
expect(response).to render_template :show
expect(subject.idv_session.selfie_check_required).to eq(false)
expect(@analytics).to have_logged_event(analytics_name, analytics_args)
end
end
end
end

describe '#update' do
Expand All @@ -260,6 +295,7 @@
analytics_id: 'Doc Auth',
redo_document_capture: nil,
skip_hybrid_handoff: nil,
selfie_check_required: sp_selfie_enabled && doc_auth_selfie_capture_enabled,
irs_reproofing: false,
telephony_response: {
errors: {},
Expand All @@ -279,6 +315,10 @@

let(:document_capture_session_uuid) { '09228b6d-dd39-4925-bf82-b69104095517' }

before do
subject.idv_session.document_capture_session_uuid = document_capture_session_uuid
end

it 'invalidates future steps' do
expect(subject).to receive(:clear_future_steps!)

Expand All @@ -292,10 +332,6 @@
expect(@analytics).to have_logged_event(analytics_name, analytics_args)
end

before do
subject.idv_session.document_capture_session_uuid = document_capture_session_uuid
end

it 'sends a doc auth link' do
expect(Telephony).to receive(:send_doc_auth_link).with(
hash_including(
Expand All @@ -319,6 +355,7 @@
redo_document_capture: nil,
skip_hybrid_handoff: nil,
irs_reproofing: false,
selfie_check_required: doc_auth_selfie_capture_enabled && sp_selfie_enabled,
}.merge(ab_test_args)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
flow_path: 'hybrid',
irs_reproofing: false,
step: 'document_capture',
selfie_check_required: boolean,
}.merge(ab_test_args)
end

Expand Down Expand Up @@ -181,6 +182,7 @@
flow_path: 'hybrid',
irs_reproofing: false,
step: 'document_capture',
selfie_check_required: boolean,
}.merge(ab_test_args)
end

Expand Down
Loading