diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 6965155c0ba..bfffcbb32ef 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1365,7 +1365,7 @@ "count": 2 }, "@typescript-eslint/prefer-nullish-coalescing": { - "count": 2 + "count": 1 } }, "packages/profile-sync-controller/src/controllers/authentication/__fixtures__/mockServices.ts": { @@ -1605,11 +1605,6 @@ "count": 1 } }, - "packages/profile-sync-controller/src/sdk/utils/validate-login-response.test.ts": { - "@typescript-eslint/explicit-function-return-type": { - "count": 1 - } - }, "packages/profile-sync-controller/src/shared/encryption/cache.ts": { "@typescript-eslint/explicit-function-return-type": { "count": 1 diff --git a/packages/profile-metrics-controller/CHANGELOG.md b/packages/profile-metrics-controller/CHANGELOG.md index c4116479689..7bb88f18809 100644 --- a/packages/profile-metrics-controller/CHANGELOG.md +++ b/packages/profile-metrics-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Move bearer token acquisition inside the retry loop in `ProfileMetricsService.submitMetrics` so each retry attempt fetches a fresh token instead of reusing a potentially stale one ([#8144](https://github.com/MetaMask/core/pull/8144)) + ## [3.0.2] ### Changed diff --git a/packages/profile-metrics-controller/src/ProfileMetricsService.ts b/packages/profile-metrics-controller/src/ProfileMetricsService.ts index af72ee6b341..38cb77117e6 100644 --- a/packages/profile-metrics-controller/src/ProfileMetricsService.ts +++ b/packages/profile-metrics-controller/src/ProfileMetricsService.ts @@ -198,11 +198,11 @@ export class ProfileMetricsService { * @returns The response from the API. */ async submitMetrics(data: ProfileMetricsSubmitMetricsRequest): Promise { - const authToken = await this.#messenger.call( - 'AuthenticationController:getBearerToken', - data.entropySourceId ?? undefined, - ); await this.#policy.execute(async () => { + const authToken = await this.#messenger.call( + 'AuthenticationController:getBearerToken', + data.entropySourceId ?? undefined, + ); const url = new URL(`${this.#baseURL}/profile/accounts`); const localResponse = await this.#fetch(url, { method: 'PUT', diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index f7d0979a7a0..e81fbf41bed 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -20,10 +20,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Add client-side JWT `exp` claim validation to prevent stale cached tokens from being returned ([#8144](https://github.com/MetaMask/core/pull/8144)) + - `validateLoginResponse` now decodes the JWT `exp` claim and rejects tokens that have actually expired, regardless of client-side TTL tracking (`obtainedAt`/`expiresIn`) + - Non-JWT access tokens are now rejected as invalid. In production this has no effect (access tokens are always JWTs from the OIDC server), but E2E test mocks that use raw identifier strings as access tokens must be updated. `getMockAuthAccessTokenResponse` now wraps identifiers in a JWT; consumers should use `getE2EIdentifierFromJwt` (newly exported) to extract the identifier from the bearer token in mock servers. - **BREAKING:** Standardize names of `AuthenticationController` and `UserStorageController` messenger action types ([#7976](https://github.com/MetaMask/core/pull/7976/)) - All existing types for messenger actions have been renamed so they end in `Action` (e.g. `AuthenticationControllerPerformSignIn` -> `AuthenticationControllerPerformSignInAction`). You will need to update imports appropriately. - This change only affects the types. The action type strings themselves have not changed, so you do not need to update the list of actions you pass when initializing `AuthenticationController` and `UserStorageController` messengers. +### Fixed + +- Fix `AuthenticationController` silently discarding tokens when `entropySourceId` is `undefined` ([#8144](https://github.com/MetaMask/core/pull/8144)) + - `getBearerToken`, `getSessionProfile`, and `getUserProfileLineage` now resolve `undefined` `entropySourceId` to the primary SRP entropy source ID via the message-signing snap before delegating to the auth SDK + - This also eliminates a login deduplication race condition where `getBearerToken(undefined)` and `getBearerToken("primary-srp-id")` would trigger two independent OIDC logins for the same identity +- Update `getUserProfileLineage` to accept an optional `entropySourceId` parameter ([#8144](https://github.com/MetaMask/core/pull/8144)) + ## [27.1.0] ### Changed diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts index 224bbb9c273..0269b2c8bfa 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.test.ts @@ -355,6 +355,95 @@ describe('AuthenticationController', () => { expect.any(Error), ); }); + + it('resolves undefined entropySourceId to primary and stores token', async () => { + const metametrics = createMockAuthMetaMetrics(); + const { messenger, mockSnapGetAllPublicKeys } = + createMockAuthenticationMessenger(); + arrangeAuthAPIs(); + + const controller = new AuthenticationController({ + messenger, + metametrics, + }); + + const result = await controller.getBearerToken(); + expect(result).toBe(MOCK_OATH_TOKEN_RESPONSE.access_token); + + expect(mockSnapGetAllPublicKeys).toHaveBeenCalled(); + expect(controller.state.isSignedIn).toBe(true); + expect( + controller.state.srpSessionData?.[MOCK_ENTROPY_SOURCE_IDS[0]], + ).toBeDefined(); + }); + + it('returns the same token for undefined and explicit primary entropySourceId', async () => { + const metametrics = createMockAuthMetaMetrics(); + const { messenger } = createMockAuthenticationMessenger(); + const originalState = mockSignedInState(); + const controller = new AuthenticationController({ + messenger, + state: originalState, + metametrics, + }); + + const resultUndefined = await controller.getBearerToken(); + const resultExplicit = await controller.getBearerToken( + MOCK_ENTROPY_SOURCE_IDS[0], + ); + expect(resultUndefined).toBe(resultExplicit); + }); + + it('caches the primary entropySourceId resolution across calls', async () => { + const metametrics = createMockAuthMetaMetrics(); + const { messenger, mockSnapGetAllPublicKeys } = + createMockAuthenticationMessenger(); + const originalState = mockSignedInState(); + const controller = new AuthenticationController({ + messenger, + state: originalState, + metametrics, + }); + + await controller.getBearerToken(); + await controller.getBearerToken(); + await controller.getBearerToken(); + + // getAllPublicKeys should only be called once despite multiple getBearerToken calls + expect(mockSnapGetAllPublicKeys).toHaveBeenCalledTimes(1); + }); + + it('throws when snap returns no entropy sources', async () => { + const metametrics = createMockAuthMetaMetrics(); + const { messenger, mockSnapGetAllPublicKeys } = + createMockAuthenticationMessenger(); + mockSnapGetAllPublicKeys.mockResolvedValue([]); + + const controller = new AuthenticationController({ + messenger, + metametrics, + }); + + await expect(controller.getBearerToken()).rejects.toThrow( + 'No entropy sources found from snap', + ); + }); + + it('throws when primary entropy source ID is undefined', async () => { + const metametrics = createMockAuthMetaMetrics(); + const { messenger, mockSnapGetAllPublicKeys } = + createMockAuthenticationMessenger(); + mockSnapGetAllPublicKeys.mockResolvedValue([[undefined, 'MOCK_KEY']]); + + const controller = new AuthenticationController({ + messenger, + metametrics, + }); + + await expect(controller.getBearerToken()).rejects.toThrow( + 'Primary entropy source ID is undefined', + ); + }); }); describe('getSessionProfile', () => { @@ -657,7 +746,7 @@ describe('metadata', () => { "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", }, "token": { - "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", "expiresIn": 1000, "obtainedAt": 0, }, @@ -669,7 +758,7 @@ describe('metadata', () => { "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", }, "token": { - "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", "expiresIn": 1000, "obtainedAt": 0, }, @@ -704,7 +793,7 @@ describe('metadata', () => { "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", }, "token": { - "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", "expiresIn": 1000, "obtainedAt": 0, }, @@ -716,7 +805,7 @@ describe('metadata', () => { "profileId": "f88227bd-b615-41a3-b0be-467dd781a4ad", }, "token": { - "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", + "accessToken": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c", "expiresIn": 1000, "obtainedAt": 0, }, diff --git a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts index 238b4a84bd2..7514f3c0118 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/AuthenticationController.ts @@ -141,6 +141,8 @@ export class AuthenticationController extends BaseController< #isUnlocked = false; + #cachedPrimaryEntropySourceId?: string; + readonly #keyringController = { setupLockedStateSubscriptions: () => { const { isUnlocked } = this.messenger.call('KeyringController:getState'); @@ -219,43 +221,33 @@ export class AuthenticationController extends BaseController< async #getLoginResponseFromState( entropySourceId?: string, ): Promise { - if (entropySourceId) { - if (!this.state.srpSessionData?.[entropySourceId]) { - return null; - } - return this.state.srpSessionData[entropySourceId]; - } - - const primarySrpLoginResponse = Object.values( - this.state.srpSessionData || {}, - )?.[0]; - - if (!primarySrpLoginResponse) { + const resolvedId = + entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + if (!this.state.srpSessionData?.[resolvedId]) { return null; } - - return primarySrpLoginResponse; + return this.state.srpSessionData[resolvedId]; } async #setLoginResponseToState( loginResponse: LoginResponse, entropySourceId?: string, ) { + const resolvedId = + entropySourceId ?? (await this.#getPrimaryEntropySourceId()); const metaMetricsId = await this.#metametrics.getMetaMetricsId(); this.update((state) => { - if (entropySourceId) { - state.isSignedIn = true; - if (!state.srpSessionData) { - state.srpSessionData = {}; - } - state.srpSessionData[entropySourceId] = { - ...loginResponse, - profile: { - ...loginResponse.profile, - metaMetricsId, - }, - }; + state.isSignedIn = true; + if (!state.srpSessionData) { + state.srpSessionData = {}; } + state.srpSessionData[resolvedId] = { + ...loginResponse, + profile: { + ...loginResponse.profile, + metaMetricsId, + }, + }; }); } @@ -265,6 +257,29 @@ export class AuthenticationController extends BaseController< } } + async #getPrimaryEntropySourceId(): Promise { + if (this.#cachedPrimaryEntropySourceId) { + return this.#cachedPrimaryEntropySourceId; + } + const allPublicKeys = await this.#snapGetAllPublicKeys(); + + if (allPublicKeys.length === 0) { + throw new Error( + '#getPrimaryEntropySourceId - No entropy sources found from snap', + ); + } + + const primaryId = allPublicKeys[0][0]; + if (!primaryId) { + throw new Error( + '#getPrimaryEntropySourceId - Primary entropy source ID is undefined', + ); + } + + this.#cachedPrimaryEntropySourceId = primaryId; + return this.#cachedPrimaryEntropySourceId; + } + public async performSignIn(): Promise { this.#assertIsUnlocked('performSignIn'); @@ -282,6 +297,7 @@ export class AuthenticationController extends BaseController< } public performSignOut(): void { + this.#cachedPrimaryEntropySourceId = undefined; this.update((state) => { state.isSignedIn = false; state.srpSessionData = undefined; @@ -297,7 +313,9 @@ export class AuthenticationController extends BaseController< public async getBearerToken(entropySourceId?: string): Promise { this.#assertIsUnlocked('getBearerToken'); - return await this.#auth.getAccessToken(entropySourceId); + const resolvedId = + entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + return await this.#auth.getAccessToken(resolvedId); } /** @@ -312,12 +330,18 @@ export class AuthenticationController extends BaseController< entropySourceId?: string, ): Promise { this.#assertIsUnlocked('getSessionProfile'); - return await this.#auth.getUserProfile(entropySourceId); + const resolvedId = + entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + return await this.#auth.getUserProfile(resolvedId); } - public async getUserProfileLineage(): Promise { + public async getUserProfileLineage( + entropySourceId?: string, + ): Promise { this.#assertIsUnlocked('getUserProfileLineage'); - return await this.#auth.getUserProfileLineage(); + const resolvedId = + entropySourceId ?? (await this.#getPrimaryEntropySourceId()); + return await this.#auth.getUserProfileLineage(resolvedId); } public isSignedIn(): boolean { diff --git a/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.test.ts b/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.test.ts new file mode 100644 index 00000000000..a0f78412f75 --- /dev/null +++ b/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.test.ts @@ -0,0 +1,77 @@ +import { + getMockAuthAccessTokenResponse, + getE2EIdentifierFromJwt, + MOCK_OATH_TOKEN_RESPONSE, +} from './mockResponses'; + +describe('getE2EIdentifierFromJwt()', () => { + it('extracts the sub claim from a valid mock JWT', () => { + const header = btoa(JSON.stringify({ alg: 'none', typ: 'JWT' })); + const payload = btoa( + JSON.stringify({ sub: 'MOCK_SRP_IDENTIFIER_1', exp: 4102444800 }), + ); + const jwt = `${header}.${payload}.mock`; + + expect(getE2EIdentifierFromJwt(jwt)).toBe('MOCK_SRP_IDENTIFIER_1'); + }); + + it('returns the raw token when it is not a JWT', () => { + expect(getE2EIdentifierFromJwt('MOCK_SRP_IDENTIFIER_1')).toBe( + 'MOCK_SRP_IDENTIFIER_1', + ); + }); + + it('returns the raw token for an empty string', () => { + expect(getE2EIdentifierFromJwt('')).toBe(''); + }); + + it('returns the raw token when JWT payload has no sub claim', () => { + const header = btoa(JSON.stringify({ alg: 'none' })); + const payload = btoa(JSON.stringify({ exp: 4102444800 })); + const jwt = `${header}.${payload}.mock`; + + expect(getE2EIdentifierFromJwt(jwt)).toBe(jwt); + }); + + it('returns the raw token when JWT payload has invalid base64', () => { + expect(getE2EIdentifierFromJwt('a.!!!invalid!!!.b')).toBe( + 'a.!!!invalid!!!.b', + ); + }); +}); + +describe('getMockAuthAccessTokenResponse()', () => { + it('wraps the e2eIdentifier in a JWT when assertion is present', () => { + const mock = getMockAuthAccessTokenResponse(); + const response = + // eslint-disable-next-line @typescript-eslint/naming-convention + (mock.response as (body?: string) => { access_token: string })( + 'assertion=MOCK_SRP_IDENTIFIER_1', + ); + + const token = response.access_token; + expect(token.split('.')).toHaveLength(3); + expect(getE2EIdentifierFromJwt(token)).toBe('MOCK_SRP_IDENTIFIER_1'); + }); + + it('returns the default OIDC access token when no assertion is present', () => { + const mock = getMockAuthAccessTokenResponse(); + const response = + // eslint-disable-next-line @typescript-eslint/naming-convention + (mock.response as (body?: string) => { access_token: string })(''); + + expect(response.access_token).toBe(MOCK_OATH_TOKEN_RESPONSE.access_token); + }); + + it('produces JWTs with a far-future exp claim', () => { + const mock = getMockAuthAccessTokenResponse(); + const response = + // eslint-disable-next-line @typescript-eslint/naming-convention + (mock.response as (body?: string) => { access_token: string })( + 'assertion=test-id', + ); + + const payload = JSON.parse(atob(response.access_token.split('.')[1])); + expect(payload.exp).toBe(4102444800); + }); +}); diff --git a/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.ts b/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.ts index 080f89d725a..81b91c44649 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/mocks/mockResponses.ts @@ -69,21 +69,63 @@ export const getMockAuthLoginResponse = () => { export const MOCK_OATH_TOKEN_RESPONSE = SDK_MOCK_OIDC_TOKEN_RESPONSE; +const MOCK_JWT_FAR_FUTURE_EXP = 4102444800; // 2100-01-01 + +/** + * Wraps a plain-text identifier in a minimal JWT so that client-side + * JWT validation (exp check) passes in E2E tests. The identifier is + * stored in the `sub` claim and can be extracted via {@link getE2EIdentifierFromJwt}. + * + * @param identifier - The plain-text E2E identifier to wrap. + * @returns A JWT-shaped string containing the identifier. + */ +const wrapInMockJwt = (identifier: string): string => { + const header = btoa(JSON.stringify({ alg: 'none', typ: 'JWT' })); + const payload = btoa( + JSON.stringify({ sub: identifier, exp: MOCK_JWT_FAR_FUTURE_EXP }), + ); + return `${header}.${payload}.mock`; +}; + +/** + * Extracts the E2E identifier (`sub` claim) from a mock JWT created + * by {@link wrapInMockJwt}. Falls back to returning the raw token if + * decoding fails (backward compatibility with raw-identifier headers). + * + * @param token - A bearer token string (JWT or raw identifier). + * @returns The decoded identifier, or the original token as-is. + */ +export const getE2EIdentifierFromJwt = (token: string): string => { + try { + const parts = token.split('.'); + if (parts.length === 3) { + const { sub } = JSON.parse(atob(parts[1])); + if (typeof sub === 'string' && sub.length > 0) { + return sub; + } + } + } catch { + // not a JWT — fall through + } + return token; +}; + export const getMockAuthAccessTokenResponse = () => { return { url: MOCK_OIDC_TOKEN_URL, requestMethod: 'POST', response: (requestJsonBody?: string) => { - // We end up setting the access token to the e2eIdentifier in the test environment - // This is then attached to every request's Authorization header - // and used to segregate data in the test environment + // We wrap the e2eIdentifier in a JWT so client-side JWT validation passes. + // The mock server extracts the identifier back via getE2EIdentifierFromJwt. const e2eIdentifier = new URLSearchParams(requestJsonBody).get( 'assertion', ); return { ...MOCK_OATH_TOKEN_RESPONSE, - access_token: e2eIdentifier ?? MOCK_OATH_TOKEN_RESPONSE.access_token, + access_token: e2eIdentifier + ? wrapInMockJwt(e2eIdentifier) + : MOCK_OATH_TOKEN_RESPONSE.access_token, }; }, } satisfies MockResponse; diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts index 550e20c84e9..7c12242a617 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts @@ -167,4 +167,46 @@ describe('SRPJwtBearerAuth rate limit handling', () => { // Should NOT apply any delay expect(mockDelay).not.toHaveBeenCalled(); }); + + it('triggers a fresh login when the cached JWT exp claim is in the past', async () => { + const expiredExp = Math.floor(Date.now() / 1000) - 3600; + const header = btoa(JSON.stringify({ alg: 'RS256', typ: 'JWT' })); + const payload = btoa(JSON.stringify({ exp: expiredExp })); + const expiredJwt = `${header}.${payload}.fake-sig`; + + const { auth, store } = createAuth(); + store.value = { + profile: MOCK_PROFILE, + token: { + accessToken: expiredJwt, + expiresIn: 86400, + obtainedAt: Date.now(), + }, + }; + + const token = await auth.getAccessToken(); + expect(token).toBe('access'); + expect(mockGetNonce).toHaveBeenCalledTimes(1); + }); + + it('returns the cached token when JWT exp claim is still in the future', async () => { + const futureExp = Math.floor(Date.now() / 1000) + 3600; + const header = btoa(JSON.stringify({ alg: 'RS256', typ: 'JWT' })); + const payload = btoa(JSON.stringify({ exp: futureExp })); + const validJwt = `${header}.${payload}.fake-sig`; + + const { auth, store } = createAuth(); + store.value = { + profile: MOCK_PROFILE, + token: { + accessToken: validJwt, + expiresIn: 86400, + obtainedAt: Date.now(), + }, + }; + + const token = await auth.getAccessToken(); + expect(token).toBe(validJwt); + expect(mockGetNonce).not.toHaveBeenCalled(); + }); }); 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 0dc39d461f0..183d90a5a47 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 @@ -77,7 +77,10 @@ export class SRPJwtBearerAuth implements IBaseAuth { readonly #metametrics?: MetaMetricsAuth; // Map to store ongoing login promises by entropySourceId - readonly #ongoingLogins = new Map>(); + readonly #ongoingLogins = new Map< + string | undefined, + Promise + >(); // Default cooldown when 429 has no Retry-After header readonly #cooldownDefaultMs: number; @@ -140,8 +143,10 @@ export class SRPJwtBearerAuth implements IBaseAuth { return await this.#options.signing.getIdentifier(entropySourceId); } - async getUserProfileLineage(): Promise { - const accessToken = await this.getAccessToken(); + async getUserProfileLineage( + entropySourceId?: string, + ): Promise { + const accessToken = await this.getAccessToken(entropySourceId); return await getUserProfileLineage(this.#config.env, accessToken); } @@ -234,11 +239,8 @@ export class SRPJwtBearerAuth implements IBaseAuth { } async #deferredLogin(entropySourceId?: string): Promise { - // Use a key that accounts for undefined entropySourceId - const loginKey = entropySourceId ?? '__default__'; - // Check if there's already an ongoing login for this entropySourceId - const existingLogin = this.#ongoingLogins.get(loginKey); + const existingLogin = this.#ongoingLogins.get(entropySourceId); if (existingLogin) { return existingLogin; } @@ -247,15 +249,14 @@ export class SRPJwtBearerAuth implements IBaseAuth { const loginPromise = this.#loginWithRetry(entropySourceId); // Store the promise in the map - this.#ongoingLogins.set(loginKey, loginPromise); + this.#ongoingLogins.set(entropySourceId, loginPromise); try { // Wait for the login to complete - const result = await loginPromise; - return result; + return await loginPromise; } finally { // Always clean up the ongoing login promise when done - this.#ongoingLogins.delete(loginKey); + this.#ongoingLogins.delete(entropySourceId); } } diff --git a/packages/profile-sync-controller/src/sdk/authentication.test.ts b/packages/profile-sync-controller/src/sdk/authentication.test.ts index fafdffa8012..c6110359316 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.test.ts @@ -690,7 +690,7 @@ describe('Authentication - rejects when calling unrelated methods', () => { function createMockStoredProfile(): LoginResponse { return { token: { - accessToken: MOCK_SRP_LOGIN_RESPONSE.token, + accessToken: MOCK_ACCESS_JWT, expiresIn: MOCK_SRP_LOGIN_RESPONSE.expires_in, obtainedAt: Date.now(), }, diff --git a/packages/profile-sync-controller/src/sdk/authentication.ts b/packages/profile-sync-controller/src/sdk/authentication.ts index d87e1798bfc..5f204e71b62 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.ts @@ -76,8 +76,10 @@ export class JwtBearerAuth implements SIWEInterface, SRPInterface { return await this.#sdk.getIdentifier(entropySourceId); } - async getUserProfileLineage(): Promise { - return await this.#sdk.getUserProfileLineage(); + async getUserProfileLineage( + entropySourceId?: string, + ): Promise { + return await this.#sdk.getUserProfileLineage(entropySourceId); } async signMessage( diff --git a/packages/profile-sync-controller/src/sdk/mocks/auth.ts b/packages/profile-sync-controller/src/sdk/mocks/auth.ts index 1fab71a1f77..3062aacdf1a 100644 --- a/packages/profile-sync-controller/src/sdk/mocks/auth.ts +++ b/packages/profile-sync-controller/src/sdk/mocks/auth.ts @@ -19,7 +19,7 @@ export const MOCK_JWT = 'eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImIwNzE2N2U2LWJjNWUtNDgyZC1hNjRhLWU1MjQ0MjY2MGU3NyJ9.eyJzdWIiOiI1MzE0ODc5YWM2NDU1OGI3OTQ5ZmI4NWIzMjg2ZjZjNjUwODAzYmFiMTY0Y2QyOWNmMmM3YzdmMjMzMWMwZTRlIiwiaWF0IjoxNzA2MTEzMDYyLCJleHAiOjE3NjkxODUwNjMsImlzcyI6ImF1dGgubWV0YW1hc2suaW8iLCJhdWQiOiJwb3J0Zm9saW8ubWV0YW1hc2suaW8ifQ.E5UL6oABNweS8t5a6IBTqTf7NLOJbrhJSmEcsr7kwLp4bGvcENJzACwnsHDkA6PlzfDV09ZhAGU_F3hlS0j-erbY0k0AFR-GAtyS7E9N02D8RgUDz5oDR65CKmzM8JilgFA8UvruJ6OJGogroaOSOqzRES_s8MjHpP47RJ9lXrUesajsbOudXbuksXWg5QmWip6LLvjwr8UUzcJzNQilyIhiEpo4WdzWM4R3VtTwr4rHnWEvtYnYCov1jmI2w3YQ48y0M-3Y9IOO0ov_vlITRrOnR7Y7fRUGLUFmU5msD8mNWRywjQFLHfJJ1yNP5aJ8TkuCK3sC6kcUH335IVvukQ'; export const MOCK_ACCESS_JWT = - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'; + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjQxMDI0NDQ4MDB9.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'; export const MOCK_NONCE_RESPONSE = { nonce: 'xGMm9SoihEKeAEfV', 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 index b38d5c2ab5b..64e2eeb6101 100644 --- 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 @@ -1,32 +1,91 @@ import { validateLoginResponse } from './validate-login-response'; import type { LoginResponse } from '../authentication'; -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: {} }); +/** + * Creates a minimal JWT string with the given payload claims. + * The signature is fake — only the payload matters for expiration checks. + * + * @param payload - The payload claims to include in the JWT. + * @returns A JWT string with the given payload claims. + */ +function createTestJwt(payload: Record): string { + const header = btoa(JSON.stringify({ alg: 'RS256', typ: 'JWT' })); + const body = btoa(JSON.stringify(payload)); + return `${header}.${body}.fake-signature`; +} + +function createValidLoginResponse( + accessToken: string = createTestJwt({ + exp: Math.floor(Date.now() / 1000) + 3600, + }), +): LoginResponse { + return { + profile: { identifierId: '', metaMetricsId: '', profileId: '' }, + token: { accessToken, expiresIn: 3600, obtainedAt: Date.now() }, + }; +} + +describe('validateLoginResponse()', () => { + it('returns true for a valid, non-expired LoginResponse', () => { + expect(validateLoginResponse(createValidLoginResponse())).toBe(true); + }); + + it('returns false for null/undefined/empty input', () => { + expect(validateLoginResponse(null)).toBe(false); + expect(validateLoginResponse(undefined)).toBe(false); + expect(validateLoginResponse({})).toBe(false); + }); + + it('returns false when token or profile is missing', () => { + expect(validateLoginResponse({ profile: {} })).toBe(false); + expect(validateLoginResponse({ token: {} })).toBe(false); + }); + + it('returns false when the JWT exp claim is in the past', () => { + const expiredJwt = createTestJwt({ + exp: Math.floor(Date.now() / 1000) - 3600, + }); + expect(validateLoginResponse(createValidLoginResponse(expiredJwt))).toBe( + false, + ); + }); + + it('returns false when the JWT has no exp claim', () => { + const noExpJwt = createTestJwt({ sub: 'user-123' }); + expect(validateLoginResponse(createValidLoginResponse(noExpJwt))).toBe( + false, + ); + }); + + it('returns false when the JWT exp claim is not a number', () => { + const badExpJwt = createTestJwt({ exp: 'not-a-number' }); + expect(validateLoginResponse(createValidLoginResponse(badExpJwt))).toBe( + false, + ); + }); + + it('returns false when the JWT exp claim is a float', () => { + const floatExpJwt = createTestJwt({ + exp: Math.floor(Date.now() / 1000) + 3600.5, + }); + expect(validateLoginResponse(createValidLoginResponse(floatExpJwt))).toBe( + false, + ); + }); + + it('returns false when the access token is malformed', () => { + expect(validateLoginResponse(createValidLoginResponse('not-a-jwt'))).toBe( + false, + ); + expect(validateLoginResponse(createValidLoginResponse(''))).toBe(false); + expect(validateLoginResponse(createValidLoginResponse('a.b'))).toBe(false); + }); + + it('returns false when the JWT payload has invalid base64', () => { + expect( + validateLoginResponse( + createValidLoginResponse('header.!!!invalid!!!.sig'), + ), + ).toBe(false); }); }); 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 index 3f48c2ddaed..c8ee45ca260 100644 --- a/packages/profile-sync-controller/src/sdk/utils/validate-login-response.ts +++ b/packages/profile-sync-controller/src/sdk/utils/validate-login-response.ts @@ -1,11 +1,15 @@ 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 + * Validates that the input is a well-formed, non-expired LoginResponse. + * + * Checks structural shape (token + profile objects exist) and verifies + * the JWT access token's `exp` claim is still in the future. This acts + * as a hard guard against stale cached tokens regardless of client-side + * TTL tracking (obtainedAt / expiresIn), which can be corrupted. * * @param input - unknown/untyped input - * @returns boolean if input is LoginResponse + * @returns boolean if input is a valid, non-expired LoginResponse */ export function validateLoginResponse(input: unknown): input is LoginResponse { const assumedInput = input as LoginResponse; @@ -18,5 +22,29 @@ export function validateLoginResponse(input: unknown): input is LoginResponse { return false; } + if (isJwtExpired(assumedInput.token.accessToken)) { + return false; + } + return true; } + +/** + * Checks whether a JWT has expired by decoding its `exp` claim. + * + * @param token - A JWT string. + * @returns true if the token is expired or cannot be decoded; false if still valid. + */ +function isJwtExpired(token: string): boolean { + try { + const parts = token.split('.'); + if (parts.length !== 3) { + return true; + } + const base64 = parts[1].replace(/-/gu, '+').replace(/_/gu, '/'); + const { exp } = JSON.parse(atob(base64)); + return !Number.isInteger(exp) || exp * 1000 <= Date.now(); + } catch { + return true; + } +}