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

Merged
merged 30 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9af0916
init relax vendor redirect if in test mode
amirbey Dec 19, 2024
b9c13d4
fix doc cap standard test
amirbey Dec 20, 2024
8fd3671
fix doc cap socure hybrid test
amirbey Dec 20, 2024
81105b1
fix doc cap hybrid test
amirbey Dec 20, 2024
c21efbf
in test mode do not redirect except if socure and facial match req'd
amirbey Dec 20, 2024
ef4c1c5
fix specs to not allow socure to have flow in test mode for selfie ca…
amirbey Dec 21, 2024
e1d5f7d
update happy path specs to be able to upload links while enforcing do…
amirbey Dec 23, 2024
2d40398
allow step without forcing doc cap redirect
amirbey Dec 23, 2024
3c57139
disable invalid test
amirbey Dec 23, 2024
98c9a6e
fix hybrid mobile spec
amirbey Jan 2, 2025
4730e34
create flag for disable redirect_to_correct_vendor
amirbey Jan 2, 2025
b8b344b
undo spec changes
amirbey Jan 2, 2025
e7ef3ce
add redirect spec
amirbey Jan 2, 2025
fc62995
when redirect to correct vendor is disabled on hybrid mobile spec
amirbey Jan 2, 2025
f8066ca
undo spec preconditions change
amirbey Jan 2, 2025
d453b1d
use default redirect to correct vendor in dev
amirbey Jan 3, 2025
af0673c
only bypass redirect correct vendor if disabled
amirbey Jan 3, 2025
f0e6fbd
only bypass redirect correct vendor if disabled - remove spec
amirbey Jan 3, 2025
af1cf31
test disable vendor redirect for socure standard controller
amirbey Jan 3, 2025
dc7e129
test redirect correct vendor in socure doc auth controllers
amirbey Jan 3, 2025
f41883c
test redirect correct vendor in standard doc auth controllers
amirbey Jan 3, 2025
9f3801f
happy linting
amirbey Jan 3, 2025
aadaa46
rename env var doc_auth_disable_redirect_to_correct_vendor to doc_aut…
amirbey Jan 3, 2025
4044b62
rename env var doc_auth_disable_redirect_to_correct_vendor to doc_aut…
amirbey Jan 3, 2025
9fc1446
lint config file
amirbey Jan 3, 2025
a6b8968
remove unused variable
amirbey Jan 3, 2025
0a03187
fix spec for document request api calls to test hybrid flow
amirbey Jan 3, 2025
e1f99f5
happy linting
amirbey Jan 3, 2025
f7ffaaa
add spec to test when socure hybrid mobile when redirect is enforced
amirbey Jan 10, 2025
ace1915
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 '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,30 @@
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 👍🏿

let(:idv_vendor) { Idp::Constants::Vendors::LEXIS_NEXIS }

it 'redirects to the LN/Mock controller' do
get :show

expect(response).to redirect_to(idv_hybrid_mobile_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 'renders 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