diff --git a/KNOWN-ISSUES.md b/KNOWN-ISSUES.md index 00f4f8e..864f252 100644 --- a/KNOWN-ISSUES.md +++ b/KNOWN-ISSUES.md @@ -48,6 +48,8 @@ works the same way — and dropped where 0.15 was the whole subject. Support for 0.15 was removed on 2026-08-26; the last release that runs on it is tagged [`stalwart-0.15-support`](https://github.com/Coffey-Labs/ihasmail/releases/tag/stalwart-0.15-support). +- **Push subscriptions are not replaced by a repeated `deviceClientId`, and an account holds fifteen.** ihasmail registered a new subscription on every renewal believing the old one would be replaced, as the mock did. **Confirmed live (0.16.22, 2026-09-16)**: a second create with the same `deviceClientId` leaves both in place, the sixteenth create is refused with `overQuota`, "There are too many subscriptions, please delete some before adding a new one.", and `update` of `expires` is accepted. `PushSubscription/get` does not return `url` (nor `keys`), so a subscription can only be matched by its `deviceClientId`. A `types` of `[]` or `null` is stored as *every* type, not none. Read from the 0.16.22 source: `EmailDelivery` changes only on delivery, a delivery reaches a subscription with an `emailPush` filter as an EmailPush alone, and the payload carries `id` and `threadId` only when they are named in `properties`. Browsers now subscribe to `EmailDelivery` only, extend rather than re-create, clear their own duplicates and make room on `overQuota`; the server removes what its previous process registered. The mock follows all of it ([#375](https://github.com/Coffey-Labs/ihasmail/issues/375)). + - **Administration was built from Stalwart's source, and the first live run found the one thing the source reading got wrong.** Accounts and Domains were written on 2026-09-13 against the 0.16.22 source and a mock reproducing it, deployed the same day, and exercised against the live server from an administrator's session. On that server the Accounts list did not load: `x:Account/query` answered **`unsupportedFilter - type`**. A registry filter is keyed by the property's name *as it appears on the object*, and the discriminator is `@type`, so `{"type": "User"}` names nothing the server knows and fails the whole query; `{"@type": "User"}` is accepted. The research that fed the build had listed the field as `type`, and the mock took it without complaint — which is how it shipped. Fixed in [#336](https://github.com/Coffey-Labs/ihasmail/pull/336), and the mock now refuses any filter name the real server does not index, answering the way Stalwart does. Everything else was **confirmed live (2026-09-13)**, mostly read-only, with the domain writes made on a throwaway domain created for the purpose and removed afterwards: - **Permissions** come from `GET /api/account` in camelCase (`sysAccountGet`); an administrator's list held 641 of them and none were kebab-case, whatever the documentation shows. The menu gates on these. diff --git a/server/src/mock/handlers.ts b/server/src/mock/handlers.ts index c7a637f..ada85f3 100644 --- a/server/src/mock/handlers.ts +++ b/server/src/mock/handlers.ts @@ -7,6 +7,11 @@ import { ACCOUNT, MASKED, MAX_DELAYED_SEND, MOCK_LOCALE, NO_FUTURE_RELEASE, Obj, import { NO_KEYWORD_SORT, abRights, blobs, booksFor, calendarsFor, cards, compareBy, emails, eventsFor, fileNodes, fr, identities, mailboxes, mb, nodesFor, participantIdentities, principals, pushSubscriptions, putBlob, recount, rightsCal, seq, sharedCards, sieveScripts, vacationBox } from "./data.js"; import { Handler, MethodError, applyPatch, calendarEventParse, calendarEventSet, directory, genericGet, genericSet, hideShareWithUnlessAsked, matchFilter, matchSubmissionFilter, pick, resolveEvent, setResp, submissionView, submissions } from "./engine.js"; +/** Stalwart's limit per account (0.16.22). */ +const MAX_PUSH_SUBSCRIPTIONS = 15; +/** What an empty or missing `types` list is taken to mean: everything. */ +const ALL_PUSH_TYPES = ["Email", "EmailDelivery", "Mailbox", "Thread", "Identity", "EmailSubmission", "VacationResponse", "CalendarEvent", "Calendar", "ContactCard", "AddressBook", "FileNode", "Quota", "SieveScript", "PushSubscription"]; + export const handlers: Record = { // 0.16 exposes the account locale here, under a permission ordinary users // actually have (unlike x:Account below, which needs sysAccountGet). @@ -197,10 +202,17 @@ export const handlers: Record = { notCreated[cid] = { type: "invalidArguments", properties: ["emailPush"], description: "Invalid filter." }; continue; } - // One per device: re-subscribing replaces rather than accumulates. + /* + * As Stalwart does (checked live on 0.16.22, 2026-09-16): a repeated + * deviceClientId is a second subscription, not a replacement -- this mock + * used to replace, which is how the client's pile-up never showed here + * (#375) -- and an account holds at most fifteen. + */ const deviceId = String(o.deviceClientId ?? ""); - const clash = pushSubscriptions.findIndex((s) => s.deviceClientId === deviceId); - if (clash >= 0) pushSubscriptions.splice(clash, 1); + if (pushSubscriptions.length >= MAX_PUSH_SUBSCRIPTIONS) { + notCreated[cid] = { type: "overQuota", description: "There are too many subscriptions, please delete some before adding a new one." }; + continue; + } const id = `ps${randomUUID().slice(0, 6)}`; /* * A subscription expires, and this used to hand back `expires: null`. @@ -212,7 +224,9 @@ export const handlers: Record = { * so "does this client renew?" is a question the mock can answer. */ const expires = new Date(Date.now() + PUSH_TTL_MS).toISOString(); - pushSubscriptions.push({ id, deviceClientId: deviceId, url: o.url, types: o.types ?? null, emailPush: o.emailPush ?? null, expires, keys, verified: false, code: `v${randomUUID().slice(0, 8)}` }); + // An empty or missing list means every type, not none. + const types = Array.isArray(o.types) && o.types.length ? o.types : ALL_PUSH_TYPES; + pushSubscriptions.push({ id, deviceClientId: deviceId, url: o.url, types, emailPush: o.emailPush ?? null, expires, keys, verified: false, code: `v${randomUUID().slice(0, 8)}` }); created[cid] = { id, expires }; state.n++; } @@ -224,6 +238,13 @@ export const handlers: Record = { if (code !== s.code) { notUpdated[id] = { type: "invalidProperties", properties: ["verificationCode"], description: "Verification code does not match." }; continue; } s.verified = true; } + // An expiry can be extended, up to the same seven days a new one gets. + const wanted = (patch as Obj).expires; + if (typeof wanted === "string") { + const at = Math.min(Date.parse(wanted), Date.now() + PUSH_TTL_MS); + if (Number.isNaN(at)) { notUpdated[id] = { type: "invalidProperties", properties: ["expires"] }; continue; } + s.expires = new Date(at).toISOString(); + } updated[id] = null; state.n++; } diff --git a/server/src/push.test.ts b/server/src/push.test.ts index 5621f7d..e37f950 100644 --- a/server/src/push.test.ts +++ b/server/src/push.test.ts @@ -133,3 +133,55 @@ test("a tab on the relay is moved to fan-out when its account verifies, and its assert.match(out.written.at(-1) ?? "", /StateChange/, "the same browser stream now receives fan-out"); } finally { restore(); } }); + +test("a new subscription clears what this installation left behind, and only that", async () => { + // What a restart finds: its own subscription from the last process, another + // installation's on the same server, a browser's, and the old id format. + const calls: Array<[string, Record]> = []; + let ownPrefix = ""; + const real = globalThis.fetch; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + if (url.endsWith("/.well-known/jmap") || url.includes("/jmap/session")) { + return new Response(JSON.stringify({ apiUrl: "http://127.0.0.1:1/jmap/", primaryAccounts: { "urn:ietf:params:jmap:mail": "a" }, + accounts: { a: {} }, capabilities: {}, eventSourceUrl: "", downloadUrl: "", uploadUrl: "", state: "s" }), { status: 200, headers: { "content-type": "application/json" } }); + } + const { methodCalls } = JSON.parse(String(init?.body)) as { methodCalls: [string, Record, string][] }; + const [name, args, id] = methodCalls[0]!; + calls.push([name, args]); + let result: Record = {}; + if (name === "PushSubscription/get") { + result = { list: [ + { id: "mine-before", deviceClientId: `${ownPrefix}oldtoken` }, + { id: "other-install", deviceClientId: "ihasmail-proxy-ZZZZZZZZZZ-12345678" }, + { id: "a-browser", deviceClientId: "ihasmail-00000000-0000-4000-8000-000000000001" }, + { id: "old-format", deviceClientId: "ihasmail-Ab3_x9Qz" }, + ] }; + } else if (name === "PushSubscription/set" && args.create) { + const body = (args.create as Record).s!; + result = { created: { s: { id: "fresh", expires: new Date(Date.now() + 7 * 86_400_000).toISOString() } } }; + calls.at(-1)![1] = { ...args, deviceClientId: body.deviceClientId }; + } else { + result = { destroyed: args.destroy }; + } + return new Response(JSON.stringify({ methodResponses: [[name, result, id]] }), { status: 200, headers: { "content-type": "application/json" } }); + }) as typeof fetch; + try { + // The installation's prefix, learned the way the server makes it: from its first create. + push.prepare("probe-prefix@example.com", "a", "Basic p"); + await new Promise((r) => setTimeout(r, 30)); + const firstCreate = calls.find(([n, a]) => n === "PushSubscription/set" && a.create); + const deviceId = String(firstCreate?.[1].deviceClientId ?? ""); + assert.match(deviceId, /^ihasmail-proxy-[A-Za-z0-9_-]{10}-[A-Za-z0-9_-]{8}$/, "the server's own prefix, naming the installation"); + ownPrefix = deviceId.slice(0, deviceId.lastIndexOf("-") + 1); + + calls.length = 0; + push.prepare("restart@example.com", "a", "Basic r"); + await new Promise((r) => setTimeout(r, 30)); + const destroyed = calls.filter(([n, a]) => n === "PushSubscription/set" && a.destroy).flatMap(([, a]) => a.destroy as string[]); + assert.deepEqual(destroyed, ["mine-before"], "only this installation's leftover goes"); + assert.ok(calls.some(([n, a]) => n === "PushSubscription/set" && a.create), "and a new one is made"); + } finally { + globalThis.fetch = real; + } +}); diff --git a/server/src/push.ts b/server/src/push.ts index d681644..a6caeca 100644 --- a/server/src/push.ts +++ b/server/src/push.ts @@ -23,7 +23,7 @@ * transition loses no events, because a tab opened before verification keeps * its own relay for its whole life. */ -import { randomBytes } from "node:crypto"; +import { createHash, randomBytes } from "node:crypto"; import type { ServerResponse } from "node:http"; import { config } from "./config.js"; import { absoluteUpstream, getUpstreamSession, upstreamFor } from "./upstream.js"; @@ -71,10 +71,58 @@ async function jmap(entry: AccountPush, calls: unknown[]) { return (await res.json()) as { methodResponses: [string, Record, string][] }; } +/* + * Whose subscriptions are whose. + * + * Each process used to register a subscription per account and forget it when + * it stopped -- state here is in memory, and an immutable deployment restarts + * on every deploy -- so each restart left one more behind, receiving 404s until + * it expired. Stalwart keeps them all and allows fifteen per account (checked + * live on 0.16.22, 2026-09-16), which the browser subscriptions count against + * too (#375). + * + * So the device id names the installation -- a hash of the address Stalwart + * posts to, stable across restarts and different for another installation on + * the same server -- and a new subscription first removes the ones this + * installation left before. The `ihasmail-proxy-` prefix keeps them apart from + * the browsers' own, which the web client may clear to make room. + */ +function installationId(): string { + return createHash("sha256").update(`${config.pushUrl}${config.basePath}`).digest("base64url").slice(0, 10); +} + +function deviceIdFor(entry: AccountPush): string { + return `ihasmail-proxy-${installationId()}-${entry.token.slice(0, 8)}`; +} + +async function removeLeftovers(entry: AccountPush) { + const mine = `ihasmail-proxy-${installationId()}-`; + const r = await jmap(entry, [["PushSubscription/get", { ids: null, properties: ["id", "deviceClientId"] }, "0"]]); + const list = (r.methodResponses[0]?.[1] as { list?: Array<{ id: string; deviceClientId?: string }> }).list ?? []; + const stale = list.filter((s) => s.id !== entry.subscriptionId && String(s.deviceClientId ?? "").startsWith(mine)).map((s) => s.id); + if (stale.length) await jmap(entry, [["PushSubscription/set", { destroy: stale }, "0"]]); +} + +/** Give the live subscription another week, rather than registering a second one. */ +async function renew(entry: AccountPush) { + const expires = new Date(Date.now() + 7 * 86_400_000).toISOString().replace(/\.\d+Z$/, "Z"); + const r = await jmap(entry, [["PushSubscription/set", { update: { [entry.subscriptionId!]: { expires } } }, "0"]]); + const res = r.methodResponses[0]?.[1] as { updated?: Record; notUpdated?: Record }; + if (!res.updated || !(entry.subscriptionId! in res.updated)) throw new Error("subscription not extended"); + const got = await jmap(entry, [["PushSubscription/get", { ids: [entry.subscriptionId], properties: ["expires"] }, "0"]]); + const after = (got.methodResponses[0]?.[1] as { list?: Array<{ expires?: string | null }> }).list?.[0]?.expires; + entry.expires = after ? Date.parse(after) : Date.parse(expires); +} + async function subscribe(entry: AccountPush) { + try { + await removeLeftovers(entry); + } catch (err) { + console.warn(`[ihasmail] push: could not clear old subscriptions for ${entry.username}: ${(err as Error).message}`); + } const url = `${config.pushUrl!.replace(/\/$/, "")}${config.basePath}/api/push/${entry.token}`; const r = await jmap(entry, [["PushSubscription/set", { - create: { s: { deviceClientId: `ihasmail-${entry.token.slice(0, 8)}`, url, + create: { s: { deviceClientId: deviceIdFor(entry), url, types: ["Email", "Mailbox", "Thread", "Identity", "EmailSubmission", "VacationResponse"] } }, }, "0"]]); const created = (r.methodResponses[0]?.[1] as { created?: Record }).created?.s; @@ -184,8 +232,13 @@ function startSweeper() { console.warn(`[ihasmail] push: no verification for ${entry.username} within ${VERIFY_TIMEOUT_MS / 1000}s; relay in use`); } if (entry.state === "verified" && entry.expires - now < RENEW_BEFORE_MS) { - entry.state = "pending"; entry.since = now; - subscribe(entry).catch(() => { entry.state = "failed"; }); + // Extended in place, which keeps it verified. Only if the server will + // not is a new one registered, and that one has to verify again. + entry.expires = now + RENEW_BEFORE_MS; + renew(entry).catch(() => { + entry.state = "pending"; entry.since = Date.now(); + subscribe(entry).catch(() => { entry.state = "failed"; }); + }); } if (entry.tabs.size === 0 && (entry.state === "failed" || now - entry.since > 10 * 60_000)) { void unsubscribe(entry); diff --git a/web/public/sw.js b/web/public/sw.js index 2134961..af0539b 100644 --- a/web/public/sw.js +++ b/web/public/sw.js @@ -346,6 +346,14 @@ self.addEventListener("push", (event) => { const emails = (data && data["@type"] === "EmailPush" && Array.isArray(data.emails)) ? data.emails : []; event.waitUntil((async () => { + /* + * Someone reading the app already knows. A focused, visible window of this + * app gets its new mail from its own event stream, so a notification on + * top of it is a second telling of the same thing (#375). Chrome does not + * require one while the site is in the foreground. + */ + const windows = await self.clients.matchAll({ type: "window" }); + if (windows.some((w) => w.focused && w.visibilityState === "visible")) return; const facts = await readFacts(); const strings = facts?.strings ?? { newMail: "New mail", newMessage: "New message", noSubject: "(no subject)" }; /* @@ -361,8 +369,10 @@ self.addEventListener("push", (event) => { if ("setAppBadge" in self.navigator) await self.navigator.setAppBadge().catch(() => {}); if (!emails.length) { - // A StateChange, or a payload too large to carry the message. Say - // something true rather than inventing a sender. + // A delivery from a server that sends StateChange rather than EmailPush + // -- the subscription asks for `EmailDelivery` only, so it is new mail -- + // or a payload too large to carry the message. Say something true + // rather than inventing a sender. await self.registration.showNotification(strings.newMail, { icon: `${BASE}/img/icon-192.png`, badge: `${BASE}/img/favicon-64.png`, tag: "ihasmail-mail", data: { url: `${BASE}/mail` }, }); @@ -382,7 +392,8 @@ self.addEventListener("push", (event) => { // be drawn. actions: email.id ? actionsFor(facts) : [], data: { - url: email.id ? `${BASE}/mail/inbox/${email.id}` : `${BASE}/mail`, + // The route names a conversation, and `m` the message in it. + url: email.id && email.threadId ? `${BASE}/mail/inbox/${email.threadId}?m=${encodeURIComponent(email.id)}` : `${BASE}/mail`, id: email.id || null, title, accountId: facts?.accountId ?? null, diff --git a/web/src/lib/notify/__tests__/push-registration.test.ts b/web/src/lib/notify/__tests__/push-registration.test.ts new file mode 100644 index 0000000..61464bc --- /dev/null +++ b/web/src/lib/notify/__tests__/push-registration.test.ts @@ -0,0 +1,164 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { client } from "@/jmap/client"; +import type { JmapSession } from "@/jmap/types"; +import { setDeviceTrusted } from "@/lib/storage"; +import { deviceClientId, isBrowserSubscription, rememberEndpoint, roomToMake, setPushEnabledHere, type JmapPushSubscription } from "@/lib/notify/webpush"; +import { renewWebPush } from "@/lib/notify/webpushEnable"; + +/** + * #375: every renewal registered another subscription, on the belief that a + * repeated deviceClientId replaces the old one. Stalwart keeps both and allows + * fifteen per account, so accounts filled up with "too many subscriptions". + * + * The server below behaves as a live 0.16.22 was seen to: duplicates are kept, + * the sixteenth is refused with overQuota, and an expiry can be extended. + */ + +const KEY = "BBvig2GPmqohMJJHMzp6bTKviHibYiVCyAY8gdq2fPhS-9YfO9_0TnhMyZ0a0JxTsbCqd3zm1rEiXsXsL3jveJY"; +const DAY = 24 * 60 * 60 * 1000; +const OTHER = (n: number) => `ihasmail-00000000-0000-4000-8000-${String(n).padStart(12, "0")}`; + +let server: Array; +let writes: Array<[string, Record]>; +let seq: number; + +const fakeSub = (endpoint: string) => ({ + endpoint, + toJSON: () => ({ endpoint, keys: { p256dh: "BPub", auth: "auth" } }), + getKey: () => null, +}); +let browserSub: ReturnType | null; + +function install() { + client.session = { capabilities: { "urn:ietf:params:jmap:core": { maxCallsInRequest: 16 }, "urn:ietf:params:jmap:webpush-vapid": { applicationServerKey: KEY } }, accounts: {}, primaryAccounts: {}, state: "s" } as unknown as JmapSession; + vi.stubGlobal("PushManager", function PushManager() {}); + vi.stubGlobal("Notification", { permission: "granted" }); + const reg = { + pushManager: { + getSubscription: async () => browserSub, + subscribe: async () => (browserSub = fakeSub("https://push.example/new-endpoint")), + }, + }; + Object.defineProperty(navigator, "serviceWorker", { + configurable: true, + value: { ready: Promise.resolve(reg), getRegistration: async () => reg, addEventListener: () => {} }, + }); + vi.stubGlobal("fetch", vi.fn(async (_url: string, init: RequestInit) => { + const { methodCalls } = JSON.parse(init.body as string) as { methodCalls: [string, Record, string][] }; + const methodResponses = methodCalls.map(([name, args, id]) => { + if (name === "PushSubscription/get") return [name, { list: server.map((s) => ({ ...s })), notFound: [] }, id]; + writes.push([name, args]); + const created: Record = {}; + const notCreated: Record = {}; + const updated: Record = {}; + for (const [cid, body] of Object.entries((args.create ?? {}) as Record>)) { + if (server.length >= 15) { notCreated[cid] = { type: "overQuota", description: "There are too many subscriptions, please delete some before adding a new one." }; continue; } + const sub = { id: `p${seq++}`, deviceClientId: String(body.deviceClientId), expires: new Date(Date.now() + 7 * DAY).toISOString(), verificationCode: null, types: body.types as string[] }; + server.push(sub); + created[cid] = { id: sub.id, expires: sub.expires }; + } + for (const [sid, patch] of Object.entries((args.update ?? {}) as Record>)) { + const s = server.find((x) => x.id === sid); + if (s && typeof patch.expires === "string") { s.expires = patch.expires; updated[sid] = null; } + } + const destroy = (args.destroy ?? []) as string[]; + server = server.filter((s) => !destroy.includes(s.id)); + return [name, { created, notCreated, updated, destroyed: destroy }, id]; + }); + return { ok: true, status: 200, json: async () => ({ methodResponses, sessionState: "s" }) } as Response; + })); +} + +beforeEach(() => { + localStorage.clear(); + server = []; + writes = []; + seq = 1; + browserSub = fakeSub("https://push.example/endpoint-a"); + setDeviceTrusted(true); + setPushEnabledHere(true); + install(); +}); + +afterEach(() => { + client.session = null; + vi.unstubAllGlobals(); +}); + +const mine = () => server.filter((s) => s.deviceClientId === deviceClientId()); +const ours = (expiresIn: number, id = `m${seq++}`) => ({ id, deviceClientId: deviceClientId(), expires: new Date(Date.now() + expiresIn).toISOString(), verificationCode: "done" }); + +describe("keeping this browser registered", () => { + it("registers once, for new mail only, and remembers the endpoint", async () => { + await renewWebPush(); + expect(mine()).toHaveLength(1); + expect(mine()[0]!.types).toEqual(["EmailDelivery"]); + // Started again straight away: nothing more to do. + writes = []; + await renewWebPush(); + expect(writes).toEqual([]); + expect(mine()).toHaveLength(1); + }); + + it("leaves a subscription with time on it alone", async () => { + rememberEndpoint(browserSub!.endpoint); + server.push(ours(6 * DAY)); + await renewWebPush(); + expect(writes).toEqual([]); + }); + + it("extends one that is close to expiring instead of adding another", async () => { + rememberEndpoint(browserSub!.endpoint); + server.push(ours(1 * DAY, "keep")); + await renewWebPush(); + expect(writes.map(([n, a]) => `${n} ${Object.keys(a).join(",")}`)).toEqual(["PushSubscription/set update"]); + expect(mine()).toHaveLength(1); + expect(Date.parse(mine()[0]!.expires!) - Date.now()).toBeGreaterThan(6 * DAY); + }); + + it("clears the copies earlier versions left, keeping the newest", async () => { + rememberEndpoint(browserSub!.endpoint); + server.push(ours(1 * DAY), ours(3 * DAY), ours(6 * DAY, "newest")); + await renewWebPush(); + expect(mine().map((s) => s.id)).toEqual(["newest"]); + }); + + it("replaces its registrations when the browser's endpoint has changed", async () => { + rememberEndpoint("https://push.example/an-old-endpoint"); + server.push(ours(6 * DAY, "old1"), ours(6 * DAY, "old2")); + await renewWebPush(); + expect(mine()).toHaveLength(1); + expect(mine()[0]!.id).not.toMatch(/^old/); + expect(localStorage.getItem("ihasmail:pushEndpoint")).toBe(browserSub!.endpoint); + }); + + it("makes room when the account is full, taking another browser's never-verified one first", async () => { + for (let i = 0; i < 13; i++) server.push({ id: `o${i}`, deviceClientId: OTHER(i), expires: new Date(Date.now() + (i + 1) * DAY / 4).toISOString(), verificationCode: "done" }); + server.push({ id: "unverified", deviceClientId: OTHER(99), expires: new Date(Date.now() + 6 * DAY).toISOString(), verificationCode: null }); + server.push({ id: "proxy", deviceClientId: "ihasmail-proxy-abcdefghij-12345678", expires: new Date(Date.now() + DAY).toISOString(), verificationCode: "done" }); + await renewWebPush(); + expect(mine()).toHaveLength(1); + expect(server.find((s) => s.id === "unverified")).toBeUndefined(); + expect(server.find((s) => s.id === "proxy")).toBeDefined(); + expect(server).toHaveLength(15); + }); +}); + +describe("telling subscriptions apart", () => { + it("recognizes a browser's id, and not the server's or another client's", () => { + const sub = (deviceClientId: string) => ({ id: "x", deviceClientId, expires: null }) as JmapPushSubscription; + expect(isBrowserSubscription(sub(OTHER(1)))).toBe(true); + expect(isBrowserSubscription(sub("ihasmail-proxy-abcdefghij-12345678"))).toBe(false); + expect(isBrowserSubscription(sub("ihasmail-Ab3_x9Qz"))).toBe(false); + expect(isBrowserSubscription(sub("some-other-client"))).toBe(false); + }); + + it("chooses the soonest to expire when every candidate is verified", () => { + const subs = [ + { id: "later", deviceClientId: OTHER(1), expires: new Date(Date.now() + 5 * DAY).toISOString(), verificationCode: "v" }, + { id: "sooner", deviceClientId: OTHER(2), expires: new Date(Date.now() + DAY).toISOString(), verificationCode: "v" }, + { id: "me", deviceClientId: OTHER(3), expires: new Date(Date.now()).toISOString(), verificationCode: "v" }, + ]; + expect(roomToMake(subs, OTHER(3))).toEqual(["sooner"]); + }); +}); diff --git a/web/src/lib/notify/__tests__/webpush.test.ts b/web/src/lib/notify/__tests__/webpush.test.ts index 2845675..a3a22d2 100644 --- a/web/src/lib/notify/__tests__/webpush.test.ts +++ b/web/src/lib/notify/__tests__/webpush.test.ts @@ -125,9 +125,15 @@ describe("what gets registered", () => { expect(subscriptionPayload(fakeSub, null)).not.toHaveProperty("emailPush"); }); - it("subscribes to Email changes only, since EventSource covers an open tab", () => { + it("subscribes to deliveries only, so reading or moving mail elsewhere sends nothing", () => { client.session = session({ "urn:ietf:params:jmap:webpush-vapid": { applicationServerKey: LIVE_KEY } }); - expect((subscriptionPayload(fakeSub, "a1") as Record).types).toEqual(["Email"]); + expect((subscriptionPayload(fakeSub, "a1") as Record).types).toEqual(["EmailDelivery"]); + }); + + it("asks for the message and conversation ids, which Stalwart only sends when named", () => { + client.session = session({ "urn:ietf:params:jmap:webpush-vapid": { applicationServerKey: LIVE_KEY }, "urn:ietf:params:jmap:emailpush": {} }); + const payload = subscriptionPayload(fakeSub, "a1") as { emailPush: Record }; + expect(payload.emailPush.a1!.properties).toEqual(expect.arrayContaining(["id", "threadId"])); }); }); diff --git a/web/src/lib/notify/notify.ts b/web/src/lib/notify/notify.ts index 2762e69..55230ea 100644 --- a/web/src/lib/notify/notify.ts +++ b/web/src/lib/notify/notify.ts @@ -82,14 +82,30 @@ export async function requestNotificationPermission(): Promise`), so if both ever show it, the second + * replaces the first instead of stacking beside it. + */ export function showNotification(title: string, opts: NotificationOptions & { onClick?: () => void } = {}): void { if (!("Notification" in window) || Notification.permission !== "granted") return; if (document.visibilityState === "visible" && document.hasFocus()) return; + const { onClick, ...options } = opts; + const full = { icon: withBase("/img/icon-192.png"), badge: withBase("/img/favicon-64.png"), ...options }; + const viaWorker = navigator.serviceWorker?.controller ? navigator.serviceWorker.ready : null; + if (viaWorker) { + void viaWorker.then((reg) => reg.showNotification(title, full)).catch(() => undefined); + return; + } try { - const n = new Notification(title, { icon: withBase("/img/icon-192.png"), badge: withBase("/img/favicon-64.png"), ...opts }); + const n = new Notification(title, full); n.onclick = () => { window.focus(); - opts.onClick?.(); + onClick?.(); n.close(); }; setTimeout(() => n.close(), 8000); diff --git a/web/src/lib/notify/webpush.ts b/web/src/lib/notify/webpush.ts index 5e37a26..5c6970a 100644 --- a/web/src/lib/notify/webpush.ts +++ b/web/src/lib/notify/webpush.ts @@ -24,17 +24,36 @@ import { isDeviceTrusted } from "@/lib/storage"; export const VAPID_CAP = "urn:ietf:params:jmap:webpush-vapid"; export const EMAILPUSH_CAP = "urn:ietf:params:jmap:emailpush"; -/** Which Email properties to put in the payload, best first. */ -const PAYLOAD_PROPS = ["from", "subject", "preview", "receivedAt"]; +/** + * Which Email properties to put in the payload, best first. + * + * `id` and `threadId` have to be asked for: Stalwart sends only what is named + * (0.16.22 source). Without them a notification could not be tagged by + * message, carried no Archive or Mark-read button, and opened the inbox rather + * than the message. + */ +const PAYLOAD_PROPS = ["id", "threadId", "from", "subject", "preview", "receivedAt"]; export interface JmapPushSubscription { id: Id; deviceClientId: string; - url: string; + /** Write-only: Stalwart never returns it, so a subscription cannot be matched by endpoint. */ + url?: string; expires: string | null; verificationCode?: string | null; } +/** A `PushSubscription/set` refusal, with the server's type kept for deciding what to do. */ +export class PushSetError extends Error { + constructor( + readonly type: string, + message: string, + ) { + super(message); + this.name = "PushSetError"; + } +} + /** The VAPID key this server signs with, or null if it does not do Web Push. */ export function applicationServerKey(): string | null { const cap = client.session?.capabilities?.[VAPID_CAP] as { applicationServerKey?: string } | undefined; @@ -126,9 +145,19 @@ export function subscriptionPayload(sub: PushSubscription, accountId: Id | null, deviceClientId: deviceClientId(), url: sub.endpoint, keys: { p256dh: json.keys?.p256dh ?? encodeKey(sub.getKey("p256dh")), auth: json.keys?.auth ?? encodeKey(sub.getKey("auth")) }, - // StateChange notifications are not wanted: the app already has EventSource - // while it is open, and this channel exists for when it is not. - types: ["Email"], + /* + * New mail, and nothing else. + * + * `EmailDelivery` changes only when a message is delivered; `Email` changes + * on every read, flag and move, from any client, and each of those arrived + * here as a push the worker could only show as "New mail" (#375). With an + * `emailPush` filter, Stalwart sends a delivery as an EmailPush alone; a + * server without emailpush turns it into a StateChange naming + * `EmailDelivery`, which is then a true "New mail". An empty or null list + * is not "none": Stalwart takes it as every type there is (checked live on + * 0.16.22, 2026-09-16). + */ + types: ["EmailDelivery"], }; if (accountId && supportsEmailPush()) { body.emailPush = { @@ -185,9 +214,51 @@ export function setPushEnabledHere(on: boolean): void { */ export const RENEW_WITHIN_MS = 2 * 24 * 60 * 60 * 1000; +/** + * This browser's registered subscriptions, the one with the most time left + * first. + * + * Plural because Stalwart keeps every create: a second subscription with the + * same `deviceClientId` sits beside the first rather than replacing it + * (checked live on 0.16.22, 2026-09-16), so an account holds as many as were + * ever registered until each one expires. + */ +export function mySubscriptions(subs: JmapPushSubscription[], deviceId: string): JmapPushSubscription[] { + const left = (s: JmapPushSubscription) => (s.expires ? Date.parse(s.expires) || 0 : Number.MAX_SAFE_INTEGER); + return subs.filter((s) => s.deviceClientId === deviceId).sort((a, b) => left(b) - left(a)); +} + /** This browser's registered subscription, out of everything the account has. */ export function findSubscription(subs: JmapPushSubscription[], deviceId: string): JmapPushSubscription | null { - return subs.find((s) => s.deviceClientId === deviceId) ?? null; + return mySubscriptions(subs, deviceId)[0] ?? null; +} + +/** + * Whether a subscription was registered by a browser running ihasmail, rather + * than by the ihasmail server (`ihasmail-proxy-`, or the older eight-character + * form) or by another client altogether. + */ +export function isBrowserSubscription(s: JmapPushSubscription): boolean { + return /^ihasmail-[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(s.deviceClientId); +} + +/** + * Which subscriptions to let go of when the account is at its limit. + * + * Stalwart allows fifteen per account and refuses the sixteenth with + * `overQuota` (checked live on 0.16.22, 2026-09-16). Only browser + * subscriptions are candidates, never this browser's and never the server's: + * one that never verified first, then the one closest to expiring. A device + * that loses its subscription this way registers again the next time the app + * is opened there, because it no longer finds its own. + */ +export function roomToMake(subs: JmapPushSubscription[], deviceId: string, count = 1): Id[] { + const expiry = (s: JmapPushSubscription) => (s.expires ? Date.parse(s.expires) || 0 : Number.MAX_SAFE_INTEGER); + return subs + .filter((s) => s.deviceClientId !== deviceId && isBrowserSubscription(s)) + .sort((a, b) => Number(Boolean(a.verificationCode)) - Number(Boolean(b.verificationCode)) || expiry(a) - expiry(b)) + .slice(0, count) + .map((s) => s.id); } /** @@ -225,10 +296,55 @@ export async function createSubscription(body: Record): Promise { create: { s: body } }, [CAP.core, VAPID_CAP, EMAILPUSH_CAP], ); - if (res.notCreated?.s) throw new Error(String(res.notCreated.s.description ?? res.notCreated.s.type)); + const refused = res.notCreated?.s; + if (refused) throw new PushSetError(String(refused.type), String(refused.description ?? refused.type)); return (res.created?.s as { id?: Id } | undefined)?.id ?? null; } +/** + * Give a registered subscription more time, rather than registering another. + * + * Seven days is JMAP's ceiling and what Stalwart grants a new one; the server + * may shorten what is asked for, and whatever it keeps is what counts. + */ +export async function extendSubscription(id: Id, now: number = Date.now()): Promise { + const expires = new Date(now + 7 * 24 * 60 * 60 * 1000).toISOString().replace(/\.\d+Z$/, "Z"); + const res = await client.call>("PushSubscription/set", { update: { [id]: { expires } } }, [CAP.core, VAPID_CAP]); + const err = res.notUpdated?.[id]; + if (err) throw new PushSetError(String(err.type), String(err.description ?? err.type)); +} + +export async function destroySubscriptions(ids: Id[]): Promise { + if (!ids.length) return; + await client.call>("PushSubscription/set", { destroy: ids }, [CAP.core, VAPID_CAP]); +} + +/** + * The push endpoint this browser last registered with the server. + * + * The server never returns a subscription's URL, so this is the only way to + * tell a subscription that still points at this browser's endpoint from one + * made for an endpoint the browser has since replaced. + */ +const ENDPOINT_KEY = "ihasmail:pushEndpoint"; + +export function registeredEndpoint(): string | null { + try { + return localStorage.getItem(ENDPOINT_KEY); + } catch { + return null; + } +} + +export function rememberEndpoint(endpoint: string | null): void { + try { + if (endpoint) localStorage.setItem(ENDPOINT_KEY, endpoint); + else localStorage.removeItem(ENDPOINT_KEY); + } catch { + /* private mode: every start is then a fresh registration, which still works */ + } +} + /** * Hand back the code the server pushed. * @@ -262,10 +378,10 @@ export async function unsubscribeThisDevice(): Promise { /* the browser end is gone or was never there; still clear the server end */ } try { - const subs = await listSubscriptions(); - for (const s of subs) if (s.deviceClientId === mine) await destroySubscription(s.id); + await destroySubscriptions(mySubscriptions(await listSubscriptions(), mine).map((s) => s.id)); } catch { /* signing out must not fail over this */ } + rememberEndpoint(null); setPushEnabledHere(false); } diff --git a/web/src/lib/notify/webpushEnable.ts b/web/src/lib/notify/webpushEnable.ts index 3f40dbe..296c651 100644 --- a/web/src/lib/notify/webpushEnable.ts +++ b/web/src/lib/notify/webpushEnable.ts @@ -15,11 +15,18 @@ import { applicationServerKey, createSubscription, decodeApplicationServerKey, + destroySubscriptions, deviceClientId, + extendSubscription, findSubscription, listSubscriptions, - needsRenewal, + mySubscriptions, + PushSetError, pushEnabledHere, + registeredEndpoint, + rememberEndpoint, + RENEW_WITHIN_MS, + roomToMake, setPushEnabledHere, subscriptionPayload, unsubscribeThisDevice, @@ -64,8 +71,7 @@ async function collectStoredVerification(): Promise { } /** - * Subscribe this browser. Safe to call again — the deviceClientId makes a - * repeat replace rather than accumulate. + * Subscribe this browser. Safe to call again: see `registerThisBrowser`. * * Returns why it could not, rather than throwing, because every reason is * something to tell the user plainly: an old server, a browser without push, a @@ -98,11 +104,23 @@ export async function enableWebPush(): Promise<{ ok: true } | { ok: false; reaso } /** - * Get this browser subscribed at the push service and registered at Stalwart. + * Get this browser subscribed at the push service and registered at Stalwart, + * with exactly one subscription there, and that one current. * - * Shared by turning push on and by renewing it, because they are the same - * call: `deviceClientId` makes a repeat registration replace rather than - * accumulate, so there is no separate "update" path to get wrong. + * Shared by turning push on and by renewing it. It used to create a new + * subscription every time, on the belief that a repeated `deviceClientId` + * replaces the old one. Stalwart keeps both (checked live on 0.16.22), so each + * renewal added one, every start inside the renewal window added another, and + * the account reached its limit of fifteen -- "too many subscriptions" (#375). + * Now: + * + * - the same endpoint as last time, already registered: extend the newest one + * when it is close to expiring, and remove any extra copies; + * - anything else -- a new endpoint, nothing registered, an extension the + * server refused: remove this browser's old ones and register afresh. + * + * A registration refused for `overQuota` makes room among other browsers' + * subscriptions (`roomToMake`) and is tried once more. * * The local subscription is created when it is missing rather than only reused. * A browser may drop or rotate one on its own -- a `pushsubscriptionchange` @@ -117,9 +135,35 @@ async function registerThisBrowser(key: string): Promise { userVisibleOnly: true, applicationServerKey: decodeApplicationServerKey(key), })); - const accountId = useSession.getState().ownAccountFor(CAP.mail); - const inboxId = useMail.getState().roleId("inbox"); - await createSubscription(subscriptionPayload(sub, accountId, inboxId)); + const deviceId = deviceClientId(); + const subs = await listSubscriptions(); + const mine = mySubscriptions(subs, deviceId); + const [newest, ...extra] = mine; + + if (newest && registeredEndpoint() === sub.endpoint) { + if (extra.length) await destroySubscriptions(extra.map((s) => s.id)); + const at = newest.expires ? Date.parse(newest.expires) : Number.NaN; + if (!newest.expires || (!Number.isNaN(at) && at - Date.now() > RENEW_WITHIN_MS)) return; + try { + await extendSubscription(newest.id); + return; + } catch { + /* not extendable: replaced below */ + } + } + + if (mine.length) await destroySubscriptions(mine.map((s) => s.id)); + const payload = subscriptionPayload(sub, useSession.getState().ownAccountFor(CAP.mail), useMail.getState().roleId("inbox")); + try { + await createSubscription(payload); + } catch (err) { + if (!(err instanceof PushSetError) || err.type !== "overQuota") throw err; + const room = roomToMake(subs.filter((s) => !mine.includes(s)), deviceId); + if (!room.length) throw err; + await destroySubscriptions(room); + await createSubscription(payload); + } + rememberEndpoint(sub.endpoint); } /** @@ -141,7 +185,8 @@ export async function renewWebPush(): Promise { const key = applicationServerKey(); if (!key) return; try { - if (!needsRenewal(await listSubscriptions(), deviceClientId())) return; + // Cheap when nothing is due: one read, and a write only when a + // subscription is close to expiring, missing, or duplicated. await registerThisBrowser(key); listenForVerification(); } catch { diff --git a/web/src/store/mail/index.ts b/web/src/store/mail/index.ts index cd25d4c..0d7388d 100644 --- a/web/src/store/mail/index.ts +++ b/web/src/store/mail/index.ts @@ -29,6 +29,7 @@ import { withBase } from "@/lib/basePath"; import { MAILBOX_PROPS, LIST_PROPS, FULL_PROPS, BODY_PROPS } from "./props"; import { type ListQuery, type MailState } from "./types"; import { playNewMailSound, showNotification } from "@/lib/notify/notify"; +import { pushEnabledHere } from "@/lib/notify/webpush"; /* * `@/store/mail` stays the one public entry. The split below is about file @@ -1262,12 +1263,15 @@ async function notifyNewMail(created: Id[], get: () => MailState) { const fresh = emails.filter((e) => e.mailboxIds[inbox] && !e.keywords.$seen && !e.keywords.$draft); if (!fresh.length) return; if (s.notificationSound) playNewMailSound(); - if (s.desktopNotifications) { + // Where background notifications are on in this browser, the service worker + // shows these already; showing them here too was the duplicate in #375. + if (s.desktopNotifications && !pushEnabledHere()) { for (const e of fresh.slice(0, 3)) { const from = e.from?.[0]; showNotification(from?.name || from?.email || "New message", { body: `${e.subject || "(no subject)"}\n${e.preview ?? ""}`.trim(), - tag: e.id, + tag: `ihasmail-${e.id}`, + data: { url: withBase(`/mail/${inbox}/${e.threadId}?m=${encodeURIComponent(e.id)}`) }, onClick: () => { window.location.hash = ""; // The one navigation that does not go through wouter -- it is