From 2c462c304e60111d7b23cea04d7014da70125c7b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:48:39 +0100 Subject: [PATCH] fix: show the right preview when an account id is in more than one login --- app/api/push/preview/route.ts | 13 +++--- .../push-preview-route-ambiguous.test.ts | 43 ++++++++++++++++--- lib/__tests__/sw-push-delivery.test.ts | 14 ++++++ public/sw.js | 3 ++ 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/app/api/push/preview/route.ts b/app/api/push/preview/route.ts index c862eeb99..2c214351c 100644 --- a/app/api/push/preview/route.ts +++ b/app/api/push/preview/route.ts @@ -45,11 +45,12 @@ interface ResolvedTarget { // session of everyone it is shared with, who sees only that folder - not its // Inbox, nor the message that just arrived there. // -// The push names nothing but the account id, and ids repeat across servers -// (and a group account is listed by each member). When that leaves more than -// one login, answer 'ambiguous' rather than guess: a wrong guess previews +// Ids repeat across servers (and a group account is listed by each member), so +// the account id alone can leave several logins. The push's label names the login +// that registered it, which settles it among those that have the account. Without +// one that fits, answer 'ambiguous' rather than guess: a wrong guess previews // another login's mail and opens it there. -async function resolveTargetForAccount(accountId: string, forwarded: Record): Promise { +async function resolveTargetForAccount(accountId: string, login: string | null, forwarded: Record): Promise { const cookieStore = await cookies(); const probes: Promise<{ target: ResolvedTarget; own: boolean } | null>[] = []; for (let slot = 0; slot < MAX_ACCOUNT_SLOTS; slot++) { @@ -91,6 +92,8 @@ async function resolveTargetForAccount(accountId: string, forwarded: Record r !== null); + const named = login ? results.filter((r) => r.target.username.toLowerCase() === login.toLowerCase()) : []; + if (named.length === 1) return named[0].target; const owners = results.filter((r) => r.own); if (owners.length === 1) return owners[0].target; if (results.length === 1) return results[0].target; @@ -141,7 +144,7 @@ export async function GET(request: NextRequest) { let target: ResolvedTarget | null = null; let authHeader: string; if (requestedAccountId) { - const resolved = await resolveTargetForAccount(requestedAccountId, forwarded); + const resolved = await resolveTargetForAccount(requestedAccountId, request.nextUrl.searchParams.get('login'), forwarded); if (!resolved) { return NextResponse.json({ error: 'Not authenticated' }, { status: 401 }); } diff --git a/lib/__tests__/push-preview-route-ambiguous.test.ts b/lib/__tests__/push-preview-route-ambiguous.test.ts index f83e8bb77..93e74d40f 100644 --- a/lib/__tests__/push-preview-route-ambiguous.test.ts +++ b/lib/__tests__/push-preview-route-ambiguous.test.ts @@ -26,9 +26,9 @@ vi.mock('@/lib/account-utils', () => ({ MAX_ACCOUNT_SLOTS: 3 })); // slot 0: alice on one.example, slot 1: bob on two.example, slot 2: carol on one.example const LOGINS = [ - { serverUrl: 'https://one.example', authHeader: 'Basic alice' }, - { serverUrl: 'https://two.example', authHeader: 'Basic bob' }, - { serverUrl: 'https://one.example', authHeader: 'Basic carol' }, + { serverUrl: 'https://one.example', authHeader: 'Basic alice', username: 'alice@one.example' }, + { serverUrl: 'https://two.example', authHeader: 'Basic bob', username: 'bob@two.example' }, + { serverUrl: 'https://one.example', authHeader: 'Basic carol', username: 'carol@one.example' }, ]; vi.mock('@/lib/stalwart/auth-context', () => ({ readStalwartAuthContextFromStore: (_store: unknown, slot: number) => LOGINS[slot] ?? null, @@ -73,10 +73,10 @@ vi.mock('@/lib/admin/config-manager', async () => { return { configManager: { ensureLoaded: async () => {}, getPolicy: () => DEFAULT_POLICY } }; }); -async function callRoute(accountId: string) { +async function callRoute(accountId: string, login?: string) { const { GET } = await import('@/app/api/push/preview/route'); const request = { - nextUrl: { searchParams: { get: (k: string) => (k === 'accountId' ? accountId : k === 'emailId' ? 'e1' : null) } }, + nextUrl: { searchParams: { get: (k: string) => (k === 'accountId' ? accountId : k === 'emailId' ? 'e1' : k === 'login' ? login ?? null : null) } }, headers: new Headers(), }; const res = (await GET(request as unknown as Parameters[0])) as unknown as { @@ -93,6 +93,39 @@ describe('push preview when the account id is in more than one login', () => { fetchJmapServer.mockClear(); }); + it('takes the login the push names when two servers use the same id', async () => { + SESSIONS = { + 'Basic alice': session('https://one.example', 'a'), + 'Basic bob': session('https://two.example', 'z'), + 'Basic carol': session('https://one.example', 'z'), + }; + const { status, body } = await callRoute('z', 'BOB@two.example'); + expect(status).toBe(200); + expect(body.slot).toBe(1); + expect((body.email as { subject: string }).subject).toBe('mail of Basic bob'); + }); + + it('takes the login the push names for a group two logins belong to', async () => { + SESSIONS = { + 'Basic alice': session('https://one.example', 'a', ['team']), + 'Basic bob': session('https://two.example', 'b'), + 'Basic carol': session('https://one.example', 'c', ['team']), + }; + const { status, body } = await callRoute('team', 'carol@one.example'); + expect(status).toBe(200); + expect(body.slot).toBe(2); + }); + + it('does not take a login that does not have the account', async () => { + SESSIONS = { + 'Basic alice': session('https://one.example', 'a', ['team']), + 'Basic bob': session('https://two.example', 'b'), + 'Basic carol': session('https://one.example', 'c', ['team']), + }; + const { status } = await callRoute('team', 'bob@two.example'); + expect(status).toBe(409); + }); + it('shows no preview and names no login when two servers use the same id', async () => { SESSIONS = { 'Basic alice': session('https://one.example', 'a'), diff --git a/lib/__tests__/sw-push-delivery.test.ts b/lib/__tests__/sw-push-delivery.test.ts index 96b1d737f..d2d00f7a4 100644 --- a/lib/__tests__/sw-push-delivery.test.ts +++ b/lib/__tests__/sw-push-delivery.test.ts @@ -173,4 +173,18 @@ describe('with notifications off or do not disturb on', () => { }); }); +describe('which login a push is for', () => { + it('names the login in the preview request', async () => { + const { push, previews } = loadWorker(() => ({ unreadTotal: 1, slot: 0, email: mail('e1') })); + await push(delivery('a', 'e1')); + expect(previews[0].searchParams.get('login')).toBe('me@example.org'); + }); + + it('leaves it out when the push has no label', async () => { + const { push, previews } = loadWorker(() => ({ unreadTotal: 1, slot: 0, email: mail('e1') })); + await push({ kind: 'jmap-email-push', accountId: 'a', emailIds: ['e1'] }); + expect(previews[0].searchParams.has('login')).toBe(false); + }); +}); + // -- end of cases -- diff --git a/public/sw.js b/public/sw.js index 0b9ebc9e2..14d135af6 100644 --- a/public/sw.js +++ b/public/sw.js @@ -642,6 +642,9 @@ async function notifyAccount(accountId, emailIds, accountLabel) { if (!lockHidesContent) try { const query = new URLSearchParams(); if (accountId) query.set("accountId", accountId); + // The relay record's label is the login that registered. Account and message + // ids repeat across logins, and this tells them apart. + if (accountLabel) query.set("login", accountLabel); if (freshIds.length > 0) query.set("emailId", freshIds[freshIds.length - 1]); const qs = query.toString(); const previewUrl = `${BASE_PATH}/api/push/preview${qs ? `?${qs}` : ""}`;