Merge pull request #15 from arnauda-gh/fix/coalesce-token-refresh

fix(auth): coalesce concurrent token refresh requests

src/api/jmap-client.ts

@@ -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,

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

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);
+  });
+});

src/lib/oauth.ts

@@ -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();