fix: drop tokens refreshed for an account that signed out meanwhile

src/lib/__tests__/oauth.test.ts

@@ -126,6 +126,38 @@ describe('oauth token refresh deduplication', () => {
     expect(mockSecureFetch).not.toHaveBeenCalled();
   });
 
+  it('does not bring back a chain forgotten while its refresh was in flight', async () => {
+    const tokens: OAuthTokens = {
+      accessToken: 'a1',
+      refreshToken: 'r1',
+      tokenEndpoint: 'https://auth.example.com/token',
+      clientId: 'bulwark-webmail',
+    };
+    let answer!: () => void;
+    mockSecureFetch.mockImplementationOnce(async () => {
+      await new Promise<void>((resolve) => { answer = resolve; });
+      return {
+        ok: true,
+        status: 200,
+        json: async () => ({ access_token: 'a2', refresh_token: 'r2', expires_in: 3600 }),
+      } as Response;
+    });
+
+    const inFlight = refreshOAuthAccessToken(tokens);
+    await vi.waitFor(() => expect(mockSecureFetch).toHaveBeenCalledTimes(1));
+    // The account signs out before the token endpoint answers.
+    forgetRefreshRotations('r1');
+    answer();
+    await inFlight;
+
+    // A stale copy refreshed later doesn't get the signed-out account's
+    // successor, nor one of the successor's.
+    mockSecureFetch.mockResolvedValue({ ok: false, status: 400 } as Response);
+    await expect(refreshOAuthAccessToken(tokens)).rejects.toThrow('400');
+    await expect(refreshOAuthAccessToken({ ...tokens, accessToken: 'a2', refreshToken: 'r2' })).rejects.toThrow('400');
+    expect(mockSecureFetch).toHaveBeenCalledTimes(3);
+  });
+
   it('keeps refreshing with the same token on a server that does not rotate', async () => {
     const tokens: OAuthTokens = {
       accessToken: 'a1',

src/lib/__tests__/stored-token-refresh.test.ts

@@ -14,7 +14,7 @@ vi.mock('../../api/jmap-client', () => ({
 }));
 
 import { jmapClient, type StoredCredentials } from '../../api/jmap-client';
-import { resetRefreshRotations } from '../oauth';
+import { forgetRefreshRotations, refreshOAuthAccessToken, resetRefreshRotations } from '../oauth';
 import { freshStoredCredentials } from '../stored-token-refresh';
 
 const ID = 'u@example.org@https://mail.example.com';
@@ -99,6 +99,25 @@ describe('freshStoredCredentials', () => {
     expect(jmapClient.setStoredCredentials).not.toHaveBeenCalled();
   });
 
+  it('hands out nothing when the account signs out while the refresh runs', async () => {
+    store.set(ID, bundle(1));
+    const rotate = mockSecureFetch.getMockImplementation()!;
+    mockSecureFetch.mockImplementationOnce(async (url: string, init: RequestInit) => {
+      // The sign-out, as the auth store does it, before the endpoint answers.
+      forgetRefreshRotations('r1');
+      store.delete(ID);
+      return rotate(url, init);
+    });
+
+    await expect(freshStoredCredentials(ID, bundle(1))).rejects.toThrow();
+
+    expect(store.has(ID)).toBe(false);
+    // Nor does a stale copy of the bundle get the successor out of memory.
+    await expect(
+      refreshOAuthAccessToken({ accessToken: 'a1', refreshToken: 'r1', tokenEndpoint: 'https://mail.example.com/auth/token', clientId: 'swc1.x' }),
+    ).rejects.toThrow('400');
+  });
+
   it('does not overwrite a newer bundle stored while the refresh ran', async () => {
     store.set(ID, bundle(1));
     // The iOS notification extension rotates the grant meanwhile (it can't

src/lib/oauth.ts

@@ -547,9 +547,26 @@ const ROTATED_MAX = 64;
 // A successor whose access token has less left than this is refreshed in
 // turn rather than handed out.
 const SUCCESSOR_MIN_LIFETIME_MS = 60_000;
+// Refresh tokens whose chain was dropped on sign-out. A refresh that was in
+// flight then returns afterwards and must not put the chain back.
+const forgotten = new Set<string>();
+const FORGOTTEN_MAX = 256;
+
+function markForgotten(refreshToken: string): void {
+  forgotten.delete(refreshToken);
+  if (forgotten.size >= FORGOTTEN_MAX) {
+    const oldest = forgotten.values().next().value;
+    if (oldest !== undefined) forgotten.delete(oldest);
+  }
+  forgotten.add(refreshToken);
+}
 
 function rememberRotation(previous: string, next: OAuthTokens): void {
   if (!next.refreshToken || next.refreshToken === previous) return;
+  if (forgotten.has(previous)) {
+    markForgotten(next.refreshToken);
+    return;
+  }
   if (!rotatedTo.has(previous) && rotatedTo.size >= ROTATED_MAX) {
     const oldest = rotatedTo.keys().next().value;
     if (oldest !== undefined) rotatedTo.delete(oldest);
@@ -572,13 +589,15 @@ function latestRotation(refreshToken: string): OAuthTokens | null {
 /** Test hook. */
 export function resetRefreshRotations(): void {
   rotatedTo.clear();
+  forgotten.clear();
 }
 
 /**
  * Drop the rotation chain `refreshToken` is part of, its predecessors and
  * successors both, when its account signs out. A stale copy of the bundle
  * refreshed later would otherwise get the successor from here and could
- * write the removed account's tokens back.
+ * write the removed account's tokens back. A refresh of the chain still in
+ * flight doesn't add it again when it returns.
  */
 export function forgetRefreshRotations(refreshToken: string | undefined): void {
   if (!refreshToken) return;
@@ -593,7 +612,10 @@ export function forgetRefreshRotations(refreshToken: string | undefined): void {
       if (next.refreshToken && !chain.has(next.refreshToken)) { chain.add(next.refreshToken); grew = true; }
     }
   }
-  for (const token of chain) rotatedTo.delete(token);
+  for (const token of chain) {
+    rotatedTo.delete(token);
+    markForgotten(token);
+  }
 }
 
 // OAuth refresh — exchanges the refresh token at the original token endpoint

src/lib/stored-token-refresh.ts

@@ -16,7 +16,7 @@
 //   someone else wrote.
 
 import { jmapClient, type StoredCredentials } from '../api/jmap-client';
-import { refreshOAuthAccessToken } from './oauth';
+import { forgetRefreshRotations, refreshOAuthAccessToken } from './oauth';
 
 const TOKEN_REFRESH_LEEWAY_MS = 60_000;
 
@@ -29,7 +29,8 @@ function expiresSoon(expiresAt: number | undefined): boolean {
  * stored when needed. Password credentials come back as they are; so does
  * an account whose credentials are gone from storage (signed out). A failed
  * refresh returns what there is, and the request that uses it fails on its
- * own.
+ * own. When the account signs out while the refresh runs, its fresh tokens
+ * are dropped and this throws.
  */
 export async function freshStoredCredentials(
   accountId: string,
@@ -58,6 +59,7 @@ export async function freshStoredCredentials(
     if (!expiresSoon(base.expiresAt)) return base;
   }
 
+  let signedOut = false;
   try {
     const next = await refreshOAuthAccessToken({
       accessToken: base.accessToken!,
@@ -75,11 +77,20 @@ export async function freshStoredCredentials(
       clientId: next.clientId,
     };
     const current = await jmapClient.getStoredCredentials(accountId);
-    if (current && current.refreshToken === base.refreshToken && current.accessToken !== tokens.accessToken) {
+    if (!current) {
+      // Signed out meanwhile: the sign-out forgot the chain before this
+      // refresh returned. Nobody may use the tokens it bought.
+      forgetRefreshRotations(base.refreshToken);
+      forgetRefreshRotations(tokens.refreshToken);
+      signedOut = true;
+      throw new Error('Account signed out');
+    }
+    if (current.refreshToken === base.refreshToken && current.accessToken !== tokens.accessToken) {
       await jmapClient.setStoredCredentials(accountId, { ...current, ...tokens });
     }
     return { ...base, ...tokens };
-  } catch {
+  } catch (err) {
+    if (signedOut) throw err;
     return base;
   }
 }