From 322274fff8fe3884a2900915deb555a710ae779d Mon Sep 17 00:00:00 2001 From: Shuki Vaknin Date: Thu, 29 Oct 2026 16:37:06 +0000 Subject: [PATCH] fix: let a mailbox be opened through the handoff more than five times Each impersonation handoff makes an app password on the target mailbox that expires after 8 hours. Stalwart keeps an app password after its expiresAt and counts it against the mailbox's limit (5 by default), and nothing deleted the expired ones, so after five handoffs every further one failed with "You have exceeded your quota of 5 app passwords" and the mailbox could no longer be opened that way. Before making a new one, the handoff now lists the mailbox's app passwords and deletes its own expired ones, recognised by the "Support session " description. The user's own app passwords and a session still running are never touched. It is best effort: if the list or the delete fails, the handoff tries to create the password as before. --- app/api/auth/impersonate/route.ts | 4 +-- lib/__tests__/impersonation-session.test.ts | 38 ++++++++++++++++++++- lib/impersonation/app-password.ts | 32 ++++++++++++++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/app/api/auth/impersonate/route.ts b/app/api/auth/impersonate/route.ts index d5627198a..8fbe67393 100644 --- a/app/api/auth/impersonate/route.ts +++ b/app/api/auth/impersonate/route.ts @@ -13,7 +13,7 @@ import { readImpersonationConfig, resolveImpersonationServerUrl, } from '@/lib/impersonation/master-config'; -import { mintImpersonationCredential } from '@/lib/impersonation/app-password'; +import { IMPERSONATION_DESCRIPTION_PREFIX, mintImpersonationCredential } from '@/lib/impersonation/app-password'; import { IMPERSONATION_GRANT_COOKIE, sealImpersonationGrant } from '@/lib/impersonation/grant-cookie'; import { IMPERSONATION_MARKER_COOKIE, IMPERSONATION_MARKER_MAX_AGE_SECONDS } from '@/lib/impersonation/marker-cookie'; @@ -107,7 +107,7 @@ export async function GET(request: NextRequest) { mailbox: claims.mailbox, masterUser: config.masterUser, masterPassword: config.masterPassword, - description: `Support session ${claims.jti}`.slice(0, 100), + description: `${IMPERSONATION_DESCRIPTION_PREFIX}${claims.jti}`.slice(0, 100), }); } catch (err) { logger.error('Impersonation credential could not be created', { diff --git a/lib/__tests__/impersonation-session.test.ts b/lib/__tests__/impersonation-session.test.ts index aa44a2171..245a85a5a 100644 --- a/lib/__tests__/impersonation-session.test.ts +++ b/lib/__tests__/impersonation-session.test.ts @@ -51,6 +51,9 @@ const SAME_ORIGIN = { 'sec-fetch-site': 'same-origin', 'sec-fetch-mode': 'cors', type Call = { url: string; auth: string | null; body: unknown }; let calls: Call[]; let createResult: Record; +// The mailbox's app passwords, as x:AppPassword/get lists them; null makes the +// listing fail. +let existing: { id: string; description: string; expiresAt?: string }[] | null; function stalwartFetch(input: RequestInfo | URL, init?: RequestInit): Promise { const url = String(input); @@ -64,6 +67,11 @@ function stalwartFetch(input: RequestInfo | URL, init?: RequestInit): Promise, string][] }).methodCalls[0]; + if (method[0] === 'x:AppPassword/get') { + return Promise.resolve(Response.json({ + methodResponses: [existing ? ['x:AppPassword/get', { list: existing }, '0'] : ['error', { type: 'serverFail' }, '0']], + })); + } if (method[1].create) { return Promise.resolve(Response.json({ methodResponses: [['x:AppPassword/set', createResult, '0']] })); } @@ -96,6 +104,7 @@ describe('impersonation handoff', () => { jar.clear(); calls = []; createResult = { created: { imp: { id: 'cred-1', secret: APP_SECRET } } }; + existing = []; vi.stubGlobal('fetch', vi.fn(stalwartFetch)); }); @@ -121,7 +130,7 @@ describe('impersonation handoff', () => { const masterAuth = `Basic ${Buffer.from(`alice@example.org%master@example.org:${MASTER_PASSWORD}`).toString('base64')}`; expect(calls.every((c) => c.auth === masterAuth)).toBe(true); - const create = calls.find((c) => c.url.endsWith('/jmap/'))!.body as { + const create = calls.find((c) => c.url.endsWith('/jmap/') && JSON.stringify(c.body).includes('"create"'))!.body as { methodCalls: [string, { accountId: string; create: { imp: { expiresAt: string } } }, string][]; }; const [method, args] = create.methodCalls[0]; @@ -136,6 +145,33 @@ describe('impersonation handoff', () => { expect(expires).toBeLessThanOrEqual(after + 8 * 3600_000); }); + it("deletes the mailbox's expired handoff app passwords before making a new one", async () => { + const past = '2026-01-01T00:00:00Z'; + const future = new Date(Date.now() + 3600_000).toISOString().replace(/\.\d{3}Z$/, 'Z'); + existing = [ + { id: 'old-1', description: 'Support session aaa', expiresAt: past }, + { id: 'old-2', description: 'Support session bbb', expiresAt: past }, + { id: 'live', description: 'Support session ccc', expiresAt: future }, + { id: 'theirs', description: 'Thunderbird on the laptop', expiresAt: past }, + { id: 'forever', description: 'Support session ddd' }, + ]; + const res = await impersonate(); + expect(res.status).toBe(303); + + const jmap = calls.filter((c) => c.url.endsWith('/jmap/')).map((c) => + (c.body as { methodCalls: [string, Record, string][] }).methodCalls[0]); + expect(jmap.map(([name]) => name)).toEqual(['x:AppPassword/get', 'x:AppPassword/set', 'x:AppPassword/set']); + expect(jmap[1][1].destroy).toEqual(['old-1', 'old-2']); + expect(jmap[2][1].create).toBeDefined(); + }); + + it('still hands over a session when the expired ones cannot be listed', async () => { + existing = null; + const res = await impersonate(); + expect(res.status).toBe(303); + expect(calls.some((c) => JSON.stringify(c.body ?? null).includes('"destroy"'))).toBe(false); + }); + it('refuses the handoff when Stalwart does not create the app password', async () => { createResult = { notCreated: { imp: { type: 'forbidden', description: 'not allowed' } } }; const res = await impersonate(); diff --git a/lib/impersonation/app-password.ts b/lib/impersonation/app-password.ts index c4727bf65..c79e904d9 100644 --- a/lib/impersonation/app-password.ts +++ b/lib/impersonation/app-password.ts @@ -9,6 +9,9 @@ const STALWART_USING = ['urn:ietf:params:jmap:core', 'urn:stalwart:jmap']; */ export const IMPERSONATION_SESSION_TTL_SEC = 8 * 60 * 60; +/** Every impersonation app password's description starts with this. */ +export const IMPERSONATION_DESCRIPTION_PREFIX = 'Support session '; + export class ImpersonationCredentialError extends Error { constructor(message: string) { super(message); @@ -139,15 +142,42 @@ export async function mintImpersonationCredential(opts: { }): Promise { const now = opts.now ?? new Date(); const expiresAt = utcDate(new Date(now.getTime() + IMPERSONATION_SESSION_TTL_SEC * 1000)); + const authHeader = masterAuthHeader(opts.mailbox, opts.masterUser, opts.masterPassword); + await pruneExpiredImpersonationCredentials(opts.serverUrl, authHeader, now); const created = await createAppPassword({ serverUrl: opts.serverUrl, - authHeader: masterAuthHeader(opts.mailbox, opts.masterUser, opts.masterPassword), + authHeader, description: opts.description, expiresAt, }); return { ...created, expiresAt }; } +/** + * Delete the mailbox's own expired impersonation app passwords. Stalwart keeps + * an app password after its `expiresAt` and counts it against the mailbox's + * limit (5 by default), so a mailbox opened that many times through the + * handoff could never be opened again. Only passwords this module made, by + * their description, and only expired ones go: the user's own app passwords + * and a session still running are never touched. Best effort: a failure here + * leaves the handoff to try as before. + */ +async function pruneExpiredImpersonationCredentials(serverUrl: string, authHeader: string, now: Date): Promise { + try { + const result = await callStalwart(serverUrl, authHeader, ['x:AppPassword/get', { ids: null }, '0']); + const list = (result.list as { id?: string; description?: string; expiresAt?: string }[] | undefined) ?? []; + const expired = list + .filter((p) => p.id && p.description?.startsWith(IMPERSONATION_DESCRIPTION_PREFIX)) + .filter((p) => p.expiresAt && Date.parse(p.expiresAt) <= now.getTime()) + .map((p) => p.id as string); + if (expired.length > 0) { + await callStalwart(serverUrl, authHeader, ['x:AppPassword/set', { destroy: expired }, '0']); + } + } catch { + // The create below reports anything that matters. + } +} + /** Best-effort revocation of a credential minted by {@link mintImpersonationCredential}. */ export async function revokeImpersonationCredential(opts: { serverUrl: string;