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:
@@ -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
@@ -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,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 */
|
||||||
|
|||||||
Reference in New Issue
Block a user