diff --git a/src/api/__tests__/jmap-client-hardening.test.ts b/src/api/__tests__/jmap-client-hardening.test.ts index d011d8f2..35497fc4 100644 --- a/src/api/__tests__/jmap-client-hardening.test.ts +++ b/src/api/__tests__/jmap-client-hardening.test.ts @@ -520,6 +520,78 @@ describe('JMAPClient hardening', () => { (SecureStore.getItemAsync as ReturnType).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).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).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).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).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 diff --git a/src/api/jmap-client.ts b/src/api/jmap-client.ts index a12922ac..b009d6be 100644 --- a/src/api/jmap-client.ts +++ b/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 { - 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);