Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 7 additions & 7 deletions app/forms/api/profile_creation_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def submit

response = FormResponse.new(
success: form_valid?,
errors: errors.to_hash,
errors: errors,
extra: extra_attributes,
)
[response, personal_key]
Expand Down Expand Up @@ -105,20 +105,20 @@ def session

def valid_jwt
@user_bundle = Api::UserBundleDecorator.new(user_bundle: jwt, public_key: public_key)
rescue JWT::DecodeError => err
errors.add(:jwt, "decode error: #{err.message}", type: :invalid)
rescue ::Api::UserBundleError => err
errors.add(:jwt, "malformed user bundle: #{err.message}", type: :invalid)
rescue JWT::DecodeError
errors.add(:jwt, I18n.t('idv.failure.exceptions.internal_error'), type: :decode_error)
Copy link
Contributor

Choose a reason for hiding this comment

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

🙂

rescue ::Api::UserBundleError
errors.add(:jwt, I18n.t('idv.failure.exceptions.internal_error'), type: :user_bundle_error)
end

def valid_user
return if user
errors.add(:user, 'user not found', type: :invalid)
errors.add(:user, I18n.t('devise.failure.unauthenticated'), type: :invalid_user)
end

def valid_password
return if user&.valid_password?(password)
errors.add(:password, 'invalid password', type: :invalid)
errors.add(:password, I18n.t('idv.errors.incorrect_password'), type: :invalid_password)
end

def form_valid?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function DocumentSideAcuantCapture({
}
onCameraAccessDeclined={() => {
onError(new CameraAccessDeclinedError(), { field: side });
onError(new CameraAccessDeclinedError({ isDetail: true }));
onError(new CameraAccessDeclinedError(undefined, { isDetail: true }));
}}
errorMessage={error ? error.message : undefined}
name={side}
Expand Down
9 changes: 0 additions & 9 deletions app/javascript/packages/document-capture/services/upload.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,6 @@ import { FormError } from '@18f/identity-form-steps';
export class UploadFormEntryError extends FormError {
/** @type {string} */
field = '';

/**
* @param {string} message
*/
constructor(message) {
super();

this.message = message;
}
}

export class UploadFormEntriesError extends FormError {
Expand Down
39 changes: 39 additions & 0 deletions app/javascript/packages/form-steps/form-error.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import FormError from './form-error';

describe('FormError', () => {
it('constructs with a message', () => {
const error = new FormError('message');

expect(error.message).to.equal('message');
expect(error.isDetail).to.be.false();
expect(error.field).to.be.undefined();
});

it('constructs as detailed error', () => {
const error = new FormError('message', { isDetail: true });

expect(error.message).to.equal('message');
expect(error.isDetail).to.be.true();
expect(error.field).to.be.undefined();
});

it('constructs as associated with a field', () => {
const error = new FormError('message', { field: 'field' });

expect(error.message).to.equal('message');
expect(error.isDetail).to.be.false();
expect(error.field).to.equal('field');
});

it('supports message on subclass property initializer', () => {
class ExampleFormError extends FormError {
message = 'message';
}

const error = new ExampleFormError();

expect(error.message).to.equal('message');
expect(error.isDetail).to.be.false();
expect(error.field).to.be.undefined();
});
});
12 changes: 10 additions & 2 deletions app/javascript/packages/form-steps/form-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,23 @@ export interface FormErrorOptions {
* text description.
*/
isDetail?: boolean;

/**
* Field associated with the error.
*/
field?: string;
}

class FormError extends Error {
field?: string;

isDetail: boolean;

constructor(options?: { isDetail: boolean }) {
super();
constructor(message?: string, options?: FormErrorOptions) {
super(message);

this.isDetail = Boolean(options?.isDetail);
this.field = options?.field;
}
}

Expand Down
18 changes: 17 additions & 1 deletion app/javascript/packages/verify-flow/services/api.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { SinonStub } from 'sinon';
import { useSandbox } from '@18f/identity-test-helpers';
import { post } from './api';
import { isErrorResponse, post } from './api';

describe('post', () => {
const sandbox = useSandbox();
Expand Down Expand Up @@ -73,3 +73,19 @@ describe('post', () => {
});
});
});

describe('isErrorResponse', () => {
it('returns false if object is not an error response', () => {
const response = {};
const result = isErrorResponse(response);

expect(result).to.be.false();
});

it('returns true if object is an error response', () => {
const response = { error: { field: ['message'] } };
const result = isErrorResponse(response);

expect(result).to.be.true();
});
});
8 changes: 8 additions & 0 deletions app/javascript/packages/verify-flow/services/api.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
export interface ErrorResponse<Field extends string> {
error: Record<Field, [string, ...string[]]>;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this this [string, ...string[]] type a way to say "string[] with length at least 1"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this this [string, ...string[]] type a way to say "string[] with length at least 1"?

Yes, exactly that. I don't know that it's strictly necessary here, but it seems reasonable to expect that if the response is an error response, there's at least one error message.

}

interface PostOptions {
/**
* Whether to send the request as a JSON request.
Expand Down Expand Up @@ -41,3 +45,7 @@ export async function post<Response = any>(

return options.json ? response.json() : response.text();
}

export const isErrorResponse = <F extends string>(
response: object | ErrorResponse<F>,
): response is ErrorResponse<F> => 'error' in response;
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import type { ChangeEvent } from 'react';
import { t } from '@18f/identity-i18n';
import { FormStepsButton } from '@18f/identity-form-steps';
import { Alert } from '@18f/identity-components';
import type { FormStepComponentProps } from '@18f/identity-form-steps';
import type { VerifyFlowValues } from '../../verify-flow';

interface PasswordConfirmStepStepProps extends FormStepComponentProps<VerifyFlowValues> {}

function PasswordConfirmStep({ registerField, onChange }: PasswordConfirmStepStepProps) {
function PasswordConfirmStep({ errors, registerField, onChange }: PasswordConfirmStepStepProps) {
return (
<>
{errors.map(({ error }) => (
<Alert key={error.message} type="error" className="margin-bottom-4">
{error.message}
</Alert>
))}
<input
ref={registerField('password')}
aria-label={t('idv.form.password')}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,53 @@
import { FormError } from '@18f/identity-form-steps';
import { useSandbox } from '@18f/identity-test-helpers';
import submit, { API_ENDPOINT } from './submit';

describe('submit', () => {
const sandbox = useSandbox();

beforeEach(() => {
sandbox
.stub(window, 'fetch')
.withArgs(
API_ENDPOINT,
sandbox.match({ body: JSON.stringify({ user_bundle_token: '..', password: 'hunter2' }) }),
)
.resolves({
json: () => Promise.resolve({ personal_key: '0000-0000-0000-0000' }),
} as Response);
context('with successful submission', () => {
beforeEach(() => {
sandbox
.stub(window, 'fetch')
.withArgs(
API_ENDPOINT,
sandbox.match({ body: JSON.stringify({ user_bundle_token: '..', password: 'hunter2' }) }),
)
.resolves({
json: () => Promise.resolve({ personal_key: '0000-0000-0000-0000' }),
} as Response);
});

it('sends with password confirmation values', async () => {
const patch = await submit({ userBundleToken: '..', password: 'hunter2' });

expect(patch).to.deep.equal({ personalKey: '0000-0000-0000-0000' });
});
});

it('sends with password confirmation values', async () => {
const patch = await submit({ userBundleToken: '..', password: 'hunter2' });
context('error submission', () => {
beforeEach(() => {
sandbox
.stub(window, 'fetch')
.withArgs(
API_ENDPOINT,
sandbox.match({ body: JSON.stringify({ user_bundle_token: '..', password: 'hunter2' }) }),
)
.resolves({
json: () => Promise.resolve({ error: { password: ['incorrect password'] } }),
} as Response);
});

it('throws error for the offending field', async () => {
const didError = await submit({ userBundleToken: '..', password: 'hunter2' }).catch(
(error: FormError) => {
expect(error.field).to.equal('password');
expect(error.message).to.equal('incorrect password');
return true;
},
);

expect(patch).to.deep.equal({ personalKey: '0000-0000-0000-0000' });
expect(didError).to.be.true();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { post } from '../../services/api';
import { FormError } from '@18f/identity-form-steps';
import { post, ErrorResponse, isErrorResponse } from '../../services/api';
import type { VerifyFlowValues } from '../../verify-flow';

/**
Expand All @@ -7,19 +8,34 @@ import type { VerifyFlowValues } from '../../verify-flow';
export const API_ENDPOINT = '/api/verify/v2/password_confirm';

/**
* API response shape.
* Successful API response shape.
*/
interface PasswordConfirmResponse {
interface PasswordConfirmSuccessResponse {
personal_key: string;
}

/**
* Failed API response shape.
*/
type PasswordConfirmErrorResponse = ErrorResponse<'password'>;

/**
* API response shape.
*/
type PasswordConfirmResponse = PasswordConfirmSuccessResponse | PasswordConfirmErrorResponse;

async function submit({ userBundleToken, password }: VerifyFlowValues) {
const payload = { user_bundle_token: userBundleToken, password };
const json = await post<PasswordConfirmResponse>(API_ENDPOINT, payload, {
json: true,
csrf: true,
});

if (isErrorResponse(json)) {
const [field, [error]] = Object.entries(json.error)[0];
throw new FormError(error, { field });
}

return { personalKey: json.personal_key };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ def stub_idv_session

it 'does not create a profile and return a key when it has the wrong password' do
post :create, params: { password: 'iamnotbatman', user_bundle_token: jwt }
expect(JSON.parse(response.body)['personal_key']).to be_nil
response_json = JSON.parse(response.body)
expect(response_json['personal_key']).to be_nil
expect(response_json['error']['password']).to eq([I18n.t('idv.errors.incorrect_password')])
expect(response.status).to eq 400
end
end
Expand Down
15 changes: 9 additions & 6 deletions spec/forms/api/profile_creation_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@

expect(response.success?).to be false
expect(personal_key).to be_nil
expect(response.errors[:password]).to eq ['invalid password']
expect(response.errors[:password]).to eq [I18n.t('idv.errors.incorrect_password')]
end
end

Expand All @@ -131,7 +131,7 @@

expect(response.success?).to be false
expect(personal_key).to be_nil
expect(response.errors[:user]).to eq ['user not found']
expect(response.errors[:user]).to eq [I18n.t('devise.failure.unauthenticated')]
end
end

Expand All @@ -143,7 +143,7 @@

expect(response.success?).to be false
expect(personal_key).to be_nil
expect(response.errors[:jwt]).to eq ['decode error: Signature has expired']
expect(response.errors[:jwt]).to eq [I18n.t('idv.failure.exceptions.internal_error')]
end
end
end
Expand Down Expand Up @@ -183,7 +183,8 @@

it 'is an invalid form' do
expect(subject.valid?).to be false
expect(subject.errors.to_a.join(' ')).to match(%r{decode error})
expect(subject.errors[:jwt]).to eq [I18n.t('idv.failure.exceptions.internal_error')]
expect(subject.errors).to include { |error| error.options[:type] == :decode_error }
end
end

Expand All @@ -199,7 +200,8 @@

it 'is an invalid form' do
expect(subject.valid?).to be false
expect(subject.errors.to_a.join(' ')).to match(%r{pii is missing})
expect(subject.errors[:jwt]).to eq [I18n.t('idv.failure.exceptions.internal_error')]
expect(subject.errors).to include { |error| error.options[:type] == :user_bundle_error }
end
end

Expand All @@ -215,7 +217,8 @@

it 'is an invalid form' do
expect(subject.valid?).to be false
expect(subject.errors.to_a.join(' ')).to match(%r{metadata is missing})
expect(subject.errors[:jwt]).to eq [I18n.t('idv.failure.exceptions.internal_error')]
expect(subject.errors).to include { |error| error.options[:type] == :user_bundle_error }
end
end
end
Expand Down