diff --git a/messages/de.json b/messages/de.json index 028a6958a..30d655577 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9758,6 +9758,9 @@ "notFound": "Diesen Zugriff gibt es nicht mehr.", "revokeFailed": "Der Zugriff konnte nicht beendet werden.", "renounceFailed": "Der Zugriff konnte nicht zurückgegeben werden." + }, + "anamnesis": { + "relativeNote": "Ein Eintrag zur Familienanamnese beschreibt die Angehörigen dieser Person. Trage ihn nur ein, wenn sie das hier festgehalten haben möchte." } } } diff --git a/messages/en.json b/messages/en.json index ce085b6b8..d82410d75 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9758,6 +9758,9 @@ "notFound": "That access is no longer there.", "revokeFailed": "The access could not be ended.", "renounceFailed": "The access could not be handed back." + }, + "anamnesis": { + "relativeNote": "A family history entry describes this person's relatives. Add it only when they would want it recorded here." } } } diff --git a/messages/es.json b/messages/es.json index f48a513ca..089f1ca4f 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9758,6 +9758,9 @@ "notFound": "Ese acceso ya no existe.", "revokeFailed": "No se pudo terminar el acceso.", "renounceFailed": "No se pudo devolver el acceso." + }, + "anamnesis": { + "relativeNote": "Una entrada de antecedentes familiares describe a los familiares de esta persona. Añádela solo si querría que quedara registrada aquí." } } } diff --git a/messages/fr.json b/messages/fr.json index 4c8d71687..53f77823a 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9758,6 +9758,9 @@ "notFound": "Cet accès n'existe plus.", "revokeFailed": "L'accès n'a pas pu être terminé.", "renounceFailed": "L'accès n'a pas pu être rendu." + }, + "anamnesis": { + "relativeNote": "Un antécédent familial décrit les proches de cette personne. Ne l'ajoutez que si elle voudrait le voir consigné ici." } } } diff --git a/messages/it.json b/messages/it.json index 28297a6e6..91cfcacba 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9758,6 +9758,9 @@ "notFound": "Questo accesso non esiste più.", "revokeFailed": "Non è stato possibile terminare l'accesso.", "renounceFailed": "Non è stato possibile restituire l'accesso." + }, + "anamnesis": { + "relativeNote": "Una voce di anamnesi familiare descrive i parenti di questa persona. Aggiungila solo se vorrebbe vederla registrata qui." } } } diff --git a/messages/pl.json b/messages/pl.json index 0577eed2d..e8eb37c86 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9758,6 +9758,9 @@ "notFound": "Tego dostępu już nie ma.", "revokeFailed": "Nie udało się zakończyć dostępu.", "renounceFailed": "Nie udało się zwrócić dostępu." + }, + "anamnesis": { + "relativeNote": "Wpis w wywiadzie rodzinnym opisuje bliskich tej osoby. Dodaj go tylko wtedy, gdy chciałaby, żeby został tu zapisany." } } } diff --git a/src/__tests__/managed-record-navigation.test.tsx b/src/__tests__/managed-record-navigation.test.tsx index 09dec936a..1f0c56634 100644 --- a/src/__tests__/managed-record-navigation.test.tsx +++ b/src/__tests__/managed-record-navigation.test.tsx @@ -6,6 +6,10 @@ import { classifySettingsDestination } from "@/lib/record-settings"; const pathnameRef = { value: "/settings/security" }; const activeRecordRef = { value: true }; +/** The record on screen. `managed` unless a case says otherwise. */ +const recordRef: { value: { kind: "managed" | "shared"; level: string } } = { + value: { kind: "managed", level: "manage" }, +}; vi.mock("next/navigation", () => ({ usePathname: () => pathnameRef.value, @@ -23,8 +27,8 @@ vi.mock("@/hooks/use-auth", () => ({ username: "managed-record", displayName: "Managed record", access: "write", - level: "manage", - recordKind: "managed", + level: recordRef.value.level, + recordKind: recordRef.value.kind, sections: null, canWrite: true, }, @@ -63,18 +67,36 @@ function renderSettings(pathname = "/settings/security") { ); } +/** + * The destinations a record offers, by what kind of record it is. + * + * v1.37.0 — a managed profile offers guardian configuration AND the record + * content a MANAGE holder may write, because a Guardian holds MANAGE. The + * `manage-writable` kind is what carries the second half; before it existed + * the anamnesis form had no page a Guardian could open, while the routes + * behind it had admitted the write since the level shipped. + */ +const MANAGED_KINDS = new Set(["managed-guardian", "manage-writable"]); + describe("managed record navigation", () => { it("shows only managed settings destinations while retaining an unavailable deep-link heading", () => { + recordRef.value = { kind: "managed", level: "manage" }; const html = renderSettings(); - const managedDestinations = SETTINGS_SECTIONS.filter( - (section) => - classifySettingsDestination(section.slug).kind === "managed-guardian", + const managedDestinations = SETTINGS_SECTIONS.filter((section) => + MANAGED_KINDS.has(classifySettingsDestination(section.slug).kind), ); const unavailableDestinations = SETTINGS_SECTIONS.filter( (section) => - classifySettingsDestination(section.slug).kind !== "managed-guardian", + !MANAGED_KINDS.has(classifySettingsDestination(section.slug).kind), ); + // The partition is non-empty on both sides. A classification change that + // emptied either would make one of the loops below prove nothing. + expect(managedDestinations.length).toBeGreaterThan(1); + expect(unavailableDestinations.length).toBeGreaterThan(1); + // And the destination this release opened is really on the offered side. + expect(managedDestinations.map((s) => s.slug)).toContain("anamnesis"); + expect(html).toContain('data-active-record-id="managed-record"'); expect(html).toContain('id="settings-section-security-title"'); expect(html).not.toMatch(/href="[^"]*managed-record/); @@ -91,7 +113,46 @@ describe("managed record navigation", () => { } }); + it("offers an adult manager the record content and none of the configuration", () => { + // The other half of the same classification, and the half that is easiest + // to get wrong in the generous direction: a delegate manages somebody's + // health record, not their account. Anamnese is offered; modules, + // thresholds and notification routing are not, even though a Guardian + // reaches all three. + recordRef.value = { kind: "shared", level: "manage" }; + const html = renderSettings(); + + expect(html).toContain('href="/settings/anamnesis"'); + for (const guardianOnly of [ + "account", + "modules", + "notifications", + "thresholds", + "coach", + ]) { + expect(html, guardianOnly).not.toContain( + `href="/settings/${guardianOnly}"`, + ); + } + }); + + it("offers a WRITE delegate no settings destination at all", () => { + // The refusal side. `manage-writable` is admitted by LEVEL, not by being + // in a shared record: a WRITE grant reaches no Settings page, and a + // destination it could see would be one the route below refuses. + recordRef.value = { kind: "shared", level: "write" }; + const html = renderSettings(); + + expect(html).not.toContain('href="/settings/anamnesis"'); + for (const section of SETTINGS_SECTIONS) { + expect(html, section.slug).not.toContain( + `href="/settings/${section.slug}"`, + ); + } + }); + it("keeps the exit control present while administering a managed record", () => { + recordRef.value = { kind: "managed", level: "manage" }; const html = renderToStaticMarkup( diff --git a/src/__tests__/record-settings-contract.test.ts b/src/__tests__/record-settings-contract.test.ts index 5173a575e..50d3781ab 100644 --- a/src/__tests__/record-settings-contract.test.ts +++ b/src/__tests__/record-settings-contract.test.ts @@ -7,6 +7,7 @@ import { import { classifySettingsDestination, isGuardianSettingsWriteAllowed, + isManageDelegateSettingsDestination, SETTINGS_DESTINATION_INVENTORY, } from "@/lib/record-settings/classification"; import { toRecordSettingsDto } from "@/lib/record-settings/dto"; @@ -74,6 +75,55 @@ describe("record settings contract", () => { expect(isGuardianSettingsWriteAllowed("ai")).toBe(false); }); + /** + * v1.37.0 — the anamnesis destination, which is the one piece of record + * CONTENT under `/settings`. + * + * `POST /api/allergies` and `POST /api/family-history` have resolved + * `requireRecordAuth("manage", "profile")` since MANAGE shipped, while the + * only form that posts to either was classified unavailable — an admitted + * write with no reachable caller. These three legs are the classification + * half of closing that; the integration file drives the routes themselves. + */ + it("opens the anamnesis destination to everybody the routes already admit", () => { + expect(classifySettingsDestination("anamnesis")).toEqual({ + kind: "manage-writable", + guardianWritable: true, + }); + // The Guardian half. Before this the destination existed and was + // read-only for the one person the profile exists for. + expect(isGuardianSettingsWriteAllowed("anamnesis")).toBe(true); + // The adult-delegate half, and its boundary in the same breath: a MANAGE + // grant opens the record's health background and nothing else under + // Settings. Record CONFIGURATION — modules, thresholds, notification + // routing — stays with the owner even though a Guardian may change it. + expect(isManageDelegateSettingsDestination("anamnesis")).toBe(true); + for (const guardianOnly of [ + "account", + "modules", + "notifications", + "thresholds", + "insights", + "coach", + "integrations", + ]) { + expect( + isManageDelegateSettingsDestination(guardianOnly), + guardianOnly, + ).toBe(false); + } + }); + + it("leaves exactly one destination on the manage-writable list", () => { + // The list is short on purpose and a second entry is a consent decision, + // not a classification tidy-up. Pinning the count means a destination + // cannot join it as a side effect of somebody reclassifying a neighbour. + const manageWritable = Object.entries(SETTINGS_DESTINATION_INVENTORY) + .filter(([, value]) => value.kind === "manage-writable") + .map(([slug]) => slug); + expect(manageWritable).toEqual(["anamnesis"]); + }); + it("does not report a synthetic ledger row as a managed connection", () => { const connected = { withings: false, diff --git a/src/app/api/allergies/route.ts b/src/app/api/allergies/route.ts index 4617cdcd6..f4e69e40c 100644 --- a/src/app/api/allergies/route.ts +++ b/src/app/api/allergies/route.ts @@ -67,39 +67,23 @@ export const GET = apiHandler(async (request: NextRequest) => { export const POST = apiHandler(withIdempotency<[NextRequest]>(postAllergy)); async function postAllergy(request: NextRequest): Promise { - // v1.36.x — the GET above is delegable and this is not, which is the - // opposite of where the classification landed and worth the paragraph. + // v1.37.0 — MANAGE, and reachable. // - // Nothing about the row changed: it is still a plain statement about the - // record's own body, still the single most useful thing a caregiver could - // contribute, and the argument for admitting it still holds. What it does - // not have is a caller. The only place in the product that posts here is the - // allergy manager in Settings → Anamnese, and `/settings/*` is not a - // shared-record destination — the shell shows the "not part of what was - // shared" panel there, so no delegate can reach the form at any level. + // Recording an allergy for somebody whose record you manage is the case the + // level exists for; on a managed profile it is the guardian's ordinary act. + // Additive, audited, and the row belongs to the record owner. // - // An admitted write with no reachable surface is the one-ended change this - // repository keeps rediscovering (CLAUDE.md, "A two-ended change carries - // both ends"): the permission ships, the consumer is the follow-up, and - // nothing in the gate notices because every other check proves the other - // end. The frozen list is built the other way round on purpose — its own - // actor-surface note says the rest "arrive as their own diffs; naming them - // before they exist would freeze a guess." + // The surface is Settings → Anamnese inside the record + // (`src/components/settings/record-anamnesis-section.tsx`, reached through + // `record-settings-section-gate.tsx` for any destination classified + // `manage-writable`). This arm carried a long paragraph through v1.36.x + // arguing that the write was deliberately unreachable and waiting for that + // surface; the surface exists now, so the paragraph is gone rather than left + // to send the next reader back to close what it opens. // - // So this arm waits for the surface that would exercise it, and the two - // land together. Choosing that surface is design work, not a fix: allergies - // and family history have exactly one home today, that home is a personal - // account surface a switch rightly closes, and bolting a second copy onto a - // shared page would split one concept across two places. Re-admitting is - // one line here plus one entry in `sharing-surface-guard.test.ts` plus the - // paragraph that argues it — which is exactly the reviewed diff that guard - // exists to force. - // - // The half that was always the point is untouched: a caregiver can still - // READ the allergy list inside the record. - // v1.37.0 — MANAGE. Recording an allergy for somebody whose record you - // manage is the case the level exists for; on a managed profile it is the - // guardian's ordinary act. Additive, audited, and the row is the record's. + // A READ or WRITE delegate reaches neither: the destination is not on their + // Settings list, and this line refuses them with `sharing.access.denied` + // regardless. const { user } = await requireRecordAuth("manage", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { diff --git a/src/app/api/family-history/route.ts b/src/app/api/family-history/route.ts index 3bf1435df..661907833 100644 --- a/src/app/api/family-history/route.ts +++ b/src/app/api/family-history/route.ts @@ -62,23 +62,18 @@ export const POST = apiHandler( ); async function postFamilyHistory(request: NextRequest): Promise { - // v1.36.x — not a delegated write, though the GET above is delegable. The - // reasoning is written out at the sibling arm in `api/allergies/route.ts` - // and is the same here: the only surface that posts to either route is the - // manager in Settings → Anamnese, and `/settings/*` is closed inside a - // shared record, so no delegate can reach the form at any level. An - // admitted write with no reachable caller is a permission frozen ahead of - // the surface that would exercise it. + // v1.37.0 — MANAGE, and reachable. Third-party data by design, admitted on + // the same reasoning the read arm was: it is the record's own health + // background, and the surface is Settings → Anamnese inside the record + // (`src/components/settings/record-anamnesis-section.tsx`). // - // This arm carried an extra reason to wait. The row describes the record's - // RELATIVES, so it is the one admitted write where third-party health - // information is present by design — and frequently the delegate is that - // relative, asserting a condition about themselves into somebody else's - // record. The classification admitted it and named the discomfort. Landing - // it together with the surface that offers it means the copy on that surface - // can say whose statement the row is, which no route comment can. - // v1.37.0 — MANAGE. Third-party data by design, admitted on the same - // reasoning the read arm was: it is the record's own health background. + // This arm carried an extra reason to wait, and that reason is now answered + // where it can be. The row describes the record subject's RELATIVES, so it + // is the one admitted write where third-party health information is present + // by design — and frequently the delegate IS that relative, asserting a + // condition about themselves into somebody else's record. No route comment + // can say that to the person filling the form; the card above the form says + // it (`recordSharing.anamnesis.relativeNote`). const { user } = await requireRecordAuth("manage", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index aeb9bddba..1849f4e61 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -14,6 +14,7 @@ import { useRecordCapabilities } from "@/hooks/use-record-capabilities"; import { useRecordSessionTransition } from "@/hooks/use-record-session-transition"; import { useTranslations } from "@/lib/i18n/context"; import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context"; +import { isManageDelegateSettingsDestination } from "@/lib/record-settings"; import { isDestinationInSharedRecord } from "./nav-model"; import { BottomNav } from "./bottom-nav"; import { DemoBanner } from "./demo-banner"; @@ -84,15 +85,35 @@ export function AuthShell({ const { inSharedRecord, recordKind, + level, sections, accessRefused, recordSessionPending, } = useRecordCapabilities(); - // A managed profile has a deliberately small Settings surface of its own. - // Let that route family reach its record-aware gate; adult shared records - // stay in the generic refusal branch even at MANAGE. - const isManagedRecordSettingsPath = - recordKind === "managed" && pathname.startsWith("/settings"); + // A shared record has a deliberately small Settings surface of its own, and + // which routes belong to it depends on the record. + // + // A managed profile keeps the whole `/settings` family reaching its + // record-aware gate: guardian configuration lives there and the gate refuses + // per destination. + // + // v1.37.0 — an ordinary shared record at MANAGE reaches exactly the + // destinations classified `manage-writable`, which today is Settings → + // Anamnese and nothing else. That is not a widening of what a delegate may + // do: `POST /api/allergies` and `POST /api/family-history` already resolve + // `requireRecordAuth("manage", "profile")` and have done since the level + // shipped. What was missing was any page from which to call them, and an + // admitted write with no reachable caller is the half-wired state this + // repository keeps rediscovering. Every other `/settings` path in an adult + // share stays in the generic refusal branch, at every level. + const settingsSlug = pathname.startsWith("/settings") + ? (pathname.split("/")[2] ?? "") + : null; + const isRecordSettingsPath = + settingsSlug !== null && + (recordKind === "managed" || + (level === "manage" && + isManageDelegateSettingsDestination(settingsSlug))); // v1.36.0 — a page reached while acting on somebody else's record that // sharing does not cover. The nav already drops these entries, so arriving // here means a bookmark or a browser back button; the panel below explains @@ -101,7 +122,7 @@ export function AuthShell({ const outsideSharedRecord = inSharedRecord && !isDestinationInSharedRecord(pathname, sections) && - !isManagedRecordSettingsPath; + !isRecordSettingsPath; const isOnboardingPage = pathname === "/onboarding"; const showUnlockNotifier = isAuthenticated && !isPublicPage && !!user?.id; diff --git a/src/components/settings/record-anamnesis-section.tsx b/src/components/settings/record-anamnesis-section.tsx new file mode 100644 index 000000000..f6cce2663 --- /dev/null +++ b/src/components/settings/record-anamnesis-section.tsx @@ -0,0 +1,78 @@ +"use client"; + +import { ShieldAlert, Users } from "lucide-react"; + +import { SettingsCard } from "@/components/settings/settings-card"; +import { SettingsCardHeader } from "@/components/settings/_card-header"; +import { AllergyManager } from "@/components/records/allergy-manager"; +import { FamilyHistoryManager } from "@/components/records/family-history-manager"; +import { useTranslations } from "@/lib/i18n/context"; + +/** + * v1.37.0 — Settings → Anamnese, inside somebody else's record. + * + * ## Why a second component instead of the personal one + * + * `AnamnesisSection` renders five managers, and only two of them belong to the + * record. The other three write through `/api/coach/about-me` and + * `/api/anamnesis/facts`, both of which resolve a bare `requireAuth()` and + * therefore refuse outright under a switch. Mounting the personal section here + * would put three forms on screen that answer 403 on submit — the precise + * failure `useRecordCapabilities` was written to end ("a control that appeared + * before its route answered"). So this surface is the intersection of the + * section and what the record routes admit, and it is a separate component so + * that intersection is visible rather than expressed as four conditionals + * inside the personal one. + * + * ## What is here + * + * Allergies and family history. Both post to routes that resolve + * `requireRecordAuth("manage", "profile")`, so both land under the record + * owner with the delegate named as actor, and a READ or WRITE delegate is + * refused by the route as well as never reaching this page. + * + * Family history is the one admitted write where third-party health + * information is present by design — the row describes the record subject's + * relatives, and often enough the delegate IS that relative, asserting a + * condition about themselves into somebody else's record. The route's own + * comment named that discomfort and left it for the surface to say out loud, + * because no route comment can. The sentence under the card is where it gets + * said. + * + * The cache is not a concern here and the reason is worth naming: switching + * records clears the whole query client, so the allergy and family-history + * lists cannot carry one record's rows into another's view. + */ +export function RecordAnamnesisSection() { + const { t } = useTranslations(); + + return ( +
+ + + + + + + + +

+ {t("recordSharing.anamnesis.relativeNote")} +

+ + +

{t("records.disclaimer")}

+
+ ); +} diff --git a/src/components/settings/record-settings-section-gate.tsx b/src/components/settings/record-settings-section-gate.tsx index 2bfac5803..449c55bf6 100644 --- a/src/components/settings/record-settings-section-gate.tsx +++ b/src/components/settings/record-settings-section-gate.tsx @@ -10,6 +10,7 @@ import { classifySettingsDestination } from "@/lib/record-settings"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { ManagedIntegrationStatus } from "@/components/settings/managed-integration-status"; import { ManagedRecordSettingsSection } from "@/components/settings/managed-record-settings-section"; +import { RecordAnamnesisSection } from "@/components/settings/record-anamnesis-section"; import { SettingsCard } from "@/components/settings/settings-card"; import type { SettingsSectionSlug } from "@/components/settings/section-slugs"; @@ -35,7 +36,7 @@ export function RecordSettingsSectionGate({ children: ReactNode; }) { const { isLoading } = useAuth(); - const { inSharedRecord, recordKind } = useRecordCapabilities(); + const { inSharedRecord, recordKind, level } = useRecordCapabilities(); const { t } = useTranslations(); // A shared session must never begin by mounting the actor's Settings @@ -53,6 +54,21 @@ export function RecordSettingsSectionGate({ if (!inSharedRecord) return children; const destination = classifySettingsDestination(section); + + // v1.37.0 — the one destination that is record CONTENT rather than record + // configuration, and therefore the one every MANAGE holder reaches: a + // Guardian inside the profile they administer, and an adult delegate whose + // grant is MANAGE. Both post to routes that already resolve + // `requireRecordAuth("manage", "profile")`; before this the forms had no + // reachable page and the permission had no caller. + // + // Checked before the managed branches so a Guardian gets the same record + // surface an adult manager does, rather than the guardian configuration + // panel, which has no family for it. + if (destination.kind === "manage-writable" && level === "manage") { + return ; + } + if ( recordKind === "managed" && destination.kind === "managed-guardian" && diff --git a/src/components/settings/settings-shell.tsx b/src/components/settings/settings-shell.tsx index 810b527d5..7d1a03655 100644 --- a/src/components/settings/settings-shell.tsx +++ b/src/components/settings/settings-shell.tsx @@ -439,19 +439,32 @@ export function SettingsShell({ modules?.[section.moduleGate] !== false, ); - // The server has already resolved which record is active. In a managed - // profile, only guardian settings are a navigation destination; the section - // gate remains responsible for refusing any direct URL that is not here. + // The server has already resolved which record is active, and which + // destinations that record offers depends on what kind of record it is. The + // section gate remains responsible for refusing any direct URL that is not + // on the list; this filter only stops the nav from offering one. + // // Keep `allVisibleSections` for the active heading so a direct unavailable // link identifies itself before the gate explains why it cannot open. + // + // * A managed profile shows its guardian configuration AND the record + // content a MANAGE holder may write — the guardian holds MANAGE, so the + // second set is theirs as well. + // * An ordinary shared record at MANAGE shows only the record content. A + // delegate manages somebody's health record, not their account: modules, + // thresholds and notification routing stay with the owner. + // * Every other shared record shows nothing here, because it reaches no + // Settings destination at all. const visibleSections = - activeRecord?.recordKind === "managed" - ? allVisibleSections.filter( - (section) => - classifySettingsDestination(section.slug).kind === - "managed-guardian", - ) - : allVisibleSections; + activeRecord === null + ? allVisibleSections + : allVisibleSections.filter((section) => { + const kind = classifySettingsDestination(section.slug).kind; + if (activeRecord.recordKind === "managed") { + return kind === "managed-guardian" || kind === "manage-writable"; + } + return kind === "manage-writable" && activeRecord.level === "manage"; + }); const activeSection = allVisibleSections.find( (section) => section.slug === activeSlug, ); diff --git a/src/lib/record-settings/classification.ts b/src/lib/record-settings/classification.ts index 9413fd128..2c600d42d 100644 --- a/src/lib/record-settings/classification.ts +++ b/src/lib/record-settings/classification.ts @@ -1,7 +1,11 @@ import type { SettingsSectionSlug } from "@/components/settings/section-slugs"; export type SettingsDestinationKind = - "personal" | "managed-guardian" | "adult-shared-unavailable" | "unavailable"; + | "personal" + | "managed-guardian" + | "manage-writable" + | "adult-shared-unavailable" + | "unavailable"; export interface SettingsDestinationClassification { kind: SettingsDestinationKind; @@ -23,6 +27,32 @@ const MANAGED_GUARDIAN_STATUS: SettingsDestinationClassification = { guardianWritable: false, }; +/** + * v1.37.0 — reachable by everybody the record routes behind it already admit. + * + * The other four kinds each answer for one population. `MANAGED_GUARDIAN` is + * guardian-only; `ADULT_SHARED_UNAVAILABLE` is closed to adult shares and open + * to nobody else; neither can express "the same set the route admits", which + * for the anamnesis surface is a Guardian on a managed profile AND an adult + * delegate holding MANAGE. + * + * That gap had a consequence rather than being a taxonomy nicety. `POST + * /api/allergies` and `POST /api/family-history` both resolve + * `requireRecordAuth("manage", "profile")` — the write is admitted, audited + * and owner-scoped — while the only form in the product that posts to either + * lives under Settings → Anamnese, which was classified unavailable. An + * admitted write with no reachable caller is the one-ended change this + * repository keeps rediscovering: the permission ships, the surface is "the + * follow-up", and every other check proves the other end. + * + * `guardianWritable` is true because a Guardian writes here too; the kind is + * what says the destination is not guardian-ONLY. + */ +const MANAGE_WRITABLE: SettingsDestinationClassification = { + kind: "manage-writable", + guardianWritable: true, +}; + const ADULT_SHARED_UNAVAILABLE: SettingsDestinationClassification = { kind: "adult-shared-unavailable", guardianWritable: false, @@ -55,7 +85,7 @@ export const SETTINGS_DESTINATION_INVENTORY = { labs: ADULT_SHARED_UNAVAILABLE, illness: ADULT_SHARED_UNAVAILABLE, environment: UNAVAILABLE, - anamnesis: ADULT_SHARED_UNAVAILABLE, + anamnesis: MANAGE_WRITABLE, score: ADULT_SHARED_UNAVAILABLE, vorsorge: ADULT_SHARED_UNAVAILABLE, thresholds: MANAGED_GUARDIAN, @@ -83,10 +113,36 @@ export function classifySettingsDestination( ); } +/** + * May a Guardian change this destination inside the profile they administer. + * + * Two kinds qualify, and the second is not a widening of the first: a + * `manage-writable` destination is one the record routes already admit at + * MANAGE, and a Guardian holds MANAGE by construction. Naming only + * `managed-guardian` here would have left the anamnesis destination visible + * and read-only for the one person the record exists for. + */ export function isGuardianSettingsWriteAllowed(destination: string): boolean { const classification = classifySettingsDestination(destination); return ( - classification.kind === "managed-guardian" && + (classification.kind === "managed-guardian" || + classification.kind === "manage-writable") && classification.guardianWritable ); } + +/** + * Is this destination part of the record surface an adult MANAGE grant opens. + * + * Deliberately narrower than {@link isGuardianSettingsWriteAllowed}: a + * Guardian administers a profile and reaches the record's configuration — + * modules, thresholds, notification routing — while an adult delegate reaches + * only the health record itself. `manage-writable` is the one kind that is + * record content rather than record configuration, so it is the only kind an + * ordinary shared record opens under `/settings`. + */ +export function isManageDelegateSettingsDestination( + destination: string, +): boolean { + return classifySettingsDestination(destination).kind === "manage-writable"; +} diff --git a/src/lib/record-settings/index.ts b/src/lib/record-settings/index.ts index c349f9355..293842925 100644 --- a/src/lib/record-settings/index.ts +++ b/src/lib/record-settings/index.ts @@ -8,6 +8,7 @@ export { resolveManagedIntegrationState } from "./integrations"; export { classifySettingsDestination, isGuardianSettingsWriteAllowed, + isManageDelegateSettingsDestination, SETTINGS_DESTINATION_INVENTORY, type SettingsDestinationClassification, type SettingsDestinationKind, diff --git a/tests/integration/sharing-delegable-writes.test.ts b/tests/integration/sharing-delegable-writes.test.ts index c3ac2452c..735a99f86 100644 --- a/tests/integration/sharing-delegable-writes.test.ts +++ b/tests/integration/sharing-delegable-writes.test.ts @@ -117,7 +117,7 @@ async function signIn(userId: string) { async function switchInto( ownerId: string, delegateId: string, - access: "READ" | "WRITE", + access: "READ" | "WRITE" | "MANAGE", ) { const { inviteGrant, acceptGrant } = await import("@/lib/sharing/grants"); const invited = await inviteGrant({ @@ -421,82 +421,155 @@ writeContract("POST /api/biomarkers", { }); /* -------------------------------------------------------------------------- */ -/* 4 + 5 — allergies and family history, the two that are NOT delegable */ +/* 4 + 5 — allergies and family history, admitted at MANAGE */ /* -------------------------------------------------------------------------- */ /** - * The opposite contract, and it earns its place beside the admitted ones. + * The MANAGE contract, and why the WRITE one will not do. * - * Both of these verbs were admitted and then withdrawn, and the argument for - * admitting them was never wrong: an allergy is a plain statement about the - * record's own body and the single most useful thing a caregiver could - * contribute. What they lack is a caller. The only surface in the product that - * posts to either lives in Settings, and a switch closes `/settings` — so no - * delegate can reach the form at any grant level, and admitting the write - * would freeze a permission ahead of anything that exercises it. + * `writeContract` above asserts that a WRITE grant writes. A route that + * declares `requireRecordAuth("manage", …)` must REFUSE a WRITE grant, so the + * two contracts differ in their first leg and agree on every other: the row + * lands under the owner and under nobody else, the audit row is filed under the + * owner with the delegate named as actor, the refusals are the stable + * `sharing.access.denied`, revocation takes effect on the very next request, + * and an unswitched owner is unaffected. * - * Both READ arms stay delegable; the read suite pins them. What is pinned here - * is the pair that has to move together: the route refuses, and the owner's own - * unswitched write still lands. Withdrawing a delegated write by breaking the - * ordinary one would be the worse bug, and no other test in this file would - * have noticed. + * Two legs are new coverage rather than a rewrite, and they are the reason this + * block exists. Nothing in the repository drove a MANAGE delegate against + * either of these routes before, and nothing drove a Guardian against them at + * all — the routes had admitted the write since the level shipped while the + * only form that posts to them was classified unavailable, so the permission + * had no reachable caller. Plan 13 opened the surface; these are the legs that + * say the write it leads to actually lands. * - * Re-admitting means flipping `requireAuth()` back to `requireRecordAuth`, - * re-listing the route in `sharing-surface-guard.test.ts`, and replacing this - * block with a `writeContract` — in the same diff as the caregiver-reachable - * surface that made it worth doing. + * What this block replaces is a `refusedWriteContract` whose legs asserted + * `sharing.not_permitted` — the refusal a route that declares NO sharing mode + * returns. These routes declare one, so a READ or WRITE delegate gets + * `sharing.access.denied` instead, and those four assertions had been failing + * against the shipped routes for a release. */ -function refusedWriteContract( - name: string, - c: { - /** Seed whatever the write needs, owned by the RECORD. Mirrors `writeContract`. */ - prepare?: (recordOwnerId: string) => Promise; - call: (ctx: C) => Promise; - ok: number; - count: (userId: string) => Promise; - }, -) { +function manageWriteContract(name: string, c: WriteCase) { const prepare = c.prepare ?? (async () => undefined as C); describe(name, () => { + it("writes into the owner's record under a live MANAGE grant", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + await switchInto(owner.id, delegate.id, "MANAGE"); + const response = await c.call(ctx); + + expect(response.status).toBe(c.ok); + // Under the owner, and under nobody else. + expect(await c.count(owner.id)).toBe(1); + expect(await c.count(delegate.id)).toBe(0); + }); + + it("writes into a managed profile for its Guardian", async () => { + // The population the surface exists for, driven through the shipped + // creation service rather than a hand-planted grant: a Guardian is a + // MANAGE holder on a credential-less record, and this is the only place + // in the suite that puts one against these routes. + const guardian = await makeUser("guardian"); + const { createManagedProfile } = + await import("@/lib/managed-profiles/create"); + const { profile } = await createManagedProfile({ + creatorId: guardian.id, + displayName: "Managed profile", + dateOfBirth: null, + locale: "de", + timezone: "Europe/Berlin", + }); + const ctx = await prepare(profile.id); + + const session = await signIn(guardian.id); + await switchSessionTo(session.id, profile.id); + + expect((await c.call(ctx)).status).toBe(c.ok); + expect(await c.count(profile.id)).toBe(1); + expect(await c.count(guardian.id)).toBe(0); + }); + + it("files the audit row under the owner and names the delegate as actor", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + await switchInto(owner.id, delegate.id, "MANAGE"); + expect((await c.call(ctx)).status).toBe(c.ok); + + const row = await getPrismaClient().auditLog.findFirst({ + where: { action: c.auditAction }, + orderBy: { createdAt: "desc" }, + }); + expect(row, `no ${c.auditAction} row was written`).not.toBeNull(); + expect(row?.userId).toBe(owner.id); + expect(row?.actorUserId).toBe(delegate.id); + }); + for (const access of ["READ", "WRITE"] as const) { - it(`refuses a delegate holding a ${access} grant, and writes nothing`, async () => { + it(`refuses a ${access} delegate with sharing.access.denied, and writes nothing`, async () => { const owner = await makeUser("owner"); const delegate = await makeUser("delegate"); const ctx = await prepare(owner.id); await switchInto(owner.id, delegate.id, access); - const response = await c.call(ctx); + // The insufficient-level refusal, not the undeclared-mode one. The + // route names a mode; what the delegate lacks is the level. + await expectDenied(await c.call(ctx)); - // The undeclared-mode refusal, not the no-grant one: the route names - // no sharing mode at all, so the carrier is refused before any grant - // is consulted. A WRITE grant makes no difference, which is the point. - expect(response.status).toBe(403); - expect((await payload(response)).meta?.errorCode).toBe( - "sharing.not_permitted", - ); - - // Neither account, not just not the owner's — a handler that fell - // back to the caller would have written a row somewhere. expect(await c.count(owner.id)).toBe(0); expect(await c.count(delegate.id)).toBe(0); }); } - it("still lets the owner write it themselves", async () => { + it("refuses a caller who names a record they were never granted", async () => { const owner = await makeUser("owner"); + const stranger = await makeUser("stranger"); const ctx = await prepare(owner.id); - await signIn(owner.id); + + await claimWithoutGrant(owner.id, stranger.id); + await expectDenied(await c.call(ctx)); + + expect(await c.count(owner.id)).toBe(0); + expect(await c.count(stranger.id)).toBe(0); + }); + + it("refuses the next request after the grant is revoked", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + const { grant } = await switchInto(owner.id, delegate.id, "MANAGE"); + expect((await c.call(ctx)).status).toBe(c.ok); + + await revoke(grant.id, owner.id); + await expectDenied(await c.call(ctx)); + expect(await c.count(owner.id)).toBe(1); + }); + + it("is unchanged for a caller who has not switched", async () => { + const plain = await makeUser("plain"); + const ctx = await prepare(plain.id); + await signIn(plain.id); const response = await c.call(ctx); - expect(response.status).toBe(c.ok); - expect(await c.count(owner.id)).toBe(1); + expect(await c.count(plain.id)).toBe(1); + + const row = await getPrismaClient().auditLog.findFirst({ + where: { action: c.auditAction }, + orderBy: { createdAt: "desc" }, + }); + expect(row?.userId).toBe(plain.id); + expect(row?.actorUserId).toBeNull(); }); }); } -refusedWriteContract("POST /api/allergies", { +manageWriteContract("POST /api/allergies", { call: async () => { const { POST } = await import("@/app/api/allergies/route"); return post(POST as Handler, "/api/allergies", { @@ -506,10 +579,11 @@ refusedWriteContract("POST /api/allergies", { }); }, ok: 201, + auditAction: "allergy.create", count: (userId) => getPrismaClient().allergy.count({ where: { userId } }), }); -refusedWriteContract("POST /api/family-history", { +manageWriteContract("POST /api/family-history", { call: async () => { const { POST } = await import("@/app/api/family-history/route"); return post(POST as Handler, "/api/family-history", { @@ -518,6 +592,7 @@ refusedWriteContract("POST /api/family-history", { }); }, ok: 201, + auditAction: "family-history.create", count: (userId) => getPrismaClient().familyHistoryEntry.count({ where: { userId } }), }); @@ -578,6 +653,69 @@ describe("POST /api/illness/episodes — the module gate", () => { /* 7 — custom-metric entries, the third that is NOT delegable */ /* -------------------------------------------------------------------------- */ +/** + * The contract for a route that declares NO sharing mode at all. + * + * Its legs assert `sharing.not_permitted` — the refusal the carrier collects + * before any grant is consulted — which is a different fact from + * `sharing.access.denied` and is what makes the pair worth keeping apart. A + * route that names a mode and refuses a level answers the second; a route that + * names nothing answers the first, and a WRITE grant makes no difference to it. + * + * v1.37.0 — allergies and family history left this helper for + * `manageWriteContract` above, in the same diff as the surface that made their + * admitted writes reachable. One caller remains. + */ +function refusedWriteContract( + name: string, + c: { + /** Seed whatever the write needs, owned by the RECORD. Mirrors `writeContract`. */ + prepare?: (recordOwnerId: string) => Promise; + call: (ctx: C) => Promise; + ok: number; + count: (userId: string) => Promise; + }, +) { + const prepare = c.prepare ?? (async () => undefined as C); + + describe(name, () => { + for (const access of ["READ", "WRITE"] as const) { + it(`refuses a delegate holding a ${access} grant, and writes nothing`, async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + const ctx = await prepare(owner.id); + + await switchInto(owner.id, delegate.id, access); + const response = await c.call(ctx); + + // The undeclared-mode refusal, not the no-grant one: the route names + // no sharing mode at all, so the carrier is refused before any grant + // is consulted. A WRITE grant makes no difference, which is the point. + expect(response.status).toBe(403); + expect((await payload(response)).meta?.errorCode).toBe( + "sharing.not_permitted", + ); + + // Neither account, not just not the owner's — a handler that fell + // back to the caller would have written a row somewhere. + expect(await c.count(owner.id)).toBe(0); + expect(await c.count(delegate.id)).toBe(0); + }); + } + + it("still lets the owner write it themselves", async () => { + const owner = await makeUser("owner"); + const ctx = await prepare(owner.id); + await signIn(owner.id); + + const response = await c.call(ctx); + + expect(response.status).toBe(c.ok); + expect(await c.count(owner.id)).toBe(1); + }); + }); +} + /** * The third withdrawal, one release after the other two, and the delay is what * makes it worth a paragraph: the rule that removed allergies and family