-
Notifications
You must be signed in to change notification settings - Fork 166
LG-11718: make sure err http status is handled. #9957
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
Changes from all commits
63ebf90
96185fe
d023ea1
a03d80f
a290133
618c2be
0b624ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -186,6 +186,8 @@ | |
| complete_doc_auth_steps_before_document_capture_step | ||
| mock_doc_auth_trueid_http_non2xx_status(438) | ||
| attach_and_submit_images | ||
| # verify it's a network error | ||
| expect(page).to have_content(I18n.t('doc_auth.errors.general.network_error')) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 |
||
| click_try_again | ||
| end | ||
|
|
||
|
|
@@ -199,6 +201,8 @@ | |
| complete_doc_auth_steps_before_document_capture_step | ||
| mock_doc_auth_trueid_http_non2xx_status(500) | ||
| attach_and_submit_images | ||
| # verify it's a network error | ||
| expect(page).to have_content(I18n.t('doc_auth.errors.general.network_error')) | ||
| click_try_again | ||
| end | ||
| it_behaves_like 'image re-upload allowed' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -268,8 +268,10 @@ | |
| extra: { remaining_attempts: IdentityConfig.store.doc_auth_max_attempts - 1 }, | ||
| ) | ||
| end | ||
| let(:doc_auth_client) { double(DocAuth::LexisNexis::LexisNexisClient) } | ||
| before do | ||
| allow(subject).to receive(:post_images_to_client).and_return(failed_response) | ||
| subject.instance_variable_set(:@doc_auth_client, doc_auth_client) | ||
| allow(doc_auth_client).to receive(:post_images) { failed_response } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch here! 💪🏻 |
||
| end | ||
|
|
||
| it 'is not successful' do | ||
|
|
@@ -279,6 +281,7 @@ | |
| expect(response.success?).to eq(false) | ||
| expect(response.attention_with_barcode?).to eq(false) | ||
| expect(response.pii_from_doc).to eq({}) | ||
| expect(response.doc_auth_success?).to eq(false) | ||
| end | ||
|
|
||
| it 'includes remaining_attempts' do | ||
|
|
@@ -300,6 +303,24 @@ | |
| expect(response.errors).to have_key(:front) | ||
| expect(response.errors).to have_value([I18n.t('doc_auth.errors.doc.resubmit_failed_image')]) | ||
| end | ||
|
|
||
| it 'logs analytics event' do | ||
| form.submit | ||
| expect(fake_analytics).to have_logged_event( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: alot of the values here are nil due to the test case ... is it possible we could use something like hash_including instead and only check relevant values? |
||
| 'IdV: doc auth image upload vendor submitted', | ||
| hash_including( | ||
| doc_auth_result: nil, | ||
| errors: { front: 'glare' }, | ||
| success: false, | ||
| doc_type_supported: boolean, | ||
| doc_auth_success: boolean, | ||
| liveness_checking_required: boolean, | ||
| selfie_status: :not_processed, | ||
| selfie_live: boolean, | ||
| selfie_quality_good: boolean, | ||
| ), | ||
| ) | ||
| end | ||
| end | ||
|
|
||
| context 'PII validation from client response fails' do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ | |
| trueid_account_id: 'test_account', | ||
| trueid_noliveness_cropping_workflow: 'NOLIVENESS.CROPPING.WORKFLOW', | ||
| trueid_noliveness_nocropping_workflow: 'NOLIVENESS.NOCROPPING.WORKFLOW', | ||
| trueid_liveness_cropping_workflow: 'LIVENESS.CROPPING.WORKFLOW', | ||
| trueid_liveness_nocropping_workflow: 'LIVENESS.NOCROPPING.WORKFLOW', | ||
| ) | ||
| end | ||
|
|
||
|
|
@@ -117,4 +119,85 @@ | |
| ) | ||
| end | ||
| end | ||
|
|
||
| context 'with selfie check enabled' do | ||
| ## enable feature | ||
| let(:workflow) { 'LIVENESS.CROPPING.WORKFLOW' } | ||
| before do | ||
| allow(FeatureManagement).to receive(:idv_allow_selfie_check?).and_return(true) | ||
| end | ||
| describe 'when success response returned' do | ||
| before do | ||
| stub_request(:post, image_upload_url).to_return( | ||
| body: LexisNexisFixtures.true_id_response_success_with_liveness, | ||
| ) | ||
| end | ||
| it 'returns a successful response' do | ||
| result = client.post_images( | ||
| front_image: DocAuthImageFixtures.document_front_image, | ||
| back_image: DocAuthImageFixtures.document_back_image, | ||
| image_source: image_source, | ||
| selfie_image: DocAuthImageFixtures.selfie_image, | ||
| liveness_checking_required: true, | ||
| ) | ||
| expect(result.success?).to eq(true) | ||
| expect(result.class).to eq(DocAuth::LexisNexis::Responses::TrueIdResponse) | ||
| expect(result.doc_auth_success?).to eq(true) | ||
| expect(result.selfie_live?).to eq(true) | ||
| expect(result.selfie_quality_good?).to eq(true) | ||
| expect(result.selfie_check_performed?).to eq(true) | ||
| end | ||
| end | ||
|
|
||
| describe 'when selfie failure response returned' do | ||
| before do | ||
| stub_request(:post, image_upload_url).to_return( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is non-blocking feedback. I peeked at the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eileen-nava , the portrait match result part are the same.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks. |
||
| body: LexisNexisFixtures.true_id_response_failure_with_liveness, | ||
| ) | ||
| end | ||
|
|
||
| it 'returns a response indicate all failures' do | ||
| result = client.post_images( | ||
| front_image: DocAuthImageFixtures.document_front_image, | ||
| back_image: DocAuthImageFixtures.document_back_image, | ||
| image_source: image_source, | ||
| selfie_image: DocAuthImageFixtures.selfie_image, | ||
| liveness_checking_required: true, | ||
| ) | ||
| expect(result.success?).to eq(false) | ||
| expect(result.class).to eq(DocAuth::LexisNexis::Responses::TrueIdResponse) | ||
| expect(result.doc_auth_success?).to eq(false) | ||
| result_hash = result.to_h | ||
| expect(result_hash[:selfie_status]).to eq(:fail) | ||
| expect(result.selfie_live?).to eq(true) | ||
| expect(result.selfie_quality_good?).to eq(false) | ||
| expect(result.selfie_check_performed?).to eq(true) | ||
| end | ||
| end | ||
|
|
||
| describe 'when http request failed' do | ||
| it 'return failed response with correct statuses' do | ||
| stub_request(:post, image_upload_url).to_return(body: '', status: 401) | ||
|
|
||
| result = client.post_images( | ||
| front_image: DocAuthImageFixtures.document_front_image, | ||
| back_image: DocAuthImageFixtures.document_back_image, | ||
| image_source: image_source, | ||
| selfie_image: DocAuthImageFixtures.selfie_image, | ||
| liveness_checking_required: true, | ||
| ) | ||
|
|
||
| expect(result.success?).to eq(false) | ||
| expect(result.errors).to eq(network: true) | ||
| expect(result.exception.message).to eq( | ||
| 'DocAuth::LexisNexis::Requests::TrueIdRequest Unexpected HTTP response 401', | ||
| ) | ||
| result_hash = result.to_h | ||
| expect(result_hash[:vendor]).to eq('TrueID') | ||
| expect(result_hash[:doc_auth_success]).to eq(false) | ||
| expect(result_hash[:selfie_status]).to eq(:not_processed) | ||
| expect(result.class).to eq(DocAuth::Response) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,13 +87,18 @@ | |
| expect(response.class).to eq(DocAuth::Response) | ||
| end | ||
|
|
||
| it 'includes information on the error' do | ||
| response = subject.fetch | ||
| it 'includes information on the error and notifies NewRelic' do | ||
| expected_message = [ | ||
| subject.class.name, | ||
| 'Unexpected HTTP response', | ||
| status, | ||
| ].join(' ') | ||
| expect(NewRelic::Agent).to receive(:notice_error) do |arg| | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't used do blocks like this in testing very much. It like it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eileen-nava , it has issue with multiple parameters when using |
||
| expect(arg).to be_an_instance_of(DocAuth::RequestError) | ||
| expect(arg.message).to eq(expected_message) | ||
| expect(arg.error_code).to eq(status) | ||
| end | ||
| response = subject.fetch | ||
|
|
||
| expect(response.exception.message).to eq(expected_message) | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,19 +3,24 @@ | |
| RSpec.describe DocAuth::LexisNexis::Requests::TrueIdRequest do | ||
| let(:image_source) { nil } | ||
| let(:account_id) { 'test_account' } | ||
| let(:workflow) { nil } | ||
| let(:base_url) { 'https://lexis.nexis.example.com' } | ||
| let(:workflow) { subject.send(:workflow) } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious why you made this change? Not against it, just curious about it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eileen-nava , to verify workflows being used (which is determined by a private method) |
||
| let(:path) { "/restws/identity/v3/accounts/#{account_id}/workflows/#{workflow}/conversations" } | ||
| let(:full_url) { base_url + path } | ||
| let(:applicant) { { uuid: SecureRandom.uuid, uuid_prefix: '123' } } | ||
| let(:non_cropping_non_liveness_flow) { 'test_workflow' } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 |
||
| let(:cropping_non_liveness_flow) { 'test_workflow_cropping' } | ||
| let(:non_cropping_liveness_flow) { 'test_workflow_liveness' } | ||
| let(:cropping_liveness_flow) { 'test_workflow_liveness_cropping' } | ||
|
|
||
| let(:config) do | ||
| DocAuth::LexisNexis::Config.new( | ||
| trueid_account_id: account_id, | ||
| base_url: base_url, | ||
| trueid_noliveness_cropping_workflow: 'test_workflow_cropping', | ||
| trueid_noliveness_nocropping_workflow: 'test_workflow', | ||
| trueid_liveness_cropping_workflow: 'test_workflow_liveness_cropping', | ||
| trueid_liveness_nocropping_workflow: 'test_workflow_liveness', | ||
| trueid_noliveness_cropping_workflow: cropping_non_liveness_flow, | ||
| trueid_noliveness_nocropping_workflow: non_cropping_non_liveness_flow, | ||
| trueid_liveness_cropping_workflow: cropping_liveness_flow, | ||
| trueid_liveness_nocropping_workflow: non_cropping_liveness_flow, | ||
| ) | ||
| end | ||
| let(:selfie_image) { DocAuthImageFixtures.selfie_image } | ||
|
|
@@ -100,35 +105,40 @@ def include_liveness_expected | |
| context 'when liveness checking is NOT required' do | ||
| let(:liveness_checking_required) { false } | ||
| context 'with acuant image source' do | ||
| let(:workflow) { 'test_workflow' } | ||
| let(:image_source) { DocAuth::ImageSources::ACUANT_SDK } | ||
| it_behaves_like 'a successful request' | ||
|
|
||
| it 'uses non-cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(non_cropping_non_liveness_flow) | ||
| end | ||
| it 'does not include a nil selfie in the request body sent to TrueID' do | ||
| body_as_json = subject.send(:body) | ||
| body_as_hash = JSON.parse(body_as_json) | ||
| expect(body_as_hash['Document']).not_to have_key('Selfie') | ||
| end | ||
| end | ||
| context 'with unknown image source' do | ||
| let(:workflow) { 'test_workflow_cropping' } | ||
| let(:image_source) { DocAuth::ImageSources::UNKNOWN } | ||
|
|
||
| it 'uses cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| end | ||
|
|
||
| context 'when liveness checking is required' do | ||
| let(:liveness_checking_required) { true } | ||
| context 'with acuant image source' do | ||
| let(:workflow) { 'test_workflow' } | ||
| let(:image_source) { DocAuth::ImageSources::ACUANT_SDK } | ||
| it 'uses non-cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(non_cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| context 'with unknown image source' do | ||
| let(:workflow) { 'test_workflow_cropping' } | ||
| let(:image_source) { DocAuth::ImageSources::UNKNOWN } | ||
|
|
||
| it 'uses cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| end | ||
|
|
@@ -144,60 +154,67 @@ def include_liveness_expected | |
| context 'when liveness checking is NOT required' do | ||
| let(:liveness_checking_required) { false } | ||
| context 'with acuant image source' do | ||
| let(:workflow) { 'test_workflow' } | ||
| let(:image_source) { DocAuth::ImageSources::ACUANT_SDK } | ||
| it 'use non-cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(non_cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| context 'with unknown image source' do | ||
| let(:workflow) { 'test_workflow_cropping' } | ||
| let(:image_source) { DocAuth::ImageSources::UNKNOWN } | ||
|
|
||
| it 'use cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| end | ||
|
|
||
| context 'when liveness checking is required' do | ||
| let(:liveness_checking_required) { true } | ||
| context 'with acuant image source' do | ||
| let(:workflow) { 'test_workflow_liveness' } | ||
| let(:image_source) { DocAuth::ImageSources::ACUANT_SDK } | ||
|
|
||
| it 'use non-cropping liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(non_cropping_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| context 'with unknown image source' do | ||
| let(:workflow) { 'test_workflow_liveness_cropping' } | ||
| let(:image_source) { DocAuth::ImageSources::UNKNOWN } | ||
|
|
||
| it 'use cropping liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(cropping_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
|
|
||
| context 'when hosted env is prod' do | ||
| let(:selfie_check_allowed) { false } | ||
| context 'with acuant image source' do | ||
| let(:workflow) { 'test_workflow' } | ||
| let(:image_source) { DocAuth::ImageSources::ACUANT_SDK } | ||
| it 'use non-cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(non_cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| context 'with unknown image source' do | ||
| let(:workflow) { 'test_workflow_cropping' } | ||
| let(:image_source) { DocAuth::ImageSources::UNKNOWN } | ||
|
|
||
| it 'use cropping non-liveness workflow' do | ||
| expect(subject.send(:workflow)).to eq(cropping_non_liveness_flow) | ||
| end | ||
| it_behaves_like 'a successful request' | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'with non 200 http status code' do | ||
| let(:workflow) { 'test_workflow' } | ||
| let(:image_source) { DocAuth::ImageSources::ACUANT_SDK } | ||
| it 'is a network error with 5xx status' do | ||
| stub_request(:post, full_url).to_return(body: '{}', status: 500) | ||
| response = subject.fetch | ||
| expect(response.network_error?).to eq(true) | ||
| end | ||
| it 'is not a network error with 440, 438, 439' do | ||
| stub_request(:post, full_url).to_return(body: '{}', status: 443) | ||
| it 'is a network error with non 5xx error' do | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Phasing out Acuant, there 438, 439 etc have special meanings. |
||
| stub_request(:post, full_url).to_return(body: '{}', status: 401) | ||
| response = subject.fetch | ||
| expect(response.network_error?).to eq(true) | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with some defaults for http error.