From e95612e3d35c01b6f7f6d5a348e255abfefb1301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 9 Sep 2026 19:42:27 +0200 Subject: [PATCH] fix(sharing): show a guardian the controls the routes accept (#939) useRecordCapabilities answers per section now. canWriteDomain and canManageDomain read the two lists the account payload publishes; in one's own record they are always true, and under a refused, pending or unproven context always false. canManage becomes the coarse switch, true when any section is manageable, and is what the banner and the navigation read. Every add, edit and delete control names its own section: labs, mind, profile, illness, cycle, medications, measurements, documents. Creates that a WRITE route answers (visits, practitioners) ask canWriteDomain. The generic delete button, the selection bar, the capture picker, the dashboard quick entry and the bottom nav take the section from their callers. Controls whose routes resolve the caller rather than the record (settings shortcuts, chart overlay preferences, custom metrics, the Today rail, OCR, efficacy targets, credential tabs) ask for the caller's own record instead, and the customise-shortcut guard now requires that shape. The vault has no delegated write route, so its controls stay withheld under every grant, and the documents page says so in one sentence where the upload control would be, in all seven locales. --- messages/de.json | 1 + messages/en.json | 1 + messages/es.json | 1 + messages/fr.json | 1 + messages/it.json | 1 + messages/ko.json | 1 + messages/pl.json | 1 + .../customize-shortcut-guard.test.ts | 60 ++++--- .../sharing-legacy-client-contract.test.ts | 10 +- src/app/labs/page.tsx | 8 +- src/app/medications/page-client.tsx | 4 +- src/app/mood/page-client.tsx | 14 +- .../delegated-write-affordances.test.tsx | 167 ++++++++++++++---- .../charts/chart-overlay-controls.tsx | 4 +- .../custom-metrics/custom-metric-detail.tsx | 1 + .../custom-metric-history-list.tsx | 1 + .../custom-metrics/custom-metric-list.tsx | 6 +- .../__tests__/cycle-view-error-state.test.tsx | 6 +- src/components/cycle/cycle-view.tsx | 13 +- src/components/daily/today-hero.tsx | 9 +- src/components/dashboard/dashboard-header.tsx | 8 +- .../dashboard/quick-entry-sheets.tsx | 15 +- src/components/data-list/delete-button.tsx | 22 ++- .../data-list/selection-action-bar.tsx | 20 ++- .../documents/document-detail-sheet.tsx | 31 ++-- src/components/documents/documents-view.tsx | 34 ++-- .../documents/episode-documents-card.tsx | 11 +- .../__tests__/visits-section.test.tsx | 7 +- src/components/encounters/visits-section.tsx | 6 +- .../illness/episode-visits-card.tsx | 8 +- .../illness/illness-day-timeline.tsx | 5 +- .../illness/illness-episode-detail.tsx | 5 +- src/components/illness/illness-view.tsx | 16 +- .../__tests__/ocr-capability-gate.test.ts | 4 +- src/components/labs/biomarker-manager.tsx | 1 + src/components/labs/lab-biomarker-detail.tsx | 6 +- src/components/labs/lab-history-list.tsx | 6 +- src/components/labs/use-ocr-extract.ts | 7 +- .../__tests__/capture-picker-discard.test.tsx | 2 + .../layout/__tests__/capture-picker.test.tsx | 2 + src/components/layout/bottom-nav.tsx | 19 +- src/components/layout/capture-picker.tsx | 19 +- ...rsorge-dashboard-card-error-state.test.tsx | 6 +- .../vorsorge-dashboard-card.tsx | 5 +- .../vorsorge-section.tsx | 29 +-- .../measurements/measurement-list.tsx | 3 + .../detail/efficacy/efficacy-tab.tsx | 6 +- .../detail/medication-detail-tabs.tsx | 29 ++- .../medications/dose-history-ledger.tsx | 7 +- .../medications/intake-history-editable.tsx | 5 +- .../medications/intake-history-list-v2.tsx | 5 +- .../medications/medication-card-menu.tsx | 9 +- .../sections/intake-history-preview.tsx | 5 +- .../sections/inventory-section.tsx | 34 ++-- .../medications/side-effects-section.tsx | 5 +- .../mental-health/instrument-card.tsx | 5 +- .../mental-health/instrument-detail.tsx | 5 +- src/components/mood/mood-list.tsx | 18 +- .../__tests__/practitioner-list.test.tsx | 6 +- .../practitioners/practitioner-list.tsx | 9 +- src/components/records/allergy-manager.tsx | 1 + .../records/family-history-manager.tsx | 1 + .../__tests__/use-record-capabilities.test.ts | 116 ++++++++++-- src/hooks/use-record-capabilities.ts | 106 +++++++---- 64 files changed, 690 insertions(+), 289 deletions(-) diff --git a/messages/de.json b/messages/de.json index 58a32857f..eef3089fe 100644 --- a/messages/de.json +++ b/messages/de.json @@ -9548,6 +9548,7 @@ "title": "Dokumente", "subtitle": "Jeder Brief, Befund und Scan an einem durchsuchbaren Ort — verschlüsselt gespeichert.", "pageUpload": "Hochladen", + "ownerOnlyUpload": "Dokumente kann nur die Inhaberin oder der Inhaber der Akte hinzufügen.", "kind": { "DOCTOR_REPORT": "Arztbericht", "DISCHARGE_LETTER": "Entlassungsbrief", diff --git a/messages/en.json b/messages/en.json index 3ba64450b..e75e342a8 100644 --- a/messages/en.json +++ b/messages/en.json @@ -9548,6 +9548,7 @@ "title": "Documents", "subtitle": "Every letter, report, and scan in one searchable place — encrypted at rest.", "pageUpload": "Upload", + "ownerOnlyUpload": "Documents can only be added by the record owner.", "kind": { "DOCTOR_REPORT": "Doctor report", "DISCHARGE_LETTER": "Discharge letter", diff --git a/messages/es.json b/messages/es.json index 9515185a7..91f6187d8 100644 --- a/messages/es.json +++ b/messages/es.json @@ -9548,6 +9548,7 @@ "title": "Documentos", "subtitle": "Cada carta, informe y escaneo en un solo lugar con búsqueda — cifrado en reposo.", "pageUpload": "Subir", + "ownerOnlyUpload": "Solo el titular del historial puede añadir documentos.", "kind": { "DOCTOR_REPORT": "Informe médico", "DISCHARGE_LETTER": "Informe de alta", diff --git a/messages/fr.json b/messages/fr.json index 8724f604f..23921c63d 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -9548,6 +9548,7 @@ "title": "Documents", "subtitle": "Chaque courrier, compte rendu et scan au même endroit, consultable — chiffré au repos.", "pageUpload": "Téléverser", + "ownerOnlyUpload": "Seul le titulaire du dossier peut ajouter des documents.", "kind": { "DOCTOR_REPORT": "Compte rendu médical", "DISCHARGE_LETTER": "Lettre de sortie", diff --git a/messages/it.json b/messages/it.json index c2b0a981e..e17e31091 100644 --- a/messages/it.json +++ b/messages/it.json @@ -9548,6 +9548,7 @@ "title": "Documenti", "subtitle": "Ogni lettera, referto e scansione in un unico posto ricercabile — cifrato a riposo.", "pageUpload": "Carica", + "ownerOnlyUpload": "Solo il titolare della cartella può aggiungere documenti.", "kind": { "DOCTOR_REPORT": "Referto medico", "DISCHARGE_LETTER": "Lettera di dimissione", diff --git a/messages/ko.json b/messages/ko.json index d82cc8fe9..66d228404 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -9548,6 +9548,7 @@ "title": "문서", "subtitle": "모든 소견서·리포트·영상을 한곳에서 검색해요 — 저장할 때 암호화돼요.", "pageUpload": "업로드", + "ownerOnlyUpload": "문서는 기록 소유자만 추가할 수 있습니다.", "kind": { "DOCTOR_REPORT": "진료용 리포트", "DISCHARGE_LETTER": "퇴원 소견서", diff --git a/messages/pl.json b/messages/pl.json index 4ebd23e32..7ec4d5cbc 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -9548,6 +9548,7 @@ "title": "Dokumenty", "subtitle": "Każde pismo, wynik i skan w jednym przeszukiwalnym miejscu — zaszyfrowane w spoczynku.", "pageUpload": "Prześlij", + "ownerOnlyUpload": "Dokumenty może dodawać tylko właściciel kartoteki.", "kind": { "DOCTOR_REPORT": "Raport lekarski", "DISCHARGE_LETTER": "Wypis ze szpitala", diff --git a/src/__tests__/customize-shortcut-guard.test.ts b/src/__tests__/customize-shortcut-guard.test.ts index 9945974b4..9e4f93341 100644 --- a/src/__tests__/customize-shortcut-guard.test.ts +++ b/src/__tests__/customize-shortcut-guard.test.ts @@ -12,16 +12,24 @@ * it, so this states it. It is structural rather than rendered because these * pages early-return a loading gate under SSR — the markup a delegate would * see is not reachable without a browser, and the property is about the source - * either way: the link is inside a `canManage` conditional, or it is not. + * either way: the link is inside an `inSharedRecord` conditional, or it is + * not. + * + * v1.38.12 — the guard used to name the manage capability, which was true + * only in one's own record. It answers per section now and is true for a + * guardian, so a wrench behind it would send a guardian into the wall. The + * customise pages live under `/settings`, which is the CALLER's and never the + * record's, so the link is withheld on `inSharedRecord` — the one answer no + * grant level moves. * * ## What decides * * The AST. The `href` is found as a string literal on a JSX attribute and the * ancestor chain above it is walked for a `&&` whose left-hand side names - * `canManage`. A comment about gating cannot satisfy that, and a `canManage` - * used elsewhere in the same file cannot either — which a "does this file - * mention the symbol" matcher would have accepted, and which is exactly what - * `/mood` looked like the day it shipped ungated. + * `inSharedRecord`. A comment about gating cannot satisfy that, and an + * `inSharedRecord` used elsewhere in the same file cannot either — which a + * "does this file mention the symbol" matcher would have accepted, and which + * is exactly what `/mood` looked like the day it shipped ungated. */ import { readdirSync, readFileSync } from "node:fs"; import { join, sep } from "node:path"; @@ -34,8 +42,8 @@ const SRC = join(process.cwd(), "src"); /** The prefix every customise shortcut points at. */ const LAYOUT_PREFIX = "/settings/layout/"; -/** The capability a shared-record surface withholds these on. */ -const GUARD = "canManage"; +/** The answer a shared-record surface withholds these on: `!inSharedRecord`. */ +const GUARD = "inSharedRecord"; /** * The surfaces that carry a layout shortcut and need no gate. @@ -66,16 +74,16 @@ function allSourceFiles(): string[] { .sort(); } -/** Does any ancestor of `node` gate it behind ` && …`? */ -function guardedByCanManage(node: ts.Node): boolean { +/** Does any ancestor of `node` gate it behind `! && …`? */ +function guardedByOwnRecord(node: ts.Node): boolean { for (let cur: ts.Node | undefined = node; cur; cur = cur.parent) { const parent: ts.Node | undefined = cur.parent; if ( parent && ts.isBinaryExpression(parent) && parent.operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken && - // Only the RIGHT side is the guarded branch. `canManage && x` guards `x`; - // `x && canManage` is `canManage` being the guarded thing. + // Only the RIGHT side is the guarded branch. `!inSharedRecord && x` + // guards `x`; `x && !inSharedRecord` is the answer being the guarded thing. parent.right === cur ) { const names = new Set(); @@ -87,7 +95,7 @@ function guardedByCanManage(node: ts.Node): boolean { if (names.has(GUARD)) return true; } // A conditional whose whenTrue branch holds the link, guarded on the - // capability: `canManage ? : null`, the shape `vorsorge-section` + // answer: `!inSharedRecord ? : null`, the shape `vorsorge-section` // uses. if ( parent && @@ -120,7 +128,7 @@ function unguardedShortcuts(source: string, fileName: string): string[] { if ( ts.isStringLiteralLike(node) && node.text.startsWith(LAYOUT_PREFIX) && - !guardedByCanManage(node) + !guardedByOwnRecord(node) ) { found.push(node.text); } @@ -160,20 +168,20 @@ describe("the customise shortcut is withheld inside somebody else's record", () } expect( offenders, - `a customise link outside a \`${GUARD}\` branch sends a delegate to /settings, which a switch closes`, + `a customise link outside a \`!${GUARD}\` branch sends a delegate to /settings, which a switch closes`, ).toEqual([]); }); it("the matcher reads the branch, not the file", () => { - // The shape that shipped: the capability is resolved and used elsewhere in + // The shape that shipped: the answer is resolved and used elsewhere in // the module while the link itself sits outside any branch. A matcher - // asking "does this file name canManage" calls this clean. + // asking "does this file name inSharedRecord" calls this clean. const UNGATED = ` export function Page() { - const { canManage } = useRecordCapabilities(); + const { inSharedRecord } = useRecordCapabilities(); return ( <> - {canManage && )} - {/* v1.36.x — a mood entry is not one of the verbs a grant - admits, so it stays with the account whose record it is. A - delegate at any level gets no add path here. */} - {canManage && ( + {/* A mood entry is a MANAGE create: a WRITE delegate gets no add + path here, a guardian or a MANAGE delegate does. */} + {canAddMood && ( deleteMutation.mutate(r.id)} title={t("customMetrics.entry.deleteConfirmTitle")} description={t( diff --git a/src/components/custom-metrics/custom-metric-list.tsx b/src/components/custom-metrics/custom-metric-list.tsx index 9363e4511..73d0cdacf 100644 --- a/src/components/custom-metrics/custom-metric-list.tsx +++ b/src/components/custom-metrics/custom-metric-list.tsx @@ -38,7 +38,9 @@ export function CustomMetricList() { // Logging a value against a custom metric was admitted for a release and is // not any more: this paragraph named the gap, the release shipped over it, // and the write withdrew rather than the sentence staying true. - const { canManage } = useRecordCapabilities(); + // Every custom-metric write resolves the caller (`requireAuth`); no grant + // reaches it at any level. + const { inSharedRecord } = useRecordCapabilities(); const router = useRouter(); const [addOpen, setAddOpen] = useState(false); const [addFooterEl, setAddFooterEl] = useState(null); @@ -57,7 +59,7 @@ export function CustomMetricList() { router.push(`/custom-metrics/${saved.id}`); } - if (!canManage) return null; + if (inSharedRecord) return null; return (
({ })); vi.mock("@/hooks/use-record-capabilities", () => ({ - useRecordCapabilities: () => ({ canManage: true, canAdd: true }), + useRecordCapabilities: () => ({ + canManage: true, + canAdd: true, + canManageDomain: () => true, + }), })); const errorQuery = { diff --git a/src/components/cycle/cycle-view.tsx b/src/components/cycle/cycle-view.tsx index c268f4a20..b8c084508 100644 --- a/src/components/cycle/cycle-view.tsx +++ b/src/components/cycle/cycle-view.tsx @@ -93,7 +93,8 @@ export function CycleView() { // v1.36.x — no grant level admits a cycle write. Inside somebody else's // record the page stays readable and every path into the log-day sheet and // the profile settings is absent, rather than opening onto a refusal. - const { canManage } = useRecordCapabilities(); + const { canManageDomain } = useRecordCapabilities(); + const canManageCycle = canManageDomain("cycle"); // Deep-link support: `/cycle?tab=insights` opens the insights tab. The value // is read ONCE for the initial `defaultValue` (uncontrolled tabs — so normal @@ -188,7 +189,7 @@ export function CycleView() { // One action. The settings glyph that used to sit beside it was a // second door to the tab directly below, and no other page in the // app carries a gear in that corner. - canManage ? ( + canManageCycle ? ( ) : null} - {canManage && ( + {canManageDocuments && ( deleteMutation.mutate(marker.id)} title={t("labs.biomarker.deleteConfirmTitle")} description={t("labs.biomarker.deleteConfirmDescription")} diff --git a/src/components/labs/lab-biomarker-detail.tsx b/src/components/labs/lab-biomarker-detail.tsx index dbcfce3ed..97e0dcd3d 100644 --- a/src/components/labs/lab-biomarker-detail.tsx +++ b/src/components/labs/lab-biomarker-detail.tsx @@ -95,7 +95,8 @@ const READINGS_PAGE_SIZE = 200; */ export function LabBiomarkerDetail({ biomarkerId }: { biomarkerId: string }) { const { user } = useAuth(); - const { canAdd, canManage } = useRecordCapabilities(); + const { canAdd, canManageDomain } = useRecordCapabilities(); + const canManageLabs = canManageDomain("labs"); const { t, locale } = useTranslations(); const router = useRouter(); const queryClient = useQueryClient(); @@ -332,6 +333,7 @@ export function LabBiomarkerDetail({ biomarkerId }: { biomarkerId: string }) { full marker editor behind the pencil, so the separate range control was dropped). */} deleteMarker.mutate()} title={t("labs.biomarker.deleteConfirmTitle")} description={t("labs.biomarker.deleteConfirmDescription")} @@ -340,7 +342,7 @@ export function LabBiomarkerDetail({ biomarkerId }: { biomarkerId: string }) { className="min-h-11 min-w-11 sm:min-h-9 sm:min-w-9" iconClassName="h-4 w-4" /> - {canManage && ( + {canManageLabs && ( @@ -864,7 +866,8 @@ function VorsorgeCard({ }) { const { t } = useTranslations(); const fmt = useFormatters(); - const { canManage } = useRecordCapabilities(); + const { canManageDomain } = useRecordCapabilities(); + const canManageMeasurements = canManageDomain("measurements"); // Issue #490 — day-boundary zone for the relative "today / yesterday" // bucket must match the zone `fmt.date` renders in (mirror → Berlin). // The next-due phrase reads off the SAME zone as the last-done date below @@ -938,7 +941,7 @@ function VorsorgeCard({ // Single med-style kebab: Edit + Delete. The Delete item is the shared // confirm-on-delete control rendered as a full-width menu row. - const headerActions = canManage ? ( + const headerActions = canManageMeasurements ? ( )} deleteMutation.mutate(m.id)} title={t("measurements.deleteConfirmTitle")} @@ -1535,6 +1537,7 @@ export function MeasurementList({ {/* v1.15.13 — page-scoped multi-select action bar. */} (""); const [busy, setBusy] = useState(false); @@ -443,7 +445,7 @@ function RetargetControl({ } }; - if (!canManage || items.length === 0) return null; + if (inSharedRecord || items.length === 0) return null; return (
= new Set([ + // Schedule times are a MANAGE route, but the tab also carries the reminder + // and notification settings, which resolve the caller. "zeitplan", - "bestand", + // Per-medication tokens: a credential surface, never delegated. "api", + // Phase config, export and the token-adjacent rows resolve the caller. "erweitert", ]); +/** + * v1.38.12 — tabs whose every write is a MANAGE route under `medications`, + * so a grant that reaches those routes is shown the tab. The supply ledger + * is one; the three above are not, because each mixes in a route that + * resolves the caller and would 403 under a switch. + */ +const MANAGE_TABS: ReadonlySet = new Set(["bestand"]); + /** * Legacy slugs that point at a dissolved tab. The Erinnerung tab folded * into Zeitplan in v1.15.20; old deep-links keep landing on the surface @@ -242,7 +253,8 @@ export function MedicationDetailTabs({ }) { const { t } = useTranslations(); const fmt = useFormatters(); - const { canManage } = useRecordCapabilities(); + const { canManageDomain, inSharedRecord } = useRecordCapabilities(); + const canManageMedications = canManageDomain("medications"); const router = useRouter(); const searchParams = useSearchParams(); @@ -308,9 +320,16 @@ export function MedicationDetailTabs({ (slug !== "injektion" || isInjectable) && (slug !== "zeitplan" || !asNeeded) && (slug !== "wirkung" || hasEfficacyTarget) && - (canManage || !OWNER_ONLY_TABS.has(slug)), + (!inSharedRecord || !OWNER_ONLY_TABS.has(slug)) && + (canManageMedications || !MANAGE_TABS.has(slug)), ), - [isInjectable, asNeeded, hasEfficacyTarget, canManage], + [ + isInjectable, + asNeeded, + hasEfficacyTarget, + inSharedRecord, + canManageMedications, + ], ); const requestedRaw = searchParams?.get("tab"); @@ -908,7 +927,7 @@ export function MedicationDetailTabs({ (`PUT /api/medications/[id]` is not delegable), so the deep link is gated exactly like the hero button it stands in for. */} void; }) { const { t } = useTranslations(); - const { canAdd, canManage } = useRecordCapabilities(); + const { canAdd, canManageDomain } = useRecordCapabilities(); + const canManageMedications = canManageDomain("medications"); const fmt = useFormatters(); const actionable = isSlotActionable(row); @@ -733,7 +734,7 @@ export function LedgerRowItem({ items in one menu, a delegate gets skip as its own control beside take, which is the shape the card already uses. The owner keeps it where it was. */} - {showTakenButton && canAdd && !canManage && ( + {showTakenButton && canAdd && !canManageMedications && (
+ {/* `/settings` is not a shared-record destination, so the cross-link + is withheld under a switch rather than opening onto the "not part + of what was shared" panel. */} + {!inSharedRecord && ( +
+ + {lowStockDays !== null + ? t("medications.detail.bestand.lowStockLinkOn", { + days: lowStockDays, + }) + : t("medications.detail.bestand.lowStockLinkOff")} + +
+ )} {/* v1.16.11 — packaging economics surfaced where the stock lives: @@ -443,6 +450,7 @@ export function InventorySection({ {t("medications.detail.bestand.adjustButton")} void deleteItem(item)} title={t("medications.detail.bestand.deleteTitle")} description={t("medications.detail.bestand.deleteDescription")} diff --git a/src/components/medications/side-effects-section.tsx b/src/components/medications/side-effects-section.tsx index 1e376d0be..ebc9f3418 100644 --- a/src/components/medications/side-effects-section.tsx +++ b/src/components/medications/side-effects-section.tsx @@ -112,7 +112,8 @@ function entryI18nKey(entry: MedicationSideEffectEntry): string { export function SideEffectsSection({ medicationId }: SideEffectsSectionProps) { const { t } = useTranslations(); - const { canAdd, canManage } = useRecordCapabilities(); + const { canAdd, canManageDomain } = useRecordCapabilities(); + const canManageMedications = canManageDomain("medications"); const fmt = useFormatters(); const queryClient = useQueryClient(); @@ -346,7 +347,7 @@ export function SideEffectsSection({ medicationId }: SideEffectsSectionProps) {

)} - {canManage && ( + {canManageMedications && ( - ) : onAddFirst && canManage ? ( + ) : onAddFirst && canManageMind ? ( deleteMutation.mutate(row.id)} title={t("records.allergies.deleteConfirmTitle")} description={t("records.allergies.deleteConfirmDescription")} diff --git a/src/components/records/family-history-manager.tsx b/src/components/records/family-history-manager.tsx index ba007a21b..1b865bc8a 100644 --- a/src/components/records/family-history-manager.tsx +++ b/src/components/records/family-history-manager.tsx @@ -91,6 +91,7 @@ export function FamilyHistoryManager() { deleteMutation.mutate(row.id)} title={t("records.family.deleteConfirmTitle")} description={t("records.family.deleteConfirmDescription")} diff --git a/src/hooks/__tests__/use-record-capabilities.test.ts b/src/hooks/__tests__/use-record-capabilities.test.ts index 4a7609b06..ef791c475 100644 --- a/src/hooks/__tests__/use-record-capabilities.test.ts +++ b/src/hooks/__tests__/use-record-capabilities.test.ts @@ -14,6 +14,8 @@ * nothing" goes red. * - make the `!active` branch return `canAdd: false` → "the caller's own * record keeps everything" goes red. + * - answer `canManageDomain` from `level === "manage"` instead of the list → + * "the vault is never manageable" goes red. */ import { describe, expect, it } from "vitest"; @@ -22,6 +24,8 @@ import { resolveRecordCapabilities, } from "@/hooks/use-record-capabilities"; import type { AccountAccessEntry } from "@/lib/sharing/account-access-view"; +import { delegatedDomains } from "@/lib/sharing/domain-write-support"; +import { SHARE_DOMAINS } from "@/lib/sharing/scope"; const READ_ONLY: AccountAccessEntry = { accountId: "acct-owner", @@ -33,13 +37,20 @@ const READ_ONLY: AccountAccessEntry = { sections: null, recordKind: "shared", canWrite: false, + writableDomains: [], + manageableDomains: [], }; +// The lists are what the server publishes for each level over the whole +// record: the same helper `resolveAccountAccess` runs, so the fixture is the +// payload and not a guess at it. const WRITABLE: AccountAccessEntry = { ...READ_ONLY, access: "write", level: "write", canWrite: true, + writableDomains: delegatedDomains("write", null, "write"), + manageableDomains: [], }; const MANAGING: AccountAccessEntry = { @@ -47,6 +58,8 @@ const MANAGING: AccountAccessEntry = { access: "write", level: "manage", canWrite: true, + writableDomains: delegatedDomains("manage", null, "write"), + manageableDomains: delegatedDomains("manage", null, "manage"), }; const SCOPED: AccountAccessEntry = { @@ -59,7 +72,8 @@ describe("resolveRecordCapabilities", () => { // `null` is not "no access", it is "this is mine". Both an unswitched // session and a payload from a server that has never heard of sharing // land here, which is why the absent case must be the permissive one. - expect(resolveRecordCapabilities(null)).toEqual({ + const own = resolveRecordCapabilities(null); + expect(own).toMatchObject({ inSharedRecord: false, canWrite: false, canAdd: true, @@ -71,13 +85,21 @@ describe("resolveRecordCapabilities", () => { sections: null, recordKind: "self", }); - expect(resolveRecordCapabilities(undefined)).toEqual( - resolveRecordCapabilities(null), - ); + for (const domain of SHARE_DOMAINS) { + expect(own.canWriteDomain(domain), domain).toBe(true); + expect(own.canManageDomain(domain), domain).toBe(true); + } + expect(resolveRecordCapabilities(undefined)).toMatchObject({ + inSharedRecord: false, + canAdd: true, + canManage: true, + recordKind: "self", + }); }); it("refuses a malformed published block instead of treating it as own record", () => { - expect(resolveRecordCapabilities(null, true)).toEqual({ + const refused = resolveRecordCapabilities(null, true); + expect(refused).toMatchObject({ inSharedRecord: true, canWrite: false, canAdd: false, @@ -87,10 +109,15 @@ describe("resolveRecordCapabilities", () => { recordKind: "shared", accessRefused: true, }); + for (const domain of SHARE_DOMAINS) { + expect(refused.canWriteDomain(domain), domain).toBe(false); + expect(refused.canManageDomain(domain), domain).toBe(false); + } }); it("a read-only delegate adds nothing", () => { - expect(resolveRecordCapabilities(READ_ONLY)).toEqual({ + const reader = resolveRecordCapabilities(READ_ONLY); + expect(reader).toMatchObject({ inSharedRecord: true, canWrite: false, canAdd: false, @@ -99,12 +126,17 @@ describe("resolveRecordCapabilities", () => { sections: null, recordKind: "shared", }); + for (const domain of SHARE_DOMAINS) { + expect(reader.canWriteDomain(domain), domain).toBe(false); + expect(reader.canManageDomain(domain), domain).toBe(false); + } }); it("a delegate may not change what is already there", () => { // The asymmetry the whole design rests on: a WRITE grant adds, and that // is all it does. Not even to an entry the delegate made a minute ago. - expect(resolveRecordCapabilities(WRITABLE)).toEqual({ + const writer = resolveRecordCapabilities(WRITABLE); + expect(writer).toMatchObject({ inSharedRecord: true, canWrite: true, canAdd: true, @@ -113,6 +145,14 @@ describe("resolveRecordCapabilities", () => { sections: null, recordKind: "shared", }); + // Writable where a WRITE route exists, manageable nowhere. + expect(writer.canWriteDomain("measurements")).toBe(true); + expect(writer.canWriteDomain("labs")).toBe(true); + expect(writer.canWriteDomain("mind")).toBe(false); + expect(writer.canWriteDomain("documents")).toBe(false); + for (const domain of SHARE_DOMAINS) { + expect(writer.canManageDomain(domain), domain).toBe(false); + } }); it("carries the level and the sections through untouched", () => { @@ -128,22 +168,45 @@ describe("resolveRecordCapabilities", () => { expect(resolveRecordCapabilities(MANAGING).level).toBe("manage"); }); - it("does not hand a manage grant the affordances its routes cannot answer yet", () => { - // The deliberate gap, pinned so it cannot close by accident. MANAGE will - // bring the edit and delete controls back, and it does so in the release - // where the routes behind them answer — not in the one that merely - // publishes the level. A `canManage: true` here today would paint - // controls that 403, which is the exact failure these booleans exist to - // prevent. - expect(resolveRecordCapabilities(MANAGING)).toEqual({ + it("hands a manage grant exactly the sections its routes answer for", () => { + // The v1.37.0 hold-back answered `canManage: false` here until the routes + // behind the edit and delete controls existed. They exist; the answer is + // the server's per-section list, and the coarse boolean is "is that list + // non-empty". + const manager = resolveRecordCapabilities(MANAGING); + expect(manager).toMatchObject({ inSharedRecord: true, canWrite: true, canAdd: true, - canManage: false, + canManage: true, level: "manage", sections: null, recordKind: "shared", }); + expect(manager.canManageDomain("mind")).toBe(true); + expect(manager.canManageDomain("profile")).toBe(true); + expect(manager.canManageDomain("labs")).toBe(true); + expect(manager.canWriteDomain("mind")).toBe(true); + }); + + it("never lets a shared record manage the vault", () => { + // The vault has no delegated write route, so the server's list never + // names it — and the hook binds the list rather than the level. Answering + // from `level === "manage"` would paint the upload control that 403s. + expect( + resolveRecordCapabilities(MANAGING).canManageDomain("documents"), + ).toBe(false); + expect( + resolveRecordCapabilities(MANAGING).canWriteDomain("documents"), + ).toBe(false); + const claimsEverything: AccountAccessEntry = { + ...MANAGING, + manageableDomains: [], + }; + // An empty list from the server is an empty answer, whatever the level. + const held = resolveRecordCapabilities(claimsEverything); + expect(held.canManage).toBe(false); + expect(held.canManageDomain("mind")).toBe(false); }); it("binds the server's boolean rather than the grant's label", () => { @@ -157,6 +220,19 @@ describe("resolveRecordCapabilities", () => { canWrite: false, }; expect(resolveRecordCapabilities(disagreeing).canAdd).toBe(false); + // The same posture for the lists: a scoped WRITE grant is writable only + // where its list says so, not wherever its level would reach. + const scopedWriter: AccountAccessEntry = { + ...WRITABLE, + sections: ["labs"], + writableDomains: ["labs"], + }; + expect(resolveRecordCapabilities(scopedWriter).canWriteDomain("labs")).toBe( + true, + ); + expect( + resolveRecordCapabilities(scopedWriter).canWriteDomain("measurements"), + ).toBe(false); }); }); @@ -242,8 +318,10 @@ describe("an unprovable record context withholds every control", () => { it("leaves every existing arm untouched when the two agree", () => { // The fourth argument defaults to false, so every case above this block // means exactly what it meant before the fence existed. - expect(resolveRecordCapabilities(READ_ONLY)).toEqual( - resolveRecordCapabilities(READ_ONLY, false, false, false), - ); + expect(resolveRecordCapabilities(READ_ONLY)).toMatchObject({ + ...resolveRecordCapabilities(READ_ONLY, false, false, false), + canWriteDomain: expect.any(Function), + canManageDomain: expect.any(Function), + }); }); }); diff --git a/src/hooks/use-record-capabilities.ts b/src/hooks/use-record-capabilities.ts index ddfdbcf90..524c075a8 100644 --- a/src/hooks/use-record-capabilities.ts +++ b/src/hooks/use-record-capabilities.ts @@ -6,6 +6,10 @@ import { accountLabel, type AccountAccessEntry, } from "@/lib/sharing/account-access-view"; +import type { ShareDomain } from "@/lib/sharing/scope"; + +const NEVER = () => false; +const ALWAYS = () => true; /** * What the person at the keyboard may do to the record on screen. @@ -20,7 +24,7 @@ import { * still taught them the product was broken. This hook is what every mutation * affordance asks before it renders. * - * ## The two answers, and why there are two + * ## The answers, and why there are several * * A grant at WRITE admits a short, closed list of verbs: entering readings, * lab results, an illness entry, a biomarker, a side effect, a medication, and @@ -32,34 +36,44 @@ import { * reason one release later: its only form sits on `/custom-metrics/{id}`, * which a switch closes too. * `sharing-surface-guard.test.ts` carries that argument - * in full and is the list that binds; this paragraph has to agree with it. Editing, deleting, restoring, purging, importing and - * bulk-acting stay with the owner, including on an entry the delegate added - * themselves a minute ago, and so does every create the list does not name - * (a mood entry, a screener, a cycle day). Two booleans carry that: + * in full and is the list that binds; this paragraph has to agree with it. + * Editing, deleting, restoring, purging, importing and bulk-acting are MANAGE + * verbs, and so is every create the list does not name (a mood entry, a + * screener, a cycle day). Which sections answer at which level is the + * server's table (`domain-write-support.ts`), published per grant as two + * lists on `accountAccess.active`. Four answers carry that: * * - `canAdd` — may this person add one of the admitted kinds. - * - `canManage` — may this person change what is already there, or add - * something the delegation does not cover. True only in one's own record. + * - `canWriteDomain(domain)` — is there a delegated write in this section + * the grant satisfies. + * - `canManageDomain(domain)` — may this person change what is already + * there in this section, or add what a WRITE grant does not cover. + * - `canManage` — the coarse switch: is there ANY section this person may + * manage. The banner and the navigation ask it; a control never should, + * because a control belongs to one section and the answer differs per + * section (the vault is read-only under every grant). * - * A surface that offers an admitted create asks `canAdd`. Everything else - * asks `canManage`. Neither is a permission decision made here: `canWrite` - * arrives resolved from the server on `accountAccess.active` and is bound, - * never recomputed (`account-access-view.ts:11-15`). What this file decides is - * only which class of affordance a resolved level covers, and that mapping is - * a property of the release, not of the caller. + * A surface that offers an admitted create asks `canAdd`. A control that + * edits, deletes or creates what WRITE does not cover asks `canManageDomain` + * for its own section. A control whose route resolves the caller rather than + * the record (settings, credentials, AI budget, chart preferences) asks + * `!inSharedRecord`: no grant reaches it at any level. None of these is a + * permission decision made here: `canWrite` and the two lists arrive resolved + * from the server on `accountAccess.active` and are bound, never recomputed + * (`account-access-view.ts`). What this file decides is only which class of + * affordance a resolved answer covers. * - * ## The two facts this hook carries but does not yet act on + * ## Why the lists are per section * - * v1.37.0 gives a grant a section set and a third level, and both arrive here - * (`sections`, `level`) as resolved values beside the booleans. Nothing in - * this file turns either into a capability, and that is the state of the - * release rather than an omission: the nav narrowing that reads `sections` - * and the return of the edit and delete affordances that reads `level` are - * the shared-session UX work, which lands after the routes that answer for a - * MANAGE grant exist. Publishing the facts here first is what lets those be - * a rendering change rather than a second access decision — and a control - * that appeared before its route answered would be the failure the two - * booleans above were written to end. + * v1.37.0 published `level` and `sections` and answered `canManage: false` + * for every shared record, a hold-back for the release where the level + * arrived before the routes behind it. The routes landed and the hold-back + * stayed, so a guardian of a managed profile saw no add, edit or delete + * control on Mood, screeners, visits, allergies and labs while the server + * accepted every one of those writes (#939). A single boolean could not have + * closed that without opening the vault, whose routes accept none; the lists + * close it section by section, and a section with no delegated route stays + * closed by the same mechanism. * * ## Absent, not disabled * @@ -82,17 +96,35 @@ export interface RecordCapabilities { canWrite: boolean; /** May the caller add an entry of a kind the delegation admits. */ canAdd: boolean; - /** May the caller change what exists, or add what the delegation excludes. */ + /** + * May the caller change what exists somewhere in this record, or add what + * the delegation excludes. In one's own record, always. In a shared record, + * true when `manageableDomains` is non-empty: the coarse switch the banner + * and the navigation read. A control asks {@link canManageDomain} for its + * own section instead, because the answer differs per section. + */ canManage: boolean; + /** + * v1.38.12 — is there a delegated write in `domain` this grant satisfies. + * Own record: always. Shared record: membership in + * `accountAccess.active.writableDomains`. Refused, pending or unproven + * context: never. + */ + canWriteDomain: (domain: ShareDomain) => boolean; + /** + * v1.38.12 — may the caller edit, delete or bulk-act in `domain`, or add + * what a WRITE grant does not cover there. Own record: always. Shared + * record: membership in `accountAccess.active.manageableDomains`. Refused, + * pending or unproven context: never. + */ + canManageDomain: (domain: ShareDomain) => boolean; /** * v1.37.0 — the resolved level of the grant on screen, or `null` in one's * own record (which is not a grant and has no level). * - * Bound from `accountAccess.active.level`, never derived. `canManage` is - * deliberately NOT computed from it in this release: the edit and delete - * affordances come back for `"manage"` when the routes that answer them - * land, and a control that appears before its route answers is the exact - * failure the two booleans above exist to prevent. + * Bound from `accountAccess.active.level`, never derived, and not what the + * controls read: the two domain lists are, because a level says what was + * granted and the lists say which routes answer for it. */ level: AccountAccessEntry["level"] | null; /** @@ -136,6 +168,8 @@ export function resolveRecordCapabilities( canWrite: false, canAdd: false, canManage: false, + canWriteDomain: NEVER, + canManageDomain: NEVER, level: null, sections: [], recordKind: "shared", @@ -147,6 +181,8 @@ export function resolveRecordCapabilities( canWrite: false, canAdd: true, canManage: true, + canWriteDomain: ALWAYS, + canManageDomain: ALWAYS, // One's own record has no grant, so it has no level, and it is open in // full. Null on both counts rather than a fabricated "manage" over all // eight sections: the caller is not a delegate, and a consumer that had @@ -156,11 +192,19 @@ export function resolveRecordCapabilities( recordKind: "self", }; } + // Bound, not derived: the two lists are the server's intersection of the + // grant with the routes that exist. Membership is the whole of the client's + // part, and `canManage` is only the question "is either list non-empty" + // asked of the manage one. + const writable = new Set(active.writableDomains); + const manageable = new Set(active.manageableDomains); return { inSharedRecord: true, canWrite: active.canWrite, canAdd: active.canWrite, - canManage: false, + canManage: manageable.size > 0, + canWriteDomain: (domain) => writable.has(domain), + canManageDomain: (domain) => manageable.has(domain), level: active.level, sections: active.sections, recordKind: active.recordKind,