Skip to content

Commit ee3a8c0

Browse files
authored
feat: add rate limit (429) handling to AuthenticationController (#6993)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References Related to: [MUL-1214](https://consensyssoftware.atlassian.net/browse/MUL-1214) ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds 429-aware error handling with configurable retry/cooldown to SRP JWT auth and centralizes HTTP error parsing, with tests and changelog updates. > > - **SDK Auth (SRP)**: > - Add retry on `RateLimitedError` (429) with cooldown and `maxLoginRetries` options in `SRPJwtBearerAuth` (`#loginWithRetry`, default cooldown 10s). > - Continue to coalesce concurrent logins via deferred login path. > - **Services**: > - Centralize HTTP error handling via `handleErrorResponse`, parsing `Retry-After` and throwing `RateLimitedError` on 429. > - Improve error messages to include HTTP status codes across `getNonce`, `authenticate`, `authorizeOIDC`, `pairIdentifiers`, `getUserProfileLineage`. > - **Utils & Errors**: > - Add `HTTP_STATUS_CODES` and `utils/time.delay`. > - Introduce `RateLimitedError` with optional `retryAfterMs` and `isRateLimitError` type guard. > - **Tests**: > - Add `flow-srp.test.ts` covering retry with cooldown, exhausting retries, no-retry on non-429, and concurrent call coalescing. > - **Docs**: > - Update `CHANGELOG.md` with new 429 handling and retry behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fb123d2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [MUL-1214]: https://consensyssoftware.atlassian.net/browse/MUL-1214?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
1 parent 777121a commit ee3a8c0

File tree

7 files changed

+364
-26
lines changed

7 files changed

+364
-26
lines changed

packages/profile-sync-controller/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Changed
11+
12+
- Add rate limit (429) handling with automatic retry in authentication flow ([#6993](https://github.com/MetaMask/core/pull/6993))
13+
- Update authentication services to throw `RateLimitedError` when encountering 429 responses.
14+
- Improve Authentication errors by adding the HTTP code in error messages.
15+
- Add rate limit retry logic to `SRPJwtBearerAuth` with configurable cooldown via `rateLimitRetry.cooldownDefaultMs` option (defaults to 10 seconds).
16+
- Non-429 errors are thrown immediately without retry, delegating retry logic to consumers.
17+
1018
## [26.0.0]
1119

1220
### Changed
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
import { SRPJwtBearerAuth } from './flow-srp';
2+
import {
3+
AuthType,
4+
type AuthConfig,
5+
type LoginResponse,
6+
type UserProfile,
7+
} from './types';
8+
import * as timeUtils from './utils/time';
9+
import { Env, Platform } from '../../shared/env';
10+
import { RateLimitedError } from '../errors';
11+
12+
jest.setTimeout(15000);
13+
14+
// Mock the time utilities to avoid real delays in tests
15+
jest.mock('./utils/time', () => ({
16+
delay: jest.fn(),
17+
}));
18+
19+
const mockDelay = timeUtils.delay as jest.MockedFunction<
20+
typeof timeUtils.delay
21+
>;
22+
23+
// Mock services
24+
const mockGetNonce = jest.fn();
25+
const mockAuthenticate = jest.fn();
26+
const mockAuthorizeOIDC = jest.fn();
27+
28+
jest.mock('./services', () => ({
29+
authenticate: (...args: unknown[]) => mockAuthenticate(...args),
30+
authorizeOIDC: (...args: unknown[]) => mockAuthorizeOIDC(...args),
31+
getNonce: (...args: unknown[]) => mockGetNonce(...args),
32+
getUserProfileLineage: jest.fn(),
33+
}));
34+
35+
describe('SRPJwtBearerAuth rate limit handling', () => {
36+
const config: AuthConfig & { type: AuthType.SRP } = {
37+
type: AuthType.SRP,
38+
env: Env.DEV,
39+
platform: Platform.EXTENSION,
40+
};
41+
42+
// Mock data constants
43+
const MOCK_PROFILE: UserProfile = {
44+
profileId: 'p1',
45+
metaMetricsId: 'm1',
46+
identifierId: 'i1',
47+
};
48+
49+
const MOCK_NONCE_RESPONSE = {
50+
nonce: 'nonce-1',
51+
identifier: 'identifier-1',
52+
expiresIn: 60,
53+
};
54+
55+
const MOCK_AUTH_RESPONSE = {
56+
token: 'jwt-token',
57+
expiresIn: 60,
58+
profile: MOCK_PROFILE,
59+
};
60+
61+
const MOCK_OIDC_RESPONSE = {
62+
accessToken: 'access',
63+
expiresIn: 60,
64+
obtainedAt: Date.now(),
65+
};
66+
67+
// Helper to create a rate limit error
68+
const createRateLimitError = (retryAfterMs?: number) =>
69+
new RateLimitedError('rate limited', retryAfterMs);
70+
71+
const createAuth = (overrides?: {
72+
cooldownDefaultMs?: number;
73+
maxLoginRetries?: number;
74+
}) => {
75+
const store: { value: LoginResponse | null } = { value: null };
76+
77+
const auth = new SRPJwtBearerAuth(config, {
78+
storage: {
79+
getLoginResponse: async () => store.value,
80+
setLoginResponse: async (val) => {
81+
store.value = val;
82+
},
83+
},
84+
signing: {
85+
getIdentifier: async () => 'identifier-1',
86+
signMessage: async () => 'signature-1',
87+
},
88+
rateLimitRetry: overrides,
89+
});
90+
91+
return { auth, store };
92+
};
93+
94+
beforeEach(() => {
95+
jest.clearAllMocks();
96+
mockGetNonce.mockResolvedValue(MOCK_NONCE_RESPONSE);
97+
mockAuthenticate.mockResolvedValue(MOCK_AUTH_RESPONSE);
98+
mockAuthorizeOIDC.mockResolvedValue(MOCK_OIDC_RESPONSE);
99+
});
100+
101+
it('coalesces concurrent calls into a single login attempt', async () => {
102+
const { auth } = createAuth();
103+
104+
const p1 = auth.getAccessToken();
105+
const p2 = auth.getAccessToken();
106+
const p3 = auth.getAccessToken();
107+
108+
const [t1, t2, t3] = await Promise.all([p1, p2, p3]);
109+
110+
expect(t1).toBe('access');
111+
expect(t2).toBe('access');
112+
expect(t3).toBe('access');
113+
114+
// single sequence of service calls
115+
expect(mockGetNonce).toHaveBeenCalledTimes(1);
116+
expect(mockAuthenticate).toHaveBeenCalledTimes(1);
117+
expect(mockAuthorizeOIDC).toHaveBeenCalledTimes(1);
118+
});
119+
120+
it('applies cooldown and retries once on 429 with Retry-After', async () => {
121+
const cooldownDefaultMs = 20;
122+
const maxLoginRetries = 1;
123+
const { auth } = createAuth({ cooldownDefaultMs, maxLoginRetries });
124+
125+
mockAuthenticate
126+
.mockRejectedValueOnce(createRateLimitError(cooldownDefaultMs))
127+
.mockResolvedValueOnce(MOCK_AUTH_RESPONSE);
128+
129+
const p1 = auth.getAccessToken();
130+
const p2 = auth.getAccessToken();
131+
132+
const [t1, t2] = await Promise.all([p1, p2]);
133+
expect(t1).toBe('access');
134+
expect(t2).toBe('access');
135+
136+
// Should retry after rate limit error
137+
expect(mockAuthenticate).toHaveBeenCalledTimes(maxLoginRetries + 1);
138+
// Should apply cooldown delay
139+
expect(mockDelay).toHaveBeenCalledWith(cooldownDefaultMs);
140+
});
141+
142+
it('throws 429 after exhausting all retries', async () => {
143+
const cooldownDefaultMs = 20;
144+
const maxLoginRetries = 1;
145+
const { auth } = createAuth({ cooldownDefaultMs, maxLoginRetries });
146+
147+
mockAuthenticate.mockRejectedValue(createRateLimitError(cooldownDefaultMs));
148+
await expect(auth.getAccessToken()).rejects.toThrow('rate limited');
149+
150+
// Should attempt initial + maxLoginRetries
151+
expect(mockAuthenticate).toHaveBeenCalledTimes(1 + maxLoginRetries);
152+
// Should apply cooldown delay
153+
expect(mockDelay).toHaveBeenCalledTimes(maxLoginRetries);
154+
});
155+
156+
it('throws transient errors immediately without retry', async () => {
157+
const { auth, store } = createAuth();
158+
159+
// Force a login by clearing session
160+
store.value = null;
161+
162+
const transientError = new Error('transient network error');
163+
mockAuthenticate.mockRejectedValue(transientError);
164+
165+
await expect(auth.getAccessToken()).rejects.toThrow(
166+
'transient network error',
167+
);
168+
169+
// Should NOT retry on transient errors
170+
expect(mockAuthenticate).toHaveBeenCalledTimes(1);
171+
// Should NOT apply any delay
172+
expect(mockDelay).not.toHaveBeenCalled();
173+
});
174+
});

packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ import type {
1616
UserProfile,
1717
UserProfileLineage,
1818
} from './types';
19+
import * as timeUtils from './utils/time';
1920
import type { MetaMetricsAuth } from '../../shared/types/services';
20-
import { ValidationError } from '../errors';
21+
import { ValidationError, RateLimitedError } from '../errors';
2122
import { getMetaMaskProviderEIP6963 } from '../utils/eip-6963-metamask-provider';
2223
import {
2324
MESSAGE_SIGNING_SNAP,
@@ -30,6 +31,10 @@ import { validateLoginResponse } from '../utils/validate-login-response';
3031
type JwtBearerAuth_SRP_Options = {
3132
storage: AuthStorageOptions;
3233
signing?: AuthSigningOptions;
34+
rateLimitRetry?: {
35+
cooldownDefaultMs?: number; // default cooldown when 429 has no Retry-After
36+
maxLoginRetries?: number; // maximum number of login retries on rate limit
37+
};
3338
};
3439

3540
const getDefaultEIP6963Provider = async () => {
@@ -64,13 +69,22 @@ const getDefaultEIP6963SigningOptions = (
6469
export class SRPJwtBearerAuth implements IBaseAuth {
6570
readonly #config: AuthConfig;
6671

67-
readonly #options: Required<JwtBearerAuth_SRP_Options>;
72+
readonly #options: {
73+
storage: AuthStorageOptions;
74+
signing: AuthSigningOptions;
75+
};
6876

6977
readonly #metametrics?: MetaMetricsAuth;
7078

7179
// Map to store ongoing login promises by entropySourceId
7280
readonly #ongoingLogins = new Map<string, Promise<LoginResponse>>();
7381

82+
// Default cooldown when 429 has no Retry-After header
83+
readonly #cooldownDefaultMs: number;
84+
85+
// Maximum number of login retries on rate limit errors
86+
readonly #maxLoginRetries: number;
87+
7488
#customProvider?: Eip1193Provider;
7589

7690
constructor(
@@ -89,6 +103,11 @@ export class SRPJwtBearerAuth implements IBaseAuth {
89103
getDefaultEIP6963SigningOptions(this.#customProvider),
90104
};
91105
this.#metametrics = options.metametrics;
106+
107+
// Apply rate limit retry config if provided
108+
this.#cooldownDefaultMs =
109+
options.rateLimitRetry?.cooldownDefaultMs ?? 10000;
110+
this.#maxLoginRetries = options.rateLimitRetry?.maxLoginRetries ?? 1;
92111
}
93112

94113
setCustomProvider(provider: Eip1193Provider) {
@@ -225,7 +244,7 @@ export class SRPJwtBearerAuth implements IBaseAuth {
225244
}
226245

227246
// Create a new login promise
228-
const loginPromise = this.#performLogin(entropySourceId);
247+
const loginPromise = this.#loginWithRetry(entropySourceId);
229248

230249
// Store the promise in the map
231250
this.#ongoingLogins.set(loginKey, loginPromise);
@@ -240,6 +259,34 @@ export class SRPJwtBearerAuth implements IBaseAuth {
240259
}
241260
}
242261

262+
async #loginWithRetry(entropySourceId?: string): Promise<LoginResponse> {
263+
// Allow max attempts: initial + maxLoginRetries on 429
264+
for (let attempt = 0; attempt < 1 + this.#maxLoginRetries; attempt += 1) {
265+
try {
266+
return await this.#performLogin(entropySourceId);
267+
} catch (e) {
268+
// Only retry on rate-limit (429) errors
269+
if (!RateLimitedError.isRateLimitError(e)) {
270+
throw e;
271+
}
272+
273+
// If we've exhausted attempts, rethrow
274+
if (attempt >= this.#maxLoginRetries) {
275+
throw e;
276+
}
277+
278+
// Wait for Retry-After or default cooldown
279+
const waitMs = e.retryAfterMs ?? this.#cooldownDefaultMs;
280+
await timeUtils.delay(waitMs);
281+
282+
// Loop continues to retry
283+
}
284+
}
285+
286+
// Should never reach here due to loop logic, but TypeScript needs a return
287+
throw new Error('Unexpected: login loop exhausted without result');
288+
}
289+
243290
#createSrpLoginRawMessage(
244291
nonce: string,
245292
publicKey: string,

0 commit comments

Comments
 (0)