Merge pull request #213 from Coffey-Labs/fix/push-verification-cache-key

Name the push verification entry absolutely, from both sides
This commit is contained in:
Coffey Labs
2026-09-02 01:11:32 -07:00
committed by GitHub
4 changed files with 69 additions and 3 deletions
+6
View File
@@ -1045,6 +1045,12 @@ would have quietly ended the WCAG AA claim two sections down.
run. Where the server also implements `emailpush`, the payload carries the run. Where the server also implements `emailpush`, the payload carries the
sender, subject and preview; without it the notification says only that mail sender, subject and preview; without it the notification says only that mail
arrived. Offered only on a device you said was yours. arrived. Offered only on a device you said was yours.
- The verification code a subscription needs is handed to an open tab, or left
in the browser's cache under a key **anchored to where the app is mounted**
for the next tab to collect. Both sides name it absolutely: a relative key is
resolved against the URL of whoever asks, so the worker at `<base>/sw.js` and
a tab at `/mail/inbox/…` were naming two different entries, and agreed only
when the open page happened to be the root.
- **The subscription is renewed on every app start**, because a JMAP push - **The subscription is renewed on every app start**, because a JMAP push
subscription expires — seven days is the ceiling — and re-registering before subscription expires — seven days is the ceiling — and re-registering before
it lapses is the client's job. Renewal can only happen with a page open: it lapses is the client's job. Renewal can only happen with a page open:
+12 -1
View File
@@ -74,7 +74,18 @@ self.addEventListener("fetch", (event) => {
* credentials), so it is stashed for a tab to collect and confirm. * credentials), so it is stashed for a tab to collect and confirm.
*/ */
const VERIFY_KEY = "ihasmail-push-verification"; /*
* Absolute, and anchored to the mount rather than to whatever page happens to
* be open.
*
* A relative key is resolved against the URL of whoever is asking: the worker
* lives at `<base>/sw.js`, so it stored this under `<base>/…`, while a tab at
* `/mail/inbox/abc` looked for it under `/mail/inbox/…`. The two only ever
* agreed when the open page was the root, so a verification code that arrived
* with no tab open was written where the next tab would not look -- and the
* subscription stayed silent, which is the same thing push failing looks like.
*/
const VERIFY_KEY = `${BASE}/ihasmail-push-verification`;
function textOf(email) { function textOf(email) {
const from = email?.from?.[0]; const from = email?.from?.[0];
@@ -0,0 +1,45 @@
import { describe, expect, it } from "vitest";
import { withBase, BASE_PATH } from "@/lib/basePath";
/**
* The verification code a push subscription needs is written by the service
* worker when no tab is open, and collected by the next tab to start. Both
* sides have to name the same cache entry.
*
* A relative key does not do that. It is resolved against the URL of whoever
* is asking: the worker lives at `<base>/sw.js`, so it wrote under `<base>/…`,
* while a tab at `/mail/inbox/abc` looked under `/mail/inbox/…`. They agreed
* only when the open page happened to be the root — and a subscription that
* never gets its code back stays silent, which is indistinguishable from push
* simply not working.
*
* These tests pin the shape of the key rather than the plumbing: what matters
* is that it is absolute and anchored to the mount, so it cannot vary with the
* route.
*/
const KEY = "/ihasmail-push-verification";
describe("the push verification cache key", () => {
it("is absolute, so it does not depend on which page is open", () => {
expect(withBase(KEY).startsWith("/")).toBe(true);
});
it("is the same string wherever it is asked for", () => {
// The bug was that this was not true: the page and the worker each
// resolved a relative key against their own URL.
expect(withBase(KEY)).toBe(withBase(KEY));
});
it("is anchored to the mount, which is what the worker anchors to", () => {
// The worker builds `${BASE}/ihasmail-push-verification`, where BASE comes
// from `new URL("./", self.location)` — the same mount this derives from.
expect(withBase(KEY)).toBe(`${BASE_PATH}${KEY}`);
});
it("carries no route in it", () => {
for (const route of ["mail", "inbox", "calendar", "settings"]) {
expect(withBase(KEY)).not.toContain(`/${route}/`);
}
});
});
+6 -2
View File
@@ -6,6 +6,7 @@
* permission prompt, none of which exists under a test runner. * permission prompt, none of which exists under a test runner.
*/ */
import { CAP } from "@/jmap/client"; import { CAP } from "@/jmap/client";
import { withBase } from "./basePath";
import { isDeviceTrusted } from "@/lib/storage"; import { isDeviceTrusted } from "@/lib/storage";
import { useSession } from "@/store/session"; import { useSession } from "@/store/session";
import { useMail } from "@/store/mail"; import { useMail } from "@/store/mail";
@@ -48,10 +49,13 @@ export function listenForVerification(): void {
async function collectStoredVerification(): Promise<void> { async function collectStoredVerification(): Promise<void> {
try { try {
const cache = await caches.open("ihasmail-v2"); const cache = await caches.open("ihasmail-v2");
const hit = await cache.match("ihasmail-push-verification"); // The same absolute key the worker writes. Relative would be resolved
// against this document's URL, which is a different place on every route.
const key = withBase("/ihasmail-push-verification");
const hit = await cache.match(key);
if (!hit) return; if (!hit) return;
const { id, code } = (await hit.json()) as { id?: string; code?: string }; const { id, code } = (await hit.json()) as { id?: string; code?: string };
await cache.delete("ihasmail-push-verification"); await cache.delete(key);
if (id && code) await verifySubscription(id, code); if (id && code) await verifySubscription(id, code);
} catch { } catch {
/* nothing waiting, or no cache: not a failure */ /* nothing waiting, or no cache: not a failure */