From 4858eb091583213a2bdf15c713ed488bb19c47cb Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 16:12:20 +0200 Subject: [PATCH] fix: open a push notification's message in the account it came from Clicking a new-mail notification opened `/mail/message/` in whichever account was active. With several accounts signed in, that looked the id up in the wrong account: nothing opened, or a different message did and was marked read. The preview API now returns the cookie slot of the login that owns the notified account, the service worker adds it to the permalink (or links that account's Inbox for a group notification), and the app switches to the login on that slot before opening the message. --- app/api/push/preview/route.ts | 9 ++++++++- components/mail/mail-app.tsx | 12 ++++++++---- lib/__tests__/deep-links.test.ts | 14 ++++++++++++++ lib/__tests__/push-preview-route.test.ts | 7 +++++++ lib/deep-links.ts | 21 ++++++++++++++------- public/sw.js | 15 ++++++++++++--- 6 files changed, 63 insertions(+), 15 deletions(-) diff --git a/app/api/push/preview/route.ts b/app/api/push/preview/route.ts index 8273df184..17fcd190d 100644 --- a/app/api/push/preview/route.ts +++ b/app/api/push/preview/route.ts @@ -19,6 +19,12 @@ interface ResolvedTarget { accountId: string; /** See StalwartCredentials.trusted - false routes through the guarded fetch. */ trusted: boolean; + /** + * Cookie slot of the login that owns the account. Handed back so a click + * on the notification opens the message in that login: the worker knows + * only the JMAP account id, which can repeat across servers. + */ + slot?: number; } // When the SW passes ?accountId=, we need the slot whose JMAP session owns @@ -54,7 +60,7 @@ async function resolveTargetForAccount(accountId: string): Promise { // A permalink can name the account it belongs to. Ids are only meaningful // within their account, so switch first - but only to a login that is - // actually connected; we can't authenticate on someone's behalf. - if (link.accountId && link.accountId !== useAuthStore.getState().activeAccountId) { + // actually connected; we can't authenticate on someone's behalf. A push + // notification names the login by its cookie slot. + const linkAccountId = link.accountId ?? (link.slot !== undefined + ? useAccountStore.getState().accounts.find((a) => a.cookieSlot === link.slot)?.id ?? `slot:${link.slot}` + : undefined); + if (linkAccountId && linkAccountId !== useAuthStore.getState().activeAccountId) { const target = useAccountStore.getState().accounts.find( - (a) => a.id === link.accountId && a.isConnected, + (a) => a.id === linkAccountId && a.isConnected, ); if (!target) { toast.error(t('deep_link.account_unavailable')); return; } - await switchAccount(link.accountId); + await switchAccount(linkAccountId); } const activeClient = useAuthStore.getState().client; diff --git a/lib/__tests__/deep-links.test.ts b/lib/__tests__/deep-links.test.ts index 6c0bb17ad..3b988c289 100644 --- a/lib/__tests__/deep-links.test.ts +++ b/lib/__tests__/deep-links.test.ts @@ -150,6 +150,20 @@ describe('mail paths', () => { }); }); + it('reads the login slot a push notification names', () => { + expect(parseMailPath(['message', 'm1'], new URLSearchParams('slot=2'))).toEqual({ + kind: 'message', id: 'm1', accountId: undefined, slot: 2, + }); + expect(parseMailPath(['folder', 'inbox'], new URLSearchParams('slot=0'))).toEqual({ + kind: 'folder', ref: 'inbox', accountId: undefined, slot: 0, + }); + for (const bad of ['slot=', 'slot=-1', 'slot=1.5', 'slot=x']) { + expect(parseMailPath(['message', 'm1'], new URLSearchParams(bad))).toEqual({ + kind: 'message', id: 'm1', accountId: undefined, + }); + } + }); + it('falls back to the bare mail path with nothing selected', () => { expect(buildMailPath({ mailboxId: null, emailId: null, threadId: null })).toBe('/mail'); expect(parseMailPath([])).toBeNull(); diff --git a/lib/__tests__/push-preview-route.test.ts b/lib/__tests__/push-preview-route.test.ts index 569da2a0c..3664c49f5 100644 --- a/lib/__tests__/push-preview-route.test.ts +++ b/lib/__tests__/push-preview-route.test.ts @@ -104,6 +104,13 @@ describe('push preview route account resolution', () => { expect(mailboxQuery?.methodCalls[0][1].accountId).toBe('g'); }); + it('names the login (cookie slot) the account belongs to', async () => { + // The worker only knows the JMAP account id, which can repeat across + // servers; the click must open the message in this login. + const { body } = await callRoute('a'); + expect(body.slot).toBe(0); + }); + it('rejects an account the session does not know', async () => { const { status } = await callRoute('stranger'); expect(status).toBe(401); diff --git a/lib/deep-links.ts b/lib/deep-links.ts index 9ed61b1ea..5ced1dfc1 100644 --- a/lib/deep-links.ts +++ b/lib/deep-links.ts @@ -131,10 +131,14 @@ const VIRTUAL_ALIAS_BY_ID: Record = Object.fromEntries( /** Mailbox roles that get a readable alias instead of their opaque JMAP id. */ const ALIASED_ROLES = new Set(['inbox', 'sent', 'drafts', 'trash', 'archive', 'junk']); +/** + * `accountId` is the local account (AccountEntry.id). `slot` names the + * login by its cookie slot instead - what a push notification knows. + */ export type MailDeepLink = - | { kind: 'folder'; ref: string; accountId?: string } - | { kind: 'message'; id: string; accountId?: string; fullscreen?: boolean } - | { kind: 'thread'; id: string; accountId?: string }; + | { kind: 'folder'; ref: string; accountId?: string; slot?: number } + | { kind: 'message'; id: string; accountId?: string; slot?: number; fullscreen?: boolean } + | { kind: 'thread'; id: string; accountId?: string; slot?: number }; export interface MailLinkState { mailboxId: string | null; @@ -210,6 +214,9 @@ export function parseMailPath( search?: URLSearchParams, ): MailDeepLink | null { const accountId = search?.get('account') ?? undefined; + const rawSlot = search?.get('slot'); + const slotNumber = rawSlot != null && rawSlot !== '' ? Number(rawSlot) : NaN; + const slot = Number.isInteger(slotNumber) && slotNumber >= 0 ? slotNumber : undefined; // `?view=fullscreen` asks for the message alone, no sidebar or list - what // a mail dragged out into a new browser tab opens as. The Pro shell always // opens message links as fullscreen email tabs and ignores the flag. @@ -219,14 +226,14 @@ export function parseMailPath( if (kind && value) { const id = decodeSegment(value); if (id) { - if (kind === 'message') return { kind: 'message', id, accountId, fullscreen }; - if (kind === 'thread') return { kind: 'thread', id, accountId }; - if (kind === 'folder') return { kind: 'folder', ref: id, accountId }; + if (kind === 'message') return { kind: 'message', id, accountId, slot, fullscreen }; + if (kind === 'thread') return { kind: 'thread', id, accountId, slot }; + if (kind === 'folder') return { kind: 'folder', ref: id, accountId, slot }; } } const legacyEmail = search?.get('email'); - if (legacyEmail) return { kind: 'message', id: legacyEmail, accountId }; + if (legacyEmail) return { kind: 'message', id: legacyEmail, accountId, slot }; return null; } diff --git a/public/sw.js b/public/sw.js index 57ec0c71a..7d31d609f 100644 --- a/public/sw.js +++ b/public/sw.js @@ -155,6 +155,9 @@ async function handlePush(event) { const unreadTotal = preview && typeof preview.unreadTotal === "number" ? preview.unreadTotal : 0; + // The login (cookie slot) that owns this account, so the click opens the + // message there and not in whichever account happens to be active. + const slot = preview && Number.isInteger(preview.slot) ? preview.slot : undefined; // Push subscription is scoped to EmailDelivery, but stragglers from the // older broader-types subscription, marking-as-read races and verification @@ -182,7 +185,7 @@ async function handlePush(event) { const groupTag = "bulwark-mail:" + (accountId || "default"); let title; let body; - let data = { kind: "mail-list", accountId }; + let data = { kind: "mail-list", accountId, slot }; if (email) { const sender = email.from && email.from[0]; @@ -196,7 +199,7 @@ async function handlePush(event) { body += "\n" + (more === 1 ? "+1 more message" : `+${more} more messages`); } else { // Exactly one unread: deep-link straight to that message on click. - data = { kind: "email", emailId: email.id, threadId: email.threadId }; + data = { kind: "email", emailId: email.id, threadId: email.threadId, slot }; } } else { title = accountLabel ? `New mail (${accountLabel})` : "New mail"; @@ -458,10 +461,16 @@ function getReusableClientScore(state) { function buildClickUrl(data) { if (!data) return `${BASE_PATH}/`; + // `?slot=` names the login the message belongs to (see the preview API). + const slotQuery = Number.isInteger(data.slot) ? `?slot=${data.slot}` : ""; if (data.kind === "email" && data.emailId) { // Permalink (#733). Under NEXT_PUBLIC_LOCALE_PREFIX=always the proxy // redirects this to the localised path; the worker has no locale to add. - return `${BASE_PATH}/mail/message/${encodeURIComponent(data.emailId)}`; + return `${BASE_PATH}/mail/message/${encodeURIComponent(data.emailId)}${slotQuery}`; + } + // A group of new mail in a known login: that login's Inbox. + if (data.kind === "mail-list" && slotQuery) { + return `${BASE_PATH}/mail/folder/inbox${slotQuery}`; } // Generic "New mail" toast (preview API failed or returned no email): land // the user on the latest unread message in their Inbox rather than just the