diff --git a/src/api/jmap-client.ts b/src/api/jmap-client.ts index 22143eab..c70269d5 100644 --- a/src/api/jmap-client.ts +++ b/src/api/jmap-client.ts @@ -151,6 +151,7 @@ export class JMAPClient { private async persistRefreshedTokens(next: OAuthTokens): Promise { if (!this.credentials) return; + if (this.credentials.accessToken === next.accessToken) return; this.credentials = { ...this.credentials, accessToken: next.accessToken, diff --git a/src/api/unified-inbox.ts b/src/api/unified-inbox.ts index 75b441fd..1e5619f9 100644 --- a/src/api/unified-inbox.ts +++ b/src/api/unified-inbox.ts @@ -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 diff --git a/src/lib/__tests__/oauth.test.ts b/src/lib/__tests__/oauth.test.ts new file mode 100644 index 00000000..d31dd751 --- /dev/null +++ b/src/lib/__tests__/oauth.test.ts @@ -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); + }); +}); diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 015027be..fea94ed2 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -207,6 +207,8 @@ export async function redeemPairingCode(webmailUrl: string, code: string): Promi }; } +const activeRefreshes = new Map>(); + // 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 { + 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();