From 873e9cb4fbcc98791405c5f7d1b8cc4ae8d4405d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 13 Sep 2026 14:45:31 +0200 Subject: [PATCH] fix(pwa): keep a subscribed worker registered behind a gateway A same-origin auth gateway that answers an expired session with a 200 sign-in page returns HTML for both the navigation and the /api/version probe, which read as another app. The worker then unregistered and destroyed the Web Push subscription, which nothing re-creates, so reminders stopped without notice. The worker no longer unregisters while the registration holds a push subscription, or when the lookup is unavailable. It deletes its caches and passes requests to the network for the rest of its life instead. The probe now sends same-origin credentials, so a valid gateway cookie reaches HealthLog. Refs #847 --- public/sw.js | 42 ++++++++++++++++-- src/__tests__/service-worker.test.ts | 64 +++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/public/sw.js b/public/sw.js index 67c604453..44d6f8ed4 100644 --- a/public/sw.js +++ b/public/sw.js @@ -399,6 +399,14 @@ function isForeignNavigationResponse(response) { * an auth gateway, a redirect) proves nothing, and the worker resumes normal * service. * + * One gateway shape still reads as "another app": a same-origin auth gateway + * that answers an expired session with its sign-in page and status 200 (nginx + * `auth_request` with `error_page 401 = /oauth2/sign_in` does this). Both the + * navigation and this probe then come back as HTML. The probe sends the + * gateway cookie (`credentials: "same-origin"`), so a valid gateway session + * reaches HealthLog; with an expired one the verdict is wrong, which is why + * `retireFromForeignOrigin` never unregisters while a push subscription exists. + * * The probe is the worker's own `fetch`, which never passes through this * worker's fetch handler; `no-store` keeps the HTTP cache out of it and * `redirect: "manual"` keeps a login redirect from reading as an answer. @@ -409,7 +417,7 @@ async function confirmForeignOrigin() { response = await fetch("/api/version", { cache: "no-store", redirect: "manual", - credentials: "omit", + credentials: "same-origin", }); } catch { return false; @@ -456,13 +464,38 @@ function checkForeignOrigin() { return foreignOriginCheck; } +/** + * Whether this registration holds a Web Push subscription. Unregistering + * destroys the subscription and nothing in the app re-creates it (the push + * card only reads it), so reminders would stop without notice. A missing + * `pushManager` or a throwing lookup counts as "has one": keeping a worker + * that only passes requests through costs nothing, losing reminders does. + */ +async function hasPushSubscription() { + try { + const pushManager = self.registration.pushManager; + if (!pushManager || typeof pushManager.getSubscription !== "function") { + return true; + } + return (await pushManager.getSubscription()) !== null; + } catch { + return true; + } +} + /** * Leave an origin that no longer serves HealthLog: delete every HealthLog * cache (only ours, matched by name, never another app's CacheStorage) and * unregister, so the next load of that origin runs without this worker. The - * `foreignOrigin` flag set by the caller covers the requests the current page - * still makes before it unloads. Each step is best-effort and independent: a - * failed cache delete must not keep the worker registered. + * `foreignOrigin` flag set by the caller stays set, so the requests the + * current page still makes go to the network. Each step is best-effort and + * independent: a failed cache delete must not keep the worker registered. + * + * With a push subscription the worker does not unregister: the verdict may + * be a gateway's 200 sign-in page (see `confirmForeignOrigin`), and + * unregistering would destroy the subscription. It deletes the caches and + * stays in pass-through for the rest of its life instead, so another app on + * the origin still gets network content. The next worker start probes again. */ async function retireFromForeignOrigin() { try { @@ -475,6 +508,7 @@ async function retireFromForeignOrigin() { } catch { // CacheStorage unavailable; unregistering still matters more. } + if (await hasPushSubscription()) return; try { await self.registration.unregister(); } catch { diff --git a/src/__tests__/service-worker.test.ts b/src/__tests__/service-worker.test.ts index 0ec33c658..c64e25563 100644 --- a/src/__tests__/service-worker.test.ts +++ b/src/__tests__/service-worker.test.ts @@ -136,6 +136,8 @@ interface SwHarness { cacheStorage: FakeCacheStorage; navPreload: { enabled: boolean; enable: () => Promise }; unregisterCalls: { count: number }; + /** What `pushManager.getSubscription()` does; default: no subscription. */ + push: { getSubscription: () => Promise }; lifetimePromises: Promise[]; context: Record; } @@ -145,6 +147,7 @@ function bootServiceWorker(): SwHarness { const cacheStorage = new FakeCacheStorage(); const lifetimePromises: Promise[] = []; const unregisterCalls = { count: 0 }; + const push = { getSubscription: async (): Promise => null }; const navPreload = { enabled: false, enable: async () => { @@ -167,6 +170,9 @@ function bootServiceWorker(): SwHarness { unregisterCalls.count += 1; return true; }, + pushManager: { + getSubscription: () => push.getSubscription(), + }, showNotification: async (title: string, opts: { tag?: string }) => { shown.push({ title, tag: opts?.tag, closed: false }); }, @@ -219,6 +225,7 @@ function bootServiceWorker(): SwHarness { cacheStorage, navPreload, unregisterCalls, + push, lifetimePromises, context, }; @@ -803,7 +810,7 @@ describe("sw.js — foreign origin (#847)", () => { expect(harness.unregisterCalls.count).toBe(1); }); - it("probes with no-store and a manual redirect", async () => { + it("probes with no-store, a manual redirect and same-origin credentials", async () => { const harness = bootServiceWorker(); let probeInit: RequestInit | undefined; harness.context.fetch = async ( @@ -826,6 +833,8 @@ describe("sw.js — foreign origin (#847)", () => { expect(probeInit?.cache).toBe("no-store"); expect(probeInit?.redirect).toBe("manual"); + // A valid same-origin gateway cookie must reach /api/version. + expect(probeInit?.credentials).toBe("same-origin"); }); it("does not retire when a header-stripping proxy hides the marker from a real HealthLog", async () => { @@ -924,6 +933,59 @@ describe("sw.js — foreign origin (#847)", () => { expect(harness.unregisterCalls.count).toBe(1); }); + it("keeps a worker that holds a push subscription: caches deleted, no unregister, pass-through", async () => { + const harness = bootServiceWorker(); + await seedHealthlogCaches(harness); + harness.push.getSubscription = async () => ({ + endpoint: "https://push.example/abc", + }); + // A gateway's 200 sign-in page for both the navigation and the probe. + routeFetch(harness, foreignHtml, foreignHtml); + + await dispatchNavigationFetch(harness, "/"); + await Promise.all(harness.lifetimePromises); + + expect(harness.unregisterCalls.count).toBe(0); + expect([...harness.cacheStorage.stores.keys()]).toEqual([ + "next-app-runtime", + ]); + expect( + dispatchAnyFetch(harness, "/_next/static/chunks/app.js").responded, + ).toBe(false); + expect( + dispatchAnyFetch(harness, "/about", { accept: "text/html" }).responded, + ).toBe(false); + }); + + it.each([ + [ + "the subscription lookup throws", + (h: SwHarness) => { + h.push.getSubscription = async () => { + throw new Error("push service unavailable"); + }; + }, + ], + [ + "pushManager is missing", + (h: SwHarness) => { + delete (h.context.self as { registration: { pushManager?: unknown } }) + .registration.pushManager; + }, + ], + ])("does not unregister when %s", async (_label, arrange) => { + const harness = bootServiceWorker(); + await seedHealthlogCaches(harness); + arrange(harness); + routeFetch(harness, foreignHtml, foreignHtml); + + await dispatchNavigationFetch(harness, "/"); + await Promise.all(harness.lifetimePromises); + + expect(harness.unregisterCalls.count).toBe(0); + expect(harness.cacheStorage.stores.has(CURRENT_STATIC_CACHE)).toBe(false); + }); + it("still unregisters when deleting the caches fails", async () => { const harness = bootServiceWorker(); harness.cacheStorage.keys = async () => {