mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-04 00:06:53 +00:00
fix(auth): coalesce concurrent token refresh requests
Coalesces concurrent OAuth token refreshes using a Promise-based lock map keyed by the refresh token. This prevents the app from firing duplicate refresh requests with the same expired refresh token, which causes replay revocations on OIDC providers (e.g. Authelia/Hydra) enforcing strict rotation. Also optimizes persistRefreshedTokens and ensureFreshCredentials to prevent redundant SecureStore/disk writes when concurrent operations complete. Fixes #14
This commit is contained in:
@@ -151,6 +151,7 @@ export class JMAPClient {
|
||||
|
||||
private async persistRefreshedTokens(next: OAuthTokens): Promise<void> {
|
||||
if (!this.credentials) return;
|
||||
if (this.credentials.accessToken === next.accessToken) return;
|
||||
this.credentials = {
|
||||
...this.credentials,
|
||||
accessToken: next.accessToken,
|
||||
|
||||
@@ -101,7 +101,10 @@ async function ensureFreshCredentials(
|
||||
tokenEndpoint: next.tokenEndpoint,
|
||||
clientId: next.clientId,
|
||||
};
|
||||
await jmapClient.setStoredCredentials(accountId, updated);
|
||||
const current = await jmapClient.getStoredCredentials(accountId);
|
||||
if (!current || current.accessToken !== updated.accessToken) {
|
||||
await jmapClient.setStoredCredentials(accountId, updated);
|
||||
}
|
||||
return updated;
|
||||
} catch {
|
||||
// Fall back to the existing (possibly expired) token; the request will
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { mockSecureFetch } = vi.hoisted(() => ({
|
||||
mockSecureFetch: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../client-cert', () => ({
|
||||
secureFetch: mockSecureFetch,
|
||||
}));
|
||||
|
||||
import { refreshOAuthAccessToken, type OAuthTokens } from '../oauth';
|
||||
|
||||
describe('oauth token refresh deduplication', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should only send a single HTTP request when refreshOAuthAccessToken is called concurrently with the same refresh token', async () => {
|
||||
const tokens: OAuthTokens = {
|
||||
accessToken: 'old-access',
|
||||
refreshToken: 'same-refresh-token',
|
||||
expiresAt: 123456,
|
||||
tokenEndpoint: 'https://auth.example.com/token',
|
||||
clientId: 'client-id-123',
|
||||
};
|
||||
|
||||
// Set up a mock response that takes a brief moment to resolve
|
||||
mockSecureFetch.mockImplementation(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
return {
|
||||
ok: true,
|
||||
status: 200,
|
||||
json: async () => ({
|
||||
access_token: 'new-access-token',
|
||||
refresh_token: 'rotated-refresh-token',
|
||||
expires_in: 3600,
|
||||
}),
|
||||
} as Response;
|
||||
});
|
||||
|
||||
// Fire two refreshes concurrently
|
||||
const [res1, res2] = await Promise.all([
|
||||
refreshOAuthAccessToken(tokens),
|
||||
refreshOAuthAccessToken(tokens),
|
||||
]);
|
||||
|
||||
// Check that secureFetch was called exactly once
|
||||
expect(mockSecureFetch).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Verify both calls received the exact same updated token bundle
|
||||
expect(res1.accessToken).toBe('new-access-token');
|
||||
expect(res1.refreshToken).toBe('rotated-refresh-token');
|
||||
expect(res2.accessToken).toBe('new-access-token');
|
||||
expect(res2.refreshToken).toBe('rotated-refresh-token');
|
||||
|
||||
// Fire another refresh after the first completes (now the map should be empty for this token)
|
||||
mockSecureFetch.mockClear();
|
||||
const res3 = await refreshOAuthAccessToken(tokens);
|
||||
expect(mockSecureFetch).toHaveBeenCalledTimes(1);
|
||||
expect(res3.accessToken).toBe('new-access-token');
|
||||
});
|
||||
|
||||
it('should propagate errors to all waiters if the HTTP request fails', async () => {
|
||||
const tokens: OAuthTokens = {
|
||||
accessToken: 'old-access',
|
||||
refreshToken: 'failing-refresh-token',
|
||||
expiresAt: 123456,
|
||||
tokenEndpoint: 'https://auth.example.com/token',
|
||||
clientId: 'client-id-123',
|
||||
};
|
||||
|
||||
mockSecureFetch.mockImplementation(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
return {
|
||||
ok: false,
|
||||
status: 400,
|
||||
} as Response;
|
||||
});
|
||||
|
||||
// Fire two refreshes concurrently and check that both reject with the same error
|
||||
const promise1 = refreshOAuthAccessToken(tokens);
|
||||
const promise2 = refreshOAuthAccessToken(tokens);
|
||||
|
||||
await expect(promise1).rejects.toThrow('Token refresh failed: 400');
|
||||
await expect(promise2).rejects.toThrow('Token refresh failed: 400');
|
||||
|
||||
expect(mockSecureFetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
+47
-30
@@ -207,6 +207,8 @@ export async function redeemPairingCode(webmailUrl: string, code: string): Promi
|
||||
};
|
||||
}
|
||||
|
||||
const activeRefreshes = new Map<string, Promise<OAuthTokens>>();
|
||||
|
||||
// OAuth refresh — exchanges the refresh token at the original token endpoint
|
||||
// for a new access token. Returns the updated bundle so the caller can
|
||||
// persist it.
|
||||
@@ -214,37 +216,52 @@ export async function refreshOAuthAccessToken(tokens: OAuthTokens): Promise<OAut
|
||||
if (!tokens.refreshToken) {
|
||||
throw new HandoffError('No refresh token available');
|
||||
}
|
||||
const body = new URLSearchParams({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokens.refreshToken,
|
||||
client_id: tokens.clientId,
|
||||
});
|
||||
const response = await secureFetch(tokens.tokenEndpoint, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Content-Type': 'application/x-www-form-urlencoded',
|
||||
Accept: 'application/json',
|
||||
},
|
||||
body: body.toString(),
|
||||
});
|
||||
if (!response.ok) {
|
||||
throw new HandoffError(`Token refresh failed: ${response.status}`);
|
||||
|
||||
const cacheKey = tokens.refreshToken;
|
||||
let promise = activeRefreshes.get(cacheKey);
|
||||
|
||||
if (!promise) {
|
||||
promise = (async () => {
|
||||
try {
|
||||
const body = new URLSearchParams({
|
||||
grant_type: 'refresh_token',
|
||||
refresh_token: tokens.refreshToken!,
|
||||
client_id: tokens.clientId,
|
||||
});
|
||||
const response = await secureFetch(tokens.tokenEndpoint, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Content-Type': 'application/x-www-form-urlencoded',
|
||||
Accept: 'application/json',
|
||||
},
|
||||
body: body.toString(),
|
||||
});
|
||||
if (!response.ok) {
|
||||
throw new HandoffError(`Token refresh failed: ${response.status}`);
|
||||
}
|
||||
const data = (await response.json()) as {
|
||||
access_token?: string;
|
||||
refresh_token?: string;
|
||||
expires_in?: number;
|
||||
};
|
||||
if (!data.access_token) {
|
||||
throw new HandoffError('Token refresh response missing access_token');
|
||||
}
|
||||
return {
|
||||
accessToken: data.access_token,
|
||||
refreshToken: data.refresh_token ?? tokens.refreshToken!,
|
||||
expiresAt: data.expires_in ? Date.now() + data.expires_in * 1000 : undefined,
|
||||
tokenEndpoint: tokens.tokenEndpoint,
|
||||
clientId: tokens.clientId,
|
||||
};
|
||||
} finally {
|
||||
activeRefreshes.delete(cacheKey);
|
||||
}
|
||||
})();
|
||||
activeRefreshes.set(cacheKey, promise);
|
||||
}
|
||||
const data = (await response.json()) as {
|
||||
access_token?: string;
|
||||
refresh_token?: string;
|
||||
expires_in?: number;
|
||||
};
|
||||
if (!data.access_token) {
|
||||
throw new HandoffError('Token refresh response missing access_token');
|
||||
}
|
||||
return {
|
||||
accessToken: data.access_token,
|
||||
refreshToken: data.refresh_token ?? tokens.refreshToken,
|
||||
expiresAt: data.expires_in ? Date.now() + data.expires_in * 1000 : undefined,
|
||||
tokenEndpoint: tokens.tokenEndpoint,
|
||||
clientId: tokens.clientId,
|
||||
};
|
||||
|
||||
return promise;
|
||||
}
|
||||
|
||||
WebBrowser.maybeCompleteAuthSession();
|
||||
|
||||
Reference in New Issue
Block a user