diff --git a/stores/__tests__/auth-store-refresh-per-account.test.ts b/stores/__tests__/auth-store-refresh-per-account.test.ts new file mode 100644 index 000000000..4d20f3876 --- /dev/null +++ b/stores/__tests__/auth-store-refresh-per-account.test.ts @@ -0,0 +1,228 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Each test loads a fresh store: the clients and refresh timers live at module level. +let useAuthStore: typeof import('../auth-store').useAuthStore; +let useAccountStore: typeof import('../account-store').useAccountStore; + +type FetchInput = Parameters[0]; +type FetchInit = Parameters[1]; + +const SERVER = 'https://mail.example.test'; + +function account(id: string, slot: number, active: boolean) { + return { + id, + label: id, + serverUrl: SERVER, + username: `${id}@example.test`, + authMode: 'oauth' as const, + rememberMe: true, + displayName: id, + email: `${id}@example.test`, + lastLoginAt: Date.now(), + isConnected: false, + hasError: false, + isDefault: active, + cookieSlot: slot, + avatarColor: '#000', + }; +} + +function session(username: string) { + return { + username, + accounts: { acc: { name: username } }, + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acc' }, + apiUrl: `${SERVER}/jmap`, + downloadUrl: `${SERVER}/download`, + uploadUrl: `${SERVER}/upload`, + capabilities: {}, + }; +} + +function jsonResponse(status: number, body: unknown) { + const res = { + ok: status >= 200 && status < 300, + status, + redirected: false, + url: '', + headers: { get: () => null }, + json: async () => body, + text: async () => JSON.stringify(body), + clone() { return res; }, + }; + return res; +} + +/** + * Reproduction for the silent account drop (#989). + * + * Two OAuth accounts: A (active, slot 0) and B (background, slot 1). B's + * cached access token is already expired as far as the mail server is + * concerned, so B's first JMAP call gets a 401 and its client asks the store + * for a fresh token. The store must refresh SLOT 1 (B's refresh token) and + * hand B a token minted for B - not force-rotate the active account's + * refresh token and hand B the active account's token. + */ +describe('auth-store: token refresh is scoped to the account that needs it', () => { + const forcedRefreshSlots: number[] = []; + const deletedSessions: string[] = []; + let freshCounter = 0; + let bCachedExpired = true; + let slot1Refused = false; + + beforeEach(async () => { + vi.restoreAllMocks(); + vi.resetModules(); + ({ useAuthStore } = await import('../auth-store')); + ({ useAccountStore } = await import('../account-store')); + const browserNavigation = await import('@/lib/browser-navigation'); + sessionStorage.clear(); + localStorage.clear(); + forcedRefreshSlots.length = 0; + deletedSessions.length = 0; + freshCounter = 0; + bCachedExpired = true; + slot1Refused = false; + window.history.pushState({}, '', '/en'); + vi.spyOn(browserNavigation, 'replaceWindowLocation').mockImplementation(() => {}); + + useAccountStore.setState({ + accounts: [account('A', 0, true), account('B', 1, false)], + activeAccountId: 'A', + defaultAccountId: 'A', + }); + useAuthStore.setState({ + isAuthenticated: true, + isLoading: false, + error: null, + serverUrl: SERVER, + username: 'A@example.test', + client: null, + authMode: 'oauth', + rememberMe: true, + accessToken: null, + tokenExpiresAt: null, + connectionLost: false, + activeAccountId: 'A', + }); + + const fetchMock = vi.fn(async (input: FetchInput, init?: FetchInit) => { + const url = String(input); + const method = init?.method ?? 'GET'; + const headers = (init?.headers ?? {}) as Record; + + // Bulwark token route: plain PUT hands back the cached token for the + // slot; force=true mints a fresh one for that slot. + const m = url.match(/^\/api\/auth\/token\?slot=(\d+)(&force=true)?$/); + if (m && method === 'PUT') { + const slot = Number(m[1]); + const who = slot === 0 ? 'A' : 'B'; + if (m[2]) { + forcedRefreshSlots.push(slot); + if (slot === 1 && slot1Refused) return jsonResponse(401, { error: 'invalid_grant' }); + freshCounter += 1; + return jsonResponse(200, { access_token: `${who}-fresh-${freshCounter}`, expires_in: 3600 }); + } + return jsonResponse(200, { access_token: `${who}-cached`, expires_in: 3600 }); + } + if (url.startsWith('/api/auth/session') && method === 'DELETE') { + deletedSessions.push(url); + return jsonResponse(200, {}); + } + if (url.startsWith('/api/auth/token') && method === 'DELETE') { + return jsonResponse(200, {}); + } + if (url === '/api/auth/stalwart-context') { + return jsonResponse(200, { ok: true }); + } + + // Mail server: B's cached token is expired; everything else is accepted + // and answered with the session of the account the token belongs to. + if (url === `${SERVER}/.well-known/jmap`) { + const auth = headers.Authorization ?? ''; + if (auth === 'Bearer B-cached' && bCachedExpired) return jsonResponse(401, {}); + const owner = auth.includes('Bearer A') ? 'A' : 'B'; + return jsonResponse(200, session(`${owner}@example.test`)); + } + // Anything else (display-name lookups etc.) is irrelevant here. + return jsonResponse(404, {}); + }); + vi.stubGlobal('fetch', fetchMock); + }); + + afterEach(() => { + for (const client of useAuthStore.getState().getAllConnectedClients().values()) { + try { client.disconnect(); } catch { /* ignore */ } + } + vi.unstubAllGlobals(); + }); + + it('refreshes the background account\'s own slot and hands its client its own token', async () => { + await useAuthStore.getState().checkAuth(); + + // Background restore of B runs fire-and-forget after A is up. + await vi.waitFor(() => { + expect(forcedRefreshSlots.length).toBeGreaterThan(0); + }); + // Give the retry-after-refresh a moment to land. + await vi.waitFor(() => { + expect(useAuthStore.getState().getClientForAccount('B')).toBeDefined(); + }); + + // The refresh B asked for must be spent on B's slot, never on A's. + expect(forcedRefreshSlots).toEqual([1]); + + // B ends up holding a token minted for B, not A's. + const clientB = useAuthStore.getState().getClientForAccount('B')!; + expect(clientB.getAuthHeader()).toMatch(/^Bearer B-fresh-/); + + // Nobody got evicted and the active session is untouched. + const ids = useAccountStore.getState().accounts.map((a) => a.id).sort(); + expect(ids).toEqual(['A', 'B']); + expect(useAuthStore.getState().isAuthenticated).toBe(true); + expect(deletedSessions).toEqual([]); + }); + + it('renews each account through its own slot when the scheduled refresh fires', async () => { + bCachedExpired = false; + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + await useAuthStore.getState().checkAuth(); + await vi.waitFor(() => { + expect(useAuthStore.getState().getClientForAccount('B')).toBeDefined(); + }); + expect(forcedRefreshSlots).toEqual([]); + + await vi.advanceTimersByTimeAsync(3600_000); + + expect([...forcedRefreshSlots].sort()).toEqual([0, 1]); + expect(useAuthStore.getState().getClientForAccount('A')!.getAuthHeader()).toMatch(/^Bearer A-fresh-/); + expect(useAuthStore.getState().getClientForAccount('B')!.getAuthHeader()).toMatch(/^Bearer B-fresh-/); + expect(useAuthStore.getState().accessToken).toMatch(/^A-fresh-/); + } finally { + vi.useRealTimers(); + } + }); + + it('drops only the background account whose refresh token is refused', async () => { + bCachedExpired = false; + slot1Refused = true; + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + await useAuthStore.getState().checkAuth(); + await vi.waitFor(() => { + expect(useAuthStore.getState().getClientForAccount('B')).toBeDefined(); + }); + + await vi.advanceTimersByTimeAsync(3600_000); + + expect(useAccountStore.getState().accounts.map((a) => a.id)).toEqual(['A']); + expect(useAuthStore.getState().isAuthenticated).toBe(true); + expect(useAuthStore.getState().activeAccountId).toBe('A'); + expect(deletedSessions).not.toContain('/api/auth/session?slot=0'); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index 1644a5e7c..f80480a88 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -92,13 +92,13 @@ interface AuthState { loginWithServerSso: (code: string, state: string) => Promise; loginDemo: () => Promise; /** - * Obtain a usable access token for the active account. + * Obtain a usable access token for `accountId`, or the active account. * * Renews against the IdP by default. `allowCached` lets a session restore * reuse the token the server still holds for this slot - IdPs that gate * refresh tokens behind an `nbf` claim reject an early renewal outright. */ - refreshAccessToken: (options?: { allowCached?: boolean }) => Promise; + refreshAccessToken: (options?: { allowCached?: boolean; accountId?: string }) => Promise; /** * Sign out of the active account. Switches to the next signed-in account * when there is one; otherwise ends the identity provider's session too @@ -1383,7 +1383,7 @@ export const useAuthStore = create()( } if (bearerToken) { - client = JMAPClient.withBearer(serverUrl, bearerToken, username, () => get().refreshAccessToken()); + client = JMAPClient.withBearer(serverUrl, bearerToken, username, () => get().refreshAccessToken({ accountId })); await client.connect(); oauthAccessToken = bearerToken; upgradedToOAuth = true; @@ -1517,7 +1517,7 @@ export const useAuthStore = create()( // Schedule token refresh for TOTP-upgraded sessions if (upgradedToOAuth && oauthExpiresIn > 0) { - scheduleRefresh(oauthExpiresIn, get().refreshAccessToken, accountId); + scheduleRefresh(oauthExpiresIn, () => get().refreshAccessToken({ accountId }), accountId); } // Sync settings from server (only if enabled) @@ -1775,8 +1775,10 @@ export const useAuthStore = create()( const { access_token, expires_in } = await tokenRes.json(); - const refreshFn = get().refreshAccessToken; - const client = JMAPClient.withBearer(serverUrl, access_token, '', () => refreshFn()); + // The account id comes from the session, so the refresh hook reads + // it once it is known below. + let boundAccountId: string | undefined; + const client = JMAPClient.withBearer(serverUrl, access_token, '', () => get().refreshAccessToken({ accountId: boundAccountId })); await client.connect(); const jmapUsername = client.getUsername(); @@ -1793,6 +1795,7 @@ export const useAuthStore = create()( // Register in account store const accountId = generateAccountId(username, serverUrl); + boundAccountId = accountId; // Snapshot current account if switching away and clear stores so // the new account starts with a clean email/contact/calendar state. @@ -1852,7 +1855,7 @@ export const useAuthStore = create()( }); }).catch(() => {}); - scheduleRefresh(expires_in, get().refreshAccessToken, accountId); + scheduleRefresh(expires_in, () => get().refreshAccessToken({ accountId }), accountId); notifyParent('sso:auth-success', { username }); @@ -1928,8 +1931,8 @@ export const useAuthStore = create()( throw new Error('Server URL not configured'); } - const refreshFn = get().refreshAccessToken; - const client = JMAPClient.withBearer(ssoServerUrl, access_token, '', () => refreshFn()); + let boundAccountId: string | undefined; + const client = JMAPClient.withBearer(ssoServerUrl, access_token, '', () => get().refreshAccessToken({ accountId: boundAccountId })); await client.connect(); const jmapUsername = client.getUsername(); @@ -1941,6 +1944,7 @@ export const useAuthStore = create()( initializeFeatureStores(client); const accountId = generateAccountId(username, ssoServerUrl); + boundAccountId = accountId; const prevAccountId = get().activeAccountId; if (prevAccountId && prevAccountId !== accountId) { @@ -1996,7 +2000,7 @@ export const useAuthStore = create()( }); }).catch(() => {}); - scheduleRefresh(expires_in, get().refreshAccessToken, accountId); + scheduleRefresh(expires_in, () => get().refreshAccessToken({ accountId }), accountId); notifyParent('sso:auth-success', { username }); @@ -2027,15 +2031,20 @@ export const useAuthStore = create()( }, refreshAccessToken: async (options) => { - if (refreshPromise) return refreshPromise; - - const accountId = get().activeAccountId; - if (accountId && refreshPromises.has(accountId)) { - return refreshPromises.get(accountId)!; + // Every multi-account client and timer passes its own account: renewing + // the active slot on a background account's behalf would hand that + // client another account's token (#989). + const accountId = options?.accountId ?? get().activeAccountId; + if (accountId) { + const inFlight = refreshPromises.get(accountId); + if (inFlight) return inFlight; + } else if (refreshPromise) { + return refreshPromise; } const account = accountId ? useAccountStore.getState().getAccountById(accountId) : null; const slot = account?.cookieSlot ?? 0; + const isActive = !accountId || accountId === get().activeAccountId; // The account is being signed out: a renewal now would only undo that. if (closingSlots.has(slot)) return null; @@ -2054,6 +2063,11 @@ export const useAuthStore = create()( // maintenance windows and offline spells. if (res.status === 401) { resetRefreshBackoff(accountId ?? undefined); + // Only this login's refresh token was refused. + if (accountId && !isActive) { + get().removeAccount(accountId); + return null; + } notifyParent('sso:session-expired'); markSessionExpired(); get().logout({ endProviderSession: false }); @@ -2075,7 +2089,7 @@ export const useAuthStore = create()( if (shouldRetryRefresh(slot, accountId ?? undefined)) { const retryIn = nextRefreshRetrySeconds(accountId ?? undefined); debug.error(`Token refresh unavailable (${res.status}), retrying with backoff`); - scheduleRefresh(retryIn, get().refreshAccessToken, accountId ?? undefined); + scheduleRefresh(retryIn, () => get().refreshAccessToken({ accountId: accountId ?? undefined }), accountId ?? undefined); } return null; } @@ -2085,7 +2099,8 @@ export const useAuthStore = create()( // Signed out while the renewal was in flight: nothing to keep alive. if (closingSlots.has(slot)) return null; - get().client?.updateAccessToken(access_token); + const owner = (accountId ? clients.get(accountId) : undefined) ?? (isActive ? get().client : null); + owner?.updateAccessToken(access_token); if (account) { await syncStalwartAuthContext( @@ -2096,13 +2111,15 @@ export const useAuthStore = create()( ); } - set({ - accessToken: access_token, - tokenExpiresAt: Date.now() + expires_in * 1000, - }); + if (isActive) { + set({ + accessToken: access_token, + tokenExpiresAt: Date.now() + expires_in * 1000, + }); + } resetRefreshBackoff(accountId ?? undefined); - scheduleRefresh(expires_in, get().refreshAccessToken, accountId ?? undefined); + scheduleRefresh(expires_in, () => get().refreshAccessToken({ accountId: accountId ?? undefined }), accountId ?? undefined); if (accountId) void authHooks.onTokenRefresh.emit({ accountId }); return access_token; } catch (error) { @@ -2110,17 +2127,17 @@ export const useAuthStore = create()( // not a rejection. Keep the session and retry with backoff. debug.error('Token refresh failed, retrying with backoff:', error); if (shouldRetryRefresh(slot, accountId ?? undefined)) { - scheduleRefresh(nextRefreshRetrySeconds(accountId ?? undefined), get().refreshAccessToken, accountId ?? undefined); + scheduleRefresh(nextRefreshRetrySeconds(accountId ?? undefined), () => get().refreshAccessToken({ accountId: accountId ?? undefined }), accountId ?? undefined); } return null; } finally { - refreshPromise = null; if (accountId) refreshPromises.delete(accountId); + else refreshPromise = null; } })(); - refreshPromise = promise; if (accountId) refreshPromises.set(accountId, promise); + else refreshPromise = promise; return promise; }, @@ -2371,12 +2388,11 @@ export const useAuthStore = create()( const res = await fetchSlotAccessToken(targetAccount.cookieSlot); if (res.ok) { const { access_token, expires_in } = await res.json(); - const refreshFn = get().refreshAccessToken; - targetClient = JMAPClient.withBearer(targetAccount.serverUrl, access_token, targetAccount.username, () => refreshFn()); + targetClient = JMAPClient.withBearer(targetAccount.serverUrl, access_token, targetAccount.username, () => get().refreshAccessToken({ accountId })); bindClientStatusHandlers(targetClient, set, get, accountId); await targetClient.connect(); clients.set(accountId, targetClient); - scheduleRefresh(expires_in, get().refreshAccessToken, accountId); + scheduleRefresh(expires_in, () => get().refreshAccessToken({ accountId }), accountId); await syncStalwartAuthContext( targetAccount.serverUrl, targetAccount.username, @@ -2654,13 +2670,12 @@ export const useAuthStore = create()( const res = await fetchSlotAccessToken(account.cookieSlot); if (res.ok) { const { access_token, expires_in } = await res.json(); - const refreshFn = get().refreshAccessToken; - const client = JMAPClient.withBearer(account.serverUrl, access_token, account.username, () => refreshFn()); + const client = JMAPClient.withBearer(account.serverUrl, access_token, account.username, () => get().refreshAccessToken({ accountId: account.id })); bindClientStatusHandlers(client, set, get, account.id); const contextSync = syncStalwartAuthContext(account.serverUrl, account.username, client.getAuthHeader(), account.cookieSlot); await client.connect(); clients.set(account.id, client); - scheduleRefresh(expires_in, get().refreshAccessToken, account.id); + scheduleRefresh(expires_in, () => get().refreshAccessToken({ accountId: account.id }), account.id); await contextSync; accountStore.updateAccount(account.id, { isConnected: true, hasError: false }); void syncAccountDisplayName(account.id, client);