From e8a52c4080f13ad2ebd680e99f29b1291a4c79df Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 9 Sep 2020 09:37:25 -0400 Subject: [PATCH 1/7] Allow AcuantCapture to receive non-image files in non-production environments **Why**: As a developer, I expect that I can upload files which simulate expected failures which can occur during document capture, so that I can reliably test non-happy-path flows. --- .../components/acuant-capture.jsx | 44 +++++++----- .../components/acuant-capture-spec.jsx | 72 ++++++++++++++++++- .../components/documents-step-spec.jsx | 9 --- .../components/selfie-step-spec.jsx | 9 --- spec/javascripts/spec_helper.js | 2 - 5 files changed, 99 insertions(+), 37 deletions(-) diff --git a/app/javascript/packages/document-capture/components/acuant-capture.jsx b/app/javascript/packages/document-capture/components/acuant-capture.jsx index 05f1585b92a..ea5f58efb69 100644 --- a/app/javascript/packages/document-capture/components/acuant-capture.jsx +++ b/app/javascript/packages/document-capture/components/acuant-capture.jsx @@ -28,9 +28,6 @@ import FileBase64CacheContext from '../context/file-base64-cache'; * @prop {'user'|'environment'=} capture Facing mode of capture. If capture is not specified and a * camera is supported, defaults to the Acuant environment camera capture. * @prop {string=} className Optional additional class names. - * @prop {number=} minimumGlareScore Minimum glare score to be considered acceptable. - * @prop {number=} minimumSharpnessScore Minimum sharpness score to be considered acceptable. - * @prop {number=} minimumFileSize Minimum file size (in bytes) to be considered acceptable. * @prop {boolean=} allowUpload Whether to allow file upload. Defaults to `true`. * @prop {ReactNode=} errorMessage Error to show. */ @@ -40,24 +37,42 @@ import FileBase64CacheContext from '../context/file-base64-cache'; * * @type {number} */ -const DEFAULT_ACCEPTABLE_GLARE_SCORE = 50; +const ACCEPTABLE_GLARE_SCORE = 50; /** * The minimum sharpness score value to be considered acceptable. * * @type {number} */ -const DEFAULT_ACCEPTABLE_SHARPNESS_SCORE = 50; +const ACCEPTABLE_SHARPNESS_SCORE = 50; /** * The minimum file size (bytes) for an image to be considered acceptable. * * @type {number} */ -const DEFAULT_ACCEPTABLE_FILE_SIZE_BYTES = - process.env.ACUANT_MINIMUM_FILE_SIZE === undefined - ? 250 * 1024 - : Number(process.env.ACUANT_MINIMUM_FILE_SIZE); +export const ACCEPTABLE_FILE_SIZE_BYTES = 250 * 1024; + +/** + * Returns attribute value to assign to the input field, depending on the current environment. + * + * @return {string[]=} Minimum file size, in bytes. + */ +export function getInputAccept() { + return process.env.NODE_ENV === 'production' ? ['image/*'] : undefined; +} + +/** + * Given a file, returns minimum acceptable file size in bytes, depending on the type of file and + * the current environment. + * + * @param {Blob} file File to assess. + * + * @return {number} Minimum file size, in bytes. + */ +export function getMinimumFileSize(file) { + return file.type.startsWith('image/') ? ACCEPTABLE_FILE_SIZE_BYTES : 0; +} /** * Returns an instance of File representing the given data URL. @@ -90,9 +105,6 @@ function AcuantCapture( onChange = () => {}, capture, className, - minimumGlareScore = DEFAULT_ACCEPTABLE_GLARE_SCORE, - minimumSharpnessScore = DEFAULT_ACCEPTABLE_SHARPNESS_SCORE, - minimumFileSize = DEFAULT_ACCEPTABLE_FILE_SIZE_BYTES, allowUpload = true, errorMessage, }, @@ -151,7 +163,7 @@ function AcuantCapture( * @param {Blob?} nextValue Next value candidate. */ function onChangeIfValid(nextValue) { - if (nextValue && nextValue.size < minimumFileSize) { + if (nextValue && nextValue.size < getMinimumFileSize(nextValue)) { setOwnErrorMessage(t('errors.doc_auth.photo_file_size')); } else { setOwnErrorMessage(null); @@ -190,9 +202,9 @@ function AcuantCapture( setIsCapturing(false)}> { - if (nextCapture.glare < minimumGlareScore) { + if (nextCapture.glare < ACCEPTABLE_GLARE_SCORE) { setOwnErrorMessage(t('errors.doc_auth.photo_glare')); - } else if (nextCapture.sharpness < minimumSharpnessScore) { + } else if (nextCapture.sharpness < ACCEPTABLE_SHARPNESS_SCORE) { setOwnErrorMessage(t('errors.doc_auth.photo_blurry')); } else { const dataAsBlob = toBlob(nextCapture.image.data); @@ -214,7 +226,7 @@ function AcuantCapture( label={label} hint={hasCapture || !allowUpload ? undefined : t('doc_auth.tips.document_capture_hint')} bannerText={bannerText} - accept={['image/*']} + accept={getInputAccept()} capture={capture} value={value} errorMessage={ownErrorMessage ?? errorMessage} diff --git a/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx b/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx index 8904b93d84e..b570ca1d2f3 100644 --- a/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx @@ -3,15 +3,65 @@ import { fireEvent } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { waitForElementToBeRemoved } from '@testing-library/dom'; import sinon from 'sinon'; -import AcuantCapture from '@18f/identity-document-capture/components/acuant-capture'; +import AcuantCapture, { + getInputAccept, + getMinimumFileSize, + ACCEPTABLE_FILE_SIZE_BYTES, +} from '@18f/identity-document-capture/components/acuant-capture'; import { Provider as AcuantContextProvider } from '@18f/identity-document-capture/context/acuant'; import DeviceContext from '@18f/identity-document-capture/context/device'; import I18nContext from '@18f/identity-document-capture/context/i18n'; import render from '../../../support/render'; import { useAcuant } from '../../../support/acuant'; +import { useSandbox } from '../../../support/sinon'; describe('document-capture/components/acuant-capture', () => { const { initialize } = useAcuant(); + const sandbox = useSandbox(); + + describe('getInputAccept', () => { + context('NODE_ENV=production', () => { + beforeEach(() => { + sandbox.stub(process.env, 'NODE_ENV').value('production'); + }); + + it('returns array', () => { + expect(getInputAccept()).to.be.instanceOf(Array); + }); + }); + + context('NODE_ENV=test', () => { + beforeEach(() => { + sandbox.stub(process.env, 'NODE_ENV').value('test'); + }); + + it('returns undefined', () => { + expect(getInputAccept()).to.be.undefined(); + }); + }); + + context('NODE_ENV=development', () => { + beforeEach(() => { + sandbox.stub(process.env, 'NODE_ENV').value('development'); + }); + + it('returns undefined', () => { + expect(getInputAccept()).to.be.undefined(); + }); + }); + }); + + describe('getMinimumFileSize', () => { + it('returns zero for non-image file', () => { + const file = new window.File([], 'file.yml', { type: 'application/x-yaml' }); + expect(getMinimumFileSize(file)).to.equal(0); + }); + + it('returns non-zero for image file', () => { + const file = new window.File([], 'file.png', { type: 'image/png' }); + expect(getMinimumFileSize(file)).to.be.gt(0); + }); + }); context('mobile', () => { it('renders with assumed capture button support while acuant is not ready and on mobile', () => { @@ -146,6 +196,8 @@ describe('document-capture/components/acuant-capture', () => { }); it('calls onChange with the captured image on successful capture', async () => { + sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); + const onChange = sinon.mock(); const { getByText } = render( @@ -305,6 +357,8 @@ describe('document-capture/components/acuant-capture', () => { }); it('shows at most one error message between AcuantCapture and FileInput', async () => { + sandbox.stub(process.env, 'NODE_ENV').value('production'); + const { getByLabelText, getByText, findByText } = render( @@ -343,6 +397,8 @@ describe('document-capture/components/acuant-capture', () => { }); it('removes error message once image is corrected', async () => { + sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); + const { getByText, findByText } = render( @@ -602,4 +658,18 @@ describe('document-capture/components/acuant-capture', () => { expect(defaultPrevented).to.be.false(); expect(window.AcuantCameraUI.start.called).to.be.false(); }); + + it('restricts accepted file types', () => { + sandbox.stub(process.env, 'NODE_ENV').value('production'); + + const { getByLabelText } = render( + + + , + ); + + const input = getByLabelText('Image'); + + expect(input.getAttribute('accept')).to.equal('image/*'); + }); }); diff --git a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx index 9583a310493..7bfeb66fdca 100644 --- a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx @@ -58,15 +58,6 @@ describe('document-capture/components/documents-step', () => { expect(onChange.getCall(0).args[0]).to.deep.equal({ front: file }); }); - it('restricts accepted file types', () => { - const onChange = sinon.spy(); - const { getByLabelText } = render(); - - const input = getByLabelText('doc_auth.headings.document_capture_front'); - - expect(input.getAttribute('accept')).to.equal('image/*'); - }); - it('renders device-specific instructions', () => { let { getByText } = render( diff --git a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx index 52692d6418c..98c3b036065 100644 --- a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx @@ -35,13 +35,4 @@ describe('document-capture/components/selfie-step', () => { expect(onChange.getCall(0).args[0]).to.deep.equal({ selfie: file }); }); - - it('restricts accepted file types', () => { - const onChange = sinon.spy(); - const { getByLabelText } = render(); - - const input = getByLabelText('doc_auth.headings.document_capture_selfie'); - - expect(input.getAttribute('accept')).to.equal('image/*'); - }); }); diff --git a/spec/javascripts/spec_helper.js b/spec/javascripts/spec_helper.js index cb5b08a4a81..467148a7d9f 100644 --- a/spec/javascripts/spec_helper.js +++ b/spec/javascripts/spec_helper.js @@ -18,7 +18,5 @@ global.document = window.document; global.getComputedStyle = window.getComputedStyle; global.self = window; -process.env.ACUANT_MINIMUM_FILE_SIZE = '0'; - useCleanDOM(); useConsoleLogSpy(); From 1ec639dc050703426b947abbaed0e137ee47df92 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 9 Sep 2020 17:57:48 -0400 Subject: [PATCH 2/7] Return error from YAML in non-production image upload --- app/forms/idv/api_image_upload_form.rb | 15 +++- .../idv/image_uploads_controller_spec.rb | 85 ++++++++++++++++--- spec/support/doc_auth_image_fixtures.rb | 8 ++ 3 files changed, 97 insertions(+), 11 deletions(-) diff --git a/app/forms/idv/api_image_upload_form.rb b/app/forms/idv/api_image_upload_form.rb index 98f421a7209..89978d2e1c3 100644 --- a/app/forms/idv/api_image_upload_form.rb +++ b/app/forms/idv/api_image_upload_form.rb @@ -84,7 +84,20 @@ def validate_image(image_key) file.rewind return if file.content_type.start_with?('image/') && data.present? - errors.add(image_key, t('doc_auth.errors.must_be_image')) + error = error_from_yaml(data) if !Rails.env.production? && yaml_file?(file) + error ||= t('doc_auth.errors.must_be_image') + errors.add(image_key, error) + end + + def yaml_file?(file) + file.original_filename =~ /\.ya?ml$/ + end + + def error_from_yaml(data) + parsed_yaml = YAML.safe_load(data) + parsed_yaml.dig('friendly_error') + rescue Psych::SyntaxError + nil end end end diff --git a/spec/controllers/idv/image_uploads_controller_spec.rb b/spec/controllers/idv/image_uploads_controller_spec.rb index c3ddf7fd26f..6f4cc23de07 100644 --- a/spec/controllers/idv/image_uploads_controller_spec.rb +++ b/spec/controllers/idv/image_uploads_controller_spec.rb @@ -52,27 +52,92 @@ context 'when a value is not a file' do before { params.merge!(front: 'some string') } - it 'returns an error' do - action + shared_examples 'failed non-file response' do + it 'returns an error' do + action - json = JSON.parse(response.body, symbolize_names: true) - expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('doc_auth.errors.not_a_file') }, - ] + json = JSON.parse(response.body, symbolize_names: true) + expect(json[:errors]).to eq [ + { field: 'front', message: I18n.t('doc_auth.errors.not_a_file') }, + ] + end + + context 'with a locale param' do + before { params.merge!(locale: 'es') } + + it 'translates errors using the locale param' do + action + + json = JSON.parse(response.body, symbolize_names: true) + expect(json[:errors]).to eq [ + { field: 'front', message: I18n.t('doc_auth.errors.not_a_file', locale: 'es') }, + ] + end + end + end + + context 'development' do + before do + allow(Rails.env).to receive(:production?).and_return(false) + end + + it_behaves_like 'failed non-file response' + end + + context 'production' do + before do + allow(Rails.env).to receive(:production?).and_return(true) + end + + it_behaves_like 'failed non-file response' end + end - context 'with a locale param' do - before { params.merge!(locale: 'es') } + context 'when a value is an error-formatted yaml file' do + before { params.merge!(front: DocAuthImageFixtures.error_yaml_multipart) } + + context 'development' do + before do + allow(Rails.env).to receive(:production?).and_return(false) + end - it 'translates errors using the locale param' do + it 'returns error from yaml file' do action json = JSON.parse(response.body, symbolize_names: true) expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('doc_auth.errors.not_a_file', locale: 'es') }, + { field: 'front', message: I18n.t('friendly_errors.doc_auth.barcode_could_not_be_read') }, ] end end + + context 'production' do + before do + allow(Rails.env).to receive(:production?).and_return(true) + end + + it 'returns non-image error' do + action + + json = JSON.parse(response.body, symbolize_names: true) + expect(json[:errors]).to eq [ + { field: 'front', message: I18n.t('doc_auth.errors.must_be_image') }, + ] + end + + context 'with a locale param' do + before { params.merge!(locale: 'es') } + + it 'translates errors using the locale param' do + action + + json = JSON.parse(response.body, symbolize_names: true) + expect(json[:errors]).to eq [ + { field: 'front', message: I18n.t('doc_auth.errors.must_be_image', locale: 'es') }, + ] + end + end + end end context 'when image upload succeeds' do diff --git a/spec/support/doc_auth_image_fixtures.rb b/spec/support/doc_auth_image_fixtures.rb index 88a5fea50fc..8bf0dc598d7 100644 --- a/spec/support/doc_auth_image_fixtures.rb +++ b/spec/support/doc_auth_image_fixtures.rb @@ -31,6 +31,14 @@ def self.selfie_image_multipart Rack::Test::UploadedFile.new(fixture_path('selfie.jpg'), 'image/jpeg') end + def self.error_yaml_multipart + path = File.join( + File.dirname(__FILE__), + '../fixtures/ial2_test_credential_forces_error.yml', + ) + Rack::Test::UploadedFile.new(path, Mime[:yaml]) + end + def self.fixture_path(filename) File.join( File.dirname(__FILE__), From d6c97f110ca33da709ba846249448d0b4afc8821 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 9 Sep 2020 18:23:34 -0400 Subject: [PATCH 3/7] rubocop --- spec/controllers/idv/image_uploads_controller_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/controllers/idv/image_uploads_controller_spec.rb b/spec/controllers/idv/image_uploads_controller_spec.rb index 6f4cc23de07..98212096685 100644 --- a/spec/controllers/idv/image_uploads_controller_spec.rb +++ b/spec/controllers/idv/image_uploads_controller_spec.rb @@ -106,7 +106,10 @@ json = JSON.parse(response.body, symbolize_names: true) expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('friendly_errors.doc_auth.barcode_could_not_be_read') }, + { + field: 'front', + message: I18n.t('friendly_errors.doc_auth.barcode_could_not_be_read'), + }, ] end end From 8865cd362390ec76cc6880473e4cab94a743dd3b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 9 Sep 2020 18:28:36 -0400 Subject: [PATCH 4/7] Fix client test specs --- .../components/document-capture-spec.jsx | 5 +++++ .../document-capture/components/documents-step-spec.jsx | 9 +++++++++ .../document-capture/components/selfie-step-spec.jsx | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx index 1fd1ede7076..b821d49c16f 100644 --- a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx @@ -2,6 +2,7 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import { waitFor } from '@testing-library/dom'; import { fireEvent } from '@testing-library/react'; +import { ACCEPTABLE_FILE_SIZE_BYTES } from '@18f/identity-document-capture/components/acuant-capture'; import { UploadFormEntriesError } from '@18f/identity-document-capture/services/upload'; import { AcuantProvider, DeviceContext } from '@18f/identity-document-capture'; import DocumentCapture, { @@ -9,9 +10,11 @@ import DocumentCapture, { } from '@18f/identity-document-capture/components/document-capture'; import render from '../../../support/render'; import { useAcuant } from '../../../support/acuant'; +import { useSandbox } from '../../../support/sinon'; describe('document-capture/components/document-capture', () => { const { initialize } = useAcuant(); + const sandbox = useSandbox(); function isFormValid(form) { return [...form.querySelectorAll('input')].every((input) => input.checkValidity()); @@ -21,6 +24,8 @@ describe('document-capture/components/document-capture', () => { beforeEach(() => { originalHash = window.location.hash; + sandbox.stub(process.env, 'NODE_ENV').value('production'); + sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); }); afterEach(() => { diff --git a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx index 7bfeb66fdca..9499f5fef97 100644 --- a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx @@ -1,12 +1,21 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import sinon from 'sinon'; +import { ACCEPTABLE_FILE_SIZE_BYTES } from '@18f/identity-document-capture/components/acuant-capture'; import DeviceContext from '@18f/identity-document-capture/context/device'; import DocumentsStep, { validate } from '@18f/identity-document-capture/components/documents-step'; import { RequiredValueMissingError } from '@18f/identity-document-capture/components/form-steps'; import render from '../../../support/render'; +import { useSandbox } from '../../../support/sinon'; describe('document-capture/components/documents-step', () => { + const sandbox = useSandbox(); + + beforeEach(() => { + sandbox.stub(process.env, 'NODE_ENV').value('production'); + sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); + }); + describe('validate', () => { it('returns errors if both front and back are unset', () => { const value = {}; diff --git a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx index 98c3b036065..3bdec305274 100644 --- a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx @@ -1,11 +1,20 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; import sinon from 'sinon'; +import { ACCEPTABLE_FILE_SIZE_BYTES } from '@18f/identity-document-capture/components/acuant-capture'; import SelfieStep, { validate } from '@18f/identity-document-capture/components/selfie-step'; import { RequiredValueMissingError } from '@18f/identity-document-capture/components/form-steps'; import render from '../../../support/render'; +import { useSandbox } from '../../../support/sinon'; describe('document-capture/components/selfie-step', () => { + const sandbox = useSandbox(); + + beforeEach(() => { + sandbox.stub(process.env, 'NODE_ENV').value('production'); + sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); + }); + describe('validate', () => { it('returns object with error if selfie is unset', () => { const value = {}; From 45091d24dc33e36d6f58d696dd664e4465d81789 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 11 Sep 2020 09:44:22 -0400 Subject: [PATCH 5/7] Pass mock client as detail to root upload context --- .../components/acuant-capture.jsx | 13 ++----- .../components/submission.jsx | 2 +- .../document-capture/context/upload.jsx | 20 ++++++++-- app/javascript/packs/document-capture.jsx | 2 + .../idv/shared/_document_capture.html.erb | 1 + .../components/acuant-capture-spec.jsx | 39 +------------------ .../components/document-capture-spec.jsx | 1 - .../components/documents-step-spec.jsx | 1 - .../components/selfie-step-spec.jsx | 1 - .../document-capture/context/upload-spec.jsx | 21 ++++++++-- spec/javascripts/support/render.jsx | 7 +++- 11 files changed, 47 insertions(+), 61 deletions(-) diff --git a/app/javascript/packages/document-capture/components/acuant-capture.jsx b/app/javascript/packages/document-capture/components/acuant-capture.jsx index ea5f58efb69..ef69f979c77 100644 --- a/app/javascript/packages/document-capture/components/acuant-capture.jsx +++ b/app/javascript/packages/document-capture/components/acuant-capture.jsx @@ -15,6 +15,7 @@ import Button from './button'; import useI18n from '../hooks/use-i18n'; import DeviceContext from '../context/device'; import FileBase64CacheContext from '../context/file-base64-cache'; +import UploadContext from '../context/upload'; /** @typedef {import('react').ReactNode} ReactNode */ @@ -53,15 +54,6 @@ const ACCEPTABLE_SHARPNESS_SCORE = 50; */ export const ACCEPTABLE_FILE_SIZE_BYTES = 250 * 1024; -/** - * Returns attribute value to assign to the input field, depending on the current environment. - * - * @return {string[]=} Minimum file size, in bytes. - */ -export function getInputAccept() { - return process.env.NODE_ENV === 'production' ? ['image/*'] : undefined; -} - /** * Given a file, returns minimum acceptable file size in bytes, depending on the type of file and * the current environment. @@ -112,6 +104,7 @@ function AcuantCapture( ) { const fileCache = useContext(FileBase64CacheContext); const { isReady, isError, isCameraSupported } = useContext(AcuantContext); + const { isMockClient } = useContext(UploadContext); const inputRef = useRef(/** @type {?HTMLInputElement} */ (null)); const isForceUploading = useRef(false); const [isCapturing, setIsCapturing] = useState(false); @@ -226,7 +219,7 @@ function AcuantCapture( label={label} hint={hasCapture || !allowUpload ? undefined : t('doc_auth.tips.document_capture_hint')} bannerText={bannerText} - accept={getInputAccept()} + accept={isMockClient ? undefined : ['image/*']} capture={capture} value={value} errorMessage={ownErrorMessage ?? errorMessage} diff --git a/app/javascript/packages/document-capture/components/submission.jsx b/app/javascript/packages/document-capture/components/submission.jsx index 1678b2b647e..cae65ed0a18 100644 --- a/app/javascript/packages/document-capture/components/submission.jsx +++ b/app/javascript/packages/document-capture/components/submission.jsx @@ -17,7 +17,7 @@ import CallbackOnMount from './callback-on-mount'; * @param {SubmissionProps} props Props object. */ function Submission({ payload, onError }) { - const upload = useContext(UploadContext); + const { upload } = useContext(UploadContext); const resource = useAsync(upload, payload); return ( diff --git a/app/javascript/packages/document-capture/context/upload.jsx b/app/javascript/packages/document-capture/context/upload.jsx index 24c19488560..fd019ee082a 100644 --- a/app/javascript/packages/document-capture/context/upload.jsx +++ b/app/javascript/packages/document-capture/context/upload.jsx @@ -1,7 +1,10 @@ -import React, { createContext } from 'react'; +import React, { createContext, useMemo } from 'react'; import defaultUpload from '../services/upload'; -const UploadContext = createContext(defaultUpload); +const UploadContext = createContext({ + upload: defaultUpload, + isMockClient: false, +}); /** @typedef {import('react').ReactNode} ReactNode */ @@ -45,6 +48,7 @@ const UploadContext = createContext(defaultUpload); * @typedef UploadContextProviderProps * * @prop {UploadImplementation=} upload Custom upload implementation. + * @prop {boolean=} isMockClient Whether to treat upload as a mock implementation. * @prop {string} endpoint Endpoint to which payload should be sent. * @prop {string} csrf CSRF token to send as parameter to upload implementation. * @prop {Record} formData Extra form data to merge into the payload before uploading @@ -54,10 +58,18 @@ const UploadContext = createContext(defaultUpload); /** * @param {UploadContextProviderProps} props Props object. */ -function UploadContextProvider({ upload = defaultUpload, endpoint, csrf, formData, children }) { +function UploadContextProvider({ + upload = defaultUpload, + isMockClient = false, + endpoint, + csrf, + formData, + children, +}) { const uploadWithCSRF = (payload) => upload({ ...payload, ...formData }, { endpoint, csrf }); + const value = useMemo(() => ({ upload: uploadWithCSRF, isMockClient }), [upload, isMockClient]); - return {children}; + return {children}; } export default UploadContext; diff --git a/app/javascript/packs/document-capture.jsx b/app/javascript/packs/document-capture.jsx index 8af4b90115c..49417446af0 100644 --- a/app/javascript/packs/document-capture.jsx +++ b/app/javascript/packs/document-capture.jsx @@ -27,6 +27,7 @@ document.body.classList.add('js-skip-form-validation'); loadPolyfills(['fetch']).then(() => { const appRoot = document.getElementById('document-capture-form'); const isLivenessEnabled = appRoot.hasAttribute('data-liveness'); + const isMockClient = appRoot.hasAttribute('data-mock-client'); render( { <%= tag.div id: 'document-capture-form', data: { liveness: liveness_checking_enabled?.presence, + mock_client: (DocAuth::Client.doc_auth_vendor == 'mock').presence, document_capture_session_uuid: flow_session[:document_capture_session_uuid], endpoint: api_verify_images_url } %> diff --git a/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx b/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx index b570ca1d2f3..bf7b2ec3b9f 100644 --- a/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/acuant-capture-spec.jsx @@ -4,7 +4,6 @@ import userEvent from '@testing-library/user-event'; import { waitForElementToBeRemoved } from '@testing-library/dom'; import sinon from 'sinon'; import AcuantCapture, { - getInputAccept, getMinimumFileSize, ACCEPTABLE_FILE_SIZE_BYTES, } from '@18f/identity-document-capture/components/acuant-capture'; @@ -19,38 +18,6 @@ describe('document-capture/components/acuant-capture', () => { const { initialize } = useAcuant(); const sandbox = useSandbox(); - describe('getInputAccept', () => { - context('NODE_ENV=production', () => { - beforeEach(() => { - sandbox.stub(process.env, 'NODE_ENV').value('production'); - }); - - it('returns array', () => { - expect(getInputAccept()).to.be.instanceOf(Array); - }); - }); - - context('NODE_ENV=test', () => { - beforeEach(() => { - sandbox.stub(process.env, 'NODE_ENV').value('test'); - }); - - it('returns undefined', () => { - expect(getInputAccept()).to.be.undefined(); - }); - }); - - context('NODE_ENV=development', () => { - beforeEach(() => { - sandbox.stub(process.env, 'NODE_ENV').value('development'); - }); - - it('returns undefined', () => { - expect(getInputAccept()).to.be.undefined(); - }); - }); - }); - describe('getMinimumFileSize', () => { it('returns zero for non-image file', () => { const file = new window.File([], 'file.yml', { type: 'application/x-yaml' }); @@ -357,14 +324,13 @@ describe('document-capture/components/acuant-capture', () => { }); it('shows at most one error message between AcuantCapture and FileInput', async () => { - sandbox.stub(process.env, 'NODE_ENV').value('production'); - const { getByLabelText, getByText, findByText } = render( , + { isMockClient: false }, ); initialize({ @@ -660,12 +626,11 @@ describe('document-capture/components/acuant-capture', () => { }); it('restricts accepted file types', () => { - sandbox.stub(process.env, 'NODE_ENV').value('production'); - const { getByLabelText } = render( , + { isMockClient: false }, ); const input = getByLabelText('Image'); diff --git a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx index b821d49c16f..fd23d830615 100644 --- a/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/document-capture-spec.jsx @@ -24,7 +24,6 @@ describe('document-capture/components/document-capture', () => { beforeEach(() => { originalHash = window.location.hash; - sandbox.stub(process.env, 'NODE_ENV').value('production'); sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); }); diff --git a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx index 9499f5fef97..439cf273fa9 100644 --- a/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/documents-step-spec.jsx @@ -12,7 +12,6 @@ describe('document-capture/components/documents-step', () => { const sandbox = useSandbox(); beforeEach(() => { - sandbox.stub(process.env, 'NODE_ENV').value('production'); sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); }); diff --git a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx index 3bdec305274..498ab98816b 100644 --- a/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx +++ b/spec/javascripts/packages/document-capture/components/selfie-step-spec.jsx @@ -11,7 +11,6 @@ describe('document-capture/components/selfie-step', () => { const sandbox = useSandbox(); beforeEach(() => { - sandbox.stub(process.env, 'NODE_ENV').value('production'); sandbox.stub(window.Blob.prototype, 'size').value(ACCEPTABLE_FILE_SIZE_BYTES); }); diff --git a/spec/javascripts/packages/document-capture/context/upload-spec.jsx b/spec/javascripts/packages/document-capture/context/upload-spec.jsx index c0d6163d9cd..fb4f0414ca3 100644 --- a/spec/javascripts/packages/document-capture/context/upload-spec.jsx +++ b/spec/javascripts/packages/document-capture/context/upload-spec.jsx @@ -10,8 +10,9 @@ describe('document-capture/context/upload', () => { it('defaults to the default upload service', () => { baseRender( createElement(() => { - const upload = useContext(UploadContext); + const { upload, isMockClient } = useContext(UploadContext); expect(upload).to.equal(defaultUpload); + expect(isMockClient).to.equal(false); return null; }), ); @@ -21,7 +22,7 @@ describe('document-capture/context/upload', () => { render( Promise.resolve({ ...payload, received: true })}> {createElement(() => { - const upload = useContext(UploadContext); + const { upload } = useContext(UploadContext); useEffect(() => { upload({ sent: true }).then((result) => { expect(result).to.deep.equal({ sent: true, received: true }); @@ -34,6 +35,18 @@ describe('document-capture/context/upload', () => { ); }); + it('can be overridden with isMockClient value', () => { + render( + + {createElement(() => { + const { isMockClient } = useContext(UploadContext); + expect(isMockClient).to.equal(true); + return null; + })} + , + ); + }); + it('can provide endpoint and csrf to make available to uploader', (done) => { render( { endpoint="https://example.com" > {createElement(() => { - const upload = useContext(UploadContext); + const { upload } = useContext(UploadContext); useEffect(() => { upload({ sent: true }).then((result) => { expect(result).to.deep.equal({ @@ -72,7 +85,7 @@ describe('document-capture/context/upload', () => { formData={{ foo: 'bar' }} > {createElement(() => { - const upload = useContext(UploadContext); + const { upload } = useContext(UploadContext); useEffect(() => { upload({ sent: true }).then((result) => { expect(result).to.deep.equal({ diff --git a/spec/javascripts/support/render.jsx b/spec/javascripts/support/render.jsx index bb6ee10b948..f5384fe666a 100644 --- a/spec/javascripts/support/render.jsx +++ b/spec/javascripts/support/render.jsx @@ -9,6 +9,7 @@ import { UploadContextProvider } from '@18f/identity-document-capture'; * @typedef RenderOptions * * @prop {Error=} uploadError Whether to simulate upload failure. + * @prop {boolean=} isMockClient Whether to treat upload as a mock implementation. * @prop {number=} expectedUploads Number of times upload is expected to be called. Defaults to `1`. */ @@ -24,7 +25,7 @@ import { UploadContextProvider } from '@18f/identity-document-capture'; * @return {import('@testing-library/react').RenderResult} */ function renderWithDefaultContext(element, options = {}) { - const { uploadError, expectedUploads = 1, ...baseRenderOptions } = options; + const { uploadError, expectedUploads = 1, isMockClient = true, ...baseRenderOptions } = options; const upload = sinon .stub() @@ -41,7 +42,9 @@ function renderWithDefaultContext(element, options = {}) { return render(element, { ...baseRenderOptions, wrapper: ({ children }) => ( - {children} + + {children} + ), }); } From b6389f83932a0d1ec9efeab884c98fc5e6777289 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 11 Sep 2020 10:11:21 -0400 Subject: [PATCH 6/7] Pass through non-image files to client implementation See: https://github.com/18F/identity-idp/pull/4173/files#r485970570 --- app/forms/idv/api_image_upload_form.rb | 25 +---- .../idv/image_uploads_controller_spec.rb | 104 +++++------------- spec/forms/idv/api_image_upload_form_spec.rb | 27 ----- 3 files changed, 28 insertions(+), 128 deletions(-) diff --git a/app/forms/idv/api_image_upload_form.rb b/app/forms/idv/api_image_upload_form.rb index 89978d2e1c3..1f64925569d 100644 --- a/app/forms/idv/api_image_upload_form.rb +++ b/app/forms/idv/api_image_upload_form.rb @@ -75,29 +75,8 @@ def validate_images def validate_image(image_key) file = params[image_key] - unless file.respond_to?(:content_type) - errors.add(image_key, t('doc_auth.errors.not_a_file')) - return - end - - data = file.read - file.rewind - - return if file.content_type.start_with?('image/') && data.present? - error = error_from_yaml(data) if !Rails.env.production? && yaml_file?(file) - error ||= t('doc_auth.errors.must_be_image') - errors.add(image_key, error) - end - - def yaml_file?(file) - file.original_filename =~ /\.ya?ml$/ - end - - def error_from_yaml(data) - parsed_yaml = YAML.safe_load(data) - parsed_yaml.dig('friendly_error') - rescue Psych::SyntaxError - nil + return if file.respond_to?(:read) + errors.add(image_key, t('doc_auth.errors.not_a_file')) end end end diff --git a/spec/controllers/idv/image_uploads_controller_spec.rb b/spec/controllers/idv/image_uploads_controller_spec.rb index 98212096685..0ff0b00eb1a 100644 --- a/spec/controllers/idv/image_uploads_controller_spec.rb +++ b/spec/controllers/idv/image_uploads_controller_spec.rb @@ -52,94 +52,26 @@ context 'when a value is not a file' do before { params.merge!(front: 'some string') } - shared_examples 'failed non-file response' do - it 'returns an error' do - action - - json = JSON.parse(response.body, symbolize_names: true) - expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('doc_auth.errors.not_a_file') }, - ] - end - - context 'with a locale param' do - before { params.merge!(locale: 'es') } - - it 'translates errors using the locale param' do - action - - json = JSON.parse(response.body, symbolize_names: true) - expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('doc_auth.errors.not_a_file', locale: 'es') }, - ] - end - end - end - - context 'development' do - before do - allow(Rails.env).to receive(:production?).and_return(false) - end - - it_behaves_like 'failed non-file response' - end - - context 'production' do - before do - allow(Rails.env).to receive(:production?).and_return(true) - end - - it_behaves_like 'failed non-file response' - end - end - - context 'when a value is an error-formatted yaml file' do - before { params.merge!(front: DocAuthImageFixtures.error_yaml_multipart) } - - context 'development' do - before do - allow(Rails.env).to receive(:production?).and_return(false) - end - - it 'returns error from yaml file' do - action + it 'returns an error' do + action - json = JSON.parse(response.body, symbolize_names: true) - expect(json[:errors]).to eq [ - { - field: 'front', - message: I18n.t('friendly_errors.doc_auth.barcode_could_not_be_read'), - }, - ] - end + json = JSON.parse(response.body, symbolize_names: true) + expect(json[:errors]).to eq [ + { field: 'front', message: I18n.t('doc_auth.errors.not_a_file') }, + ] end - context 'production' do - before do - allow(Rails.env).to receive(:production?).and_return(true) - end + context 'with a locale param' do + before { params.merge!(locale: 'es') } - it 'returns non-image error' do + it 'translates errors using the locale param' do action json = JSON.parse(response.body, symbolize_names: true) expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('doc_auth.errors.must_be_image') }, + { field: 'front', message: I18n.t('doc_auth.errors.not_a_file', locale: 'es') }, ] end - - context 'with a locale param' do - before { params.merge!(locale: 'es') } - - it 'translates errors using the locale param' do - action - - json = JSON.parse(response.body, symbolize_names: true) - expect(json[:errors]).to eq [ - { field: 'front', message: I18n.t('doc_auth.errors.must_be_image', locale: 'es') }, - ] - end - end end end @@ -176,6 +108,22 @@ ] end end + + context 'when a value is an error-formatted yaml file' do + before { params.merge!(back: DocAuthImageFixtures.error_yaml_multipart) } + + it 'returns error from yaml file' do + action + + json = JSON.parse(response.body, symbolize_names: true) + expect(json[:errors]).to eq [ + { + field: 'results', + message: I18n.t('friendly_errors.doc_auth.barcode_could_not_be_read'), + }, + ] + end + end end end end diff --git a/spec/forms/idv/api_image_upload_form_spec.rb b/spec/forms/idv/api_image_upload_form_spec.rb index c8a0e0105d5..7e3b4dffe44 100644 --- a/spec/forms/idv/api_image_upload_form_spec.rb +++ b/spec/forms/idv/api_image_upload_form_spec.rb @@ -59,33 +59,6 @@ end end - context 'when file does not have an image content type' do - let(:tempfile) do - Tempfile.new.tap do |f| - f.write('test') - f.close - end - end - let(:selfie_image) { Rack::Test::UploadedFile.new(tempfile.path, 'text/plain') } - - it 'is not valid' do - expect(form.valid?).to eq(false) - expect(form.errors[:selfie]).to eq(['File must be an image']) - end - end - - context 'when file is empty' do - let(:tempfile) { Tempfile.new } - let(:selfie_image) { Rack::Test::UploadedFile.new(tempfile.path, 'image/jpeg') } - - it 'is not valid' do - expect(form.valid?).to eq(false) - expect(form.errors[:selfie]).to eq(['File must be an image']) - end - - after { tempfile.unlink } - end - context 'when document_capture_session_uuid param is missing' do let(:document_capture_session_uuid) { nil } From 30f882d4fb7a79fa637dac2b0bd6c6f498f31917 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 11 Sep 2020 10:26:32 -0400 Subject: [PATCH 7/7] Remove unused i18n keys --- config/locales/doc_auth/en.yml | 1 - config/locales/doc_auth/es.yml | 1 - config/locales/doc_auth/fr.yml | 1 - 3 files changed, 3 deletions(-) diff --git a/config/locales/doc_auth/en.yml b/config/locales/doc_auth/en.yml index 454f16fc031..465b78512ab 100644 --- a/config/locales/doc_auth/en.yml +++ b/config/locales/doc_auth/en.yml @@ -13,7 +13,6 @@ en: upload_picture: Upload a photo use_phone: Use your phone errors: - must_be_image: File must be an image not_a_file: The selection was not a valid file forms: address1: Address diff --git a/config/locales/doc_auth/es.yml b/config/locales/doc_auth/es.yml index 3378797a4fd..89754a0462b 100644 --- a/config/locales/doc_auth/es.yml +++ b/config/locales/doc_auth/es.yml @@ -13,7 +13,6 @@ es: upload_picture: Sube una foto use_phone: Usa tu telefono errors: - must_be_image: El archivo debe ser una imagen not_a_file: La selección no era un archivo válido forms: address1: Dirección diff --git a/config/locales/doc_auth/fr.yml b/config/locales/doc_auth/fr.yml index 003ead2762d..b210accf89b 100644 --- a/config/locales/doc_auth/fr.yml +++ b/config/locales/doc_auth/fr.yml @@ -13,7 +13,6 @@ fr: upload_picture: Télécharger une photo use_phone: Utilisez votre téléphone errors: - must_be_image: Le fichier doit être une image not_a_file: La sélection n'était pas un fichier valide forms: address1: Adresse