diff --git a/packages/fxa-settings/package.json b/packages/fxa-settings/package.json index 5e50c19daf..8296a97011 100644 --- a/packages/fxa-settings/package.json +++ b/packages/fxa-settings/package.json @@ -79,6 +79,7 @@ "@types/react-webcam": "^3.0.0", "base32-decode": "^1.0.0", "base32-encode": "^1.2.0", + "class-validator": "^0.14.0", "classnames": "^2.3.1", "file-saver": "^2.0.5", "fxa-auth-client": "workspace:*", diff --git a/packages/fxa-settings/src/components/App/index.tsx b/packages/fxa-settings/src/components/App/index.tsx index 632c467517..cdea760850 100644 --- a/packages/fxa-settings/src/components/App/index.tsx +++ b/packages/fxa-settings/src/components/App/index.tsx @@ -187,16 +187,16 @@ const AuthAndAccountSetupRoutes = (_: RouteComponentProps) => { path="/account_recovery_confirm_key/*" linkType={LinkType['reset-password']} viewName="account-recovery-confirm-key" - getParamsFromModel={() => { + createLinkModel={() => { return CreateCompleteResetPasswordLink(); }} {...{ integration }} > - {({ setLinkStatus, params }) => ( + {({ setLinkStatus, linkModel }) => ( )} diff --git a/packages/fxa-settings/src/components/LinkExpired/index.stories.tsx b/packages/fxa-settings/src/components/LinkExpired/index.stories.tsx index daadc01d1c..68ad4803a2 100644 --- a/packages/fxa-settings/src/components/LinkExpired/index.stories.tsx +++ b/packages/fxa-settings/src/components/LinkExpired/index.stories.tsx @@ -50,10 +50,7 @@ const mockedProps: LinkExpiredProps = { export const Default = () => ; export const LinkExpiredForResetPassword = () => ( - + ); export const LinkExpiredForSignin = () => ( diff --git a/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.test.tsx b/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.test.tsx index 536d79cdbd..828bba8746 100644 --- a/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.test.tsx +++ b/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.test.tsx @@ -40,12 +40,7 @@ const renderWithHistory = (ui: any, queryParams = '', account?: Account) => { }; describe('LinkExpiredResetPassword', () => { - const component = ( - - ); + const component = ; afterEach(() => { jest.clearAllMocks(); diff --git a/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.tsx b/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.tsx index 7237a6bc58..e0ac582e4d 100644 --- a/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.tsx +++ b/packages/fxa-settings/src/components/LinkExpiredResetPassword/index.tsx @@ -3,38 +3,24 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import React, { useState } from 'react'; -import { - IntegrationType, - OAuthIntegration, - isOAuthIntegration, - useAccount, -} from '../../models'; +import { useAccount } from '../../models'; import { ResendStatus } from '../../lib/types'; import { logViewEvent } from 'fxa-settings/src/lib/metrics'; import { REACT_ENTRYPOINT } from 'fxa-settings/src/constants'; import { LinkExpired } from '../LinkExpired'; -import { IntegrationSubsetType } from '../../lib/integrations'; type LinkExpiredResetPasswordProps = { - email: string; viewName: string; - integration: LinkExpiredResetPasswordIntegration; + email: string; + service?: string; + redirectUri?: string; }; -interface LinkExpiredResetPasswordOAuthIntegration { - type: IntegrationType.OAuth; - getService: () => ReturnType; - getRedirectUri: () => ReturnType; -} - -type LinkExpiredResetPasswordIntegration = - | LinkExpiredResetPasswordOAuthIntegration - | IntegrationSubsetType; - export const LinkExpiredResetPassword = ({ - email, viewName, - integration, + email, + service, + redirectUri, }: LinkExpiredResetPasswordProps) => { // TODO in FXA-7630 add metrics event and associated tests for users hitting the LinkExpired page const account = useAccount(); @@ -45,12 +31,8 @@ export const LinkExpiredResetPassword = ({ const resendResetPasswordLink = async () => { try { - if (isOAuthIntegration(integration)) { - await account.resetPassword( - email, - integration.getService(), - integration.getRedirectUri() - ); + if (service && redirectUri) { + await account.resetPassword(email, service, redirectUri); } else { await account.resetPassword(email); } diff --git a/packages/fxa-settings/src/components/LinkValidator/index.tsx b/packages/fxa-settings/src/components/LinkValidator/index.tsx index f353fdae32..dc01e4060a 100644 --- a/packages/fxa-settings/src/components/LinkValidator/index.tsx +++ b/packages/fxa-settings/src/components/LinkValidator/index.tsx @@ -9,28 +9,36 @@ import { LinkStatus, LinkType } from '../../lib/types'; import { ResetPasswordLinkDamaged, SigninLinkDamaged } from '../LinkDamaged'; import { LinkExpiredResetPassword } from '../LinkExpiredResetPassword'; import { LinkExpiredSignin } from '../LinkExpiredSignin'; -import { ModelDataProvider } from '../../lib/model-data'; -import { Integration } from '../../models'; +import { IntegrationType, isOAuthIntegration } from '../../models'; -interface LinkValidatorChildrenProps { +interface LinkValidatorChildrenProps { setLinkStatus: React.Dispatch>; - params: T; + linkModel: TModel; } -interface LinkValidatorProps { +interface LinkValidatorIntegration { + type: IntegrationType; +} + +interface LinkValidatorProps { linkType: LinkType; viewName: string; - getParamsFromModel: () => T; - integration: Integration; - children: (props: LinkValidatorChildrenProps) => React.ReactNode; + createLinkModel: () => TModel; + integration: LinkValidatorIntegration; + children: (props: LinkValidatorChildrenProps) => React.ReactNode; } -const LinkValidator = ({ - children, +interface LinkModel { + isValid(): boolean; + email: string | undefined; +} + +const LinkValidator = ({ linkType, viewName, - getParamsFromModel, integration, + createLinkModel, + children, }: LinkValidatorProps & RouteComponentProps) => { // If `LinkValidator` is a route component receiving `path, then `children` // is a React.ReactElement @@ -38,18 +46,9 @@ const LinkValidator = ({ ? (children as React.ReactElement).props.children : children; - const params = getParamsFromModel(); - const isValid = params.isValid(); - const email = getEmailFromParams(); - - function getEmailFromParams() { - const email = params.getModelData().get('email'); - if (typeof email === 'string') { - return email; - } else { - return undefined; - } - } + const linkModel = createLinkModel(); + const isValid = linkModel.isValid(); + const email = linkModel.email; const [linkStatus, setLinkStatus] = useState( isValid ? LinkStatus.valid : LinkStatus.damaged @@ -71,7 +70,18 @@ const LinkValidator = ({ linkType === LinkType['reset-password'] && email !== undefined ) { - return ; + if (isOAuthIntegration(integration)) { + const service = integration.getService(); + const redirectUri = integration.getRedirectUri(); + + return ( + + ); + } + + return ; } if ( @@ -82,7 +92,7 @@ const LinkValidator = ({ return ; } - return <>{child({ setLinkStatus, params })}; + return <>{child({ setLinkStatus, linkModel })}; }; export default LinkValidator; diff --git a/packages/fxa-settings/src/lib/integrations/integration-factory.test.ts b/packages/fxa-settings/src/lib/integrations/integration-factory.test.ts index f27da2fab7..f0f3936c2e 100644 --- a/packages/fxa-settings/src/lib/integrations/integration-factory.test.ts +++ b/packages/fxa-settings/src/lib/integrations/integration-factory.test.ts @@ -65,6 +65,12 @@ describe('lib/integrations/integration-factory', () => { .stub(flags, 'isV3DesktopContext') .returns(!!flagOverrides.isV3DesktopContext); + urlQueryData.set('scope', 'profile'); + urlQueryData.set('client_id', '123'); + urlQueryData.set('redirect_uri', 'https://redirect.to'); + + urlHashData.set('scope', 'profile'); + // Create a factory with current state const factory = new IntegrationFactory({ window, @@ -75,9 +81,6 @@ describe('lib/integrations/integration-factory', () => { delegates, }); - urlQueryData.set('client_id', '123'); - urlQueryData.set('redirect_uri', 'https://redirect.to'); - // Create the integration const integration = factory.getIntegration(); checkInstance(integration); diff --git a/packages/fxa-settings/src/lib/integrations/integration-factory.ts b/packages/fxa-settings/src/lib/integrations/integration-factory.ts index c2d57356a8..62263f7555 100644 --- a/packages/fxa-settings/src/lib/integrations/integration-factory.ts +++ b/packages/fxa-settings/src/lib/integrations/integration-factory.ts @@ -183,8 +183,8 @@ export class IntegrationFactory { // Important! // FxDesktop declares both `entryPoint` (capital P) and // `entrypoint` (lowcase p). Normalize to `entrypoint`. - const entryPoint = integration.data.getModelData().get('entryPoint'); - const entrypoint = integration.data.getModelData().get('entrypoint'); + const entryPoint = integration.data.getModelData('entryPoint'); + const entrypoint = integration.data.getModelData('entrypoint'); if ( entryPoint != null && entrypoint != null && diff --git a/packages/fxa-settings/src/lib/model-data/bind-decorator.spec.ts b/packages/fxa-settings/src/lib/model-data/bind-decorator.spec.ts index c604061bbf..bd27a29e62 100644 --- a/packages/fxa-settings/src/lib/model-data/bind-decorator.spec.ts +++ b/packages/fxa-settings/src/lib/model-data/bind-decorator.spec.ts @@ -2,29 +2,28 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { - bind, - KeyTransforms as T, - getBoundKeys, - validateData, -} from './bind-decorator'; -import { ModelValidation as V } from './model-validation'; +import { bind, KeyTransforms as T, getBoundKeys } from './bind-decorator'; import { GenericData } from './data-stores'; import { ModelDataProvider } from './model-data-provider'; +import { IsNotEmpty, IsOptional, IsString } from 'class-validator'; /** * Example model for testing bind decorators */ class TestModel extends ModelDataProvider { - @bind([], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) testField: string | undefined; - @bind([V.isNonEmptyString], T.snakeCase) + @IsOptional() + @IsString() + @IsNotEmpty() + @bind(T.snakeCase) testValidatedField: string | undefined; } describe('bind-decorator', function () { - it('creates with empty state', () => { const data = new GenericData({}); const model1 = new TestModel(data); @@ -99,19 +98,55 @@ describe('bind-decorator', function () { expect(T.snakeCase('')).toEqual(''); }); - it('validates', () => { - const data = new GenericData({}); - const model1 = new TestModel(data); - - expect(() => { - validateData(model1); - }).toThrow(); - }); - it('gets bound keys', () => { const data = new GenericData({}); const model1 = new TestModel(data); expect(getBoundKeys(model1)).toEqual(['testField', 'testValidatedField']); }); + + it('gets data directly', () => { + const data = new GenericData({ test_field: 'foo' }); + const model1 = new TestModel(data); + expect(model1.getModelData('test_field')).toEqual('foo'); + }); + + it('sets data directly', () => { + const data = new GenericData({ test_field: 'foo' }); + const model1 = new TestModel(data); + + model1.setModelData('test_field', 'bar'); + expect(model1.getModelData('test_field')).toEqual('bar'); + }); + + it('returns undefined for unset data marked as optional', () => { + const data = new GenericData({}); + const model1 = new TestModel(data); + expect(model1.getModelData('test_field') === undefined).toBeTruthy(); + expect(model1.getModelData('test_field') !== null).toBeTruthy(); + }); + + it('will not set invalid data directly', () => { + const data = new GenericData({}); + const model1 = new TestModel(data); + expect(() => model1.setModelData('test_field', 0)).toThrow(); + }); + + it('will not get invalid data directly', () => { + const data = new GenericData({ test_validated_field: '' }); + const model1 = new TestModel(data); + expect(() => model1.getModelData('testValidatedField')).toThrow(); + }); + + it('sets invalid when validate is specified as false', () => { + const data = new GenericData({ test_field: 'foo' }); + const model1 = new TestModel(data); + expect(() => model1.setModelData('test_field', 0, false)).not.toThrow(); + }); + + it('gets invalid when validate is specified as false', () => { + const data = new GenericData({ test_field: 0 }); + const model1 = new TestModel(data); + expect(() => model1.getModelData('test_field', false)).not.toThrow(); + }); }); diff --git a/packages/fxa-settings/src/lib/model-data/bind-decorator.ts b/packages/fxa-settings/src/lib/model-data/bind-decorator.ts index c4bf55a94b..5739322983 100644 --- a/packages/fxa-settings/src/lib/model-data/bind-decorator.ts +++ b/packages/fxa-settings/src/lib/model-data/bind-decorator.ts @@ -2,11 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import 'reflect-metadata'; -import { - ModelValidationError, - ModelValidationErrors, -} from './model-validation'; -import { ModelDataProvider } from './model-data-provider'; +import { ModelDataProvider, isModelDataProvider } from './model-data-provider'; /** * Turns a field name into a lookup key. This can be one of three states. @@ -51,25 +47,6 @@ const getKey = (keyTransform: KeyTransform, defaultValue: string) => { */ export const bindMetadataKey = Symbol('bind'); -/** - * For a given object that is bound to a data store, rerun all the - * validation checks on values that bind model fields to the data values held - * in the data store. - * @param target - A model that is bound to a data store. The ModelDataProvider is - * the base class, and provides access to the underlying data store. - */ -export function validateData(target: ModelDataProvider) { - for (const key of Object.keys(Object.getPrototypeOf(target))) { - // Resolves the @bind decorator - const bind = Reflect.getMetadata(bindMetadataKey, target, key); - - // If the @bind decorator was present, run its validation checks - if (bind?.validate && bind?.key) { - bind.validate(bind.key, target.getModelData().get(bind.key)); - } - } -} - /** * Looks up a list of property names that have bindings on an object * @param target @@ -86,24 +63,6 @@ export function getBoundKeys(target: ModelDataProvider) { return result; } -/** - * Provides the data store for the model, and runs a couple sanity checks on the model state. - */ -function getModelData(model: ModelDataProvider | any) { - if (!(model instanceof ModelDataProvider)) { - throw new Error( - 'Invalid bind! Does the model is not an an instance of ModelDataProvider. Check that the model inherits from ModelDataProvider.' - ); - } - const data = model.getModelData(); - if (data == null) { - throw new Error( - 'Invalid bind! Has the data store for the model been initialized?' - ); - } - return data; -} - /** * Represents a validation check **/ @@ -111,18 +70,17 @@ export type ValidationCheck = (key: string, value: T) => void; /** * A type script decorator for binding to an underlying data store. - * @param checks A list of sequential check to validate the state of the underlying value. * @param dataKey A key in the underlying data store. Used to bind the field to a key value pair in the data store. * @returns A value of type T * @example * * class User { - * @bind([V.isString], 'first_name') + * @bind('first_name') * firstName * } * */ -export const bind = (checks: ValidationCheck[], dataKey?: KeyTransform) => { +export const bind = (dataKey?: KeyTransform) => { // The actual decorator implementation. Note we have to use 'any' as a // return type here because that's the type expected by the TS decorator // definition. @@ -135,38 +93,8 @@ export const bind = (checks: ValidationCheck[], dataKey?: KeyTransform) => { throw new Error('Invalid bind! Model inherit from ModelDataProvider!'); } - /** - * Sequentially executes validation checks. - */ - function validate(key: string, value: unknown): T { - const errors = new Array(); - checks.forEach((check) => { - try { - // Throws an error if value is bad. - value = check(key, value); - } catch (err) { - if (err instanceof ModelValidationError) { - errors.push(err); - } else { - throw err; - } - } - }); - - if (errors.length > 0) { - throw new ModelValidationErrors( - `Errors in current data store: \n${errors - .map((x) => x.toString()) - .join(';')}`, - errors - ); - } - return value as T; - } - const metadata = { memberName, - validate, key: getKey(dataKey, memberName), }; Reflect.defineMetadata(bindMetadataKey, metadata, model, memberName); @@ -176,24 +104,34 @@ export const bind = (checks: ValidationCheck[], dataKey?: KeyTransform) => { const property = { enumerable: true, set: function (value: T) { - const data = getModelData(this); - const key = getKey(dataKey, memberName); - const currentValue = data.get(key); - - // Don't bother setting state needlessly. Depending on the data - // store writes may or may not be cheap. - if (currentValue !== value) { - data.set(key, validate(key, value)); + if (!isModelDataProvider(this)) { + throw new InvalidModelInstance(); } + + const key = getKey(dataKey, memberName); + this.setModelData(key, value); }, get: function () { - const data = getModelData(this); + if (!isModelDataProvider(this)) { + throw new InvalidModelInstance(); + } + const key = getKey(dataKey, memberName); - const value = data.get(key); - return validate(key, value); + return this.getModelData(key); }, }; Object.defineProperty(model, memberName, property); return property; }; }; + +/** + * Error for situations where the model data provider object that backs the @bind is invalid. + */ +export class InvalidModelInstance extends Error { + constructor() { + super( + 'Invalid bind! Does the model is not an an instance of ModelDataProvider. Check that the model inherits from ModelDataProvider.' + ); + } +} diff --git a/packages/fxa-settings/src/lib/model-data/index.ts b/packages/fxa-settings/src/lib/model-data/index.ts index 0887012e11..b8e34d2314 100644 --- a/packages/fxa-settings/src/lib/model-data/index.ts +++ b/packages/fxa-settings/src/lib/model-data/index.ts @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ export * from './bind-decorator'; -export * from './model-validation'; export * from './data-stores'; export * from './model-data-provider'; export * from './model-data-store'; diff --git a/packages/fxa-settings/src/lib/model-data/model-data-provider.spec.ts b/packages/fxa-settings/src/lib/model-data/model-data-provider.spec.ts index ece93f3640..895a7ed52d 100644 --- a/packages/fxa-settings/src/lib/model-data/model-data-provider.spec.ts +++ b/packages/fxa-settings/src/lib/model-data/model-data-provider.spec.ts @@ -12,11 +12,6 @@ describe('model-data-provider', function () { modelDataProvider = new ModelDataProvider(new GenericData({})); }); - it('gets model data accessor', () => { - const dataAccessor = modelDataProvider.getModelData(); - expect(dataAccessor).toBeDefined(); - }); - it('validates', () => { expect(() => modelDataProvider.validate()).not.toThrow(); }); diff --git a/packages/fxa-settings/src/lib/model-data/model-data-provider.ts b/packages/fxa-settings/src/lib/model-data/model-data-provider.ts index e9b51b759e..20debd50b1 100644 --- a/packages/fxa-settings/src/lib/model-data/model-data-provider.ts +++ b/packages/fxa-settings/src/lib/model-data/model-data-provider.ts @@ -2,11 +2,24 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { validateData } from './bind-decorator'; -import { ModelValidationErrors } from './model-validation'; +import { validateSync, ValidationError } from 'class-validator'; import { ModelDataStore } from './model-data-store'; +/** + * Type guard for validating model + * @param model + * @returns + */ +export function isModelDataProvider(model: any): model is ModelDataProvider { + if (model instanceof ModelDataProvider) { + return true; + } + return false; +} + export class ModelDataProvider { + private isDirty = true; + constructor(protected readonly modelData: ModelDataStore) { if (modelData == null) { throw new Error('dataAccessor must be provided!'); @@ -15,10 +28,43 @@ export class ModelDataProvider { /** * Gets the data collection that holds the state of the model. + * @param key Key data is held under + * @param value Value for key + * @param validate Whether or not to validate. Optional and defaults to true. + * @returns underlying data + */ + setModelData(key: string, value: unknown, validate = true) { + if (this.modelData == null) { + throw new Error( + 'Invalid bind! Has the data store for the model been initialized?' + ); + } + const currentValue = this.modelData.get(key); + if (currentValue !== value) { + this.modelData.set(key, value); + this.isDirty = true; + if (validate) { + this.validate(); + } + } + } + + /** + * Fetches data from underlying data store + * @param key The key to look up + * @param validate Whether or not to validate. Optional and defaults to true. * @returns */ - getModelData() { - return this.modelData; + getModelData(key: string, validate = true) { + if (this.modelData == null) { + throw new Error( + 'Invalid bind! Has the data store for the model been initialized?' + ); + } + if (validate) { + this.validate(); + } + return this.modelData.get(key); } /** @@ -33,10 +79,25 @@ export class ModelDataProvider { /** * Checks the state of the model data is valid. Throws an error if not valid. + * @param key A specific property name to validate. * @returns */ - validate() { - return validateData(this); + validate(property?: string) { + if (!this.isDirty) { + return; + } + this.isDirty = false; + + let errors = validateSync(this); + + // If a key was provided only consider errors for that property. + if (property) { + errors = errors.filter((x) => x.property === property); + } + + if (errors.length > 0) { + throw new ModelValidationErrors(errors); + } } /** @@ -49,7 +110,9 @@ export class ModelDataProvider { return true; } catch (err) { if (err instanceof ModelValidationErrors) { - console.warn(err.errors.map((x) => `${x.key}-${x.value}-${x.message}`)); + err.errors.forEach((x) => { + console.warn(x.toString()); + }); return false; } throw err; @@ -63,16 +126,16 @@ export class ModelDataProvider { */ tryValidate(): { isValid: boolean; - error?: ModelValidationErrors; + error?: ValidationError; } { - let error: ModelValidationErrors | undefined; + let error: ValidationError | undefined; let isValid = true; try { this.validate(); } catch (err) { console.warn(err); isValid = false; - if (err instanceof ModelValidationErrors) { + if (err instanceof ValidationError) { error = err; } else { throw err; @@ -85,3 +148,12 @@ export class ModelDataProvider { }; } } + +export class ModelValidationErrors extends Error { + constructor(public readonly errors: ValidationError[]) { + super( + 'Model Validation Errors Encountered! Fields:' + + errors.map((x) => x.toString()).join(',') + ); + } +} diff --git a/packages/fxa-settings/src/lib/model-data/model-validation.spec.ts b/packages/fxa-settings/src/lib/model-data/model-validation.spec.ts deleted file mode 100644 index 6d126fe101..0000000000 --- a/packages/fxa-settings/src/lib/model-data/model-validation.spec.ts +++ /dev/null @@ -1,49 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -import { ModelValidation as V } from './model-validation'; - -describe('model-validation', function () { - //let sandbox:SinonSandbox; - - it('validates string', () => { - V.isString('', ''); - V.isString('', null); - V.isString('', undefined); - expect(() => V.isString('', {})).toThrow(); - expect(() => V.isString('', 1)).toThrow(); - expect(() => V.isString('', true)).toThrow(); - expect(() => V.isString('', false)).toThrow(); - }); - - it('validates hex string', () => { - // Based on string, so only check formatting here - V.isHex('', '123ABC'); - expect(() => V.isHex('', 'zys')).toThrow(); - }); - - it('validates non empty string', () => { - V.isNonEmptyString('', '1'); - expect(() => V.isNonEmptyString('', '')).toThrow(); - }); - - it('validates number', () => { - V.isNumber('', 1); - V.isNumber('', 1.1); - V.isNumber('', 1e2); - V.isNumber('', -1e2); - V.isNumber('', '1'); - V.isNumber('', '1.1'); - V.isNumber('', '1e2'); - V.isNumber('', '-1e2'); - V.isNumber('', null); - V.isNumber('', undefined); - expect(() => V.isNumber('', 'a')).toThrow(); - expect(() => V.isNumber('', true)).toThrow(); - expect(() => V.isNumber('', false)).toThrow(); - expect(() => V.isNumber('', {})).toThrow(); - }); - - // TODO: Complete validations and tests -}); diff --git a/packages/fxa-settings/src/lib/model-data/model-validation.ts b/packages/fxa-settings/src/lib/model-data/model-validation.ts deleted file mode 100644 index f3bdcf07c3..0000000000 --- a/packages/fxa-settings/src/lib/model-data/model-validation.ts +++ /dev/null @@ -1,199 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -// TODO: Figure out how to port Vat. Here's a simplistic implementation for POC. - -import { isEmailValid } from 'fxa-shared/email/helpers'; -import { Constants } from '../constants'; - -/** - * Dedicated error class for validation errors that occur when using the @bind decorator. - */ -export class ModelValidationError extends Error { - constructor( - public readonly key: string, - public readonly value: any, - public readonly message: string - ) { - super(message); - } - - toString() { - return `[key=${this.key}] [value=${this.value}] - ${this.message} `; - } -} - -export class ModelValidationErrors extends Error { - constructor( - public readonly message: string, - public readonly errors: ModelValidationError[] - ) { - super(message); - } -} - -/** Validations */ -export const ModelValidation = { - isRequired: (k: string, v: any) => { - if (v == null) { - throw new ModelValidationError(k, v, 'Must exist!'); - } - return v; - }, - isString: (k: string, v: any) => { - if (v == null) { - return v; - } - if (typeof v !== 'string') { - throw new ModelValidationError(k, v, 'Is not string'); - } - return v; - }, - isHex: (k: string, v: any) => { - if (v == null) { - return v; - } - if (typeof v !== 'string' || !/^(?:[a-fA-F0-9]{2})+$/.test(v)) { - throw new ModelValidationError(k, v, 'Is not a hex string'); - } - return v; - }, - isBoolean: (k: string, v: any) => { - if (v == null) { - return v; - } - - if (typeof v === 'boolean') { - return v; - } - if (typeof v === 'string') { - v = v.toLocaleLowerCase().trim(); - } - if (v === 'true') { - return true; - } - if (v === 'false') { - return false; - } - throw new ModelValidationError(k, v, 'Is not boolean'); - }, - isNumber: (k: string, v: any) => { - if (v == null) { - return v; - } - - const n = parseFloat(v); - if (isNaN(n)) { - throw new ModelValidationError(k, v, 'Is not a number'); - } - return n; - }, - - isClientId: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isAccessType: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isCodeChallenge: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isCodeChallengeMethod: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isPrompt: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isUrl: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isUri: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isNonEmptyString: (k: string, v: any) => { - v = ModelValidation.isString(k, v); - if ((v || '').length === 0) { - throw new ModelValidationError(k, v, 'Cannot be an empty string'); - } - return v; - }, - - isVerificationCode: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isAction: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isKeysJwk: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isIdToken: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isEmail: (k: string, v: any) => { - // TODO: Add validation - v = ModelValidation.isString(k, v); - if (!isEmailValid(v)) { - throw new ModelValidationError(k, v, 'Is not a valid email'); - } - return v; - }, - - isGreaterThanZero: (k: string, v: any) => { - // TODO: Add validation - v = ModelValidation.isNumber(k, v); - if (v < 0) { - throw new ModelValidationError(k, v, 'Is not a positive number'); - } - return v; - }, - - isPairingAuthorityRedirectUri: (k: string, v: any) => { - if ((v || '') !== Constants.DEVICE_PAIRING_AUTHORITY_REDIRECT_URI) { - throw new ModelValidationError( - k, - v, - 'Is not a DEVICE_PAIRING_AUTHORITY_REDIRECT_URI' - ); - } - return v; - }, - - isChannelId: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isChannelKey: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, - - isValidCountry: (k: string, v: any) => { - // TODO: Add validation - return ModelValidation.isString(k, v); - }, -}; diff --git a/packages/fxa-settings/src/models/integrations/channel-info.test.ts b/packages/fxa-settings/src/models/integrations/channel-info.test.ts index 399da23dcc..3b3f3b855b 100644 --- a/packages/fxa-settings/src/models/integrations/channel-info.test.ts +++ b/packages/fxa-settings/src/models/integrations/channel-info.test.ts @@ -10,7 +10,10 @@ describe('models/integrations/channel-info', function () { let model: ChannelInfo; beforeEach(function () { - data = new GenericData({}); + data = new GenericData({ + channel_id: Buffer.from('id123').toString('base64'), + channel_key: Buffer.from('key123').toString('base64'), + }); model = new ChannelInfo(data); }); @@ -19,11 +22,11 @@ describe('models/integrations/channel-info', function () { }); it('binds model to data store', () => { - data.set('channel_id', 'foo'); - data.set('channel_key', 'bar'); + data.set('channel_id', Buffer.from('foo').toString('base64')); + data.set('channel_key', Buffer.from('bar').toString('base64')); - expect(model.channelId).toEqual('foo'); - expect(model.channelKey).toEqual('bar'); + expect(model.channelId).toEqual(Buffer.from('foo').toString('base64')); + expect(model.channelKey).toEqual(Buffer.from('bar').toString('base64')); }); it('validates', () => { diff --git a/packages/fxa-settings/src/models/integrations/channel-info.ts b/packages/fxa-settings/src/models/integrations/channel-info.ts index b146a3fc78..176c79f0bb 100644 --- a/packages/fxa-settings/src/models/integrations/channel-info.ts +++ b/packages/fxa-settings/src/models/integrations/channel-info.ts @@ -6,13 +6,17 @@ import { bind, KeyTransforms as T, ModelDataProvider, - ModelValidation as V, } from '../../lib/model-data'; +import { IsBase64, IsNotEmpty } from 'class-validator'; export class ChannelInfo extends ModelDataProvider { - @bind([V.isChannelId], T.snakeCase) + @IsBase64() + @IsNotEmpty() + @bind(T.snakeCase) channelId: string | undefined; - @bind([V.isChannelKey], T.snakeCase) + @IsBase64() + @IsNotEmpty() + @bind(T.snakeCase) channelKey: string | undefined; } diff --git a/packages/fxa-settings/src/models/integrations/client-info.ts b/packages/fxa-settings/src/models/integrations/client-info.ts index ca2eac0577..69808f77b4 100644 --- a/packages/fxa-settings/src/models/integrations/client-info.ts +++ b/packages/fxa-settings/src/models/integrations/client-info.ts @@ -2,26 +2,43 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import { + IsBoolean, + IsHexadecimal, + IsOptional, + IsString, +} from 'class-validator'; import { bind, KeyTransforms as T, ModelDataProvider, - ModelValidation as V, } from '../../lib/model-data'; export class ClientInfo extends ModelDataProvider { - @bind([V.isString, V.isHex], 'id') + @IsOptional() + @IsHexadecimal() + @bind('id') clientId: string | undefined; - @bind([V.isString], T.snakeCase) + // TODO - Validation - Needs @IsEncodedUrl() + @IsOptional() + @IsString() + @bind(T.snakeCase) imageUri: string | undefined; - @bind([V.isString, V.isRequired], 'name') + @IsOptional() + @IsString() + @bind('name') serviceName: string | undefined; - @bind([V.isString], T.snakeCase) + // TODO - Validation - Needs @IsEncodedUrl() + @IsOptional() + @IsString() + @bind(T.snakeCase) redirectUri: string | undefined; - @bind([V.isBoolean]) + @IsOptional() + @IsBoolean() + @bind() trusted: boolean | undefined; } diff --git a/packages/fxa-settings/src/models/integrations/oauth-integration.test.ts b/packages/fxa-settings/src/models/integrations/oauth-integration.test.ts index 494bb53560..fac1cbf4a2 100644 --- a/packages/fxa-settings/src/models/integrations/oauth-integration.test.ts +++ b/packages/fxa-settings/src/models/integrations/oauth-integration.test.ts @@ -11,8 +11,12 @@ describe('models/integrations/oauth-relier', function () { let model: OAuthIntegration; beforeEach(function () { - data = new GenericData({}); - oauthData = new GenericData({}); + data = new GenericData({ + scope: 'profile', + }); + oauthData = new GenericData({ + scope: 'profile', + }); model = new OAuthIntegration(data, oauthData, { scopedKeysEnabled: true, scopedKeysValidation: {}, @@ -61,8 +65,10 @@ describe('models/integrations/oauth-relier', function () { } it('empty scope', async () => { - const integration = getIntegration(''); - await expect(integration.getPermissions()).rejects.toThrow(); + await expect(async () => { + const integration = getIntegration(''); + const _permissions = await integration.getPermissions(); + }).rejects.toThrow(); }); it('whitespace scope', async () => { diff --git a/packages/fxa-settings/src/models/integrations/oauth-integration.ts b/packages/fxa-settings/src/models/integrations/oauth-integration.ts index 5b2741c579..0b1ddece06 100644 --- a/packages/fxa-settings/src/models/integrations/oauth-integration.ts +++ b/packages/fxa-settings/src/models/integrations/oauth-integration.ts @@ -9,16 +9,25 @@ import { RelierAccount, RelierClientInfo, } from './base-integration'; -import { - ModelDataStore, - bind, - KeyTransforms as T, - ModelValidation as V, -} from '../../lib/model-data'; +import { ModelDataStore, bind, KeyTransforms as T } from '../../lib/model-data'; import { Constants } from '../../lib/constants'; import { ERRORS, OAuthError } from '../../lib/oauth'; import { IntegrationFlags } from '../../lib/integrations'; import { BaseIntegrationData } from './web-integration'; +import { + IsBoolean, + IsEmail, + IsHexadecimal, + IsIn, + IsNotEmpty, + IsOptional, + IsPositive, + IsString, + MaxLength, + MinLength, +} from 'class-validator'; + +const PKCE_CODE_CHALLENGE_LENGTH = 43; interface OAuthIntegrationFeatures extends IntegrationFeatures { webChannelSupport: boolean; @@ -45,64 +54,115 @@ export function isOAuthIntegration(integration: { // TODO: probably move this somewhere else export class OAuthIntegrationData extends BaseIntegrationData { - @bind([V.isString], T.snakeCase) + // TODO - Validation - Can we get a set of known client ids from config or api call? See https://github.com/mozilla/fxa/pull/15677#discussion_r1291534277 + @IsOptional() + @IsHexadecimal() + @bind(T.snakeCase) clientId: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() imageUri: string | undefined; - @bind([V.isBoolean]) + @IsBoolean() + @IsOptional() + @bind() trusted: boolean | undefined; - @bind([V.isAccessType], T.snakeCase) + @IsOptional() + @IsIn(['offline', 'online']) + @bind(T.snakeCase) accessType: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() acrValues: string | undefined; - @bind([V.isAction]) + // TODO - Validation - Double check actions + @IsOptional() + @IsIn(['signin', 'signup', 'email', 'force_auth', 'pairing']) + @bind() action: string | undefined; - @bind([V.isCodeChallenge], T.snakeCase) + @IsOptional() + @IsString() + @MinLength(43) + @MaxLength(128) + @bind(T.snakeCase) codeChallenge: string | undefined; - @bind([V.isCodeChallengeMethod]) + @IsOptional() + @IsIn(['S256']) + @bind(T.snakeCase) codeChallengeMethod: string | undefined; - @bind([V.isString], T.snakeCase) + // TODO - Validation - Should this be base64? + @IsOptional() + @IsString() + @bind(T.snakeCase) keysJwk: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) idTokenHint: string | undefined; - @bind([V.isGreaterThanZero], T.snakeCase) + @IsPositive() + @IsOptional() + @bind(T.snakeCase) maxAge: number | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() permissions: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() prompt: string | undefined; - @bind([V.isUrl]) + // TODO - Validation - This should be a URL, but it is encoded and must be decoded in order to validate. + @IsOptional() + @IsString() + @bind() redirectTo: string | undefined; - @bind([V.isUrl], T.snakeCase) + // TODO - Validation - This should be a URL, but it is encoded and must be decoded in order to validate. + @IsOptional() + @IsString() + @bind(T.snakeCase) redirectUrl: string | undefined; - @bind([V.isString], T.snakeCase) + // TODO - Validation - Needs custom validation, see IsRedirectUriValid in content server. + // TODO - Validation - Seems to be required for OAuth + @IsString() + @IsOptional() + @bind(T.snakeCase) redirectUri: string | undefined; - @bind([V.isString], T.snakeCase) + @IsBoolean() + @IsOptional() + @bind(T.snakeCase) returnOnError: boolean | undefined; - @bind([V.isString]) + // TODO - Validation - Should scope be required? + @IsOptional() + @IsString() + @IsNotEmpty() + @bind() scope: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() state: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsEmail() + @bind() loginHint: string | undefined; } diff --git a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts index eacc1c6ecd..4aa8aaec88 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.test.ts @@ -11,7 +11,9 @@ describe('models/integrations/pairing-authority-relier', function () { let model: PairingAuthorityIntegration; beforeEach(function () { - data = new GenericData({}); + data = new GenericData({ + scope: 'profile', + }); storageData = new GenericData({}); model = new PairingAuthorityIntegration(data, storageData, { scopedKeysEnabled: true, diff --git a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts index 104bfb2a32..8683631017 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-authority-integration.ts @@ -9,14 +9,13 @@ import { OAuthIntegrationData, OAuthIntegrationOptions, } from './oauth-integration'; -import { - bind, - KeyTransforms as T, - ModelValidation as V, -} from '../../lib/model-data'; +import { bind, KeyTransforms as T } from '../../lib/model-data'; +import { IsBase64, IsNotEmpty } from 'class-validator'; export class PairingAuthorityIntegrationData extends OAuthIntegrationData { - @bind([V.isString], T.snakeCase) + @IsBase64() + @IsNotEmpty() + @bind(T.snakeCase) channelId: string = ''; } diff --git a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts index a3ab6d4efd..9a74703042 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.test.ts @@ -11,7 +11,7 @@ describe('models/integration/pairing-supplicant-integration', function () { let model: PairingSupplicantIntegration; beforeEach(function () { - data = new GenericData({}); + data = new GenericData({ scope: 'profile' }); storageData = new GenericData({}); model = new PairingSupplicantIntegration(data, storageData, { scopedKeysEnabled: true, diff --git a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts index 76acc7eadb..5bab05d59f 100644 --- a/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts +++ b/packages/fxa-settings/src/models/integrations/pairing-supplicant-integration.ts @@ -6,11 +6,16 @@ import { OAuthIntegrationData } from '.'; import { IntegrationType } from './base-integration'; import { bind } from '../../lib/model-data'; import { OAuthIntegration, OAuthIntegrationOptions } from './oauth-integration'; +import { IsNotEmpty, IsOptional, IsString } from 'class-validator'; // TODO in the 'Pairing' React epic. This shouldn't have any `feature` overrides but feel // free to look at all of that logic with fresh eyes in case we want to do it differently. export class PairingSupplicantIntegrationData extends OAuthIntegrationData { - @bind([]) + // TODO - Validation - Should scope be required? + @IsOptional() + @IsString() + @IsNotEmpty() + @bind() scope: string | undefined = ''; } diff --git a/packages/fxa-settings/src/models/integrations/signin-signup-info.ts b/packages/fxa-settings/src/models/integrations/signin-signup-info.ts index 65701b1dba..b3ac238d85 100644 --- a/packages/fxa-settings/src/models/integrations/signin-signup-info.ts +++ b/packages/fxa-settings/src/models/integrations/signin-signup-info.ts @@ -2,11 +2,22 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import { + IsBase64, + IsBoolean, + IsEmail, + IsHexadecimal, + IsIn, + IsInt, + IsNotEmpty, + IsOptional, + IsPositive, + IsString, +} from 'class-validator'; import { bind, KeyTransforms as T, ModelDataProvider, - ModelValidation as V, } from '../../lib/model-data'; // Sign inflow @@ -14,54 +25,93 @@ import { // https://mozilla.github.io/ecosystem-platform/api#tag/OAuth-Server-API-Overview export class SignInSignUpInfo extends ModelDataProvider { - @bind([V.isAccessType], T.snakeCase) + @IsOptional() + @IsIn(['offline', 'online']) + @bind(T.snakeCase) accessType: string | undefined; - @bind([V.isString], T.snakeCase) + @IsString() + @IsOptional() + @bind(T.snakeCase) acrValues: string | undefined; - @bind([V.isAction], T.snakeCase) + // TODO - Validation - Double check actions + @IsOptional() + @IsIn(['signin', 'signup', 'email', 'force_auth', 'pairing']) + @bind(T.snakeCase) action: string | undefined; - @bind([V.isClientId], T.snakeCase) + @IsOptional() + @IsHexadecimal() + @bind(T.snakeCase) clientId: string | undefined; - @bind([V.isCodeChallenge], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) codeChallenge: string | undefined; - @bind([V.isCodeChallengeMethod], T.snakeCase) + @IsOptional() + @IsIn(['S256']) + @bind(T.snakeCase) codeChallengeMethod: string | undefined; - @bind([V.isKeysJwk], T.snakeCase) + @IsOptional() + @IsBase64() + @bind(T.snakeCase) keysJwk: string | undefined; - @bind([V.isIdToken], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) idTokenHint: string | undefined; - @bind([V.isEmail], T.snakeCase) + @IsOptional() + @IsEmail() + @bind(T.snakeCase) loginHint: string | undefined; - @bind([V.isGreaterThanZero], T.snakeCase) + @IsOptional() + @IsInt() + @IsPositive() + @bind(T.snakeCase) maxAge: number | undefined; - @bind([V.isPrompt]) + @IsOptional() + @IsIn(['consent', 'none', 'login']) + @bind() prompt: string | undefined; - @bind([V.isPairingAuthorityRedirectUri], T.snakeCase) + // TODO - Validation - Needs @IsEncodedUrl() + @IsOptional() + @IsString() + @bind(T.snakeCase) redirectUri: string | undefined; - @bind([V.isUrl], T.snakeCase) + // TODO - Validation - Needs @IsEncodedUrl() + @IsOptional() + @IsString() + @bind(T.snakeCase) redirectTo: string | undefined; - @bind([V.isBoolean], T.snakeCase) + @IsOptional() + @IsBoolean() + @bind(T.snakeCase) returnOnError: boolean | undefined; - @bind([V.isNonEmptyString]) + @IsOptional() + @IsString() + @IsNotEmpty() + @bind() scope: string | undefined; - @bind([V.isNonEmptyString]) + @IsOptional() + @IsString() + @bind() state: string | undefined; - @bind([V.isEmail]) + @IsOptional() + @IsEmail() + @bind() email: string | undefined; } diff --git a/packages/fxa-settings/src/models/integrations/supplicant-info.ts b/packages/fxa-settings/src/models/integrations/supplicant-info.ts index 8367c87401..da149d505b 100644 --- a/packages/fxa-settings/src/models/integrations/supplicant-info.ts +++ b/packages/fxa-settings/src/models/integrations/supplicant-info.ts @@ -2,29 +2,52 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import { + IsHexadecimal, + IsIn, + IsNotEmpty, + IsOptional, + IsString, + MaxLength, + MinLength, +} from 'class-validator'; import { bind, KeyTransforms as T, ModelDataProvider, - ModelValidation as V, } from '../../lib/model-data'; export class SupplicantInfo extends ModelDataProvider { - @bind([V.isAccessType], T.snakeCase) + @IsOptional() + @IsIn(['offline', 'online']) + @bind(T.snakeCase) accessType: string | undefined; - @bind([V.isClientId], T.snakeCase) + @IsOptional() + @IsHexadecimal() + @bind(T.snakeCase) clientId: string | undefined; - @bind([V.isCodeChallenge], T.snakeCase) + @IsOptional() + @IsString() + @MinLength(43) + @MaxLength(128) + @bind(T.snakeCase) codeChallenge: string | undefined; - @bind([V.isCodeChallengeMethod], T.snakeCase) + @IsOptional() + @IsIn(['S256']) + @bind(T.snakeCase) codeChallengeMethod: string | undefined; - @bind([V.isNonEmptyString]) + @IsOptional() + @IsString() + @IsNotEmpty() + @bind() scope: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind() state: string | undefined; } diff --git a/packages/fxa-settings/src/models/integrations/sync-basic-integration.ts b/packages/fxa-settings/src/models/integrations/sync-basic-integration.ts index fee1d054c1..905b330b9d 100644 --- a/packages/fxa-settings/src/models/integrations/sync-basic-integration.ts +++ b/packages/fxa-settings/src/models/integrations/sync-basic-integration.ts @@ -7,31 +7,54 @@ import { IntegrationFeatures, IntegrationType, } from './base-integration'; -import { - bind, - ModelValidation as V, - ModelDataStore, -} from '../../lib/model-data'; +import { bind, ModelDataStore } from '../../lib/model-data'; import { Constants } from '../../lib/constants'; import { BaseIntegrationData } from './web-integration'; +import { + IsBase64, + IsISO31661Alpha3, + IsIn, + IsOptional, + IsString, + Length, + MaxLength, + MinLength, +} from 'class-validator'; export class SyncBasicIntegrationData extends BaseIntegrationData { - @bind([V.isValidCountry]) + // TODO - Validation - Will @IsISO31661Alpha2() work? + @IsOptional() + @IsString() + @MinLength(2) + @MaxLength(7) + @bind() country: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsBase64() + @Length(8) + @bind() signinCode: string | undefined; - @bind([V.isAction]) + // TODO - Validation - Double check actions + @IsOptional() + @IsIn(['signin', 'signup', 'email', 'force_auth', 'pairing']) + @bind() action: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() syncPreference: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() multiService: boolean | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() tokenCode: string | undefined; } diff --git a/packages/fxa-settings/src/models/integrations/web-integration.ts b/packages/fxa-settings/src/models/integrations/web-integration.ts index 6350ccf663..708304d781 100644 --- a/packages/fxa-settings/src/models/integrations/web-integration.ts +++ b/packages/fxa-settings/src/models/integrations/web-integration.ts @@ -7,58 +7,98 @@ import { bind, KeyTransforms as T, ModelDataProvider, - ModelValidation as V, ModelDataStore, } from '../../lib/model-data'; +import { + IsEmail, + IsHexadecimal, + IsIn, + IsOptional, + IsString, + Length, +} from 'class-validator'; // TODO: move this to other file, FXA-8099 export class BaseIntegrationData extends ModelDataProvider { - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() context: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsEmail() + @bind() email: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsEmail() + @bind() emailToHashWith: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() entrypoint: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) entrypointExperiment: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) entrypointVariation: string | undefined; - @bind([V.isBoolean], T.snakeCase) + @IsOptional() + @IsIn(['true', 'false', true, false]) + @bind(T.snakeCase) resetPasswordConfirm: boolean | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() setting: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() service: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsString() + @bind() style: string | undefined; - @bind([V.isString]) + @IsOptional() + @IsHexadecimal() + @Length(32) + @bind() uid: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) utmCampaign: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) utmContent: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) utmMedium: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) utmSource: string | undefined; - @bind([V.isString], T.snakeCase) + @IsOptional() + @IsString() + @bind(T.snakeCase) utmTerm: string | undefined; } diff --git a/packages/fxa-settings/src/models/mocks.tsx b/packages/fxa-settings/src/models/mocks.tsx index ad70c53cb4..5dbbcf73ea 100644 --- a/packages/fxa-settings/src/models/mocks.tsx +++ b/packages/fxa-settings/src/models/mocks.tsx @@ -99,7 +99,7 @@ export function mockStorage() { }; } -export function mockUrlQueryData(params: Record) { +export function mockUrlQueryData(params: Record) { const window = new ReachRouterWindow(); const data = new UrlQueryData(window); for (const param of Object.keys(params)) { diff --git a/packages/fxa-settings/src/models/reset-password/verification/complete-reset-password-link.ts b/packages/fxa-settings/src/models/reset-password/verification/complete-reset-password-link.ts index 6bd408896b..7081329e3f 100644 --- a/packages/fxa-settings/src/models/reset-password/verification/complete-reset-password-link.ts +++ b/packages/fxa-settings/src/models/reset-password/verification/complete-reset-password-link.ts @@ -3,27 +3,35 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { - bind, - ModelDataProvider, - ModelValidation as V, -} from '../../../lib/model-data'; + IsEmail, + IsHexadecimal, + IsNotEmpty, + IsOptional, + IsString, +} from 'class-validator'; +import { bind, ModelDataProvider } from '../../../lib/model-data'; export class CompleteResetPasswordLink extends ModelDataProvider { - // TODO: change `isNonEmptyString` to `email` when validation is properly set up. - // This is temporary for tests/Storybook so that `email=''` shows a damaged link - @bind([V.isNonEmptyString, V.isRequired]) + @IsOptional() + @IsEmail() + @bind() email: string = ''; - // TODO: add @bind `isEmail` when validation is properly set up. - // This should be _optional_ but when this exists it should be an email. - emailToHashWith: string = ''; + @IsOptional() + @IsEmail() + @bind() + emailToHashWith: string | undefined = ''; - @bind([V.isNonEmptyString, V.isRequired]) + @IsString() + @IsNotEmpty() + @bind() code: string = ''; - @bind([V.isHex, V.isRequired]) + @IsHexadecimal() + @bind() token: string = ''; - @bind([V.isHex, V.isRequired]) + @IsHexadecimal() + @bind() uid: string = ''; } diff --git a/packages/fxa-settings/src/models/verification/verification-info.ts b/packages/fxa-settings/src/models/verification/verification-info.ts index 5252fdc572..c2c9f05b98 100644 --- a/packages/fxa-settings/src/models/verification/verification-info.ts +++ b/packages/fxa-settings/src/models/verification/verification-info.ts @@ -2,28 +2,41 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { ModelValidation, ModelDataProvider, bind } from '../../lib/model-data'; +import { + IsEmail, + IsHexadecimal, + IsNotEmpty, + IsOptional, + IsString, + Length, +} from 'class-validator'; +import { ModelDataProvider, bind } from '../../lib/model-data'; export * from './verification-info'; export type VerificationInfoLinkStatus = 'expired' | 'damaged' | 'valid'; -const { isEmail, isRequired, isVerificationCode, isHex, isString, isBoolean } = - ModelValidation; - export class VerificationInfo extends ModelDataProvider { - @bind([isRequired, isEmail]) + @IsEmail() + @bind() email: string = ''; - @bind([isRequired, isEmail]) - emailToHashWith: string = ''; + @IsOptional() + @IsEmail() + @bind() + emailToHashWith: string | undefined = ''; - @bind([isRequired, isVerificationCode]) + @IsHexadecimal() + @Length(32) + @bind() code: string = ''; - @bind([isRequired, isHex]) + @IsHexadecimal() + @bind() token: string = ''; - @bind([isRequired, isString]) + @IsHexadecimal() + @Length(32) + @bind() uid: string = ''; } diff --git a/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/index.tsx b/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/index.tsx index 605bc52248..3bd1738150 100644 --- a/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/index.tsx +++ b/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/index.tsx @@ -41,10 +41,10 @@ type SubmitData = { export const viewName = 'account-recovery-confirm-key'; const AccountRecoveryConfirmKey = ({ - params, + linkModel, setLinkStatus, }: { - params: CompleteResetPasswordLink; + linkModel: CompleteResetPasswordLink; setLinkStatus: React.Dispatch>; }) => { // TODO: grab serviceName from the relier @@ -81,8 +81,8 @@ const AccountRecoveryConfirmKey = ({ } }; - checkPasswordForgotToken(params.token); - }, [account, params.token, setLinkStatus]); + checkPasswordForgotToken(linkModel.token); + }, [account, linkModel.token, setLinkStatus]); const { handleSubmit, register } = useForm({ mode: 'onBlur', @@ -246,10 +246,10 @@ const AccountRecoveryConfirmKey = ({ const recoveryKeyStripped = recoveryKey.replace(/\s/g, ''); onSubmit({ recoveryKey: recoveryKeyStripped, - token: params.token, - code: params.code, - email: params.email, - uid: params.uid, + token: linkModel.token, + code: linkModel.code, + email: linkModel.email, + uid: linkModel.uid, }); })} data-testid="account-recovery-confirm-key-form" diff --git a/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/mocks.tsx b/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/mocks.tsx index ded41465df..0de9635ef4 100644 --- a/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/mocks.tsx +++ b/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/mocks.tsx @@ -77,14 +77,14 @@ export const getSubject = ( { + createLinkModel={() => { return new CompleteResetPasswordLink(urlQueryData); }} // TODO worth fixing this type and adding integrations for AccountRecoveryConfirmKey? integration={createMockResetPasswordWebIntegration() as Integration} > - {({ setLinkStatus, params }) => ( - + {({ setLinkStatus, linkModel }) => ( + )} ), diff --git a/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryResetPassword/index.tsx b/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryResetPassword/index.tsx index 8e56e7293d..3d5efa5628 100644 --- a/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryResetPassword/index.tsx +++ b/packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryResetPassword/index.tsx @@ -98,6 +98,17 @@ const AccountRecoveryResetPassword = ({ } if (linkStatus === 'expired') { + if (isOAuthIntegration(integration)) { + const service = integration.getService(); + const redirectUri = integration.getRedirectUri(); + return ( + + ); + } + return ( { + createLinkModel={() => { return CreateCompleteResetPasswordLink(); }} {...{ integration }} > - {({ setLinkStatus, params }) => ( + {({ setLinkStatus, linkModel }) => ( { render(, account); await enterPasswordAndSubmit(); expect(mockUseNavigateWithoutRerender).toHaveBeenCalledWith( - '/reset_password_verified?email=johndope%40example.com&emailToHashWith=&token=1111111111111111111111111111111111111111111111111111111111111111&code=11111111111111111111111111111111&uid=abc123', + '/reset_password_verified?email=johndope%40example.com&emailToHashWith=johndope%40example.com&token=1111111111111111111111111111111111111111111111111111111111111111&code=11111111111111111111111111111111&uid=abc123', { replace: true, } diff --git a/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/index.tsx b/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/index.tsx index 77ddeafa33..2d565bb957 100644 --- a/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/index.tsx +++ b/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/index.tsx @@ -55,7 +55,7 @@ import { export const viewName = 'complete-reset-password'; const CompleteResetPassword = ({ - params, + linkModel, setLinkStatus, integration, finishOAuthFlowHandler, @@ -110,7 +110,7 @@ const CompleteResetPassword = ({ const handleRecoveryKeyStatus = async () => { if (!location.state?.lostRecoveryKey) { - await checkForRecoveryKeyAndNavigate(params.email); + await checkForRecoveryKeyAndNavigate(linkModel.email); } renderCompleteResetPassword(); }; @@ -133,14 +133,14 @@ const CompleteResetPassword = ({ setShowLoadingSpinner(false); logPageViewEvent(viewName, REACT_ENTRYPOINT); }; - checkPasswordForgotToken(params.token); + checkPasswordForgotToken(linkModel.token); }, [ account, navigate, location.search, location.state?.lostRecoveryKey, - params.email, - params.token, + linkModel.email, + linkModel.token, setLinkStatus, setShowLoadingSpinner, ]); @@ -339,7 +339,7 @@ const CompleteResetPassword = ({ to correctly save the updated password. Without it, the password manager tries to save the old password as the username. */} - +
onSubmit({ newPassword, - token: params.token, - code: params.code, - email: params.email, - emailToHashWith: params.emailToHashWith, + token: linkModel.token, + code: linkModel.code, + email: linkModel.email, + emailToHashWith: linkModel.emailToHashWith, }) )} loading={false} onFocusMetricsEvent={`${viewName}.engage`} />
- + ); }; diff --git a/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/interfaces.ts b/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/interfaces.ts index 2695f47793..9b4618ebf9 100644 --- a/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/interfaces.ts +++ b/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/interfaces.ts @@ -29,7 +29,7 @@ export interface CompleteResetPasswordLocationState { export interface CompleteResetPasswordParams { email: string; - emailToHashWith: string; + emailToHashWith: string | undefined; code: string; token: string; } @@ -44,7 +44,7 @@ export type CompleteResetPasswordIntegration = | IntegrationSubsetType; export interface CompleteResetPasswordProps { - params: CompleteResetPasswordLink; + linkModel: CompleteResetPasswordLink; setLinkStatus: React.Dispatch>; integration: CompleteResetPasswordIntegration; finishOAuthFlowHandler: FinishOAuthFlowHandler; diff --git a/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/mocks.tsx b/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/mocks.tsx index d8bfc42887..4b64374257 100644 --- a/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/mocks.tsx +++ b/packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/mocks.tsx @@ -47,7 +47,7 @@ export const paramsWithMissingCode = { export const paramsWithMissingEmailToHashWith = { ...mockCompleteResetPasswordParams, - emailToHashWith: '', + emailToHashWith: undefined, }; export const paramsWithMissingToken = { @@ -68,7 +68,7 @@ export const Subject = ({ params = mockCompleteResetPasswordParams, }: { integrationType?: IntegrationType; - params?: Record; + params?: Record; }) => { const urlQueryData = mockUrlQueryData(params); @@ -90,15 +90,15 @@ export const Subject = ({ { + createLinkModel={() => { return new CompleteResetPasswordLink(urlQueryData); }} // TODO worth fixing this type? integration={completeResetPasswordIntegration as Integration} > - {({ setLinkStatus, params }) => ( + {({ setLinkStatus, linkModel }) => ( Promise.resolve({ redirect: 'someUri' }) diff --git a/yarn.lock b/yarn.lock index 04cf471cbc..9bc038c3aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -31191,6 +31191,7 @@ fsevents@~2.1.1: babel-plugin-named-exports-order: ^0.0.2 base32-decode: ^1.0.0 base32-encode: ^1.2.0 + class-validator: ^0.14.0 classnames: ^2.3.1 css-loader: ^3.6.0 eslint: ^7.32.0