diff --git a/docs/self-hosting/notifications.md b/docs/self-hosting/notifications.md index 509ac3351..08eed4b7f 100644 --- a/docs/self-hosting/notifications.md +++ b/docs/self-hosting/notifications.md @@ -472,7 +472,7 @@ once per event type, and every channel carries the same target: | Medication reminder, low supply, shared dose | `/medications` (low supply opens the medication) | | Check-up and measurement reminders | `/checkups` | | Proactive Coach nudge | the Coach conversation it started | -| Cycle reminders | `/cycle` (nothing with discreet notifications on) | +| Cycle reminders | `/cycle` (the dashboard with discreet notifications on) | | New sign-in | Settings, Security | | Integration sync failing | Settings, Integrations (an admin alert about another account opens the admin page) | | Illness red flag | the illness episode | @@ -496,8 +496,11 @@ How each channel carries it: ntfy, Telegram, the webhook and email need an absolute link, built from `APP_URL` (or `NEXT_PUBLIC_APP_URL`). Without one those channels deliver as before, just without a link. A notification about a profile you look after -as a Guardian carries no deep link, because your session opens on your own -record. +as a Guardian opens the dashboard rather than the profile's page, because +your session opens on your own record and the record switcher lives there. +A discreet notification opens the dashboard as well, so its link names +nothing. The silent background sync push for medication intake is the only +notification without a link. ### Health figures in notifications @@ -538,6 +541,23 @@ token is the only credential, so it is built to be harmless on its own: The buttons need `APP_URL`, because the ntfy app calls the URL from your phone. Your phone has to reach the instance at that address. +### Quick answers behind forward auth + +If the instance sits behind a forward-auth gateway (Authelia, Authentik, +oauth2-proxy, Cloudflare Access and the like), the gateway answers the +button's request with its login page and the answer is never logged: the +ntfy app has no session with the gateway. Exempt the one path from the +gateway: + +``` +/api/notifications/action/* +``` + +The route does not rely on the gateway. The signed token is its only +credential, it is rate-limited before the token is read, and it can log one +mood answer for one account and nothing else. Every other path stays behind +the gateway. + What the token cannot protect against is the topic itself. The button URLs, token included, travel through your ntfy server inside the message, and the message is delivered to everyone subscribed to the topic. For the six hours diff --git a/src/lib/notifications/__tests__/delivery-links.test.ts b/src/lib/notifications/__tests__/delivery-links.test.ts index b609b26b0..b1085d264 100644 --- a/src/lib/notifications/__tests__/delivery-links.test.ts +++ b/src/lib/notifications/__tests__/delivery-links.test.ts @@ -85,7 +85,9 @@ describe("withDeliveryLinks", () => { "guardian-1", ); expect(out.quickActions).toBeUndefined(); - expect(out.tap).toBeUndefined(); + // The app root, never the managed record's own surface. + expect(out.tap?.path).toBe("/"); + expect(out.tap?.url).toBe("https://health.example/"); }); it("keeps the path but drops absolute URLs and answers without a public origin", async () => { diff --git a/src/lib/notifications/__tests__/tap-target.test.ts b/src/lib/notifications/__tests__/tap-target.test.ts index 676e44e4b..258a4edce 100644 --- a/src/lib/notifications/__tests__/tap-target.test.ts +++ b/src/lib/notifications/__tests__/tap-target.test.ts @@ -6,10 +6,24 @@ * `Record` type already refuses to compile without one; this * pins that every entry is also a usable path). */ -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; -import { EVENT_TYPES, type EventType } from "@/lib/notifications/types"; +vi.mock("@/lib/db", () => ({ + prisma: { + user: { findUnique: vi.fn().mockResolvedValue({ locale: "en" }) }, + }, +})); + +import { withDeliveryLinks } from "@/lib/notifications/delivery-links"; +import { buildNtfyMessage } from "@/lib/notifications/senders/ntfy"; +import { buildWebhookBody } from "@/lib/notifications/senders/webhook"; import { + EVENT_TYPES, + type EventType, + type NotificationPayload, +} from "@/lib/notifications/types"; +import { + NEUTRAL_TAP_PATH, NOTIFICATION_TAP_PATHS, absoluteAppUrl, configuredAppOrigin, @@ -91,14 +105,15 @@ describe("resolveTapPath", () => { ).toBe("/coach"); }); - it("gives a managed Guardian delivery no path", () => { + it("gives a managed Guardian delivery the app root, not the event's path", () => { expect( resolveTapPath({ eventType: "MEDICATION_REMINDER", recordUserId: "record-1", recipientUserId: "guardian-1", + metadata: { url: "/medications/m1" }, }), - ).toBeNull(); + ).toBe(NEUTRAL_TAP_PATH); }); it("keeps a self delivery's path when both ids are the same", () => { @@ -111,10 +126,14 @@ describe("resolveTapPath", () => { ).toBe("/medications"); }); - it("gives a discreet delivery no path, so nothing names the cycle surface", () => { + it("gives a discreet delivery the app root, so nothing names the cycle surface", () => { expect( - resolveTapPath({ eventType: "CYCLE_PERIOD_SOON", discreet: true }), - ).toBeNull(); + resolveTapPath({ + eventType: "CYCLE_PERIOD_SOON", + discreet: true, + metadata: { url: "/cycle?day=1" }, + }), + ).toBe(NEUTRAL_TAP_PATH); }); it("gives the silent sync push no path even with an override", () => { @@ -164,3 +183,67 @@ describe("telegramLinkMode", () => { expect(telegramLinkMode(url)).toBe(mode); }); }); + +/** + * Every visible delivery carries a target on every channel: each event type, + * delivered to its own account, to a Guardian of a managed record, and in + * discreet mode. Only the silent sync push carries none. The channels that + * take the absolute URL are checked through their pure builders; APNs and Web + * Push take the path and are pinned in their own sender tests. + */ +describe("tap target on every kind and channel", () => { + const variants: Array<{ + name: string; + extra: Pick< + Partial, + "recordUserId" | "recipientUserId" | "discreet" + >; + neutral: boolean; + }> = [ + { name: "own account", extra: {}, neutral: false }, + { + name: "managed Guardian", + extra: { recordUserId: "record-1", recipientUserId: "guardian-1" }, + neutral: true, + }, + { name: "discreet", extra: { discreet: true }, neutral: true }, + ]; + + it.each(variants)("$name", async ({ extra, neutral }) => { + process.env.APP_URL = "https://health.example"; + let checked = 0; + for (const eventType of EVENT_TYPES) { + const payload = await withDeliveryLinks( + { userId: "u1", title: "t", message: "m", eventType, ...extra }, + "guardian-1", + ); + if (SILENT.has(eventType)) { + expect(payload.tap, `${eventType} is silent`).toBeUndefined(); + continue; + } + const expectedPath = neutral + ? NEUTRAL_TAP_PATH + : NOTIFICATION_TAP_PATHS[eventType]; + expect(payload.tap?.path, eventType).toBe(expectedPath); + const url = `https://health.example${expectedPath}`; + expect(payload.tap?.url, eventType).toBe(url); + + expect( + buildNtfyMessage("topic", payload).click, + `ntfy ${eventType}`, + ).toBe(url); + expect( + JSON.parse(buildWebhookBody({ format: "generic" }, payload)).url, + `webhook ${eventType}`, + ).toBe(url); + expect( + JSON.parse(buildWebhookBody({ format: "gotify" }, payload)).extras[ + "client::notification" + ].click.url, + `gotify ${eventType}`, + ).toBe(url); + checked += 1; + } + expect(checked).toBe(EVENT_TYPES.length - SILENT.size); + }); +}); diff --git a/src/lib/notifications/senders/__tests__/apns.test.ts b/src/lib/notifications/senders/__tests__/apns.test.ts index f60b60d25..13bd6b975 100644 --- a/src/lib/notifications/senders/__tests__/apns.test.ts +++ b/src/lib/notifications/senders/__tests__/apns.test.ts @@ -726,7 +726,7 @@ describe("sendViaApns — dispatcher fan-out", () => { expect(note.category).toBe("MOOD_REMINDER"); }); - it("never gives a managed Guardian a tap path, even when one is set", async () => { + it("gives a managed Guardian the app root, even when a deeper path is set", async () => { vi.mocked(prisma.device.findMany).mockResolvedValueOnce([ { id: "d1", apnsToken: "tok-a", apnsEnvironment: "sandbox" }, ] as never); @@ -741,7 +741,24 @@ describe("sendViaApns — dispatcher fan-out", () => { tap: { path: "/medications", url: null, label: "Open" }, }); - expect(sendMock.mock.calls[0][0].payload.url).toBeUndefined(); + expect(sendMock.mock.calls[0][0].payload.url).toBe("/"); + }); + + it("carries the resolved root on a discreet delivery", async () => { + vi.mocked(prisma.device.findMany).mockResolvedValueOnce([ + { id: "d1", apnsToken: "tok-a", apnsEnvironment: "sandbox" }, + ] as never); + sendMock.mockResolvedValueOnce({ sent: [{ device: "tok-a" }], failed: [] }); + + await sendViaApns("u-1", { + title: "t", + message: "m", + eventType: "CYCLE_PERIOD_SOON", + discreet: true, + tap: { path: "/", url: null, label: "Open" }, + }); + + expect(sendMock.mock.calls[0][0].payload.url).toBe("/"); }); it("forwards reminderId on MEASUREMENT_REMINDER and strips unlisted keys", async () => { diff --git a/src/lib/notifications/senders/apns.ts b/src/lib/notifications/senders/apns.ts index 8fb2e9ac3..cb099faa5 100644 --- a/src/lib/notifications/senders/apns.ts +++ b/src/lib/notifications/senders/apns.ts @@ -33,6 +33,7 @@ import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-at import { plainPushText } from "@/lib/notifications/strip-emoji"; import { stripHtml } from "@/lib/notifications/strip-html"; import { isManagedGuardianDelivery } from "@/lib/notifications/managed-delivery"; +import { NEUTRAL_TAP_PATH } from "@/lib/notifications/tap-target"; import type { NotificationTap } from "@/lib/notifications/types"; export interface ApnsPayload { @@ -755,10 +756,13 @@ export async function sendViaApns( ), // The resolved tap target (`tap-target.ts`): a same-origin path // the app opens on tap. Overrides a producer's `metadata.url`, - // which the resolver already took into account. Never on a - // managed delivery or a discreet one (the resolver returns null). - ...(!managedDelivery && payload.tap?.path - ? { url: payload.tap.path } + // which the resolver already took into account. A managed + // delivery and a discreet one carry the app root (the resolver + // answers `/`); the managed arm is pinned here as well, so a + // payload that bypassed the resolver still cannot deep-link a + // Guardian into the record they are only notified about. + ...(payload.tap?.path + ? { url: managedDelivery ? NEUTRAL_TAP_PATH : payload.tap.path } : {}), }, threadId: routingEvent, diff --git a/src/lib/notifications/tap-target.ts b/src/lib/notifications/tap-target.ts index 2995ecb8c..95fd59c40 100644 --- a/src/lib/notifications/tap-target.ts +++ b/src/lib/notifications/tap-target.ts @@ -70,16 +70,25 @@ export function isSafeAppPath(value: unknown): value is string { return true; } +/** + * The app root: where a delivery that may not name its event leads. + */ +export const NEUTRAL_TAP_PATH = "/"; + /** * The path a delivery opens, or `null` when it carries none. * - * - A Guardian receiving a managed-record notification gets no deep link: - * their session opens on their own record, and a path into the managed - * record's surfaces would land them on the wrong person. The app root is - * where the record switcher lives. - * - Discreet mode (cycle privacy) carries no path either, because the path - * names the surface (`/cycle`) and discreet mode promises that nothing on - * the wire names the event. + * Every visible delivery carries one (v1.43); only a silent background push + * (`null` in {@link NOTIFICATION_TAP_PATHS}) does not. Two deliveries get the + * app root instead of their event's path: + * + * - A Guardian receiving a managed-record notification: their session opens + * on their own record, and a path into the managed record's surfaces would + * land them on the wrong person. The app root is where the record switcher + * lives. + * - Discreet mode (cycle privacy): the event's path names the surface + * (`/cycle`), and discreet mode promises that nothing on the wire names the + * event. The root names nothing. */ export function resolveTapPath( payload: Pick< @@ -89,8 +98,8 @@ export function resolveTapPath( ): string | null { const fallback = NOTIFICATION_TAP_PATHS[payload.eventType]; if (fallback === null || fallback === undefined) return null; - if (isManagedGuardianDelivery(payload)) return null; - if (payload.discreet) return null; + if (isManagedGuardianDelivery(payload)) return NEUTRAL_TAP_PATH; + if (payload.discreet) return NEUTRAL_TAP_PATH; const override = payload.metadata?.url; return isSafeAppPath(override) ? override : fallback; }