diff --git a/src/__tests__/destructive-control-guard.test.ts b/src/__tests__/destructive-control-guard.test.ts index a9a1bc80d..39bf621d0 100644 --- a/src/__tests__/destructive-control-guard.test.ts +++ b/src/__tests__/destructive-control-guard.test.ts @@ -348,7 +348,7 @@ const REGISTRY: DestructiveEntry[] = [ }, ], undetected: - "No detector sees it. The DELETE goes out from `unsubscribeWebPush` in `lib/notifications/web-push-unsubscribe.ts`, which sign-out calls too, and `lib/` is an excluded root. Registered by hand.", + "No detector sees it. The DELETE goes out from `lib/notifications/web-push-subscription.ts` (`unsubscribeWebPush`, and `signOutWithoutWebPush` for sign-out), and `lib/` is an excluded root. Registered by hand.", }, // ── Measurements, mood, labs, illness — the undo-and-restore surfaces ──── diff --git a/src/app/auth/register/__tests__/page.browser.test.tsx b/src/app/auth/register/__tests__/page.browser.test.tsx index 4a66bbd99..1a67ca9e4 100644 --- a/src/app/auth/register/__tests__/page.browser.test.tsx +++ b/src/app/auth/register/__tests__/page.browser.test.tsx @@ -6,7 +6,7 @@ * existed belongs to it. The app replaces the page in the history, so Back * does not show the form again. */ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { page } from "vitest/browser"; import { queryKeys } from "@/lib/query-keys"; @@ -48,6 +48,45 @@ describe("registration page", () => { }); describe("registration page, already signed in", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("takes this browser's push subscription with it on sign-out", async () => { + // The button exists so someone else can register on this browser; a + // subscription left behind would show them the last account's reminders. + const unsubscribe = vi.fn(async () => true); + vi.spyOn(navigator.serviceWorker, "getRegistration").mockResolvedValue({ + pushManager: { + getSubscription: async () => ({ + endpoint: "https://push.example.test/send/abc", + unsubscribe, + }), + }, + } as unknown as ServiceWorkerRegistration); + const api = stubFetch() + .on("DELETE", "/api/notifications/web-push", { data: {} }) + .on("POST", "/api/auth/logout", { data: { loggedOut: true } }); + window.history.replaceState(null, "", "/auth/register?invite=hlv_x"); + const { screen } = await renderWithApp(, { + user: { username: "signed-in" }, + }); + + await screen.getByTestId("register-sign-out").click(); + + await expect.poll(() => unsubscribe.mock.calls.length).toBe(1); + const order = api.requests + .map((r) => `${r.method} ${r.path.split("?")[0]}`) + .filter((call) => !call.startsWith("GET ")); + expect(order).toEqual([ + "DELETE /api/notifications/web-push", + "POST /api/auth/logout", + ]); + expect(api.calls("DELETE", "/api/notifications/web-push")[0].body).toEqual({ + endpoint: "https://push.example.test/send/abc", + }); + }); + it("keeps the page and says so when the server refuses the sign-out", async () => { const api = stubFetch().on("POST", "/api/auth/logout", { status: 403, diff --git a/src/app/auth/register/page.tsx b/src/app/auth/register/page.tsx index 8832b0f50..99b4a9b98 100644 --- a/src/app/auth/register/page.tsx +++ b/src/app/auth/register/page.tsx @@ -12,9 +12,13 @@ import { Label } from "@/components/ui/label"; import { PasswordStrength } from "@/components/ui/password-strength"; import { useTranslations } from "@/lib/i18n/context"; import { detectBrowserTimezone } from "@/lib/tz/format"; -import { ApiError, apiFetch, apiPost } from "@/lib/api/api-fetch"; +import { ApiError, apiPost } from "@/lib/api/api-fetch"; import { localizedApiError } from "@/lib/api/localized-error"; -import { clearCachesForSessionEnd, useAuth } from "@/hooks/use-auth"; +import { + clearCachesForSessionEnd, + signOutRequest, + useAuth, +} from "@/hooks/use-auth"; export default function RegisterPage() { const router = useRouter(); @@ -89,7 +93,7 @@ export default function RegisterPage() { setError(null); setSigningOut(true); try { - await apiFetch("/api/auth/logout", { method: "POST" }); + await signOutRequest(); // Same session-end wipe the shell and the menu do: the QueryClient // outlives the navigation on this SPA and the health-data families are // not user-scoped, so nothing may survive into the next account. diff --git a/src/components/auth/__tests__/claim-profile.browser.test.tsx b/src/components/auth/__tests__/claim-profile.browser.test.tsx index b4a80a369..d4a0f7080 100644 --- a/src/components/auth/__tests__/claim-profile.browser.test.tsx +++ b/src/components/auth/__tests__/claim-profile.browser.test.tsx @@ -117,6 +117,42 @@ describe("ClaimProfile", () => { }); describe("ClaimProfile, already signed in", () => { + it("takes this browser's push subscription with it on sign-out", async () => { + // The button exists so the profile's person can take over on this + // browser; a subscription left behind would show them the last + // account's reminders. + const unsubscribe = vi.fn(async () => true); + const registration = vi + .spyOn(navigator.serviceWorker, "getRegistration") + .mockResolvedValue({ + pushManager: { + getSubscription: async () => ({ + endpoint: "https://push.example.test/send/abc", + unsubscribe, + }), + }, + } as unknown as ServiceWorkerRegistration); + api + .on("DELETE", "/api/notifications/web-push", { data: {} }) + .on("POST", "/api/auth/logout", { data: { loggedOut: true } }); + await renderWithApp(, { + user: { username: "signed-in" }, + }); + + await click("claim-sign-out"); + + await expect.poll(() => unsubscribe.mock.calls.length).toBe(1); + const order = api.requests + .map((r) => `${r.method} ${r.path.split("?")[0]}`) + .filter((call) => !call.startsWith("GET ")); + // The signed-out page then loads the link's preview. + expect(order.slice(0, 2)).toEqual([ + "DELETE /api/notifications/web-push", + "POST /api/auth/logout", + ]); + registration.mockRestore(); + }); + it("keeps the session's page and says so when the server refuses the sign-out", async () => { api.on("POST", "/api/auth/logout", { status: 403, diff --git a/src/components/auth/claim-profile.tsx b/src/components/auth/claim-profile.tsx index 54120d064..0a3b60c9e 100644 --- a/src/components/auth/claim-profile.tsx +++ b/src/components/auth/claim-profile.tsx @@ -13,8 +13,12 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Logo } from "@/components/ui/logo"; import { PasswordStrength } from "@/components/ui/password-strength"; -import { clearCachesForSessionEnd, useAuth } from "@/hooks/use-auth"; -import { ApiError, apiFetch } from "@/lib/api/api-fetch"; +import { + clearCachesForSessionEnd, + signOutRequest, + useAuth, +} from "@/hooks/use-auth"; +import { ApiError } from "@/lib/api/api-fetch"; import { showErrorToast } from "@/lib/errors/error-toast"; import { useFormatters, useTranslations } from "@/lib/i18n/context"; import { loadDocument } from "@/lib/navigation/load-document"; @@ -79,7 +83,7 @@ export function ClaimProfile({ async function signOut() { setSigningOut(true); try { - await apiFetch("/api/auth/logout", { method: "POST" }); + await signOutRequest(); clearCachesForSessionEnd(queryClient); // Back to this page with the link intact: the token only rides the URL. router.replace( diff --git a/src/components/settings/web-push-card.tsx b/src/components/settings/web-push-card.tsx index e7c19b0d7..9a73f0a9b 100644 --- a/src/components/settings/web-push-card.tsx +++ b/src/components/settings/web-push-card.tsx @@ -10,7 +10,10 @@ import { SettingsCardActions } from "@/components/settings/_card-actions"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { useTranslations } from "@/lib/i18n/context"; import { apiFetchRaw } from "@/lib/api/api-fetch"; -import { unsubscribeWebPush } from "@/lib/notifications/web-push-unsubscribe"; +import { + saveWebPushSubscription, + unsubscribeWebPush, +} from "@/lib/notifications/web-push-subscription"; /** Convert a URL-safe base64 string to an ArrayBuffer (for VAPID key). */ function urlBase64ToArrayBuffer(base64String: string): ArrayBuffer { @@ -38,27 +41,6 @@ export function needsHomeScreen(): boolean { return appleTouch && !window.matchMedia("(display-mode: standalone)").matches; } -/** - * Store this browser's subscription for the signed-in account. Also run for a - * subscription the browser already holds, so a row the server lost or gave - * to another account on this browser comes back, and a channel switched off - * by failed sends is turned on again. - */ -function saveSubscription(subscription: PushSubscription): Promise { - const subJson = subscription.toJSON(); - return apiFetchRaw("/api/notifications/web-push", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - endpoint: subJson.endpoint, - keys: { - p256dh: subJson.keys?.p256dh ?? "", - auth: subJson.keys?.auth ?? "", - }, - }), - }); -} - type Unsupported = "browser" | "install" | null; export function WebPushCard() { @@ -94,7 +76,11 @@ export function WebPushCard() { if (registration) { const subscription = await registration.pushManager.getSubscription(); setIsSubscribed(!!subscription); - if (subscription) void saveSubscription(subscription).catch(() => {}); + // Sent again on every open: the server may have lost the row or + // switched the channel off, and the browser cannot tell. + if (subscription) { + void saveWebPushSubscription(subscription).catch(() => null); + } } } catch { // Ignore errors during check @@ -133,7 +119,7 @@ export function WebPushCard() { applicationServerKey: urlBase64ToArrayBuffer(vapidPublicKey), }); - const res = await saveSubscription(subscription); + const res = await saveWebPushSubscription(subscription); if (res.ok) { setIsSubscribed(true); setMsg(t("settings.webPushSubscribed")); diff --git a/src/hooks/__tests__/use-logout.test.ts b/src/hooks/__tests__/use-logout.test.ts index 592b28e79..45fb6b415 100644 --- a/src/hooks/__tests__/use-logout.test.ts +++ b/src/hooks/__tests__/use-logout.test.ts @@ -88,16 +88,18 @@ describe("useLogout", () => { expect(showErrorToast).not.toHaveBeenCalled(); }); - it("removes this browser's push subscription before the session ends", async () => { - // Left in place, the browser kept showing the signed-out account's - // medication reminders, also after someone else signed in. - const order: string[] = []; + /** A browser holding a push subscription; returns the call order log. */ + function holdSubscription(order: string[]) { const unsubscribe = vi.fn(async () => { order.push("browser unsubscribe"); return true; }); const subscription = { endpoint: "https://push.example.com/send/abc", + toJSON: () => ({ + endpoint: "https://push.example.com/send/abc", + keys: { p256dh: "p256dh", auth: "auth" }, + }), unsubscribe, }; vi.stubGlobal("window", { PushManager: class {} }); @@ -108,6 +110,16 @@ describe("useLogout", () => { }), }, }); + return { subscription, unsubscribe }; + } + + it("takes this browser's push subscription off the account and out of the browser", async () => { + // Left in place, the browser kept showing the signed-out account's + // medication reminders, also after someone else signed in. The server + // row goes while the session can still name it; the browser's own only + // once the sign-out is confirmed. + const order: string[] = []; + const { subscription } = holdSubscription(order); fetchMock.mockImplementation(async (url, init) => { order.push(`${init?.method} ${url}`); return json(200, { data: {} }); @@ -118,8 +130,8 @@ describe("useLogout", () => { expect(order).toEqual([ "DELETE /api/notifications/web-push", - "browser unsubscribe", "POST /api/auth/logout", + "browser unsubscribe", ]); const deleteCall = fetchMock.mock.calls[0]; expect(JSON.parse(String(deleteCall[1]?.body))).toEqual({ @@ -128,6 +140,28 @@ describe("useLogout", () => { expect(push).toHaveBeenCalledWith("/auth/login"); }); + it("keeps push working when the server refuses the sign-out", async () => { + const order: string[] = []; + const { unsubscribe } = holdSubscription(order); + fetchMock.mockImplementation(async (url, init) => { + order.push(`${init?.method} ${url}`); + return url === "/api/auth/logout" + ? json(503, { data: null, error: "Unavailable" }) + : json(200, { data: {} }); + }); + + useLogout(); + await settle(captured.options!); + + expect(order).toEqual([ + "DELETE /api/notifications/web-push", + "POST /api/auth/logout", + "POST /api/notifications/web-push", + ]); + expect(unsubscribe).not.toHaveBeenCalled(); + expect(showErrorToast).toHaveBeenCalledTimes(1); + }); + it("still signs out when removing the subscription fails", async () => { vi.stubGlobal("window", { PushManager: class {} }); vi.stubGlobal("navigator", { diff --git a/src/hooks/use-auth.ts b/src/hooks/use-auth.ts index 7a162487a..9f8b58829 100644 --- a/src/hooks/use-auth.ts +++ b/src/hooks/use-auth.ts @@ -30,7 +30,7 @@ import type { TourProgress } from "@/lib/onboarding/tour-progress"; import { clearLastDay } from "@/lib/day/last-day"; import { clearCoachDeleteJournal } from "@/lib/ai/coach/delete-journal"; import { clearOfflineCachesForSessionEnd } from "@/lib/pwa/query-persister"; -import { unsubscribeWebPush } from "@/lib/notifications/web-push-unsubscribe"; +import { signOutWithoutWebPush } from "@/lib/notifications/web-push-subscription"; import { setRecordScope, setRefusedRecordScope, @@ -646,20 +646,25 @@ export function clearCachesForSessionEnd(queryClient: QueryClient): void { void clearOfflineCachesForSessionEnd(); } +/** + * End the session on the server, for every sign-out button. Takes this + * browser's push subscription with it. Throws on any non-2xx answer: a + * refused sign-out leaves the session alive, and showing the sign-in page + * then would claim otherwise. + */ +export function signOutRequest(): Promise { + return signOutWithoutWebPush(() => + apiFetch("/api/auth/logout", { method: "POST" }), + ); +} + export function useLogout() { const queryClient = useQueryClient(); const router = useRouter(); const { t } = useTranslations(); return useMutation({ - mutationFn: async () => { - // Before the session ends: the server row can only be removed while - // it is still valid. A failure here does not block signing out. - await unsubscribeWebPush().catch(() => undefined); - // Throws on any non-2xx answer: a refused sign-out leaves the session - // alive, and showing the sign-in page then would claim otherwise. - await apiFetch("/api/auth/logout", { method: "POST" }); - }, + mutationFn: signOutRequest, onSuccess: () => { clearCachesForSessionEnd(queryClient); router.push("/auth/login"); diff --git a/src/lib/notifications/web-push-subscription.ts b/src/lib/notifications/web-push-subscription.ts new file mode 100644 index 000000000..ace66218e --- /dev/null +++ b/src/lib/notifications/web-push-subscription.ts @@ -0,0 +1,77 @@ +import { apiFetchRaw } from "@/lib/api/api-fetch"; + +/** This browser's push subscription, or null when it has none. */ +async function browserSubscription(): Promise { + if (!("serviceWorker" in navigator) || !("PushManager" in window)) { + return null; + } + const registration = await navigator.serviceWorker.getRegistration("/sw.js"); + return (await registration?.pushManager.getSubscription()) ?? null; +} + +function deleteServerRow(subscription: PushSubscription): Promise { + return apiFetchRaw("/api/notifications/web-push", { + method: "DELETE", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ endpoint: subscription.endpoint }), + }); +} + +/** + * Store this browser's subscription for the signed-in account. Also run for a + * subscription the browser already holds, so a row the server lost comes + * back and a channel switched off by failed sends is turned on again. + */ +export function saveWebPushSubscription( + subscription: PushSubscription, +): Promise { + const subJson = subscription.toJSON(); + return apiFetchRaw("/api/notifications/web-push", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + endpoint: subJson.endpoint, + keys: { + p256dh: subJson.keys?.p256dh ?? "", + auth: subJson.keys?.auth ?? "", + }, + }), + }); +} + +/** + * "Disable push": remove the server row, then the browser's own + * subscription. Does nothing when the browser has none. + */ +export async function unsubscribeWebPush(): Promise { + const subscription = await browserSubscription(); + if (!subscription) return; + await deleteServerRow(subscription); + await subscription.unsubscribe(); +} + +/** + * Run a sign-out with this browser's push subscription taken off the account. + * A subscription left behind keeps showing that account's reminders on this + * browser, to whoever uses it next. + * + * The server row goes first, while the session can still name it; the + * browser's subscription only once `signOut` resolved. A refused sign-out + * stores the row again, so the person who is still signed in keeps push. + * Failures around the subscription never block the sign-out. + */ +export async function signOutWithoutWebPush( + signOut: () => Promise, +): Promise { + const subscription = await browserSubscription().catch(() => null); + if (subscription) await deleteServerRow(subscription).catch(() => null); + try { + await signOut(); + } catch (failure) { + if (subscription) { + await saveWebPushSubscription(subscription).catch(() => null); + } + throw failure; + } + await subscription?.unsubscribe().catch(() => false); +} diff --git a/src/lib/notifications/web-push-unsubscribe.ts b/src/lib/notifications/web-push-unsubscribe.ts deleted file mode 100644 index ca2ba162a..000000000 --- a/src/lib/notifications/web-push-unsubscribe.ts +++ /dev/null @@ -1,23 +0,0 @@ -import { apiFetchRaw } from "@/lib/api/api-fetch"; - -/** - * Remove this browser's Web Push subscription: the server row first, while - * the session can still name it, then the browser's own. Does nothing when - * the browser has none. - * - * Used by "Disable push" and by sign-out. A subscription left behind at - * sign-out keeps showing that account's reminders on this browser, to - * whoever uses it next. - */ -export async function unsubscribeWebPush(): Promise { - if (!("serviceWorker" in navigator) || !("PushManager" in window)) return; - const registration = await navigator.serviceWorker.getRegistration("/sw.js"); - const subscription = await registration?.pushManager.getSubscription(); - if (!subscription) return; - await apiFetchRaw("/api/notifications/web-push", { - method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ endpoint: subscription.endpoint }), - }); - await subscription.unsubscribe(); -}