From d9a386d507656ee850fd9436ec6e8b73fb1efbac Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 19 Jul 2024 12:54:55 +0100 Subject: [PATCH] refactor: add login response validation (#4541) ## Explanation Profile Syncing. Adds login response validation from the developer/users storage medium. The user could lie and pass in an invalid login response, so this makes sure that there is some validation around the response shape. NOTE - we can improve this in the future by adding a runtime parser/validator like Zod. But this is out of scope and we can add in a separate PR. ## References [NOTIFY-881](https://consensyssoftware.atlassian.net/browse/NOTIFY-881) ## Changelog ### `@metamask/profile-sync-controller` - **ADDED**: Validation around the LoginResponse shape. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../authentication-jwt-bearer/flow-siwe.ts | 3 +- .../sdk/authentication-jwt-bearer/flow-srp.ts | 3 +- .../sdk/utils/validate-login-response.test.ts | 32 +++++++++++++++++++ .../src/sdk/utils/validate-login-response.ts | 22 +++++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 packages/profile-sync-controller/src/sdk/utils/validate-login-response.test.ts create mode 100644 packages/profile-sync-controller/src/sdk/utils/validate-login-response.ts diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts index d40ac2ff44..56367d6e66 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts @@ -1,6 +1,7 @@ import { SiweMessage } from 'siwe'; import { ValidationError } from '../errors'; +import { validateLoginResponse } from '../utils/validate-login-response'; import { SIWE_LOGIN_URL, authenticate, @@ -83,7 +84,7 @@ export class SIWEJwtBearerAuth implements IBaseAuth { // convert expiresIn from seconds to milliseconds and use 90% of expiresIn async #getAuthSession(): Promise { const auth = await this.#options.storage.getLoginResponse(); - if (!auth) { + if (!validateLoginResponse(auth)) { return null; } diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts index 5771f66897..c8763aae0b 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts @@ -1,6 +1,7 @@ import { ValidationError } from '../errors'; import { getMetaMaskProviderEIP6963 } from '../utils/eip-6963-metamask-provider'; import { MESSAGE_SIGNING_SNAP } from '../utils/messaging-signing-snap-requests'; +import { validateLoginResponse } from '../utils/validate-login-response'; import { authenticate, authorizeOIDC, getNonce } from './services'; import type { AuthConfig, @@ -87,7 +88,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { // convert expiresIn from seconds to milliseconds and use 90% of expiresIn async #getAuthSession(): Promise { const auth = await this.#options.storage.getLoginResponse(); - if (!auth) { + if (!validateLoginResponse(auth)) { return null; } diff --git a/packages/profile-sync-controller/src/sdk/utils/validate-login-response.test.ts b/packages/profile-sync-controller/src/sdk/utils/validate-login-response.test.ts new file mode 100644 index 0000000000..48328db4df --- /dev/null +++ b/packages/profile-sync-controller/src/sdk/utils/validate-login-response.test.ts @@ -0,0 +1,32 @@ +import type { LoginResponse } from '../authentication'; +import { validateLoginResponse } from './validate-login-response'; + +describe('validateLoginResponse() tests', () => { + it('validates if a shape is of type LoginResponse', () => { + const input: LoginResponse = { + profile: { + identifierId: '', + metaMetricsId: '', + profileId: '', + }, + token: { + accessToken: '', + expiresIn: 3600, + obtainedAt: Date.now(), + }, + }; + + expect(validateLoginResponse(input)).toBe(true); + }); + + it('returns false if a shape is invalid', () => { + const assertInvalid = (input: unknown) => { + expect(validateLoginResponse(input)).toBe(false); + }; + + assertInvalid(null); + assertInvalid({}); + assertInvalid({ profile: {} }); + assertInvalid({ token: {} }); + }); +}); diff --git a/packages/profile-sync-controller/src/sdk/utils/validate-login-response.ts b/packages/profile-sync-controller/src/sdk/utils/validate-login-response.ts new file mode 100644 index 0000000000..3f48c2ddae --- /dev/null +++ b/packages/profile-sync-controller/src/sdk/utils/validate-login-response.ts @@ -0,0 +1,22 @@ +import type { LoginResponse } from '../authentication'; + +/** + * Validates Shape is LoginResponse + * NOTE - validation is pretty loose, we can improve this by using external libs like Zod for improved/tighter validation + * + * @param input - unknown/untyped input + * @returns boolean if input is LoginResponse + */ +export function validateLoginResponse(input: unknown): input is LoginResponse { + const assumedInput = input as LoginResponse; + + if (!assumedInput) { + return false; + } + + if (!assumedInput?.token || !assumedInput?.profile) { + return false; + } + + return true; +}