fix: waiting token refreshes re-read the token instead of spending a stale copy #150

src/api/__tests__/jmap-client-hardening.test.ts

@@ -520,6 +520,78 @@ describe('JMAPClient hardening', () => {
     (SecureStore.getItemAsync as ReturnType<typeof vi.fn>).mockReset();
   });
 
+  // The app comes back after the Android push handler refreshed twice while
+  // it was frozen: its token in memory is two rotations old, and Stalwart
+  // revokes the whole sign-in if that one is spent. Several requests find the
+  // token due at once; the first to read storage takes the stored bundle,
+  // and the others must use that, not the copy they read before waiting.
+  it('lets no caller spend the old token after another took the stored one', async () => {
+    const SecureStore = await import('expo-secure-store');
+    const client = new JMAPClient();
+    mockFetch([{ status: 200, json: SESSION }]);
+    await client.connectWithOAuth('https://mail.example.com', {
+      accessToken: 'old',
+      refreshToken: 'r1',
+      expiresAt: Date.now() + 3_600_000,
+      tokenEndpoint: 'https://mail.example.com/auth/token',
+      clientId: 'c',
+    });
+    (SecureStore.getItemAsync as ReturnType<typeof vi.fn>).mockResolvedValue(JSON.stringify({
+      serverUrl: 'https://mail.example.com',
+      username: 'user@example.com',
+      password: '',
+      accessToken: 'from-push',
+      refreshToken: 'r3',
+      expiresAt: Date.now() + 9_000_000,
+      tokenEndpoint: 'https://mail.example.com/auth/token',
+      clientId: 'c',
+    }));
+    try {
+      // Two hours frozen in the background.
+      vi.useFakeTimers({ toFake: ['Date'] });
+      vi.setSystemTime(Date.now() + 7_200_000);
+      await Promise.all(Array.from({ length: 7 }, () => client.ensureFreshToken()));
+      expect(mockRefresh).not.toHaveBeenCalled();
+      expect(client.authHeader).toBe('Bearer from-push');
+    } finally {
+      (SecureStore.getItemAsync as ReturnType<typeof vi.fn>).mockReset();
+    }
+  });
+
+  // Two requests refused with the same expired token: the first takes the
+  // stored bundle, and the second retries with it instead of refreshing a
+  // token that is good.
+  it('retries concurrent 401s with tokens another caller took, without refreshing', async () => {
+    const SecureStore = await import('expo-secure-store');
+    const client = new JMAPClient();
+    mockFetch([{ status: 200, json: SESSION }]);
+    await client.connectWithOAuth('https://mail.example.com', {
+      accessToken: 'old',
+      refreshToken: 'r1',
+      expiresAt: Date.now() + 3_600_000,
+      tokenEndpoint: 'https://mail.example.com/auth/token',
+      clientId: 'c',
+    });
+    (SecureStore.getItemAsync as ReturnType<typeof vi.fn>).mockResolvedValue(JSON.stringify({
+      serverUrl: 'https://mail.example.com',
+      username: 'user@example.com',
+      password: '',
+      accessToken: 'from-push',
+      refreshToken: 'r2',
+      expiresAt: Date.now() + 7_200_000,
+      tokenEndpoint: 'https://mail.example.com/auth/token',
+      clientId: 'c',
+    }));
+    try {
+      const results = await Promise.all([client.forceRefreshToken(), client.forceRefreshToken()]);
+      expect(results).toEqual([true, true]);
+      expect(mockRefresh).not.toHaveBeenCalled();
+      expect(client.authHeader).toBe('Bearer from-push');
+    } finally {
+      (SecureStore.getItemAsync as ReturnType<typeof vi.fn>).mockReset();
+    }
+  });
+
   // The Android push handler refreshes in the background too (#94) and may
   // spend the refresh token a moment before the app does: the server then
   // refuses the app's refresh, and the new bundle lands in storage right

src/api/jmap-client.ts

@@ -578,12 +578,16 @@ export class JMAPClient {
     if (expiresAt == null) return;
     if (expiresAt - Date.now() > TOKEN_REFRESH_LEEWAY_MS) return;
     // Another process may have refreshed already (the iOS notification
-    // extension); use its tokens rather than spend a refresh token the
-    // server has replaced.
-    if (await this.adoptStoredTokens()) {
-      current = this.currentOAuthTokens();
-      if (!current || (current.tokens.expiresAt != null && current.tokens.expiresAt - Date.now() > TOKEN_REFRESH_LEEWAY_MS)) return;
-    }
+    // extension, the Android push handler); use its tokens rather than spend
+    // a refresh token the server has replaced.
+    await this.adoptStoredTokens();
+    // Read again whatever that returned: another caller may have taken the
+    // stored tokens while this one waited, and the copy read above is then
+    // spent. Stalwart revokes the whole sign-in when a refresh token two
+    // rotations old comes back.
+    current = this.currentOAuthTokens();
+    if (!current || current.tokens.expiresAt == null) return;
+    if (current.tokens.expiresAt - Date.now() > TOKEN_REFRESH_LEEWAY_MS) return;
     try {
       const next = await refreshOAuthAccessToken(current.tokens);
       await this.persistRefreshedTokens(current.credentials, next);
@@ -599,12 +603,15 @@ export class JMAPClient {
   // the account instead of evicting it (webmail 1.7.6 "keep the session when
   // the auth server is briefly unreachable").
   async forceRefreshToken(): Promise<boolean> {
-    if (!this.currentOAuthTokens()) return false;
+    const refused = this.currentOAuthTokens();
+    if (!refused) return false;
     // A newer token in storage (refreshed by the iOS notification extension)
     // is worth a retry before refreshing again.
     if (await this.adoptStoredTokens()) return true;
     const current = this.currentOAuthTokens();
     if (!current) return false;
+    // Another caller took newer tokens while this one waited: retry with them.
+    if (current.tokens.accessToken !== refused.tokens.accessToken) return true;
     try {
       const next = await refreshOAuthAccessToken(current.tokens);
       await this.persistRefreshedTokens(current.credentials, next);