mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(settings): list record content only where the grant manages it
The anamnesis page writes through requireRecordAuth("manage",
"profile"), which refuses a grant whose sections do not reach profile.
The Settings list and the navigation entry checked only the level and
the record kind, so a MANAGE grant scoped away from profile would be
offered a page whose saves are refused.
Record-content destinations now name the section they write, and are
listed only when the active record's manageable sections include it.
A destination without a named section is never listed. The shell and
the navigation read the same predicate. MANAGE grants are whole-record
today, so managed profiles and adult MANAGE shares list what they did
before.
Refs #939
This commit is contained in:
1 parent
4a49120fcf
commit
45ee70acb7
7 files changed
+224
-27
No files matched your search
@@ -3,6 +3,7 @@ import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
import { I18nProvider } from "@/lib/i18n/context";
|
||||
import { classifySettingsDestination } from "@/lib/record-settings";
|
||||
import { delegatedDomains } from "@/lib/sharing/domain-write-support";
|
||||
|
||||
const pathnameRef = { value: "/settings/security" };
|
||||
const activeRecordRef = { value: true };
|
||||
@@ -31,6 +32,12 @@ vi.mock("@/hooks/use-auth", () => ({
|
||||
recordKind: recordRef.value.kind,
|
||||
sections: null,
|
||||
canWrite: true,
|
||||
// Bound as the server publishes it for a whole-record grant.
|
||||
manageableDomains: delegatedDomains(
|
||||
recordRef.value.level as "read" | "write" | "manage",
|
||||
null,
|
||||
"manage",
|
||||
),
|
||||
},
|
||||
accounts: [],
|
||||
canSwitch: true,
|
||||
|
||||
@@ -22,15 +22,26 @@
|
||||
* MANAGE case and the parity case go red (2);
|
||||
* - keep Notifications in the switched tail beside the remapped Settings
|
||||
* entry → "never offers Notifications under a switch" goes red, with the
|
||||
* two "offers Settings" cases and the parity case (4).
|
||||
* two "offers Settings" cases and the parity case (4);
|
||||
* - drop the manageable-section check from
|
||||
* `isSettingsDestinationListedForRecord` → "offers no Settings entry to a
|
||||
* MANAGE share whose sections do not include profile" and "keeps a
|
||||
* managed profile's guardian configuration …" go red (2).
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { SETTINGS_SECTIONS } from "@/components/settings/settings-shell";
|
||||
import {
|
||||
RECORD_CONTENT_WRITE_DOMAINS,
|
||||
SETTINGS_DESTINATION_INVENTORY,
|
||||
isSettingsDestinationListedForRecord,
|
||||
type SettingsRecordContext,
|
||||
} from "@/lib/record-settings/classification";
|
||||
import {
|
||||
DOMAIN_WRITE_SUPPORT,
|
||||
delegatedDomains,
|
||||
} from "@/lib/sharing/domain-write-support";
|
||||
import type { AccountAccessLevel } from "@/lib/sharing/account-access-view";
|
||||
|
||||
import {
|
||||
BOTTOM_NAV_PRIMARY_SLOT_HREFS,
|
||||
@@ -131,14 +142,29 @@ describe("a module the RECORD does not track", () => {
|
||||
});
|
||||
|
||||
describe("the utility tail under a switch", () => {
|
||||
const MANAGED_AT_MANAGE: SettingsRecordContext = {
|
||||
recordKind: "managed",
|
||||
level: "manage",
|
||||
};
|
||||
const SHARED_AT_MANAGE: SettingsRecordContext = {
|
||||
recordKind: "shared",
|
||||
level: "manage",
|
||||
};
|
||||
/**
|
||||
* The context as the server publishes it for a whole-record grant: the
|
||||
* manageable list is the real `delegatedDomains` answer, not a literal, so
|
||||
* a change to the write-support table reaches these cases.
|
||||
*/
|
||||
const context = (
|
||||
recordKind: "managed" | "shared",
|
||||
level: AccountAccessLevel | null,
|
||||
sections: SettingsRecordContext["manageableDomains"] | null = null,
|
||||
): SettingsRecordContext => ({
|
||||
recordKind,
|
||||
level,
|
||||
manageableDomains:
|
||||
level === null
|
||||
? []
|
||||
: delegatedDomains(
|
||||
level,
|
||||
sections === null ? null : [...sections],
|
||||
"manage",
|
||||
),
|
||||
});
|
||||
const MANAGED_AT_MANAGE = context("managed", "manage");
|
||||
const SHARED_AT_MANAGE = context("shared", "manage");
|
||||
|
||||
const hrefs = (record: SettingsRecordContext | null) =>
|
||||
visibleUtilityDestinations({ record }).map((d) => d.href);
|
||||
@@ -175,7 +201,7 @@ describe("the utility tail under a switch", () => {
|
||||
it("offers nothing to a READ or WRITE share, where the shell lists nothing", () => {
|
||||
for (const recordKind of ["shared", "managed"] as const) {
|
||||
for (const level of ["read", "write"] as const) {
|
||||
expect(hrefs({ recordKind, level })).toEqual([]);
|
||||
expect(hrefs(context(recordKind, level))).toEqual([]);
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -184,7 +210,7 @@ describe("the utility tail under a switch", () => {
|
||||
// `resolveRecordCapabilities` answers `recordKind: "shared", level: null`
|
||||
// for both; a Settings entry there would lead into a context nobody has
|
||||
// proven.
|
||||
expect(hrefs({ recordKind: "shared", level: null })).toEqual([]);
|
||||
expect(hrefs(context("shared", null))).toEqual([]);
|
||||
});
|
||||
|
||||
it("never offers Notifications under a switch", () => {
|
||||
@@ -192,11 +218,81 @@ describe("the utility tail under a switch", () => {
|
||||
// under a switch at every level and for every record kind.
|
||||
for (const recordKind of ["shared", "managed"] as const) {
|
||||
for (const level of ["read", "write", "manage", null] as const) {
|
||||
expect(hrefs({ recordKind, level })).not.toContain("/notifications");
|
||||
expect(hrefs(context(recordKind, level))).not.toContain(
|
||||
"/notifications",
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* The anamnesis forms post to `requireRecordAuth("manage", "profile")`,
|
||||
* which refuses a grant whose sections do not reach `profile`. A MANAGE
|
||||
* grant is whole-record today, so these contexts are synthetic; they pin
|
||||
* that the list stays true if a scoped one ever reaches the client.
|
||||
*/
|
||||
it("offers no Settings entry to a MANAGE share whose sections do not include profile", () => {
|
||||
const scoped = context("shared", "manage", ["labs"]);
|
||||
// Non-zero: the scoped grant still manages something, so the empty tail
|
||||
// is the domain check and not a grant that manages nothing.
|
||||
expect(scoped.manageableDomains).toEqual(["labs"]);
|
||||
expect(hrefs(scoped)).toEqual([]);
|
||||
// And the shell lists nothing for it either.
|
||||
expect(
|
||||
SETTINGS_SECTIONS.filter((section) =>
|
||||
isSettingsDestinationListedForRecord(section.slug, scoped),
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it("lands a MANAGE share that manages profile on anamnesis, as the shell lists it", () => {
|
||||
const scoped = context("shared", "manage", ["profile"]);
|
||||
expect(hrefs(scoped)).toEqual(["/settings/anamnesis"]);
|
||||
expect(
|
||||
SETTINGS_SECTIONS.filter((section) =>
|
||||
isSettingsDestinationListedForRecord(section.slug, scoped),
|
||||
).map((section) => section.slug),
|
||||
).toEqual(["anamnesis"]);
|
||||
});
|
||||
|
||||
it("keeps a managed profile's guardian configuration independent of the manageable sections", () => {
|
||||
// Guardian destinations resolve `requireGuardianAuth`, not a section, so
|
||||
// only the record-content page depends on `profile`.
|
||||
const scoped = context("managed", "manage", ["labs"]);
|
||||
const listed = SETTINGS_SECTIONS.filter((section) =>
|
||||
isSettingsDestinationListedForRecord(section.slug, scoped),
|
||||
).map((section) => section.slug);
|
||||
expect(listed).toContain("modules");
|
||||
expect(listed).not.toContain("anamnesis");
|
||||
expect(hrefs(scoped)).toEqual(["/settings/account"]);
|
||||
// The whole-record guardian grant is unchanged: anamnesis stays listed.
|
||||
expect(
|
||||
SETTINGS_SECTIONS.filter((section) =>
|
||||
isSettingsDestinationListedForRecord(section.slug, MANAGED_AT_MANAGE),
|
||||
).map((section) => section.slug),
|
||||
).toContain("anamnesis");
|
||||
});
|
||||
|
||||
it("names the managed section of every record-content destination", () => {
|
||||
const recordContent = Object.entries(SETTINGS_DESTINATION_INVENTORY)
|
||||
.filter(([, classification]) => classification.kind === "manage-writable")
|
||||
.map(([slug]) => slug);
|
||||
expect(recordContent.length).toBeGreaterThan(0);
|
||||
const domains = RECORD_CONTENT_WRITE_DOMAINS as Partial<
|
||||
Record<string, keyof typeof DOMAIN_WRITE_SUPPORT>
|
||||
>;
|
||||
for (const slug of recordContent) {
|
||||
const domain = domains[slug];
|
||||
expect(domain, slug).toBeDefined();
|
||||
// A section with no MANAGE route could never satisfy the check.
|
||||
expect(
|
||||
DOMAIN_WRITE_SUPPORT[domain as keyof typeof DOMAIN_WRITE_SUPPORT]
|
||||
.manage,
|
||||
slug,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("lands on the first section the Settings shell lists, for every record kind", () => {
|
||||
// The parity net. The navigation walks the slug registry, the shell walks
|
||||
// its own ordered section list; both filter with the same predicate, and
|
||||
@@ -205,6 +301,8 @@ describe("the utility tail under a switch", () => {
|
||||
const contexts: SettingsRecordContext[] = [
|
||||
MANAGED_AT_MANAGE,
|
||||
SHARED_AT_MANAGE,
|
||||
context("shared", "manage", ["profile"]),
|
||||
context("managed", "manage", ["labs"]),
|
||||
];
|
||||
for (const record of contexts) {
|
||||
const firstListed = SETTINGS_SECTIONS.find((section) =>
|
||||
|
||||
@@ -50,6 +50,7 @@ import { I18nProvider } from "@/lib/i18n/context";
|
||||
import { SidebarNav } from "../sidebar-nav";
|
||||
import { ADMIN_SECTIONS } from "@/components/admin/admin-shell";
|
||||
import { visibleUtilityDestinations } from "../nav-model";
|
||||
import { delegatedDomains } from "@/lib/sharing/domain-write-support";
|
||||
|
||||
function render({
|
||||
pathname = "/",
|
||||
@@ -293,8 +294,7 @@ describe("<SidebarNav> inside somebody else's record (#939)", () => {
|
||||
canWrite: active.level !== "read",
|
||||
sections: null,
|
||||
writableDomains: [],
|
||||
manageableDomains:
|
||||
active.level === "manage" ? ["measurements"] : [],
|
||||
manageableDomains: delegatedDomains(active.level, null, "manage"),
|
||||
},
|
||||
}
|
||||
: undefined;
|
||||
|
||||
@@ -316,6 +316,10 @@ export function SidebarNav() {
|
||||
recordKind,
|
||||
level,
|
||||
} = useRecordCapabilities();
|
||||
// The sections the grant may manage, bound from the server-resolved entry.
|
||||
// The Settings entry needs them: a record-content page is offered only when
|
||||
// the grant manages the section its forms write.
|
||||
const manageableDomains = user?.accountAccess?.active?.manageableDomains;
|
||||
const visibleNavItems = useMemo(
|
||||
() =>
|
||||
visibleNavDestinations(user?.modules, mounted, sharedRecord, sections),
|
||||
@@ -367,9 +371,11 @@ export function SidebarNav() {
|
||||
const footerUtilityItems = useMemo(
|
||||
() =>
|
||||
visibleUtilityDestinations({
|
||||
record: sharedRecord ? { recordKind, level } : null,
|
||||
record: sharedRecord
|
||||
? { recordKind, level, manageableDomains: manageableDomains ?? [] }
|
||||
: null,
|
||||
}).filter((d) => d.href !== "/notifications"),
|
||||
[sharedRecord, recordKind, level],
|
||||
[sharedRecord, recordKind, level, manageableDomains],
|
||||
);
|
||||
|
||||
function isUtilityActive(href: string) {
|
||||
|
||||
@@ -54,7 +54,14 @@ export function TopBar() {
|
||||
level,
|
||||
} = useRecordCapabilities();
|
||||
const utilityItems = visibleUtilityDestinations({
|
||||
record: sharedRecord ? { recordKind, level } : null,
|
||||
record: sharedRecord
|
||||
? {
|
||||
recordKind,
|
||||
level,
|
||||
manageableDomains:
|
||||
user?.accountAccess?.active?.manageableDomains ?? [],
|
||||
}
|
||||
: null,
|
||||
});
|
||||
const logout = useLogout();
|
||||
const { theme, setTheme } = useTheme();
|
||||
|
||||
@@ -25,7 +25,11 @@ const mockModulesRef: { value: Record<string, boolean> | undefined } = {
|
||||
// server has already resolved which record is active, so it applies on the SSR
|
||||
// pass and is assertable from a static render.
|
||||
const mockActiveRecordRef: {
|
||||
value: { recordKind: "managed" | "shared"; level: string } | null;
|
||||
value: {
|
||||
recordKind: "managed" | "shared";
|
||||
level: string;
|
||||
manageableDomains: string[];
|
||||
} | null;
|
||||
} = { value: null };
|
||||
vi.mock("@/hooks/use-auth", () => ({
|
||||
useAuth: () => ({
|
||||
@@ -48,6 +52,7 @@ vi.mock("@/hooks/use-auth", () => ({
|
||||
}));
|
||||
|
||||
import { I18nProvider } from "@/lib/i18n/context";
|
||||
import { delegatedDomains } from "@/lib/sharing/domain-write-support";
|
||||
import {
|
||||
SETTINGS_SECTION_SLUGS,
|
||||
SETTINGS_SECTIONS,
|
||||
@@ -60,7 +65,11 @@ function renderShell(props: {
|
||||
pathname?: string;
|
||||
locale?: "en" | "de";
|
||||
modules?: Record<string, boolean>;
|
||||
activeRecord?: { recordKind: "managed" | "shared"; level: string } | null;
|
||||
activeRecord?: {
|
||||
recordKind: "managed" | "shared";
|
||||
level: string;
|
||||
manageableDomains: string[];
|
||||
} | null;
|
||||
}) {
|
||||
mockPathnameRef.value = props.pathname ?? "/settings/account";
|
||||
mockModulesRef.value = props.modules;
|
||||
@@ -358,7 +367,11 @@ describe("<SettingsShell>", () => {
|
||||
it("offers the Modules destination to a guardian on a managed record", () => {
|
||||
const html = renderShell({
|
||||
active: "account",
|
||||
activeRecord: { recordKind: "managed", level: "manage" },
|
||||
activeRecord: {
|
||||
recordKind: "managed",
|
||||
level: "manage",
|
||||
manageableDomains: delegatedDomains("manage", null, "manage"),
|
||||
},
|
||||
});
|
||||
expect(html).toContain('href="/settings/modules"');
|
||||
// Its own configuration is not the record's, and stays away.
|
||||
@@ -372,12 +385,36 @@ describe("<SettingsShell>", () => {
|
||||
// here — both are `managed-guardian`, which an adult record never opens.
|
||||
const html = renderShell({
|
||||
active: "account",
|
||||
activeRecord: { recordKind: "shared", level: "manage" },
|
||||
activeRecord: {
|
||||
recordKind: "shared",
|
||||
level: "manage",
|
||||
manageableDomains: delegatedDomains("manage", null, "manage"),
|
||||
},
|
||||
});
|
||||
expect(html).not.toContain('href="/settings/modules"');
|
||||
expect(html).not.toContain('href="/settings/integrations"');
|
||||
expect(html).toContain('href="/settings/anamnesis"');
|
||||
});
|
||||
|
||||
it("lists nothing for a MANAGE grant that does not manage profile", () => {
|
||||
// The anamnesis forms write through `requireRecordAuth("manage",
|
||||
// "profile")`, which refuses a grant whose sections miss `profile`, so
|
||||
// the shell does not list the page. The navigation reads the same
|
||||
// predicate and offers no Settings entry either.
|
||||
const html = renderShell({
|
||||
active: "account",
|
||||
activeRecord: {
|
||||
recordKind: "shared",
|
||||
level: "manage",
|
||||
manageableDomains: ["labs"],
|
||||
},
|
||||
});
|
||||
expect(html).not.toContain('href="/settings/anamnesis"');
|
||||
expect(html).not.toContain('href="/settings/modules"');
|
||||
// Non-zero: the shell rendered, and still names the active section in
|
||||
// its heading, which it resolves from the unfiltered list.
|
||||
expect(html).toContain('id="settings-section-account-title"');
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to `account` when the pathname doesn't match a known slug", () => {
|
||||
|
||||
@@ -6,6 +6,7 @@ import type {
|
||||
AccountAccessLevel,
|
||||
AccountRecordKind,
|
||||
} from "@/lib/sharing/account-access-view";
|
||||
import type { ShareDomain } from "@/lib/sharing/scope";
|
||||
|
||||
export type SettingsDestinationKind =
|
||||
| "personal"
|
||||
@@ -146,14 +147,44 @@ export function isManageDelegateSettingsDestination(
|
||||
return classifySettingsDestination(destination).kind === "manage-writable";
|
||||
}
|
||||
|
||||
/**
|
||||
* The section each record-content destination writes to.
|
||||
*
|
||||
* A `manage-writable` destination is a page of forms whose routes resolve
|
||||
* `requireRecordAuth("manage", <section>)`, and that call refuses a grant
|
||||
* whose sections do not reach the section (`grantCoversDomain`). Level alone
|
||||
* therefore does not say the page works: the grant must manage that section.
|
||||
* The anamnesis forms post to `/api/allergies` and `/api/family-history`, both
|
||||
* declared on `profile`.
|
||||
*
|
||||
* A `manage-writable` destination missing here is never listed, so a new one
|
||||
* cannot appear in a shared record until somebody names what it writes.
|
||||
*/
|
||||
export const RECORD_CONTENT_WRITE_DOMAINS = {
|
||||
anamnesis: "profile",
|
||||
} as const satisfies Partial<Record<SettingsSectionSlug, ShareDomain>>;
|
||||
|
||||
/**
|
||||
* The record a Settings listing is drawn for: the server-resolved kind of the
|
||||
* record on screen and the level of the grant that opened it. `level` is null
|
||||
* in one's own record and in a context the client could not prove.
|
||||
* record on screen, the level of the grant that opened it, and the sections
|
||||
* that grant may manage. `level` is null in one's own record and in a context
|
||||
* the client could not prove. `manageableDomains` is bound from
|
||||
* `accountAccess.active.manageableDomains`, never derived here.
|
||||
*/
|
||||
export interface SettingsRecordContext {
|
||||
recordKind: AccountRecordKind;
|
||||
level: AccountAccessLevel | null;
|
||||
manageableDomains: readonly ShareDomain[];
|
||||
}
|
||||
|
||||
function managesRecordContentSection(
|
||||
destination: string,
|
||||
record: SettingsRecordContext,
|
||||
): boolean {
|
||||
const section = (
|
||||
RECORD_CONTENT_WRITE_DOMAINS as Partial<Record<string, ShareDomain>>
|
||||
)[destination];
|
||||
return section !== undefined && record.manageableDomains.includes(section);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -171,6 +202,15 @@ export interface SettingsRecordContext {
|
||||
* thresholds and notification routing stay with the owner.
|
||||
* * Every other context lists nothing.
|
||||
*
|
||||
* A record-content destination is listed only when the grant manages the
|
||||
* section it writes ({@link RECORD_CONTENT_WRITE_DOMAINS}). Today a MANAGE
|
||||
* grant is whole-record at every layer (`inviteGrant` refuses a scope on a
|
||||
* MANAGE invitation, no endpoint raises a live grant, and the account-access
|
||||
* schema rejects a manage entry with sections), so this changes nothing for a
|
||||
* managed profile or an adult MANAGE share as they exist. It keeps the list
|
||||
* true if a scoped MANAGE grant ever reaches the client, rather than offering a
|
||||
* page whose writes answer 403.
|
||||
*
|
||||
* Every destination on either list needs MANAGE, so the level is checked once
|
||||
* for both record kinds. A guardian grant is always MANAGE today, which is
|
||||
* exactly why the check was easy to leave out, and a managed entry that ever
|
||||
@@ -184,11 +224,13 @@ export function isSettingsDestinationListedForRecord(
|
||||
): boolean {
|
||||
if (record.level !== "manage") return false;
|
||||
const kind = classifySettingsDestination(destination).kind;
|
||||
if (record.recordKind === "managed") {
|
||||
return kind === "managed-guardian" || kind === "manage-writable";
|
||||
if (kind === "manage-writable") {
|
||||
return (
|
||||
(record.recordKind === "managed" || record.recordKind === "shared") &&
|
||||
managesRecordContentSection(destination, record)
|
||||
);
|
||||
}
|
||||
if (record.recordKind === "shared") return kind === "manage-writable";
|
||||
return false;
|
||||
return record.recordKind === "managed" && kind === "managed-guardian";
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user