Skip to content

Commit

Permalink
refactor: add login response validation (#4541)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
Prithpal-Sooriya authored Jul 19, 2024
1 parent ed7a9db commit d9a386d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { SiweMessage } from 'siwe';

import { ValidationError } from '../errors';
import { validateLoginResponse } from '../utils/validate-login-response';
import {
SIWE_LOGIN_URL,
authenticate,
Expand Down Expand Up @@ -83,7 +84,7 @@ export class SIWEJwtBearerAuth implements IBaseAuth {
// convert expiresIn from seconds to milliseconds and use 90% of expiresIn
async #getAuthSession(): Promise<LoginResponse | null> {
const auth = await this.#options.storage.getLoginResponse();
if (!auth) {
if (!validateLoginResponse(auth)) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -87,7 +88,7 @@ export class SRPJwtBearerAuth implements IBaseAuth {
// convert expiresIn from seconds to milliseconds and use 90% of expiresIn
async #getAuthSession(): Promise<LoginResponse | null> {
const auth = await this.#options.storage.getLoginResponse();
if (!auth) {
if (!validateLoginResponse(auth)) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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: {} });
});
});
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit d9a386d

Please sign in to comment.