Skip to content
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

LG-15321: relax vendor redirect in test mode #11677

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
56babf7
init relax vendor redirect if in test mode
amirbey Dec 19, 2024
f609ac7
fix doc cap standard test
amirbey Dec 20, 2024
0540593
fix doc cap socure hybrid test
amirbey Dec 20, 2024
2348654
fix doc cap hybrid test
amirbey Dec 20, 2024
25fce5a
in test mode do not redirect except if socure and facial match req'd
amirbey Dec 20, 2024
04256f5
fix specs to not allow socure to have flow in test mode for selfie ca…
amirbey Dec 21, 2024
2a6562c
update happy path specs to be able to upload links while enforcing do…
amirbey Dec 23, 2024
50318a6
allow step without forcing doc cap redirect
amirbey Dec 23, 2024
66cdf8c
disable invalid test
amirbey Dec 23, 2024
8090280
fix hybrid mobile spec
amirbey Jan 2, 2025
ed14dd5
create flag for disable redirect_to_correct_vendor
amirbey Jan 2, 2025
d6d733e
undo spec changes
amirbey Jan 2, 2025
bd87c2e
add redirect spec
amirbey Jan 2, 2025
cf90e59
when redirect to correct vendor is disabled on hybrid mobile spec
amirbey Jan 2, 2025
68c6905
undo spec preconditions change
amirbey Jan 2, 2025
ce52e96
use default redirect to correct vendor in dev
amirbey Jan 3, 2025
02910a8
only bypass redirect correct vendor if disabled
amirbey Jan 3, 2025
1e5bbfd
only bypass redirect correct vendor if disabled - remove spec
amirbey Jan 3, 2025
4a48c23
test disable vendor redirect for socure standard controller
amirbey Jan 3, 2025
1ea142b
test redirect correct vendor in socure doc auth controllers
amirbey Jan 3, 2025
7fc03f2
test redirect correct vendor in standard doc auth controllers
amirbey Jan 3, 2025
97b41c7
happy linting
amirbey Jan 3, 2025
6e767ef
rename env var doc_auth_disable_redirect_to_correct_vendor to doc_aut…
amirbey Jan 3, 2025
63ea14f
rename env var doc_auth_disable_redirect_to_correct_vendor to doc_aut…
amirbey Jan 3, 2025
c37a578
lint config file
amirbey Jan 3, 2025
ba163f7
remove unused variable
amirbey Jan 3, 2025
58b949e
fix spec for document request api calls to test hybrid flow
amirbey Jan 3, 2025
22e643b
happy linting
amirbey Jan 3, 2025
e2fdc7e
add spec to test when socure hybrid mobile when redirect is enforced
amirbey Jan 10, 2025
4c00cf0
update spec desc
amirbey Jan 10, 2025
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/document_capture_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ def selfie_requirement_met?
end

def redirect_to_correct_vendor(vendor, in_hybrid_mobile)
return if IdentityConfig.store.doc_auth_redirect_to_correct_vendor_disabled

expected_doc_auth_vendor = doc_auth_vendor
return if vendor == expected_doc_auth_vendor
return if vendor == Idp::Constants::Vendors::LEXIS_NEXIS &&
Expand Down
1 change: 1 addition & 0 deletions config/application.yml.default
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,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_read_additional_pii_attributes_enabled: false
doc_auth_redirect_to_correct_vendor_disabled: false
doc_auth_selfie_desktop_test_mode: false
doc_auth_socure_wait_polling_refresh_max_seconds: 15
doc_auth_socure_wait_polling_timeout_minutes: 2
Expand Down
1 change: 1 addition & 0 deletions lib/identity_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def self.store
config.add(:doc_auth_check_failed_image_resubmission_enabled, type: :boolean)
config.add(:doc_auth_client_glare_threshold, type: :integer)
config.add(:doc_auth_client_sharpness_threshold, type: :integer)
config.add(:doc_auth_redirect_to_correct_vendor_disabled, type: :boolean)
config.add(:doc_auth_error_dpi_threshold, type: :integer)
config.add(:doc_auth_error_glare_threshold, type: :integer)
config.add(:doc_auth_error_sharpness_threshold, type: :integer)
Expand Down
32 changes: 19 additions & 13 deletions spec/controllers/idv/document_capture_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# selfie related test flags
let(:sp_selfie_enabled) { false }
let(:flow_path) { 'standard' }
let(:doc_auth_selfie_desktop_test_mode) { false }

before do
stub_sign_in(user)
Expand All @@ -41,6 +42,9 @@
allow(IdentityConfig.store).to receive(:doc_auth_vendor_default).and_return(
Idp::Constants::Vendors::LEXIS_NEXIS,
)

allow(IdentityConfig.store).to receive(:doc_auth_selfie_desktop_test_mode)
.and_return(doc_auth_selfie_desktop_test_mode)
end

describe '#step_info' do
Expand All @@ -64,11 +68,6 @@
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))
Expand Down Expand Up @@ -172,6 +171,19 @@

expect(response).to redirect_to idv_socure_document_capture_url
end

context 'when redirect to correct vendor is disabled' do
before do
allow(IdentityConfig.store)
.to receive(:doc_auth_redirect_to_correct_vendor_disabled).and_return(true)
end

it 'redirects to the Socure controller' do
get :show

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

context 'socure is the default vendor but facial match is required' do
Expand All @@ -193,13 +205,8 @@

context 'when a selfie is requested' do
let(:sp_selfie_enabled) { true }
let(:desktop_selfie_enabled) { false }
before do
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,
Expand All @@ -216,7 +223,7 @@
end

describe 'when desktop selfie enabled' do
let(:desktop_selfie_enabled) { true }
let(:doc_auth_selfie_desktop_test_mode) { true }
it 'allows capture' do
expect(subject).to receive(:render).with(
:show,
Expand Down Expand Up @@ -321,7 +328,6 @@
let(:sp_selfie_enabled) { true }

before do
allow(IdentityConfig.store).to receive(:doc_auth_selfie_desktop_test_mode).and_return(false)
allow(Idv::InPersonConfig).to receive(:enabled_for_issuer?).with(anything).and_return(false)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@

expect(response).to redirect_to idv_hybrid_mobile_socure_document_capture_url
end

context 'when redirect to correct vendor is disabled' do
before do
allow(IdentityConfig.store)
.to receive(:doc_auth_redirect_to_correct_vendor_disabled).and_return(true)
end

it 'redirects to the Socure controller' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
it 'redirects to the Socure controller' do
it 'allows the user to use this controller' do

get :show

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

it 'renders the show template' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,23 @@
expect(document_capture_session.socure_docv_transaction_token)
.to eq(docv_transaction_token)
end

context 'when we try to use this controller but we should be using the LN/mock version' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add the test where the redirect to correct vendor is enabled, similar to the LN/mock test?

Copy link
Contributor Author

@amirbey amirbey Jan 10, 2025

Choose a reason for hiding this comment

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

spec added to this context .. thanks @solipet 👍🏿

context 'when redirect to correct vendor is disabled' do
let(:idv_vendor) { Idp::Constants::Vendors::LEXIS_NEXIS }
before do
allow(IdentityConfig.store)
.to receive(:doc_auth_redirect_to_correct_vendor_disabled).and_return(true)
end

it 'redirects to the Socure controller' do
get :show

expect(response).to have_http_status 200
expect(response.body).to have_link(href: socure_capture_app_url)
end
end
end
end
end

Expand Down
23 changes: 22 additions & 1 deletion spec/controllers/idv/socure/document_capture_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
allow(IdentityConfig.store).to receive(:doc_auth_vendor_switching_enabled)
.and_return(vendor_switching_enabled)
allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user)

allow(subject).to receive(:stored_result).and_return(stored_result)

user_session = {}
Expand Down Expand Up @@ -98,6 +97,28 @@
get :show
expect(response).to redirect_to idv_document_capture_url
end

context 'when redirect to correct vendor is disabled' do
let(:socure_capture_app_url) { 'https://verify.socure.test/' }
let(:response_body) do
{
data: {
docvTransactionToken: SecureRandom.hex(6),
url: socure_capture_app_url,
},
}
end
before do
allow(IdentityConfig.store)
.to receive(:doc_auth_redirect_to_correct_vendor_disabled).and_return(true)
end

it 'redirects to the Socure controller' do
get :show

expect(response).to have_http_status 200
end
end
end

context 'when facial match is required' do
Expand Down
18 changes: 14 additions & 4 deletions spec/features/idv/doc_auth/document_capture_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,21 @@
expect(page).to have_content(I18n.t('doc_auth.errors.general.network_error'))
end

it 'does not track state if state tracking is disabled' do
allow(IdentityConfig.store).to receive(:state_tracking_enabled).and_return(false)
attach_and_submit_images
context 'state tracking is disabled' do
before do
allow(IdentityConfig.store).to receive(:state_tracking_enabled).and_return(false)
allow(IdentityConfig.store).to receive(:socure_docv_enabled).and_return(true)
end
it 'does not track state' do
# Confirm that we end up on the LN / Mock page even if we try to
# go to the Socure one.
visit idv_socure_document_capture_url
expect(page).to have_current_path(idv_document_capture_url)

expect(DocAuthLog.find_by(user_id: @user.id).state).to be_nil
attach_and_submit_images

expect(DocAuthLog.find_by(user_id: @user.id).state).to be_nil
end
end
end

Expand Down
5 changes: 5 additions & 0 deletions spec/features/idv/doc_auth/socure_document_capture_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@
docv_transaction_token: @docv_transaction_token,
)

# Confirm that we end up on the Socure page even if we try to
# go to the LN / Mock one.
visit idv_document_capture_url
expect(page).to have_current_path(idv_socure_document_capture_url)
Comment on lines +296 to +299
Copy link
Contributor

Choose a reason for hiding this comment

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

While this does test the redirect logic and avoids the cost of another run through the feature spec, it seems somewhat buried/hidden when included in

  it 'does not track state if state tracking is disabled' do
    ...

I guess we don't really have a test that is specific to the generic happy path?


visit idv_socure_document_capture_update_path
expect(DocAuthLog.find_by(user_id: @user.id).state).to be_nil
end
Expand Down
70 changes: 31 additions & 39 deletions spec/features/idv/hybrid_mobile/hybrid_socure_mobile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@
visit idv_link_sent_url
expect(page).to have_current_path(root_url)

# Confirm that we end up on the LN / Mock page even if we try to
# go to the Socure one.
visit idv_hybrid_mobile_socure_document_capture_url
# Confirm that we end up on the Socure page even if we try to
# go to the LN / Mock one.
visit idv_hybrid_mobile_document_capture_url
Comment on lines +71 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

🤦 - nice catch!

expect(page).to have_current_path(idv_hybrid_mobile_socure_document_capture_url)

# Confirm that clicking cancel and then coming back doesn't cause errors
Expand Down Expand Up @@ -459,56 +459,48 @@
end

context 'with a network error requesting the capture app url' do
before do
allow_any_instance_of(Faraday::Connection).to receive(:post)
.and_raise(Faraday::ConnectionFailed)
end

it 'shows the network error page on the phone and the link sent page on the desktop',
js: true do
user = nil
shared_examples 'document request API failure' do
it 'shows the network error page on the phone and the link sent page on the desktop',
js: true do
perform_in_browser(:desktop) do
visit_idp_from_sp_with_ial2(sp)
sign_up_and_2fa_ial1_user

perform_in_browser(:desktop) do
visit_idp_from_sp_with_ial2(sp)
user = sign_up_and_2fa_ial1_user
complete_doc_auth_steps_before_hybrid_handoff_step
clear_and_fill_in(:doc_auth_phone, phone_number)
click_send_link
end

complete_doc_auth_steps_before_hybrid_handoff_step
clear_and_fill_in(:doc_auth_phone, phone_number)
click_send_link
end
perform_in_browser(:mobile) do
visit @sms_link

perform_in_browser(:mobile) do
visit @sms_link
expect(page).to have_text(t('doc_auth.headers.general.network_error'))
expect(page).to have_text(t('doc_auth.errors.general.new_network_error'))
expect(@analytics).to have_logged_event(:idv_socure_document_request_submitted)
end

expect(page).to have_text(t('doc_auth.headers.general.network_error'))
expect(page).to have_text(t('doc_auth.errors.general.new_network_error'))
expect(@analytics).to have_logged_event(:idv_socure_document_request_submitted)
perform_in_browser(:desktop) do
expect(page).to have_current_path(idv_link_sent_path)
end
end
end

perform_in_browser(:desktop) do
expect(page).to have_current_path(idv_link_sent_path)
context 'Faraday connection error' do
before do
allow_any_instance_of(Faraday::Connection).to receive(:post)
.and_raise(Faraday::ConnectionFailed)
end

it_behaves_like 'document request API failure'
end
end

context 'invalid request', allow_browser_log: true do
context 'getting the capture path w wrong api key' do
context 'invalid request (ie: wrong api key)', allow_browser_log: true do
before do
user = user_with_2fa
visit_idp_from_oidc_sp_with_ial2
sign_in_and_2fa_user(user)
complete_doc_auth_steps_before_document_capture_step
click_idv_continue
DocAuth::Mock::DocAuthMockClient.reset!
stub_docv_document_request(status: 401)
end

it 'correctly logs event', js: true do
visit idv_socure_document_capture_path
expect(@analytics).to have_logged_event(
:idv_socure_document_request_submitted,
)
end
it_behaves_like 'document request API failure'
end
end
end