From d155cead41befe3bbcc5107eda0f95d2c6291efd Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:13:59 +0200 Subject: [PATCH] fix: revoke Lite refresh tokens on sign-out and keep no failed-login password Signing out of Lite only deleted the refresh token from web storage, so a copy taken from the browser kept working until it expired. Sign-out now revokes it at the endpoint the mail server (or, for an external provider, its discovery document) advertises, once the token is gone from storage. A password login whose token exchange failed for any reason fell back to Basic auth and kept the password in sessionStorage. The password is now only kept for the tab when the server has no token login at all (or cannot be reached for it); otherwise the Basic session lives in memory and a reload asks to sign in again. --- app/(main)/[locale]/login/page.tsx | 1 + lib/auth/lite-oauth.ts | 12 +++- lib/auth/lite-tokens.ts | 80 ++++++++++++++++++++++++ stores/__tests__/auth-store-lite.test.ts | 57 ++++++++++++++++- stores/auth-store.ts | 25 +++++--- 5 files changed, 164 insertions(+), 11 deletions(-) diff --git a/app/(main)/[locale]/login/page.tsx b/app/(main)/[locale]/login/page.tsx index 1546e80c0..17a6bbd8b 100644 --- a/app/(main)/[locale]/login/page.tsx +++ b/app/(main)/[locale]/login/page.tsx @@ -685,6 +685,7 @@ function LoginPageContent() { clientId, redirectUri, persistent: rememberMeEnabled && rememberMe, + revocationEndpoint: discovery.metadata.revocation_endpoint, }); const authUrl = new URL(discovery.metadata.authorization_endpoint); diff --git a/lib/auth/lite-oauth.ts b/lib/auth/lite-oauth.ts index 818959204..de06ac829 100644 --- a/lib/auth/lite-oauth.ts +++ b/lib/auth/lite-oauth.ts @@ -220,6 +220,8 @@ export interface LiteOAuthFlow { redirectUri: string; /** "Remember me" was ticked: keep the refresh token across browser restarts. */ persistent: boolean; + /** Where sign-out revokes the refresh token, when discovery named it. */ + revocationEndpoint?: string; } export function saveLiteOAuthFlow(flow: LiteOAuthFlow): void { @@ -236,7 +238,15 @@ export function readLiteOAuthFlow(): LiteOAuthFlow | null { const flow = raw ? (JSON.parse(raw) as Partial) : null; if (!flow || typeof flow.tokenEndpoint !== 'string' || typeof flow.clientId !== 'string' || typeof flow.redirectUri !== 'string') return null; if (!flow.tokenEndpoint || !flow.clientId || !flow.redirectUri) return null; - return { tokenEndpoint: flow.tokenEndpoint, clientId: flow.clientId, redirectUri: flow.redirectUri, persistent: flow.persistent === true }; + return { + tokenEndpoint: flow.tokenEndpoint, + clientId: flow.clientId, + redirectUri: flow.redirectUri, + persistent: flow.persistent === true, + ...(typeof flow.revocationEndpoint === 'string' && flow.revocationEndpoint + ? { revocationEndpoint: flow.revocationEndpoint } + : {}), + }; } catch { return null; } diff --git a/lib/auth/lite-tokens.ts b/lib/auth/lite-tokens.ts index b6610ba82..b10e13946 100644 --- a/lib/auth/lite-tokens.ts +++ b/lib/auth/lite-tokens.ts @@ -77,6 +77,8 @@ interface StoredRefreshToken { * to (lib/auth/lite-oauth.ts) is refreshed at that provider. */ tokenEndpoint?: string; + /** RFC 7009 endpoint that revokes the token on sign-out, when discovery named one. */ + revocationEndpoint?: string; } interface StoredBasicSession { @@ -194,6 +196,84 @@ export function clearLiteSlot(slot: number): void { clearLiteBasicSession(slot); } +// --------------------------------------------------------------------------- +// Revocation +// --------------------------------------------------------------------------- + +// Sign-out waits for this, so an unresponsive endpoint must not hold it up. +const REVOCATION_TIMEOUT_MS = 3000; + +/** The revocation endpoint a Stalwart server advertises, if any. */ +async function discoverRevocationEndpoint(serverUrl: string): Promise { + for (const path of ['/.well-known/oauth-authorization-server', '/.well-known/openid-configuration']) { + try { + const response = await fetch(`${serverUrl}${path}`, { signal: AbortSignal.timeout(REVOCATION_TIMEOUT_MS) }); + if (!response.ok) continue; + const endpoint = ((await response.json()) as { revocation_endpoint?: unknown }).revocation_endpoint; + return typeof endpoint === 'string' && /^https?:\/\//.test(endpoint) ? endpoint : null; + } catch { + // Try the next document. + } + } + return null; +} + +async function revokeRefreshToken(entry: StoredRefreshToken): Promise { + // A token from an external provider is only revoked where its own + // discovery said; Stalwart's own tokens can be looked up. + const endpoint = entry.revocationEndpoint + ?? (entry.tokenEndpoint ? null : await discoverRevocationEndpoint(trimUrl(entry.serverUrl))); + if (!endpoint) return; + try { + await fetch(endpoint, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + token: entry.refreshToken, + token_type_hint: 'refresh_token', + client_id: entry.clientId || getLiteClientId(), + }).toString(), + signal: AbortSignal.timeout(REVOCATION_TIMEOUT_MS), + // Finishes even when sign-out navigates away first. + keepalive: true, + redirect: 'error', + }); + } catch { + // Best effort: the token is gone from this browser either way. + } +} + +/** + * Sign a slot out: forget its credentials at once, then revoke its refresh + * token so a copy taken from this browser's storage stops working too. + */ +export async function revokeLiteSlot(slot: number): Promise { + const entry = readLiteRefreshToken(slot); + clearLiteSlot(slot); + if (entry) await revokeRefreshToken(entry); +} + +/** {@link revokeLiteSlot} for every slot. */ +export async function revokeAllLiteSessions(): Promise { + const entries: StoredRefreshToken[] = []; + for (const kind of ['local', 'session'] as const) { + const store = storage(kind); + if (!store) continue; + try { + for (let i = 0; i < store.length; i++) { + const key = store.key(i); + if (!key?.startsWith(REFRESH_KEY_PREFIX)) continue; + const entry = readJson(store, key); + if (entry && typeof entry.refreshToken === 'string' && entry.refreshToken) entries.push(entry); + } + } catch { + continue; + } + } + clearAllLiteSessions(); + await Promise.all(entries.map(revokeRefreshToken)); +} + export function clearAllLiteSessions(): void { for (const kind of ['local', 'session'] as const) { const store = storage(kind); diff --git a/stores/__tests__/auth-store-lite.test.ts b/stores/__tests__/auth-store-lite.test.ts index edcb104eb..286c9184e 100644 --- a/stores/__tests__/auth-store-lite.test.ts +++ b/stores/__tests__/auth-store-lite.test.ts @@ -30,14 +30,33 @@ function jsonResponse(body: unknown, status = 200): Response { * config/policy files, and fails on anything else - in particular on any * /api/ route of our own origin. `calls` lists the mail-server requests. */ -function stalwartFetch(options: { loginAnswer?: unknown; refreshAnswer?: () => Response; loginStatus?: number } = {}) { +function stalwartFetch(options: { + loginAnswer?: unknown; + refreshAnswer?: () => Response; + loginStatus?: number; + /** Advertise an RFC 7009 revocation endpoint in the metadata document. */ + revocation?: boolean; +} = {}) { const calls: string[] = []; + const revoked: string[] = []; const mock = vi.fn(async (input: FetchInput, init?: FetchInit) => { const url = String(input); if (url === '/config.json') return jsonResponse({ jmapServerUrl: SERVER }); if (url === '/policy.json') return jsonResponse({}); if (url.startsWith('/')) throw new Error(`Lite must not call its own origin: ${url}`); + // Sign-out looks the revocation endpoint up; not counted in `calls`, as + // a sign-out in one test's cleanup may still be looking when the next + // test starts. + if (url.startsWith(`${SERVER}/.well-known/`)) { + return options.revocation + ? jsonResponse({ issuer: SERVER, revocation_endpoint: `${SERVER}/auth/revoke` }) + : new Response('not found', { status: 404 }); + } calls.push(`${init?.method ?? 'GET'} ${url}`); + if (url === `${SERVER}/auth/revoke`) { + revoked.push(new URLSearchParams(String(init?.body)).get('token') ?? ''); + return new Response(null, { status: 200 }); + } if (url === `${SERVER}/api/auth`) { if (options.loginStatus) return new Response('nope', { status: options.loginStatus }); return jsonResponse(options.loginAnswer ?? { type: 'authenticated', client_code: 'CODE' }); @@ -52,7 +71,7 @@ function stalwartFetch(options: { loginAnswer?: unknown; refreshAnswer?: () => R throw new Error(`unexpected fetch ${url}`); }); vi.stubGlobal('fetch', mock); - return { mock, calls }; + return { mock, calls, revoked }; } function liteStorageKeys(): string[] { @@ -225,6 +244,17 @@ describe('auth-store in the static Lite build', () => { expect(connectSpy).not.toHaveBeenCalled(); }); + it('keeps no password in web storage when token login exists but failed', async () => { + stalwartFetch({ loginStatus: 500 }); + + const ok = await useAuthStore.getState().login(SERVER, 'alice', 'pw', undefined, true); + + expect(ok).toBe(true); + expect(useAuthStore.getState().authMode).toBe('basic'); + expect(readLiteBasicSession(0)).toBeNull(); + expect(liteStorageKeys()).toEqual([]); + }); + it('falls back to Basic auth with a tab-scoped session when the server has no token login', async () => { const { calls } = stalwartFetch({ loginStatus: 404 }); @@ -304,6 +334,29 @@ describe('auth-store in the static Lite build', () => { expect(useAuthStore.getState().isAuthenticated).toBe(false); }); + it('logout revokes the refresh token where the mail server advertises revocation', async () => { + const { calls, revoked } = stalwartFetch({ revocation: true }); + await useAuthStore.getState().login(SERVER, 'alice', 'pw', undefined, true); + calls.length = 0; + + await useAuthStore.getState().logout(); + + expect(calls).toEqual([`POST ${SERVER}/auth/revoke`]); + expect(revoked).toEqual(['RT-1']); + expect(readLiteRefreshToken(0)).toBeNull(); + }); + + it('logoutAll revokes every slot\'s refresh token', async () => { + saveLiteRefreshToken(0, { serverUrl: SERVER, username: 'a', refreshToken: 'r0' }, true); + saveLiteRefreshToken(1, { serverUrl: SERVER, username: 'b', refreshToken: 'r1' }, false); + const { revoked } = stalwartFetch({ revocation: true }); + + await useAuthStore.getState().logoutAll(); + + expect(revoked.sort()).toEqual(['r0', 'r1']); + expect(liteStorageKeys()).toEqual([]); + }); + it('logoutAll clears every Lite slot', async () => { saveLiteRefreshToken(0, { serverUrl: SERVER, username: 'a', refreshToken: 'r0' }, true); saveLiteRefreshToken(1, { serverUrl: SERVER, username: 'b', refreshToken: 'r1' }, false); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index 7b83b6be8..9e6a88d36 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -29,15 +29,15 @@ import { IS_LITE, IS_LITE_STALWART } from '@/lib/lite'; import { toAsciiEmail } from '@/lib/idn'; import { LiteLoginError, - clearAllLiteSessions, clearLiteRefreshToken, - clearLiteSlot, getLiteClientId, liteExchangeAuthorizationCode, liteRefreshTokens, liteTokenLogin, nameLiteRefreshToken, readLiteBasicSession, + revokeAllLiteSessions, + revokeLiteSlot, saveLiteBasicSession, saveLiteRefreshToken, } from '@/lib/auth/lite-tokens'; @@ -446,6 +446,7 @@ async function exchangeOAuthCode(params: { refreshToken: tokens.refreshToken, clientId: flow.clientId, tokenEndpoint: flow.tokenEndpoint, + revocationEndpoint: flow.revocationEndpoint, }, flow.persistent); } else { clearLiteRefreshToken(slot); @@ -566,7 +567,7 @@ function clearSlotCredentials(slot: number, includeToken: boolean, endSession = closingSlots.add(slot); return clearClosingSlots(new Set([slot]), async () => { if (IS_LITE) { - clearLiteSlot(slot); + await revokeLiteSlot(slot); return null; } const [, token] = await Promise.all([ @@ -586,7 +587,7 @@ function clearAllCredentials(endSessionSlot: number | null): Promise { if (IS_LITE) { - clearAllLiteSessions(); + await revokeAllLiteSessions(); return null; } const endSession = endSessionSlot === null ? '' : `&end_session_slot=${endSessionSlot}`; @@ -1228,6 +1229,10 @@ export const useAuthStore = create()( let client: JMAPClient; let upgradedToOAuth = false; + // Lite keeps a Basic password for the tab only when the server has + // no token login to use instead; a token login that failed for + // another reason must not leave the password in web storage. + let tokenLoginUnavailable = false; let oauthAccessToken: string | null = null; let oauthExpiresIn = 0; @@ -1263,6 +1268,7 @@ export const useAuthStore = create()( oauthExpiresIn = expires_in; debug.log('auth', 'TOTP login exchanged for token-based auth (has_refresh_token=' + has_refresh_token + ')'); } else { + if (tokenRes.status === 404) tokenLoginUnavailable = true; const errorBody = await tokenRes.json().catch(() => ({ error: 'unknown' })); // A correct password with a missing/invalid MFA token surfaces as // a TOTP prompt rather than a generic failure. @@ -1273,6 +1279,8 @@ export const useAuthStore = create()( } } catch (err) { if (err instanceof Error && err.message === 'TOTP_REQUIRED') throw err; + // Unreachable (a server without /api/auth rarely answers CORS). + tokenLoginUnavailable = true; debug.warn('auth', 'TOTP login exchange error, trying legacy basic auth:', err); } @@ -1324,10 +1332,11 @@ export const useAuthStore = create()( // write and stalwart-context write are best-effort persistence; the // outer login still succeeds even if they log a warning. Errors are // caught locally so Promise.all doesn't reject on either. - // In Lite a Basic session is always kept for the tab (sessionStorage), - // so a reload does not sign the user out; the regular build only - // writes the cookie when the user asked to be remembered. - const sessionWrite: Promise = ((rememberMe || IS_LITE) && !upgradedToOAuth) + // In Lite a Basic session is kept for the tab (sessionStorage) on a + // server without token login, so a reload does not sign the user + // out; the regular build only writes the cookie when the user asked + // to be remembered. + const sessionWrite: Promise = ((IS_LITE ? tokenLoginUnavailable : rememberMe) && !upgradedToOAuth) ? persistBasicSession(cookieSlot, serverUrl, username, password) : Promise.resolve();