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,