From 706550daf492f8733249ba98502fe50ea67c6ed7 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 08:33:03 +0100 Subject: [PATCH] fix: keep local mail and the outbox when an automatic sign-out wasn't a real refusal of the login, for 30 days at most --- app/api/auth/token/route.ts | 14 +- lib/__tests__/oauth-token-route.test.ts | 15 ++ lib/offline/credentials-rejected.ts | 9 +- lib/offline/flush.ts | 4 +- lib/offline/offline-mode.ts | 59 +++-- lib/offline/queue.ts | 4 +- lib/offline/state.ts | 45 ++++ lib/sign-out-cleanup.ts | 44 ++-- .../__tests__/auth-store-keep-local.test.ts | 206 ++++++++++++++++++ stores/account-store.ts | 3 + stores/auth-store.ts | 65 ++++-- 11 files changed, 411 insertions(+), 57 deletions(-) create mode 100644 stores/__tests__/auth-store-keep-local.test.ts diff --git a/app/api/auth/token/route.ts b/app/api/auth/token/route.ts index 59b5834c7..df7aaa169 100644 --- a/app/api/auth/token/route.ts +++ b/app/api/auth/token/route.ts @@ -292,7 +292,10 @@ export async function PUT(request: NextRequest) { cookieStore.delete(refreshTokenServerCookieName(slot)); cookieStore.delete(accessTokenCookieName(slot)); clearIdToken(cookieStore, slot, request.nextUrl.basePath); - return NextResponse.json({ error: 'Refresh failed' }, { status: 401 }); + // Only invalid_grant says the login itself is over. invalid_client, or + // a provider refusing an early refresh (#552), says nothing about it, + // and the client keeps the account's local data for those. + return NextResponse.json({ error: oauthErrorCode(errorText) === 'invalid_grant' ? 'invalid_grant' : 'Refresh failed' }, { status: 401 }); } return NextResponse.json({ error: 'Token endpoint unavailable' }, { status: 503 }); } @@ -336,6 +339,15 @@ export async function PUT(request: NextRequest) { } } +function oauthErrorCode(body: string): string | null { + try { + const code = (JSON.parse(body) as { error?: unknown }).error; + return typeof code === 'string' ? code : null; + } catch { + return null; + } +} + /** * Revoke a slot's refresh token and clear its token cookies. With * `endSession`, also returns the URL that ends the provider's session, or null diff --git a/lib/__tests__/oauth-token-route.test.ts b/lib/__tests__/oauth-token-route.test.ts index 1c225faf9..0170c197b 100644 --- a/lib/__tests__/oauth-token-route.test.ts +++ b/lib/__tests__/oauth-token-route.test.ts @@ -170,6 +170,21 @@ describe('oauth token route - access token cache (#552)', () => { expect(cookieStore.get('jmap_rt')).toBeTruthy(); }); + it('names a real invalid_grant, and only that, for the client to wipe on', async () => { + const refuse = (status: number, body: string) => fetchMock.mockResolvedValueOnce(new Response(body, { status })); + cookieStore.set('jmap_rt', 'refresh-token'); + refuse(400, JSON.stringify({ error: 'invalid_grant' })); + expect(await callPut({ force: 'true' })).toMatchObject({ status: 401, body: { error: 'invalid_grant' } }); + + for (const body of [JSON.stringify({ error: 'invalid_client' }), JSON.stringify({ error: 'invalid_request', error_description: 'Token is not valid yet' }), 'Forbidden']) { + cookieStore.set('jmap_rt', 'refresh-token'); + refuse(403, body); + const { status, body: answer } = await callPut({ force: 'true' }); + expect(status).toBe(401); + expect(answer.error).not.toBe('invalid_grant'); + } + }); + it('answers 503 when the token endpoint is out of reach, which the client waits out', async () => { cookieStore.set('jmap_rt', 'refresh-token'); fetchMock.mockRejectedValue(new TypeError('fetch failed')); diff --git a/lib/offline/credentials-rejected.ts b/lib/offline/credentials-rejected.ts index 9511494f3..a42282998 100644 --- a/lib/offline/credentials-rejected.ts +++ b/lib/offline/credentials-rejected.ts @@ -1,8 +1,9 @@ /** - * The server refused this account's refresh token for good. Nothing wipes on - * this yet: a refused account is dropped, and its local data stays for the next - * sign-in. HOOK: the wipe work (lib/store/wipe) registers here to act on a - * permanent rejection. Free of the stores, so the auth store can call it. + * The server really refused this login (`invalid_grant`), which the auth store + * follows with a sign-out that wipes the account's local data. An outage or any + * other refusal doesn't come here, and keeps the data. HOOK: lib/store/wipe + * registers here for what else a permanent rejection should do. Free of the + * stores, so the auth store can call it. */ type Listener = (accountId: string) => void; diff --git a/lib/offline/flush.ts b/lib/offline/flush.ts index 021c39348..53e250173 100644 --- a/lib/offline/flush.ts +++ b/lib/offline/flush.ts @@ -42,7 +42,7 @@ import { type OutboxEntry, type OutboxError, } from './queue'; -import { useOfflineStore } from './state'; +import { visibleAccounts } from './state'; import type { OfflineNotice } from './offline-mode'; export interface FlushDeps { @@ -152,7 +152,7 @@ export async function flushQueue(account: string, client: JMAPClient, deps: Flus /** Marks sends of closed tabs uncertain (they may have gone out). */ export async function settleStaleSends(account?: string, now = Date.now()): Promise { - const accounts = account ? [account] : useOfflineStore.getState().enabled; + const accounts = account ? [account] : visibleAccounts(); await Promise.all(accounts.map((id) => getStore().area(id, 'outbox.settleStale', { before: now - SENDING_STALE_MS }))); } diff --git a/lib/offline/offline-mode.ts b/lib/offline/offline-mode.ts index 1993f154d..03b0e140d 100644 --- a/lib/offline/offline-mode.ts +++ b/lib/offline/offline-mode.ts @@ -11,13 +11,15 @@ * - Changes and sends made offline wait in the store's outbox and go out in * order once the server answers again and the organisation's policy was * fetched afresh (flush.ts). - * - Everything is wiped at a full sign-out and the automatic sign-outs - * (lib/sign-out-cleanup.ts deletes the store and its keys), for one account - * when it signs out, is removed or switches offline mode off, and when the - * organisation forbids it (checked on every policy fetch). An account the - * app drops by itself (its session cookie ran out, say) keeps its data for - * the next sign-in. Unsent messages a sign-out deletes without the user's - * word are named (lost.ts). + * - Everything is wiped at a full sign-out, at the session limits and when the + * server really refuses the login (`invalid_grant`), by + * lib/sign-out-cleanup.ts; for one account when it signs out, is removed or + * switches offline mode off, and when the organisation forbids it (checked on + * every policy fetch). An account the app drops by itself because it couldn't + * confirm the login (an outage, an unknown refusal, a session cookie that ran + * out) keeps its data for 30 days or the next sign-in, hidden from everyone + * else. Unsent messages a sign-out deletes without the user's word are named + * (lost.ts). */ import { IS_LITE } from '@/lib/lite'; @@ -52,7 +54,7 @@ import { type PolicyFingerprint, } from './queue'; import { clearHandoffs } from '@/lib/launch-handoff'; -import { isOfflineNow, OFFLINE_ACCOUNTS_KEY, readEnabledFrom, serverKey, setServerDown, useOfflineStore, writeEnabled, type OutboxRow } from './state'; +import { clearEvicted, EVICTED_KEEP_MS, isEvicted, isOfflineNow, OFFLINE_ACCOUNTS_KEY, readEnabledFrom, readEvicted, visibleAccounts, serverKey, setServerDown, useOfflineStore, writeEnabled, type OutboxRow } from './state'; import { flushQueue, settleStaleSends } from './flush'; import { LEGACY_DB_NAME, moveLegacyOutbox } from './migrate-legacy'; import { deleteShellCaches, tellServiceWorker } from './shell'; @@ -470,7 +472,7 @@ export async function refreshQueueState(): Promise { if (!offlineSupported()) return; // Nothing is kept for an account that hasn't it on; and opening the // store would create it (on the sign-in page, after a sign-out). - if (useOfflineStore.getState().enabled.length === 0) { + if (visibleAccounts().length === 0) { useOfflineStore.setState({ outbox: [], pendingChanges: 0, conflicts: [] }); return; } @@ -494,9 +496,9 @@ export async function refreshQueueState(): Promise { } export async function refreshUsage(): Promise { - if (!offlineSupported() || useOfflineStore.getState().enabled.length === 0) return; + if (!offlineSupported() || visibleAccounts().length === 0) return; try { - const usage = await Promise.all(useOfflineStore.getState().enabled.map((account) => getStore().area<{ bytes: number }>(account, 'mail.usage'))); + const usage = await Promise.all(visibleAccounts().map((account) => getStore().area<{ bytes: number }>(account, 'mail.usage'))); useOfflineStore.setState({ usageBytes: usage.reduce((sum, { bytes }) => sum + bytes, 0) }); } catch { /* deleted meanwhile */ @@ -699,10 +701,13 @@ export async function wipeAccount(account: string): Promise { const timer = syncTimers.get(account); if (timer) clearTimeout(timer); syncTimers.delete(account); + // Waited for a sign-in that never came: whoever is here now mustn't read its subjects. + if (isEvicted(account)) named.add(account); if (useOfflineStore.getState().enabled.includes(account) || useOfflineStore.getState().outbox.some((row) => row.account === account)) { await nameLost(account, useAuthStore.getState().isAuthenticated); } named.delete(account); + clearEvicted(account); if (useOfflineStore.getState().enabled.includes(account)) { writeEnabled(useOfflineStore.getState().enabled.filter((id) => id !== account)); } @@ -743,6 +748,8 @@ export async function wipeEverything(): Promise { /** Whether the policy of an account is known well enough to decide on it. */ function policyDecided(account: string): boolean { + // Dropped, with no groups to wait for: what applies to everyone reaches it. + if (isEvicted(account)) return true; const auth = useAuthStore.getState(); if (account === auth.activeAccountId || !auth.activeAccountId) return !usePolicyStore.getState().subjectPending; // Another signed-in account: its groups have to be known first (a group @@ -758,9 +765,11 @@ export function enforcePolicy(): void { for (const account of useOfflineStore.getState().enabled) { if (!policyDecided(account)) continue; if (!offlineAllowed(account)) { + // Whoever is here now wasn't signed in to a dropped account. + const someoneElses = isEvicted(account); clearHandoffs(); void wipeAccount(account); - notify({ kind: 'forbidden' }); + if (!someoneElses) notify({ kind: 'forbidden' }); } } } @@ -882,6 +891,7 @@ export function startOfflineMode(): () => void { cleanups.push(onAccountRemoved((id, reason) => { if (reason === 'explicit') void wipeAccount(id); })); + void sweepAccountsGone(); // Session, then a sync, for every account signed in with offline mode on. const syncSignedIn = () => { @@ -923,23 +933,40 @@ export function startOfflineMode(): () => void { }; } +/** + * Data no signed-in account vouches for. A dropped account's waits 30 days. One + * that left the registry on purpose before this ran (a login without "remember + * me" ends at a reload, before the host mounts) goes now, naming lost mail. + */ +async function sweepAccountsGone(now = Date.now()): Promise { + const evicted = readEvicted(); + const accounts = useAccountStore.getState().accounts; + for (const account of useOfflineStore.getState().enabled) { + const at = evicted[account]; + const gone = at !== undefined ? now - at > EVICTED_KEEP_MS : !accounts.some((entry) => entry.id === account); + if (gone) await wipeAccount(account).catch(() => {}); + } +} + /** How often the store catches up while the tab is open, on top of pushes. */ export const SYNC_INTERVAL_MS = 15 * 60 * 1000; // A full sign-out deletes the store (lib/sign-out-cleanup.ts); forget what // is held in memory as well. -onFullSignOut(() => { +onFullSignOut(({ keepLocalStore }) => { for (const timer of syncTimers.values()) clearTimeout(timer); syncTimers.clear(); - getStore().stop(); + if (!keepLocalStore) getStore().stop(); confirmed.clear(); named.clear(); - useOfflineStore.setState({ enabled: [], outbox: [], pendingChanges: 0, conflicts: [], usageBytes: 0, syncing: [], lastSyncedAt: {}, startedOffline: false }); + useOfflineStore.setState((s) => ({ enabled: keepLocalStore ? s.enabled : [], outbox: [], pendingChanges: 0, conflicts: [], usageBytes: 0, syncing: [], lastSyncedAt: {}, startedOffline: false })); }); // Every sign-out the user didn't agree to lose mail at (the session limits, // a refused token renewal, a removed account) names the unsent messages. -onBeforeSignOut(async (accountId) => { +onBeforeSignOut(async (accountId, { keepLocalStore }) => { + // Nothing is lost: it waits for the next sign-in. + if (keepLocalStore) return; const stillSignedIn = accountId !== 'all' && useAccountStore.getState().accounts.some((a) => a.id !== accountId); await nameLost(accountId, stillSignedIn); }); diff --git a/lib/offline/queue.ts b/lib/offline/queue.ts index 5686f1fc5..fccd14044 100644 --- a/lib/offline/queue.ts +++ b/lib/offline/queue.ts @@ -9,7 +9,7 @@ import { requestOutboxSync } from '@/lib/pwa-notify/outbox-sync'; import { getStore } from '@/lib/store/client/api'; import type { ConflictSummary, OutboxError, OutboxStatus, OutboxSummary, PolicyFingerprint, SendItem, StoredConflict, StoredOutboxEntry } from '@/lib/store/outbox/items'; -import { useOfflineStore } from './state'; +import { visibleAccounts } from './state'; export { LOCAL_BLOB_PREFIX, heldBlobIds, isLocalBlobId } from '@/lib/store/outbox/items'; export type { EmailSendItem, OutboxError, OutboxStatus, OutboxSummary, PolicyFingerprint, RawSendItem, SendItem } from '@/lib/store/outbox/items'; @@ -40,7 +40,7 @@ interface StoredQueue { /** Everything waiting for one account (or every account with it on), oldest first. */ export async function readQueue(account?: string): Promise<{ patches: number; outbox: OutboxEntry[]; conflicts: ConflictEntry[] }> { - const accounts = account ? [account] : useOfflineStore.getState().enabled; + const accounts = account ? [account] : visibleAccounts(); const read = await Promise.all(accounts.map(async (id) => ({ id, queue: await getStore().area(id, 'outbox.read') }))); return { patches: read.reduce((sum, { queue }) => sum + queue.patches, 0), diff --git a/lib/offline/state.ts b/lib/offline/state.ts index 1ba184282..4508c07e5 100644 --- a/lib/offline/state.ts +++ b/lib/offline/state.ts @@ -74,6 +74,51 @@ export function writeEnabled(enabled: string[]): void { } } +/** localStorage: accounts the app dropped by itself, and when. Their data waits for a new sign-in. */ +export const OFFLINE_EVICTED_KEY = 'bulwark-offline-evicted'; +/** How long that data waits. */ +export const EVICTED_KEEP_MS = 30 * 24 * 60 * 60 * 1000; + +export function readEvicted(): Record { + try { + const parsed = JSON.parse(localStorage.getItem(OFFLINE_EVICTED_KEY) ?? '{}') as unknown; + return parsed && typeof parsed === 'object' ? parsed as Record : {}; + } catch { + return {}; + } +} + +function writeEvicted(evicted: Record): void { + try { + if (Object.keys(evicted).length > 0) localStorage.setItem(OFFLINE_EVICTED_KEY, JSON.stringify(evicted)); + else localStorage.removeItem(OFFLINE_EVICTED_KEY); + } catch { + /* storage unavailable */ + } +} + +/** Only an account that keeps mail here has anything to wait for. */ +export function markEvicted(account: string, now = Date.now()): void { + if (useOfflineStore.getState().enabled.includes(account)) writeEvicted({ ...readEvicted(), [account]: now }); +} + +export function clearEvicted(account: string): void { + const evicted = readEvicted(); + if (!(account in evicted)) return; + delete evicted[account]; + writeEvicted(evicted); +} + +export function isEvicted(account: string): boolean { + return account in readEvicted(); +} + +/** The accounts whose data may show here: not those waiting for a sign-in, which may be someone else's. */ +export function visibleAccounts(): string[] { + const evicted = readEvicted(); + return useOfflineStore.getState().enabled.filter((account) => !(account in evicted)); +} + export function serverKey(url: string): string { return url.replace(/\/+$/, '').toLowerCase(); } diff --git a/lib/sign-out-cleanup.ts b/lib/sign-out-cleanup.ts index 9496e43d5..f7ca42832 100644 --- a/lib/sign-out-cleanup.ts +++ b/lib/sign-out-cleanup.ts @@ -36,25 +36,38 @@ const SIGNED_OUT_STORAGE_KEYS = [ // Offline mode's "Keep mail on this device" (lib/offline/state.ts): the // next person signing in here starts with it off. 'bulwark-offline-accounts', + // Which accounts' data waits for a new sign-in, and until when. + 'bulwark-offline-evicted', ]; +const LOCAL_STORE_KEYS = new Set(['bulwark-offline-accounts', 'bulwark-offline-evicted']); + const SIGNED_OUT_SIGNAL = 'bulwark:signed-out'; // Stores the auth store can't import (they import it) clear themselves here. -const signedOutCleanups = new Set<() => void>(); +const signedOutCleanups = new Set<(options: SignOutCleanupOptions) => void>(); + +export interface SignOutCleanupOptions { + /** + * Keep the local store and its keys. For a sign-out the app decided on + * because it couldn't confirm the login (an outage, an unknown refusal), or + * when an account that was dropped that way still has data here. + */ + keepLocalStore?: boolean; +} /** Run `cleanup` when everyone signs out in this tab. */ -export function onFullSignOut(cleanup: () => void): void { +export function onFullSignOut(cleanup: (options: SignOutCleanupOptions) => void): void { signedOutCleanups.add(cleanup); } // Work that has to reach the server before an account signs out (changes // still queued for it). The auth store waits for it within its sign-out // budget, next to the plugins' onBeforeLogout. -const beforeSignOutTasks = new Set<(accountId: string) => Promise>(); +const beforeSignOutTasks = new Set<(accountId: string, options: SignOutCleanupOptions) => Promise>(); /** Run `task` before an account signs out; `accountId` is 'all' when every account does. */ -export function onBeforeSignOut(task: (accountId: string) => Promise): () => void { +export function onBeforeSignOut(task: (accountId: string, options: SignOutCleanupOptions) => Promise): () => void { beforeSignOutTasks.add(task); return () => beforeSignOutTasks.delete(task); } @@ -81,14 +94,15 @@ export async function passSignOutGates(accountId: string): Promise { return true; } -export async function runBeforeSignOut(accountId: string): Promise { - await Promise.all([...beforeSignOutTasks].map((task) => task(accountId).catch(() => {}))); +export async function runBeforeSignOut(accountId: string, options: SignOutCleanupOptions = {}): Promise { + await Promise.all([...beforeSignOutTasks].map((task) => task(accountId, options).catch(() => {}))); } /** Remove the accounts' leftovers from browser storage. */ -export function purgeSignedOutData(): void { +export function purgeSignedOutData(options: SignOutCleanupOptions = {}): void { if (typeof window === 'undefined') return; for (const key of SIGNED_OUT_STORAGE_KEYS) { + if (options.keepLocalStore && LOCAL_STORE_KEYS.has(key)) continue; try { window.localStorage.removeItem(key); } catch { @@ -111,18 +125,20 @@ export function purgeSignedOutData(): void { // Offline mode (#523): the local store with the mail and the outbox, the keys // it was encrypted with, and the app shell the service worker kept. Also the // database of older versions, in case it wasn't moved yet. - void purgeStoreFiles(); - try { - window.indexedDB?.deleteDatabase('bulwark-offline'); - } catch { - /* storage unavailable */ + if (!options.keepLocalStore) { + void purgeStoreFiles(); + try { + window.indexedDB?.deleteDatabase('bulwark-offline'); + } catch { + /* storage unavailable */ + } + forgetOfflineShell(); } - forgetOfflineShell(); // Files and links on their way into a draft or an import. clearHandoffs(); for (const cleanup of signedOutCleanups) { try { - cleanup(); + cleanup(options); } catch { /* one store failing must not keep the others' data */ } diff --git a/stores/__tests__/auth-store-keep-local.test.ts b/stores/__tests__/auth-store-keep-local.test.ts new file mode 100644 index 000000000..c1db3bae1 --- /dev/null +++ b/stores/__tests__/auth-store-keep-local.test.ts @@ -0,0 +1,206 @@ +import 'fake-indexeddb/auto'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// What an automatic sign-out does to the mail on this device. A real refusal of +// the login wipes it, like a sign-out. An outage, or a refusal that says nothing +// about the login, keeps it for the next sign-in. + +const purge = vi.hoisted(() => ({ files: 0 })); +vi.mock('@/lib/store/client/purge', () => ({ purgeStoreFiles: async () => { purge.files += 1; }, deleteStorePool: async () => {} })); +vi.mock('@/lib/compose-flush-registry', () => ({ flushComposers: vi.fn(async () => ({ total: 0, settled: 0, failed: 0, timedOut: false })) })); + +let useAuthStore: typeof import('../auth-store').useAuthStore; +let useAccountStore: typeof import('../account-store').useAccountStore; +let state: typeof import('@/lib/offline/state'); +let offline: typeof import('@/lib/offline/offline-mode'); +let queue: typeof import('@/lib/offline/queue'); +let lost: typeof import('@/lib/offline/lost'); +let store: import('@/lib/store/__tests__/helpers/in-process-store').InProcessStore; + +const ALICE = 'alice@mail.example.test'; + +function entry(id: string, cookieSlot: number) { + return { + id, label: id, serverUrl: 'https://mail.example.test', username: id.split('@mail')[0], authMode: 'oauth', rememberMe: true, + displayName: id, email: id, lastLoginAt: Date.now(), isConnected: true, hasError: false, isDefault: cookieSlot === 0, cookieSlot, avatarColor: '#000', + }; +} + +function answer(status: number, body: unknown = {}): Response { + return new Response(typeof body === 'string' ? body : JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }); +} + +function tokenAnswers(next: () => Response): void { + vi.stubGlobal('fetch', vi.fn(async (input: unknown) => (String(input).includes('/api/auth/token') ? next() : answer(200)))); +} + +/** `times`: as often as the retry timer would run it. */ +async function refresh(times = 1): Promise { + for (let i = 0; i < times; i += 1) await useAuthStore.getState().refreshAccessToken({ accountId: ALICE }); + await vi.waitFor(() => expect(useAuthStore.getState().isAuthenticated).toBe(false)); +} + +beforeEach(async () => { + vi.resetModules(); + purge.files = 0; + localStorage.clear(); + sessionStorage.clear(); + ({ useAuthStore } = await import('../auth-store')); + ({ useAccountStore } = await import('../account-store')); + state = await import('@/lib/offline/state'); + offline = await import('@/lib/offline/offline-mode'); + queue = await import('@/lib/offline/queue'); + lost = await import('@/lib/offline/lost'); + const browserNavigation = await import('@/lib/browser-navigation'); + vi.spyOn(browserNavigation, 'replaceWindowLocation').mockImplementation(() => {}); + const { InProcessStore } = await import('@/lib/store/__tests__/helpers/in-process-store'); + const { setStoreBackend } = await import('@/lib/store/client/api'); + store = new InProcessStore(); + setStoreBackend(store); + useAccountStore.setState({ accounts: [entry(ALICE, 0)], activeAccountId: ALICE, defaultAccountId: ALICE } as never); + useAuthStore.setState({ isAuthenticated: true, activeAccountId: ALICE, authMode: 'oauth', serverUrl: 'https://mail.example.test', username: 'alice' }); + state.writeEnabled([ALICE]); + await queue.queueSend(ALICE, { subject: 'Unsent', to: [] }, { kind: 'email', args: [] }); +}, 60_000); + +function expectKept(): void { + expect(purge.files).toBe(0); + expect(state.useOfflineStore.getState().enabled).toEqual([ALICE]); + expect(localStorage.getItem(state.OFFLINE_ACCOUNTS_KEY)).toContain(ALICE); + expect(state.isEvicted(ALICE)).toBe(true); + expect(store.removed).toEqual([]); + expect(store.stopped).toBe(0); + // Nothing was lost, so nothing is named at the sign-in page. + expect(lost.consumeLost()).toEqual([]); +} + +describe('an automatic sign-out of the active account', () => { + it('keeps the data through a run of 502s from a proxy', async () => { + tokenAnswers(() => answer(502, 'Bad Gateway')); + await refresh(5); + expectKept(); + }); + + it('keeps the data through a run of the route\'s own 500s', async () => { + tokenAnswers(() => answer(500, { error: 'Internal server error' })); + await refresh(5); + expectKept(); + }); + + it('keeps the data on a refusal that says nothing about the login', async () => { + tokenAnswers(() => answer(401, { error: 'Refresh failed' })); + await refresh(); + expectKept(); + }); + + it('keeps the data on a refusal it can\'t read', async () => { + tokenAnswers(() => answer(401, 'nope')); + await refresh(); + expectKept(); + }); + + it('wipes on a real invalid_grant, and names the lost mail', async () => { + tokenAnswers(() => answer(401, { error: 'invalid_grant' })); + offline.startOfflineMode(); + await refresh(); + await vi.waitFor(() => expect(purge.files).toBeGreaterThan(0)); + expect(localStorage.getItem(state.OFFLINE_ACCOUNTS_KEY)).toBeNull(); + expect(lost.consumeLost()).toEqual(['Unsent']); + }); + + it('wipes at the organisation\'s session limit', async () => { + const { SESSION_TOO_OLD } = await import('@/lib/session-policy/constants'); + tokenAnswers(() => answer(401, { error: SESSION_TOO_OLD, maxHours: 8 })); + await refresh(); + await vi.waitFor(() => expect(purge.files).toBeGreaterThan(0)); + }); +}); + +describe('a background account', () => { + const BOB = 'bob@mail.example.test'; + const token = async () => useAuthStore.getState().refreshAccessToken({ accountId: BOB }); + + beforeEach(() => { + useAccountStore.setState({ accounts: [entry(ALICE, 0), entry(BOB, 1)] } as never); + state.writeEnabled([ALICE, BOB]); + offline.startOfflineMode(); + }); + + it('keeps its data on an unknown refusal', async () => { + tokenAnswers(() => answer(401, { error: 'Refresh failed' })); + await token(); + await vi.waitFor(() => expect(useAccountStore.getState().getAccountById(BOB)).toBeUndefined()); + expect(state.isEvicted(BOB)).toBe(true); + expect(store.removed).toEqual([]); + }); + + it('wipes on a real invalid_grant', async () => { + tokenAnswers(() => answer(401, { error: 'invalid_grant' })); + await token(); + await vi.waitFor(() => expect(store.removed).toEqual([BOB])); + expect(state.isEvicted(BOB)).toBe(false); + }); +}); + +describe('data waiting for a sign-in', () => { + const BOB = 'bob@mail.example.test'; + + beforeEach(async () => { + state.writeEnabled([ALICE, BOB]); + await queue.queueSend(BOB, { subject: 'Bob\'s', to: [] }, { kind: 'email', args: [] }); + useAccountStore.setState({ accounts: [entry(ALICE, 0), entry(BOB, 1)] } as never); + useAccountStore.getState().removeAccount(BOB, 'evicted'); + }); + + it('is hidden from anyone else, in every list', async () => { + await offline.refreshQueueState(); + expect(state.useOfflineStore.getState().outbox.map((row) => row.subject)).toEqual(['Unsent']); + expect((await queue.readQueue()).outbox.map((row) => row.account)).toEqual([ALICE]); + await offline.refreshUsage(); + expect(state.visibleAccounts()).toEqual([ALICE]); + }); + + it('shows again when the account signs in', async () => { + useAccountStore.getState().addAccount({ username: 'bob', serverUrl: 'https://mail.example.test', authMode: 'oauth', rememberMe: true, label: 'bob', displayName: 'bob', email: 'bob', lastLoginAt: 1, isConnected: true, hasError: false, isDefault: false } as never); + expect(state.isEvicted(BOB)).toBe(false); + await offline.refreshQueueState(); + expect(state.useOfflineStore.getState().outbox.map((row) => row.subject).sort()).toEqual(['Bob\'s', 'Unsent']); + }); + + it('goes after 30 days', async () => { + localStorage.setItem(state.OFFLINE_EVICTED_KEY, JSON.stringify({ [BOB]: Date.now() - state.EVICTED_KEEP_MS - 1000 })); + offline.startOfflineMode(); + await vi.waitFor(() => expect(store.removed).toEqual([BOB])); + expect(state.isEvicted(BOB)).toBe(false); + expect(state.useOfflineStore.getState().enabled).toEqual([ALICE]); + }); + + it('stays before that', async () => { + offline.startOfflineMode(); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(store.removed).toEqual([]); + expect(state.useOfflineStore.getState().enabled).toEqual([ALICE, BOB]); + }); + + it('survives someone else\'s full sign-out, whose own data goes', async () => { + offline.startOfflineMode(); + await useAuthStore.getState().logout({ endProviderSession: false }); + await vi.waitFor(() => expect(store.removed).toEqual([ALICE])); + expect(purge.files).toBe(0); + expect(state.useOfflineStore.getState().enabled).toEqual([BOB]); + expect(state.isEvicted(BOB)).toBe(true); + }); +}); + +describe('a login without "remember me"', () => { + it('ends with its data at a reload, naming lost mail, even before the host runs', async () => { + useAccountStore.setState({ accounts: [{ ...entry(ALICE, 0), rememberMe: false }] } as never); + tokenAnswers(() => answer(200)); + await useAuthStore.getState().checkAuth(); + expect(useAccountStore.getState().getAccountById(ALICE)).toBeUndefined(); + expect(state.isEvicted(ALICE)).toBe(false); + offline.startOfflineMode(); + await vi.waitFor(() => expect(purge.files).toBeGreaterThan(0)); + expect(lost.consumeLost()).toEqual(['Unsent']); + }); +}); diff --git a/stores/account-store.ts b/stores/account-store.ts index 87207fc8d..072215066 100644 --- a/stores/account-store.ts +++ b/stores/account-store.ts @@ -2,6 +2,7 @@ import { create } from 'zustand'; import { persist } from 'zustand/middleware'; import { generateAccountId, generateAvatarColor, getMaxAccounts } from '@/lib/account-utils'; import { authHooks } from '@/lib/plugin-hooks'; +import { clearEvicted, markEvicted } from '@/lib/offline/state'; export type AccountRemovalReason = 'explicit' | 'evicted'; @@ -111,6 +112,7 @@ export const useAccountStore = create()( const state = get(); const id = generateAccountId(entry.username, entry.serverUrl); + clearEvicted(id); if (state.accounts.some((a) => a.id === id)) { // Already exists - update mutable fields and return existing id set((s) => ({ @@ -163,6 +165,7 @@ export const useAccountStore = create()( removeAccount: (accountId, reason = 'explicit') => { if (!get().accounts.some((a) => a.id === accountId)) return; + if (reason === 'evicted') markEvicted(accountId); for (const listener of [...removalListeners]) listener(accountId, reason); set((s) => { const remaining = s.accounts.filter((a) => a.id !== accountId); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index 9f2cc5608..c891ab06c 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -19,7 +19,7 @@ import { replaceWindowLocation, getPathPrefix, getLocaleFromPath, apiFetch } fro import { isEmbedded, notifyParent } from '@/lib/iframe-bridge'; import { snapshotAccount, restoreAccount, clearAllStores, evictAccount, evictAll, forgetAccountRows } from '@/lib/account-state-manager'; import { clearAllPluginStorage, clearPluginStorageForAccount } from '@/lib/plugin-sandbox/storage-scope'; -import { broadcastSignOut, onSignedOutElsewhere, passSignOutGates, purgeSignedOutData, runBeforeSignOut } from '@/lib/sign-out-cleanup'; +import { broadcastSignOut, onSignedOutElsewhere, passSignOutGates, purgeSignedOutData, runBeforeSignOut, type SignOutCleanupOptions } from '@/lib/sign-out-cleanup'; import { forgetPrincipals } from '@/lib/delegation/known-principals'; import { useSearchHistoryStore } from './search-history-store'; import { useCalendarNotificationStore } from './calendar-notification-store'; @@ -31,7 +31,7 @@ import type { Identity } from '@/lib/jmap/types'; import { authHooks } from '@/lib/plugin-hooks'; import { IS_LITE, IS_LITE_STALWART } from '@/lib/lite'; import { noteCredentialsRejected } from '@/lib/offline/credentials-rejected'; -import { useOfflineStore } from '@/lib/offline/state'; +import { readEvicted, useOfflineStore } from '@/lib/offline/state'; import { toAsciiEmail } from '@/lib/idn'; import { flushComposers } from '@/lib/compose-flush-registry'; import { sortIdentities } from '@/lib/identity-order'; @@ -154,6 +154,12 @@ export interface LogoutOptions { * provider, and signing in again should stay a single click. */ endProviderSession?: boolean; + /** + * The app, not the user, ends this login, and nothing says the login itself + * is over (an outage, an unknown refusal): local mail and the outbox stay, + * hidden, for the next sign-in or 30 days. + */ + keepLocalData?: boolean; } const ERROR_PATTERNS: Array<{ key: string; matches: string[] }> = [ @@ -1163,6 +1169,20 @@ function recordPermanentRefreshFailure(status: number, accountId?: string): bool return true; } +/** + * Whether a refused renewal ends the login for real, which wipes the account's + * local data: the server said invalid_grant, or the organisation's session limit + * was reached. Anything else, including a body we can't read, keeps the data. + */ +async function loginReallyOver(res: Response): Promise { + try { + const { error } = await res.clone().json() as { error?: unknown }; + return error === 'invalid_grant' || error === SESSION_TOO_OLD; + } catch { + return false; + } +} + function notifySignInAgain(): void { void import('@/stores/toast-store').then(({ toast }) => { toast.error('Your session could not be renewed', 'Sign in again to continue.'); @@ -1358,7 +1378,7 @@ async function startLocalCopyClient(accountId: string, serverUrl: string, userna // While device-only clients get replaced, restoring goes straight to the server. let skipLocalCopy = false; -function performFullLogout(set: (state: Partial) => void): void { +function performFullLogout(set: (state: Partial) => void, options: SignOutCleanupOptions = {}): void { useSettingsStore.getState().disableSync(); // With settings sync on, the local settings and templates are a copy of the // signed-out account's server file. Left in place, the next account to sign @@ -1397,7 +1417,9 @@ function performFullLogout(set: (state: Partial) => void): void { // doesn't re-write stale values. try { localStorage.removeItem('auth-storage'); } catch { /* noop */ } try { localStorage.removeItem('account-registry'); } catch { /* noop */ } - purgeSignedOutData(); + // A dropped account's data outlives anyone else's sign-out. The account that + // signed out takes its own along when it leaves the registry. + purgeSignedOutData({ keepLocalStore: options.keepLocalStore || Object.keys(readEvicted()).length > 0 }); // Other tabs still hold the accounts' mail in memory. broadcastSignOut(); } @@ -1418,9 +1440,9 @@ function notifySignedIn(state: { activeAccountId: string | null; username: strin void authHooks.onLogin.emit({ accountId: state.activeAccountId, username: state.username ?? '' }); } -function beforeLogoutHooks(accountId: string): Promise { +function beforeLogoutHooks(accountId: string, options: SignOutCleanupOptions = {}): Promise { return Promise.race([ - Promise.all([authHooks.onBeforeLogout.emit({ accountId }), runBeforeSignOut(accountId)]).then(() => {}), + Promise.all([authHooks.onBeforeLogout.emit({ accountId }), runBeforeSignOut(accountId, options)]).then(() => {}), new Promise((resolve) => setTimeout(resolve, LOGOUT_HOOK_BUDGET_MS)), ]); } @@ -2216,16 +2238,17 @@ export const useAuthStore = create()( // maintenance windows and offline spells. if (res.status === 401) { resetRefreshBackoff(accountId ?? undefined); + const over = await loginReallyOver(res); + if (over && accountId) noteCredentialsRejected(accountId); // Only this login's refresh token was refused. - if (accountId) noteCredentialsRejected(accountId); if (accountId && !isActive) { - get().removeAccount(accountId, 'evicted'); + get().removeAccount(accountId, over ? 'explicit' : 'evicted'); return null; } await saveDraftsBeforeExpiry(slot); notifyParent('sso:session-expired'); markSessionExpired(); - get().logout({ endProviderSession: false }); + get().logout({ endProviderSession: false, keepLocalData: !over }); return null; } // A Bulwark-side failure (500/502) that keeps repeating will not @@ -2240,7 +2263,8 @@ export const useAuthStore = create()( if (accountId && accountId !== get().activeAccountId) get().removeAccount(accountId, 'evicted'); else { await saveDraftsBeforeExpiry(slot); - get().logout({ endProviderSession: false }); + // An outage on our side says nothing against the mail on this device. + get().logout({ endProviderSession: false, keepLocalData: true }); } return null; } @@ -2310,13 +2334,14 @@ export const useAuthStore = create()( const slot = account?.cookieSlot ?? 0; // `options` can be a click event when the action is wired straight to onClick. const userInitiated = options?.endProviderSession !== false; + const keepLocalData = options?.keepLocalData === true; const endProviderSession = userInitiated && !wasDemoMode && shouldEndProviderSession(account); // Signing out on purpose may be stopped by the user (unsent mail in // the offline outbox would be lost); an automatic one may not. if (userInitiated && !(await passSignOutGates(accountId ?? 'all'))) return; - await beforeLogoutHooks(accountId ?? 'all'); + await beforeLogoutHooks(accountId ?? 'all', { keepLocalStore: keepLocalData }); // From here on nothing may renew this slot's session; the cleanup // below reopens it once the credentials are gone. @@ -2334,7 +2359,7 @@ export const useAuthStore = create()( if (accountId) { clients.delete(accountId); evictAccount(accountId); - accountStore.removeAccount(accountId); + accountStore.removeAccount(accountId, keepLocalData ? 'evicted' : 'explicit'); forgetPrincipals(accountId); if (!wasDemoMode) clearPluginStorageForAccount(accountId); } @@ -2428,7 +2453,9 @@ export const useAuthStore = create()( clients.get(remaining.id)?.disconnect(); clients.delete(remaining.id); evictAccount(remaining.id); - accountStore.removeAccount(remaining.id); + // Dropped by this sign-out, not signed out by anyone: their unsent + // mail never passed a question, so it waits for their next sign-in. + accountStore.removeAccount(remaining.id, 'evicted'); } } @@ -2442,7 +2469,7 @@ export const useAuthStore = create()( ...droppedAccounts.map((dropped) => clearSlotCredentials(dropped.cookieSlot ?? 0, dropped.authMode === 'oauth')), ]); - performFullLogout(set); + performFullLogout(set, { keepLocalStore: keepLocalData }); notifyParent('sso:logout'); @@ -2845,7 +2872,8 @@ export const useAuthStore = create()( if (!account.rememberMe && !IS_LITE) { forgottenAccounts += 1; evictAccount(account.id); - accountStore.removeAccount(account.id, 'evicted'); + // The user's choice on what may be a shared computer: its local data goes too. + accountStore.removeAccount(account.id, 'explicit'); if (account.authMode === 'oauth') void clearSlotCredentials(account.cookieSlot, true); return; } @@ -2870,7 +2898,7 @@ export const useAuthStore = create()( // Repeated 500/502 (see recordPermanentRefreshFailure) falls // through here and evicts the account like a rejection. (#972) if (res.status >= 500) notifySignInAgain(); - throw new Error(`Token refresh failed: ${res.status}`); + throw Object.assign(new Error(`Token refresh failed: ${res.status}`), { loginOver: await loginReallyOver(res) }); } } else { const res = await fetchSlotSession(account.cookieSlot); @@ -2927,9 +2955,10 @@ export const useAuthStore = create()( } // Remove unrestorable accounts so the user is prompted to log in // again rather than seeing a stale error entry forever. Its - // refresh token goes with it: nothing would use or revoke it. + // refresh token goes with it: nothing would use or revoke it. Its + // local data waits, unless the server really ended the login. evictAccount(account.id); - accountStore.removeAccount(account.id, 'evicted'); + accountStore.removeAccount(account.id, (err as { loginOver?: boolean }).loginOver ? 'explicit' : 'evicted'); void clearSlotCredentials(account.cookieSlot, account.authMode === 'oauth'); } };