From 402f1e8a7e35b6656380e5de6befb697230be78f Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 29 Oct 2025 15:20:59 +0100 Subject: [PATCH 1/9] feat: (wip) add 429 handling and request throttling to authentication controller --- .../flow-srp.burst.test.ts | 174 ++++++++++++++++++ .../sdk/authentication-jwt-bearer/flow-srp.ts | 55 +++++- .../sdk/authentication-jwt-bearer/services.ts | 76 ++++++-- .../authentication-jwt-bearer/utils/time.ts | 11 ++ .../profile-sync-controller/src/sdk/errors.ts | 21 +++ 5 files changed, 316 insertions(+), 21 deletions(-) create mode 100644 packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts create mode 100644 packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts new file mode 100644 index 00000000000..a2d1412ab94 --- /dev/null +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts @@ -0,0 +1,174 @@ +import { SRPJwtBearerAuth } from './flow-srp'; +import type { AuthConfig } from './types'; +import { AuthType } from './types'; + +jest.setTimeout(15000); + +// Mock the time utilities to avoid real delays in tests +jest.mock('./utils/time', () => ({ + delay: jest.fn(), +})); + +// Import after mocking to get the mocked version +import * as timeUtils from './utils/time'; +const mockDelay = timeUtils.delay as jest.MockedFunction< + typeof timeUtils.delay +>; + +// Mock services +const mockGetNonce = jest.fn(); +const mockAuthenticate = jest.fn(); +const mockAuthorizeOIDC = jest.fn(); + +jest.mock('./services', () => ({ + authenticate: (...args: any[]) => mockAuthenticate(...args), + authorizeOIDC: (...args: any[]) => mockAuthorizeOIDC(...args), + getNonce: (...args: any[]) => mockGetNonce(...args), + getUserProfileLineage: jest.fn(), +})); + +describe('SRPJwtBearerAuth burst protection', () => { + const config: AuthConfig & { type: AuthType.SRP } = { + type: AuthType.SRP, + env: 'test' as any, + platform: 'extension' as any, + }; + + const createAuth = (overrides?: { + minIntervalMs?: number; + cooldownDefaultMs?: number; + maxRetries?: number; + }) => { + const store: any = { value: null as any }; + + const auth = new SRPJwtBearerAuth(config, { + storage: { + getLoginResponse: async () => store.value, + setLoginResponse: async (val) => { + store.value = val; + }, + }, + signing: { + getIdentifier: async () => 'identifier-1', + signMessage: async () => 'signature-1', + }, + }); + + return { auth, store }; + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockGetNonce.mockResolvedValue({ + nonce: 'nonce-1', + identifier: 'identifier-1', + expiresIn: 60, + }); + mockAuthenticate.mockResolvedValue({ + token: 'jwt-token', + expiresIn: 60, + profile: { + profileId: 'p1', + metametrics_id: 'm1', + identifier_id: 'i1', + } as any, + }); + mockAuthorizeOIDC.mockResolvedValue({ + accessToken: 'access', + expiresIn: 60, + obtainedAt: Date.now(), + }); + }); + + test('coalesces concurrent calls into a single login attempt', async () => { + const { auth } = createAuth(); + + const p1 = auth.getAccessToken(); + const p2 = auth.getAccessToken(); + const p3 = auth.getAccessToken(); + + const [t1, t2, t3] = await Promise.all([p1, p2, p3]); + + expect(t1).toBe('access'); + expect(t2).toBe('access'); + expect(t3).toBe('access'); + + // single sequence of service calls + expect(mockGetNonce).toHaveBeenCalledTimes(1); + expect(mockAuthenticate).toHaveBeenCalledTimes(1); + expect(mockAuthorizeOIDC).toHaveBeenCalledTimes(1); + }); + + test('throttles sequential login attempts within min interval', async () => { + const { auth, store } = createAuth(); + + await auth.getAccessToken(); + + // Clear the store to force a new login + store.value = null; + + await auth.getAccessToken(); + + expect(mockGetNonce).toHaveBeenCalledTimes(2); + expect(mockAuthenticate).toHaveBeenCalledTimes(2); + + // Verify that throttling delay was applied + expect(mockDelay).toHaveBeenCalled(); + }); + test('applies cooldown and retries once on 429 with Retry-After', async () => { + const { auth } = createAuth(); + + let first = true; + mockAuthenticate.mockImplementation(async () => { + if (first) { + first = false; + const e: any = new Error('rate limited'); + e.name = 'RateLimitedError'; + e.status = 429; + e.retryAfterMs = 20; + throw e; + } + return { + token: 'jwt-token', + expiresIn: 60, + profile: { + profileId: 'p1', + metametrics_id: 'm1', + identifier_id: 'i1', + } as any, + }; + }); + + const p1 = auth.getAccessToken(); + const p2 = auth.getAccessToken(); + + const [t1, t2] = await Promise.all([p1, p2]); + expect(t1).toBe('access'); + expect(t2).toBe('access'); + + // Should retry after rate limit error + expect(mockAuthenticate).toHaveBeenCalledTimes(2); + // Should apply cooldown delay + expect(mockDelay).toHaveBeenCalled(); + }); + + test('throws transient errors immediately without retry', async () => { + const { auth, store } = createAuth({ + maxRetries: 1, + minIntervalMs: 10, + }); + + // Force a login by clearing session + store.value = null; + + const transientError = new Error('transient network error'); + mockAuthenticate.mockRejectedValue(transientError); + + await expect(auth.getAccessToken()).rejects.toThrow( + 'transient network error', + ); + + // Should NOT retry on transient errors + expect(mockAuthenticate).toHaveBeenCalledTimes(1); + }); +}); 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 b8e7d68904b..f3d1f5bb8ef 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 @@ -17,7 +17,7 @@ import type { UserProfileLineage, } from './types'; import type { MetaMetricsAuth } from '../../shared/types/services'; -import { ValidationError } from '../errors'; +import { ValidationError, RateLimitedError } from '../errors'; import { getMetaMaskProviderEIP6963 } from '../utils/eip-6963-metamask-provider'; import { MESSAGE_SIGNING_SNAP, @@ -26,6 +26,7 @@ import { isSnapConnected, } from '../utils/messaging-signing-snap-requests'; import { validateLoginResponse } from '../utils/validate-login-response'; +import * as timeUtils from './utils/time'; type JwtBearerAuth_SRP_Options = { storage: AuthStorageOptions; @@ -71,6 +72,12 @@ export class SRPJwtBearerAuth implements IBaseAuth { // Map to store ongoing login promises by entropySourceId readonly #ongoingLogins = new Map>(); + // Per-entropySourceId throttling/cooldown schedule state + readonly #loginScheduleByKey = new Map(); // Maps loginKey -> nextAllowedAtTimestamp + readonly #minIntervalMs = 1000; // minimum spacing between attempts per entropy source + readonly #cooldownDefaultMs = 10000; // default cooldown when 429 has no Retry-After + readonly #maxRetries = 1; // total retries for rate-limit (429) errors + #customProvider?: Eip1193Provider; constructor( @@ -225,7 +232,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { } // Create a new login promise - const loginPromise = this.#performLogin(entropySourceId); + const loginPromise = this.#loginWithRetry(loginKey, entropySourceId); // Store the promise in the map this.#ongoingLogins.set(loginKey, loginPromise); @@ -240,6 +247,50 @@ export class SRPJwtBearerAuth implements IBaseAuth { } } + async #loginWithRetry( + loginKey: string, + entropySourceId?: string, + ): Promise { + for (let attempt = 0; ; attempt++) { + // Wait if we need to throttle + const nextAllowedAt = this.#loginScheduleByKey.get(loginKey) ?? 0; + const now = Date.now(); + const waitMs = Math.max(0, nextAllowedAt - now); + if (waitMs > 0) { + await timeUtils.delay(waitMs); + } + + // Update next allowed time + this.#loginScheduleByKey.set(loginKey, Date.now() + this.#minIntervalMs); + + try { + return await this.#performLogin(entropySourceId); + } catch (e) { + // Only retry on rate-limit (429) errors + if (!RateLimitedError.isRateLimitError(e)) { + throw e; + } + + // If we've exhausted attempts, rethrow + if (attempt >= this.#maxRetries) { + throw e; + } + + // Add cooldown based on Retry-After header or default + const additionalBackoffMs = + (e as RateLimitedError).retryAfterMs ?? this.#cooldownDefaultMs; + + // Add backoff to the already-set next allowed time + const currentNextAllowed = this.#loginScheduleByKey.get(loginKey) ?? 0; + this.#loginScheduleByKey.set( + loginKey, + currentNextAllowed + additionalBackoffMs, + ); + // Loop will continue to retry + } + } + } + #createSrpLoginRawMessage( nonce: string, publicKey: string, diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 3bc4c91265f..70643b63e07 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -13,8 +13,61 @@ import { PairError, SignInError, ValidationError, + RateLimitedError, } from '../errors'; +/** + * Parse Retry-After header into milliseconds if possible. + * Supports seconds or HTTP-date formats. + */ +function parseRetryAfter(retryAfterHeader: string | null): number | null { + if (!retryAfterHeader) { + return null; + } + const seconds = Number(retryAfterHeader); + if (!Number.isNaN(seconds)) { + return seconds * 1000; + } + const date = Date.parse(retryAfterHeader); + if (!Number.isNaN(date)) { + const diff = date - Date.now(); + return diff > 0 ? diff : 0; + } + return null; +} + +/** + * Handle HTTP error responses with rate limiting support + */ +async function handleErrorResponse( + response: Response, + errorPrefix?: string, +): Promise { + const status = response.status; + const retryAfterHeader = response.headers.get('Retry-After'); + const retryAfterMs = parseRetryAfter(retryAfterHeader); + + const responseBody = (await response.json()) as + | ErrorMessage + | { error_description: string; error: string }; + + const message = + 'message' in responseBody + ? responseBody.message + : responseBody.error_description; + const error = responseBody.error; + + if (status === 429) { + throw new RateLimitedError( + `HTTP 429: ${message} (error: ${error})`, + retryAfterMs ?? undefined, + ); + } + + const prefix = errorPrefix ? `${errorPrefix} ` : ''; + throw new Error(`${prefix}HTTP ${status} error: ${message}, error: ${error}`); +} + export const NONCE_URL = (env: Env) => `${getEnvUrls(env).authApiUrl}/api/v2/nonce`; @@ -91,10 +144,7 @@ export async function pairIdentifiers( }); if (!response.ok) { - const responseBody = (await response.json()) as ErrorMessage; - throw new Error( - `HTTP error message: ${responseBody.message}, error: ${responseBody.error}`, - ); + await handleErrorResponse(response); } } catch (e) { /* istanbul ignore next */ @@ -118,10 +168,7 @@ export async function getNonce(id: string, env: Env): Promise { try { const nonceResponse = await fetch(nonceUrl.toString()); if (!nonceResponse.ok) { - const responseBody = (await nonceResponse.json()) as ErrorMessage; - throw new Error( - `HTTP error message: ${responseBody.message}, error: ${responseBody.error}`, - ); + await handleErrorResponse(nonceResponse); } const nonceJson = await nonceResponse.json(); @@ -169,13 +216,7 @@ export async function authorizeOIDC( }); if (!response.ok) { - const responseBody = (await response.json()) as { - error_description: string; - error: string; - }; - throw new Error( - `HTTP error: ${responseBody.error_description}, error code: ${responseBody.error}`, - ); + await handleErrorResponse(response); } const accessTokenResponse = await response.json(); @@ -237,10 +278,7 @@ export async function authenticate( }); if (!response.ok) { - const responseBody = (await response.json()) as ErrorMessage; - throw new Error( - `${authType} login HTTP error: ${responseBody.message}, error code: ${responseBody.error}`, - ); + await handleErrorResponse(response, `${authType} login`); } const loginResponse = await response.json(); diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts new file mode 100644 index 00000000000..fa836de4155 --- /dev/null +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts @@ -0,0 +1,11 @@ +/** + * Delays execution for the specified number of milliseconds. + * @param ms - Number of milliseconds to delay + * @returns Promise that resolves after the delay + */ +export async function delay(ms: number): Promise { + if (ms <= 0) { + return; + } + await new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/packages/profile-sync-controller/src/sdk/errors.ts b/packages/profile-sync-controller/src/sdk/errors.ts index 40ce5bc7788..115f79cee43 100644 --- a/packages/profile-sync-controller/src/sdk/errors.ts +++ b/packages/profile-sync-controller/src/sdk/errors.ts @@ -46,3 +46,24 @@ export class NotFoundError extends Error { this.name = 'NotFoundError'; } } + +export class RateLimitedError extends Error { + readonly status = 429; + readonly retryAfterMs?: number; + + constructor(message: string, retryAfterMs?: number) { + super(message); + this.name = 'RateLimitedError'; + this.retryAfterMs = retryAfterMs; + } + + /** + * Check if an unknown error is a rate limit error (429 status) + */ + static isRateLimitError(e: unknown): e is RateLimitedError { + return ( + e instanceof RateLimitedError || + (typeof e === 'object' && e !== null && (e as any)?.status === 429) + ); + } +} From 182fffe171b2204ccb3957e7e038cf4d2635029e Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 13:24:18 +0100 Subject: [PATCH 2/9] fix: only manage 429 --- ...low-srp.burst.test.ts => flow-srp.test.ts} | 123 +++++++++--------- .../sdk/authentication-jwt-bearer/flow-srp.ts | 54 ++++---- 2 files changed, 87 insertions(+), 90 deletions(-) rename packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/{flow-srp.burst.test.ts => flow-srp.test.ts} (63%) diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts similarity index 63% rename from packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts rename to packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts index a2d1412ab94..06f04c0c58a 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.burst.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts @@ -27,18 +27,50 @@ jest.mock('./services', () => ({ getUserProfileLineage: jest.fn(), })); -describe('SRPJwtBearerAuth burst protection', () => { +describe('SRPJwtBearerAuth rate limit handling', () => { const config: AuthConfig & { type: AuthType.SRP } = { type: AuthType.SRP, env: 'test' as any, platform: 'extension' as any, }; - const createAuth = (overrides?: { - minIntervalMs?: number; - cooldownDefaultMs?: number; - maxRetries?: number; - }) => { + // Mock data constants + const MOCK_PROFILE = { + profileId: 'p1', + metametrics_id: 'm1', + identifier_id: 'i1', + } as any; + + const MOCK_NONCE_RESPONSE = { + nonce: 'nonce-1', + identifier: 'identifier-1', + expiresIn: 60, + }; + + const MOCK_AUTH_RESPONSE = { + token: 'jwt-token', + expiresIn: 60, + profile: MOCK_PROFILE, + }; + + const MOCK_OIDC_RESPONSE = { + accessToken: 'access', + expiresIn: 60, + obtainedAt: Date.now(), + }; + + // Helper to create a rate limit error + const createRateLimitError = (retryAfterMs?: number) => { + const error: any = new Error('rate limited'); + error.name = 'RateLimitedError'; + error.status = 429; + if (retryAfterMs !== undefined) { + error.retryAfterMs = retryAfterMs; + } + return error; + }; + + const createAuth = (overrides?: { cooldownDefaultMs?: number }) => { const store: any = { value: null as any }; const auth = new SRPJwtBearerAuth(config, { @@ -52,6 +84,7 @@ describe('SRPJwtBearerAuth burst protection', () => { getIdentifier: async () => 'identifier-1', signMessage: async () => 'signature-1', }, + rateLimitRetry: overrides, }); return { auth, store }; @@ -59,25 +92,9 @@ describe('SRPJwtBearerAuth burst protection', () => { beforeEach(() => { jest.clearAllMocks(); - mockGetNonce.mockResolvedValue({ - nonce: 'nonce-1', - identifier: 'identifier-1', - expiresIn: 60, - }); - mockAuthenticate.mockResolvedValue({ - token: 'jwt-token', - expiresIn: 60, - profile: { - profileId: 'p1', - metametrics_id: 'm1', - identifier_id: 'i1', - } as any, - }); - mockAuthorizeOIDC.mockResolvedValue({ - accessToken: 'access', - expiresIn: 60, - obtainedAt: Date.now(), - }); + mockGetNonce.mockResolvedValue(MOCK_NONCE_RESPONSE); + mockAuthenticate.mockResolvedValue(MOCK_AUTH_RESPONSE); + mockAuthorizeOIDC.mockResolvedValue(MOCK_OIDC_RESPONSE); }); test('coalesces concurrent calls into a single login attempt', async () => { @@ -99,44 +116,16 @@ describe('SRPJwtBearerAuth burst protection', () => { expect(mockAuthorizeOIDC).toHaveBeenCalledTimes(1); }); - test('throttles sequential login attempts within min interval', async () => { - const { auth, store } = createAuth(); - - await auth.getAccessToken(); - - // Clear the store to force a new login - store.value = null; - - await auth.getAccessToken(); - - expect(mockGetNonce).toHaveBeenCalledTimes(2); - expect(mockAuthenticate).toHaveBeenCalledTimes(2); - - // Verify that throttling delay was applied - expect(mockDelay).toHaveBeenCalled(); - }); test('applies cooldown and retries once on 429 with Retry-After', async () => { - const { auth } = createAuth(); + const { auth } = createAuth({ cooldownDefaultMs: 20 }); let first = true; mockAuthenticate.mockImplementation(async () => { if (first) { first = false; - const e: any = new Error('rate limited'); - e.name = 'RateLimitedError'; - e.status = 429; - e.retryAfterMs = 20; - throw e; + throw createRateLimitError(20); } - return { - token: 'jwt-token', - expiresIn: 60, - profile: { - profileId: 'p1', - metametrics_id: 'm1', - identifier_id: 'i1', - } as any, - }; + return MOCK_AUTH_RESPONSE; }); const p1 = auth.getAccessToken(); @@ -149,14 +138,24 @@ describe('SRPJwtBearerAuth burst protection', () => { // Should retry after rate limit error expect(mockAuthenticate).toHaveBeenCalledTimes(2); // Should apply cooldown delay - expect(mockDelay).toHaveBeenCalled(); + expect(mockDelay).toHaveBeenCalledWith(20); + }); + + test('throws 429 after exhausting one retry', async () => { + const { auth } = createAuth({ cooldownDefaultMs: 20 }); + + mockAuthenticate.mockRejectedValue(createRateLimitError(20)); + + await expect(auth.getAccessToken()).rejects.toThrow('rate limited'); + + // Should attempt initial + one retry = 2 attempts + expect(mockAuthenticate).toHaveBeenCalledTimes(2); + // Should apply cooldown delay once + expect(mockDelay).toHaveBeenCalledTimes(1); }); test('throws transient errors immediately without retry', async () => { - const { auth, store } = createAuth({ - maxRetries: 1, - minIntervalMs: 10, - }); + const { auth, store } = createAuth(); // Force a login by clearing session store.value = null; @@ -170,5 +169,7 @@ describe('SRPJwtBearerAuth burst protection', () => { // Should NOT retry on transient errors expect(mockAuthenticate).toHaveBeenCalledTimes(1); + // Should NOT apply any delay + expect(mockDelay).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 f3d1f5bb8ef..a55321bc991 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 @@ -31,6 +31,9 @@ import * as timeUtils from './utils/time'; type JwtBearerAuth_SRP_Options = { storage: AuthStorageOptions; signing?: AuthSigningOptions; + rateLimitRetry?: { + cooldownDefaultMs?: number; // default cooldown when 429 has no Retry-After + }; }; const getDefaultEIP6963Provider = async () => { @@ -65,18 +68,18 @@ const getDefaultEIP6963SigningOptions = ( export class SRPJwtBearerAuth implements IBaseAuth { readonly #config: AuthConfig; - readonly #options: Required; + readonly #options: { + storage: AuthStorageOptions; + signing: AuthSigningOptions; + }; readonly #metametrics?: MetaMetricsAuth; // Map to store ongoing login promises by entropySourceId readonly #ongoingLogins = new Map>(); - // Per-entropySourceId throttling/cooldown schedule state - readonly #loginScheduleByKey = new Map(); // Maps loginKey -> nextAllowedAtTimestamp - readonly #minIntervalMs = 1000; // minimum spacing between attempts per entropy source - readonly #cooldownDefaultMs = 10000; // default cooldown when 429 has no Retry-After - readonly #maxRetries = 1; // total retries for rate-limit (429) errors + // Default cooldown when 429 has no Retry-After header + #cooldownDefaultMs = 10000; #customProvider?: Eip1193Provider; @@ -96,6 +99,11 @@ export class SRPJwtBearerAuth implements IBaseAuth { getDefaultEIP6963SigningOptions(this.#customProvider), }; this.#metametrics = options.metametrics; + + // Apply rate limit retry config if provided + if (options.rateLimitRetry?.cooldownDefaultMs !== undefined) { + this.#cooldownDefaultMs = options.rateLimitRetry.cooldownDefaultMs; + } } setCustomProvider(provider: Eip1193Provider) { @@ -251,18 +259,8 @@ export class SRPJwtBearerAuth implements IBaseAuth { loginKey: string, entropySourceId?: string, ): Promise { - for (let attempt = 0; ; attempt++) { - // Wait if we need to throttle - const nextAllowedAt = this.#loginScheduleByKey.get(loginKey) ?? 0; - const now = Date.now(); - const waitMs = Math.max(0, nextAllowedAt - now); - if (waitMs > 0) { - await timeUtils.delay(waitMs); - } - - // Update next allowed time - this.#loginScheduleByKey.set(loginKey, Date.now() + this.#minIntervalMs); - + // Allow max 2 attempts: initial + one retry on 429 + for (let attempt = 0; attempt < 2; attempt += 1) { try { return await this.#performLogin(entropySourceId); } catch (e) { @@ -271,24 +269,22 @@ export class SRPJwtBearerAuth implements IBaseAuth { throw e; } - // If we've exhausted attempts, rethrow - if (attempt >= this.#maxRetries) { + // If we've exhausted attempts (>= 1 retry), rethrow + if (attempt >= 1) { throw e; } - // Add cooldown based on Retry-After header or default - const additionalBackoffMs = + // Wait for Retry-After or default cooldown + const waitMs = (e as RateLimitedError).retryAfterMs ?? this.#cooldownDefaultMs; + await timeUtils.delay(waitMs); - // Add backoff to the already-set next allowed time - const currentNextAllowed = this.#loginScheduleByKey.get(loginKey) ?? 0; - this.#loginScheduleByKey.set( - loginKey, - currentNextAllowed + additionalBackoffMs, - ); - // Loop will continue to retry + // Loop continues to retry } } + + // Should never reach here due to loop logic, but TypeScript needs a return + throw new Error('Unexpected: login loop exhausted without result'); } #createSrpLoginRawMessage( From 86706ff52300fedf5c94ca8767c5cd79a1d87574 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 13:35:07 +0100 Subject: [PATCH 3/9] fix: lint issues --- .../flow-srp.test.ts | 23 ++++++++++++------- .../sdk/authentication-jwt-bearer/flow-srp.ts | 16 +++++-------- .../sdk/authentication-jwt-bearer/services.ts | 14 ++++++++--- .../authentication-jwt-bearer/utils/time.ts | 1 + .../profile-sync-controller/src/sdk/errors.ts | 11 +++++++-- 5 files changed, 42 insertions(+), 23 deletions(-) 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 06f04c0c58a..118873b6071 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 @@ -1,6 +1,6 @@ import { SRPJwtBearerAuth } from './flow-srp'; -import type { AuthConfig } from './types'; -import { AuthType } from './types'; +import { AuthType, type AuthConfig } from './types'; +import * as timeUtils from './utils/time'; jest.setTimeout(15000); @@ -9,8 +9,6 @@ jest.mock('./utils/time', () => ({ delay: jest.fn(), })); -// Import after mocking to get the mocked version -import * as timeUtils from './utils/time'; const mockDelay = timeUtils.delay as jest.MockedFunction< typeof timeUtils.delay >; @@ -21,8 +19,11 @@ const mockAuthenticate = jest.fn(); const mockAuthorizeOIDC = jest.fn(); jest.mock('./services', () => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any authenticate: (...args: any[]) => mockAuthenticate(...args), + // eslint-disable-next-line @typescript-eslint/no-explicit-any authorizeOIDC: (...args: any[]) => mockAuthorizeOIDC(...args), + // eslint-disable-next-line @typescript-eslint/no-explicit-any getNonce: (...args: any[]) => mockGetNonce(...args), getUserProfileLineage: jest.fn(), })); @@ -30,7 +31,9 @@ jest.mock('./services', () => ({ describe('SRPJwtBearerAuth rate limit handling', () => { const config: AuthConfig & { type: AuthType.SRP } = { type: AuthType.SRP, + // eslint-disable-next-line @typescript-eslint/no-explicit-any env: 'test' as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any platform: 'extension' as any, }; @@ -39,6 +42,7 @@ describe('SRPJwtBearerAuth rate limit handling', () => { profileId: 'p1', metametrics_id: 'm1', identifier_id: 'i1', + // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any; const MOCK_NONCE_RESPONSE = { @@ -61,6 +65,7 @@ describe('SRPJwtBearerAuth rate limit handling', () => { // Helper to create a rate limit error const createRateLimitError = (retryAfterMs?: number) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any const error: any = new Error('rate limited'); error.name = 'RateLimitedError'; error.status = 429; @@ -71,6 +76,7 @@ describe('SRPJwtBearerAuth rate limit handling', () => { }; const createAuth = (overrides?: { cooldownDefaultMs?: number }) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any const store: any = { value: null as any }; const auth = new SRPJwtBearerAuth(config, { @@ -97,7 +103,7 @@ describe('SRPJwtBearerAuth rate limit handling', () => { mockAuthorizeOIDC.mockResolvedValue(MOCK_OIDC_RESPONSE); }); - test('coalesces concurrent calls into a single login attempt', async () => { + it('coalesces concurrent calls into a single login attempt', async () => { const { auth } = createAuth(); const p1 = auth.getAccessToken(); @@ -116,11 +122,12 @@ describe('SRPJwtBearerAuth rate limit handling', () => { expect(mockAuthorizeOIDC).toHaveBeenCalledTimes(1); }); - test('applies cooldown and retries once on 429 with Retry-After', async () => { + it('applies cooldown and retries once on 429 with Retry-After', async () => { const { auth } = createAuth({ cooldownDefaultMs: 20 }); let first = true; mockAuthenticate.mockImplementation(async () => { + // eslint-disable-next-line jest/no-conditional-in-test if (first) { first = false; throw createRateLimitError(20); @@ -141,7 +148,7 @@ describe('SRPJwtBearerAuth rate limit handling', () => { expect(mockDelay).toHaveBeenCalledWith(20); }); - test('throws 429 after exhausting one retry', async () => { + it('throws 429 after exhausting one retry', async () => { const { auth } = createAuth({ cooldownDefaultMs: 20 }); mockAuthenticate.mockRejectedValue(createRateLimitError(20)); @@ -154,7 +161,7 @@ describe('SRPJwtBearerAuth rate limit handling', () => { expect(mockDelay).toHaveBeenCalledTimes(1); }); - test('throws transient errors immediately without retry', async () => { + it('throws transient errors immediately without retry', async () => { const { auth, store } = createAuth(); // Force a login by clearing session 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 a55321bc991..36052e5bd63 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 @@ -16,6 +16,7 @@ import type { UserProfile, UserProfileLineage, } from './types'; +import * as timeUtils from './utils/time'; import type { MetaMetricsAuth } from '../../shared/types/services'; import { ValidationError, RateLimitedError } from '../errors'; import { getMetaMaskProviderEIP6963 } from '../utils/eip-6963-metamask-provider'; @@ -26,7 +27,6 @@ import { isSnapConnected, } from '../utils/messaging-signing-snap-requests'; import { validateLoginResponse } from '../utils/validate-login-response'; -import * as timeUtils from './utils/time'; type JwtBearerAuth_SRP_Options = { storage: AuthStorageOptions; @@ -79,7 +79,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { readonly #ongoingLogins = new Map>(); // Default cooldown when 429 has no Retry-After header - #cooldownDefaultMs = 10000; + readonly #cooldownDefaultMs: number; #customProvider?: Eip1193Provider; @@ -101,9 +101,8 @@ export class SRPJwtBearerAuth implements IBaseAuth { this.#metametrics = options.metametrics; // Apply rate limit retry config if provided - if (options.rateLimitRetry?.cooldownDefaultMs !== undefined) { - this.#cooldownDefaultMs = options.rateLimitRetry.cooldownDefaultMs; - } + this.#cooldownDefaultMs = + options.rateLimitRetry?.cooldownDefaultMs ?? 10000; } setCustomProvider(provider: Eip1193Provider) { @@ -240,7 +239,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { } // Create a new login promise - const loginPromise = this.#loginWithRetry(loginKey, entropySourceId); + const loginPromise = this.#loginWithRetry(entropySourceId); // Store the promise in the map this.#ongoingLogins.set(loginKey, loginPromise); @@ -255,10 +254,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { } } - async #loginWithRetry( - loginKey: string, - entropySourceId?: string, - ): Promise { + async #loginWithRetry(entropySourceId?: string): Promise { // Allow max 2 attempts: initial + one retry on 429 for (let attempt = 0; attempt < 2; attempt += 1) { try { diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 70643b63e07..45e67670fcd 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -19,6 +19,9 @@ import { /** * Parse Retry-After header into milliseconds if possible. * Supports seconds or HTTP-date formats. + * + * @param retryAfterHeader - The Retry-After header value (seconds or HTTP-date) + * @returns The retry delay in milliseconds, or null if parsing fails */ function parseRetryAfter(retryAfterHeader: string | null): number | null { if (!retryAfterHeader) { @@ -37,13 +40,18 @@ function parseRetryAfter(retryAfterHeader: string | null): number | null { } /** - * Handle HTTP error responses with rate limiting support + * Handle HTTP error responses with rate limiting support. + * + * @param response - The HTTP response object + * @param errorPrefix - Optional prefix for the error message + * @throws RateLimitedError for 429 responses + * @throws Error for other error responses */ async function handleErrorResponse( response: Response, errorPrefix?: string, ): Promise { - const status = response.status; + const { status } = response; const retryAfterHeader = response.headers.get('Retry-After'); const retryAfterMs = parseRetryAfter(retryAfterHeader); @@ -55,7 +63,7 @@ async function handleErrorResponse( 'message' in responseBody ? responseBody.message : responseBody.error_description; - const error = responseBody.error; + const { error } = responseBody; if (status === 429) { throw new RateLimitedError( diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts index fa836de4155..80d95f84347 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/utils/time.ts @@ -1,5 +1,6 @@ /** * Delays execution for the specified number of milliseconds. + * * @param ms - Number of milliseconds to delay * @returns Promise that resolves after the delay */ diff --git a/packages/profile-sync-controller/src/sdk/errors.ts b/packages/profile-sync-controller/src/sdk/errors.ts index 115f79cee43..0b7dbe35666 100644 --- a/packages/profile-sync-controller/src/sdk/errors.ts +++ b/packages/profile-sync-controller/src/sdk/errors.ts @@ -49,6 +49,7 @@ export class NotFoundError extends Error { export class RateLimitedError extends Error { readonly status = 429; + readonly retryAfterMs?: number; constructor(message: string, retryAfterMs?: number) { @@ -58,12 +59,18 @@ export class RateLimitedError extends Error { } /** - * Check if an unknown error is a rate limit error (429 status) + * Check if an unknown error is a rate limit error (429 status). + * + * @param e - The error to check + * @returns True if the error is a rate limit error */ static isRateLimitError(e: unknown): e is RateLimitedError { return ( e instanceof RateLimitedError || - (typeof e === 'object' && e !== null && (e as any)?.status === 429) + (typeof e === 'object' && + e !== null && + 'status' in e && + (e as { status: number }).status === 429) ); } } From 269027eb6e4b798e004ea300c8e79babc6766aa1 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 13:39:44 +0100 Subject: [PATCH 4/9] fix: update changelog --- packages/profile-sync-controller/CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 1abd1944990..94b7e651278 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Add rate limit (429) handling with automatic retry in authentication flow ([#6993](https://github.com/MetaMask/core/pull/6993)) + - Update authentication services to throw `RateLimitedError` when encountering 429 responses. + - Improve Authentication errors by adding the HTTP code in Error messages. + - Add rate limit retry logic to `SRPJwtBearerAuth` with configurable cooldown via `rateLimitRetry.cooldownDefaultMs` option (defaults to 10 seconds). + - Non-429 errors are thrown immediately without retry, delegating retry logic to consumers. + ## [26.0.0] ### Changed From cb71a050c312d2333a77815105c996b8353d2e7a Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 13:54:33 +0100 Subject: [PATCH 5/9] fix: re-throw bug --- .../sdk/authentication-jwt-bearer/services.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 45e67670fcd..3eb1c8816fb 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -155,6 +155,10 @@ export async function pairIdentifiers( await handleErrorResponse(response); } } catch (e) { + // Re-throw RateLimitedError to preserve 429 status and retry metadata + if (RateLimitedError.isRateLimitError(e)) { + throw e; + } /* istanbul ignore next */ const errorMessage = e instanceof Error ? e.message : JSON.stringify(e ?? ''); @@ -186,6 +190,10 @@ export async function getNonce(id: string, env: Env): Promise { expiresIn: nonceJson.expires_in, }; } catch (e) { + // Re-throw RateLimitedError to preserve 429 status and retry metadata + if (RateLimitedError.isRateLimitError(e)) { + throw e; + } /* istanbul ignore next */ const errorMessage = e instanceof Error ? e.message : JSON.stringify(e ?? ''); @@ -234,6 +242,10 @@ export async function authorizeOIDC( obtainedAt: Date.now(), }; } catch (e) { + // Re-throw RateLimitedError to preserve 429 status and retry metadata + if (RateLimitedError.isRateLimitError(e)) { + throw e; + } /* istanbul ignore next */ const errorMessage = e instanceof Error ? e.message : JSON.stringify(e ?? ''); @@ -300,6 +312,10 @@ export async function authenticate( }, }; } catch (e) { + // Re-throw RateLimitedError to preserve 429 status and retry metadata + if (RateLimitedError.isRateLimitError(e)) { + throw e; + } /* istanbul ignore next */ const errorMessage = e instanceof Error ? e.message : JSON.stringify(e ?? ''); @@ -339,6 +355,10 @@ export async function getUserProfileLineage( return profileJson; } catch (e) { + // Re-throw RateLimitedError to preserve 429 status and retry metadata + if (RateLimitedError.isRateLimitError(e)) { + throw e; + } /* istanbul ignore next */ const errorMessage = e instanceof Error ? e.message : JSON.stringify(e ?? ''); From 77cba429ba2f3bafba86efe935fbab929b0321e2 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 14:02:21 +0100 Subject: [PATCH 6/9] fix: update getUserProfileLineage to use RL management --- .../src/sdk/authentication-jwt-bearer/services.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 3eb1c8816fb..b4bb9204789 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -345,10 +345,7 @@ export async function getUserProfileLineage( }); if (!response.ok) { - const responseBody = (await response.json()) as ErrorMessage; - throw new Error( - `HTTP error message: ${responseBody.message}, error: ${responseBody.error}`, - ); + await handleErrorResponse(response, 'profile lineage'); } const profileJson: UserProfileLineage = await response.json(); From 2e99d4666888e5b95e2595d507b42e8d539e5187 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 14:07:44 +0100 Subject: [PATCH 7/9] fix: past-date bug --- .../src/sdk/authentication-jwt-bearer/services.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index b4bb9204789..6bf123824c8 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -34,7 +34,7 @@ function parseRetryAfter(retryAfterHeader: string | null): number | null { const date = Date.parse(retryAfterHeader); if (!Number.isNaN(date)) { const diff = date - Date.now(); - return diff > 0 ? diff : 0; + return diff > 0 ? diff : null; } return null; } From ee982ebe89179f661ef9661e588822cf5f6b81bc Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Fri, 7 Nov 2025 15:24:30 +0100 Subject: [PATCH 8/9] fix: better types in test file --- .../flow-srp.test.ts | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 deletions(-) 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 118873b6071..3ba99e17847 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 @@ -1,6 +1,13 @@ import { SRPJwtBearerAuth } from './flow-srp'; -import { AuthType, type AuthConfig } from './types'; +import { + AuthType, + type AuthConfig, + type LoginResponse, + type UserProfile, +} from './types'; import * as timeUtils from './utils/time'; +import { Env, Platform } from '../../shared/env'; +import { RateLimitedError } from '../errors'; jest.setTimeout(15000); @@ -19,31 +26,25 @@ const mockAuthenticate = jest.fn(); const mockAuthorizeOIDC = jest.fn(); jest.mock('./services', () => ({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - authenticate: (...args: any[]) => mockAuthenticate(...args), - // eslint-disable-next-line @typescript-eslint/no-explicit-any - authorizeOIDC: (...args: any[]) => mockAuthorizeOIDC(...args), - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getNonce: (...args: any[]) => mockGetNonce(...args), + authenticate: (...args: unknown[]) => mockAuthenticate(...args), + authorizeOIDC: (...args: unknown[]) => mockAuthorizeOIDC(...args), + getNonce: (...args: unknown[]) => mockGetNonce(...args), getUserProfileLineage: jest.fn(), })); describe('SRPJwtBearerAuth rate limit handling', () => { const config: AuthConfig & { type: AuthType.SRP } = { type: AuthType.SRP, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - env: 'test' as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - platform: 'extension' as any, + env: Env.DEV, + platform: Platform.EXTENSION, }; // Mock data constants - const MOCK_PROFILE = { + const MOCK_PROFILE: UserProfile = { profileId: 'p1', - metametrics_id: 'm1', - identifier_id: 'i1', - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any; + metaMetricsId: 'm1', + identifierId: 'i1', + }; const MOCK_NONCE_RESPONSE = { nonce: 'nonce-1', @@ -64,20 +65,11 @@ describe('SRPJwtBearerAuth rate limit handling', () => { }; // Helper to create a rate limit error - const createRateLimitError = (retryAfterMs?: number) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = new Error('rate limited'); - error.name = 'RateLimitedError'; - error.status = 429; - if (retryAfterMs !== undefined) { - error.retryAfterMs = retryAfterMs; - } - return error; - }; + const createRateLimitError = (retryAfterMs?: number) => + new RateLimitedError('rate limited', retryAfterMs); const createAuth = (overrides?: { cooldownDefaultMs?: number }) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const store: any = { value: null as any }; + const store: { value: LoginResponse | null } = { value: null }; const auth = new SRPJwtBearerAuth(config, { storage: { From 20c60742ea6d37ef8803cc148c9a6e025367d41f Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Mon, 17 Nov 2025 14:13:11 +0100 Subject: [PATCH 9/9] fix: address pr feedbacks --- packages/profile-sync-controller/CHANGELOG.md | 2 +- .../flow-srp.test.ts | 44 +++++++++---------- .../sdk/authentication-jwt-bearer/flow-srp.ts | 16 ++++--- .../sdk/authentication-jwt-bearer/services.ts | 5 ++- .../src/sdk/constants.ts | 3 ++ .../profile-sync-controller/src/sdk/errors.ts | 6 ++- 6 files changed, 43 insertions(+), 33 deletions(-) create mode 100644 packages/profile-sync-controller/src/sdk/constants.ts diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 94b7e651278..7ea8e734d0d 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add rate limit (429) handling with automatic retry in authentication flow ([#6993](https://github.com/MetaMask/core/pull/6993)) - Update authentication services to throw `RateLimitedError` when encountering 429 responses. - - Improve Authentication errors by adding the HTTP code in Error messages. + - Improve Authentication errors by adding the HTTP code in error messages. - Add rate limit retry logic to `SRPJwtBearerAuth` with configurable cooldown via `rateLimitRetry.cooldownDefaultMs` option (defaults to 10 seconds). - Non-429 errors are thrown immediately without retry, delegating retry logic to consumers. 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 3ba99e17847..8fe8b97e4c0 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 @@ -68,7 +68,10 @@ describe('SRPJwtBearerAuth rate limit handling', () => { const createRateLimitError = (retryAfterMs?: number) => new RateLimitedError('rate limited', retryAfterMs); - const createAuth = (overrides?: { cooldownDefaultMs?: number }) => { + const createAuth = (overrides?: { + cooldownDefaultMs?: number; + maxLoginRetries?: number; + }) => { const store: { value: LoginResponse | null } = { value: null }; const auth = new SRPJwtBearerAuth(config, { @@ -115,17 +118,13 @@ describe('SRPJwtBearerAuth rate limit handling', () => { }); it('applies cooldown and retries once on 429 with Retry-After', async () => { - const { auth } = createAuth({ cooldownDefaultMs: 20 }); - - let first = true; - mockAuthenticate.mockImplementation(async () => { - // eslint-disable-next-line jest/no-conditional-in-test - if (first) { - first = false; - throw createRateLimitError(20); - } - return MOCK_AUTH_RESPONSE; - }); + const cooldownDefaultMs = 20; + const maxLoginRetries = 1; + const { auth } = createAuth({ cooldownDefaultMs, maxLoginRetries }); + + mockAuthenticate + .mockRejectedValueOnce(createRateLimitError(cooldownDefaultMs)) + .mockResolvedValueOnce(MOCK_AUTH_RESPONSE); const p1 = auth.getAccessToken(); const p2 = auth.getAccessToken(); @@ -135,22 +134,23 @@ describe('SRPJwtBearerAuth rate limit handling', () => { expect(t2).toBe('access'); // Should retry after rate limit error - expect(mockAuthenticate).toHaveBeenCalledTimes(2); + expect(mockAuthenticate).toHaveBeenCalledTimes(maxLoginRetries + 1); // Should apply cooldown delay - expect(mockDelay).toHaveBeenCalledWith(20); + expect(mockDelay).toHaveBeenCalledWith(cooldownDefaultMs); }); - it('throws 429 after exhausting one retry', async () => { - const { auth } = createAuth({ cooldownDefaultMs: 20 }); - - mockAuthenticate.mockRejectedValue(createRateLimitError(20)); + it('throws 429 after exhausting all retries', async () => { + const cooldownDefaultMs = 20; + const maxLoginRetries = 1; + const { auth } = createAuth({ cooldownDefaultMs, maxLoginRetries }); + mockAuthenticate.mockRejectedValue(createRateLimitError(cooldownDefaultMs)); await expect(auth.getAccessToken()).rejects.toThrow('rate limited'); - // Should attempt initial + one retry = 2 attempts - expect(mockAuthenticate).toHaveBeenCalledTimes(2); - // Should apply cooldown delay once - expect(mockDelay).toHaveBeenCalledTimes(1); + // Should attempt initial + maxLoginRetries + expect(mockAuthenticate).toHaveBeenCalledTimes(1 + maxLoginRetries); + // Should apply cooldown delay + expect(mockDelay).toHaveBeenCalledTimes(maxLoginRetries); }); it('throws transient errors immediately without retry', async () => { 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 36052e5bd63..0dc39d461f0 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 @@ -33,6 +33,7 @@ type JwtBearerAuth_SRP_Options = { signing?: AuthSigningOptions; rateLimitRetry?: { cooldownDefaultMs?: number; // default cooldown when 429 has no Retry-After + maxLoginRetries?: number; // maximum number of login retries on rate limit }; }; @@ -81,6 +82,9 @@ export class SRPJwtBearerAuth implements IBaseAuth { // Default cooldown when 429 has no Retry-After header readonly #cooldownDefaultMs: number; + // Maximum number of login retries on rate limit errors + readonly #maxLoginRetries: number; + #customProvider?: Eip1193Provider; constructor( @@ -103,6 +107,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { // Apply rate limit retry config if provided this.#cooldownDefaultMs = options.rateLimitRetry?.cooldownDefaultMs ?? 10000; + this.#maxLoginRetries = options.rateLimitRetry?.maxLoginRetries ?? 1; } setCustomProvider(provider: Eip1193Provider) { @@ -255,8 +260,8 @@ export class SRPJwtBearerAuth implements IBaseAuth { } async #loginWithRetry(entropySourceId?: string): Promise { - // Allow max 2 attempts: initial + one retry on 429 - for (let attempt = 0; attempt < 2; attempt += 1) { + // Allow max attempts: initial + maxLoginRetries on 429 + for (let attempt = 0; attempt < 1 + this.#maxLoginRetries; attempt += 1) { try { return await this.#performLogin(entropySourceId); } catch (e) { @@ -265,14 +270,13 @@ export class SRPJwtBearerAuth implements IBaseAuth { throw e; } - // If we've exhausted attempts (>= 1 retry), rethrow - if (attempt >= 1) { + // If we've exhausted attempts, rethrow + if (attempt >= this.#maxLoginRetries) { throw e; } // Wait for Retry-After or default cooldown - const waitMs = - (e as RateLimitedError).retryAfterMs ?? this.#cooldownDefaultMs; + const waitMs = e.retryAfterMs ?? this.#cooldownDefaultMs; await timeUtils.delay(waitMs); // Loop continues to retry diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index 6bf123824c8..1659b281523 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -8,6 +8,7 @@ import { AuthType } from './types'; import type { Env, Platform } from '../../shared/env'; import { getEnvUrls, getOidcClientId } from '../../shared/env'; import type { MetaMetricsAuth } from '../../shared/types/services'; +import { HTTP_STATUS_CODES } from '../constants'; import { NonceRetrievalError, PairError, @@ -65,9 +66,9 @@ async function handleErrorResponse( : responseBody.error_description; const { error } = responseBody; - if (status === 429) { + if (status === HTTP_STATUS_CODES.TOO_MANY_REQUESTS) { throw new RateLimitedError( - `HTTP 429: ${message} (error: ${error})`, + `HTTP ${HTTP_STATUS_CODES.TOO_MANY_REQUESTS}: ${message} (error: ${error})`, retryAfterMs ?? undefined, ); } diff --git a/packages/profile-sync-controller/src/sdk/constants.ts b/packages/profile-sync-controller/src/sdk/constants.ts new file mode 100644 index 00000000000..51c1c6f70ff --- /dev/null +++ b/packages/profile-sync-controller/src/sdk/constants.ts @@ -0,0 +1,3 @@ +export const HTTP_STATUS_CODES = { + TOO_MANY_REQUESTS: 429, +}; diff --git a/packages/profile-sync-controller/src/sdk/errors.ts b/packages/profile-sync-controller/src/sdk/errors.ts index 0b7dbe35666..ac2c0a7ee3a 100644 --- a/packages/profile-sync-controller/src/sdk/errors.ts +++ b/packages/profile-sync-controller/src/sdk/errors.ts @@ -1,3 +1,5 @@ +import { HTTP_STATUS_CODES } from './constants'; + export class NonceRetrievalError extends Error { constructor(message: string) { super(message); @@ -48,7 +50,7 @@ export class NotFoundError extends Error { } export class RateLimitedError extends Error { - readonly status = 429; + readonly status = HTTP_STATUS_CODES.TOO_MANY_REQUESTS; readonly retryAfterMs?: number; @@ -70,7 +72,7 @@ export class RateLimitedError extends Error { (typeof e === 'object' && e !== null && 'status' in e && - (e as { status: number }).status === 429) + e.status === HTTP_STATUS_CODES.TOO_MANY_REQUESTS) ); } }