mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 21:32:18 +00:00
feat(sharing): name a record's owner in full to whoever they share with
The switcher and the banner named a shared record's owner by their greeting name — the value they typed at onboarding for "how should HealthLog address you". When an owner has set a full name, that is what a delegate should see: entering somebody's record, you read whose it is by who they are. - the account-access resolver publishes the owner's `fullName` alongside the greeting name and the username - `accountLabel` prefers fullName, then displayName, then username. Optional on the argument so the grant-view and actor shapes that never carried it keep the old two-field behaviour untouched - the closed presentation schema carries `fullName` and its parity matcher compares it, so the two published views of the active record cannot name the same owner differently - the OpenAPI contract regenerates with the additive field This deliberately discloses the owner's full name to every delegate, read-only included — it was patient-identity data behind a MANAGE-gated export until now. That is the intended behaviour, documented at `accountLabel` so it is not mistaken for a leak and tightened back. The resolver, the schema parity, the label fallback and a read-only delegate's banner are each pinned by a test.
This commit is contained in:
1 parent
73ea1b03f3
commit
e33700c7a9
20 files changed
+202
-13
No files matched your search
@@ -38831,6 +38831,10 @@ components:
|
||||
anyOf:
|
||||
- type: string
|
||||
- type: "null"
|
||||
fullName:
|
||||
anyOf:
|
||||
- type: string
|
||||
- type: "null"
|
||||
access:
|
||||
type: string
|
||||
enum:
|
||||
@@ -38868,6 +38872,7 @@ components:
|
||||
- accountId
|
||||
- username
|
||||
- displayName
|
||||
- fullName
|
||||
- access
|
||||
- level
|
||||
- recordKind
|
||||
@@ -38888,6 +38893,10 @@ components:
|
||||
anyOf:
|
||||
- type: string
|
||||
- type: "null"
|
||||
fullName:
|
||||
anyOf:
|
||||
- type: string
|
||||
- type: "null"
|
||||
access:
|
||||
type: string
|
||||
enum:
|
||||
@@ -38925,6 +38934,7 @@ components:
|
||||
- accountId
|
||||
- username
|
||||
- displayName
|
||||
- fullName
|
||||
- access
|
||||
- level
|
||||
- recordKind
|
||||
|
||||
@@ -69,6 +69,7 @@ import {
|
||||
DELEGATE_STORAGE_STATE_PATH,
|
||||
E2E_DELEGATE_MARKER_KG,
|
||||
E2E_OWNER,
|
||||
E2E_OWNER_FULL_NAME,
|
||||
E2E_USER,
|
||||
OWNER_STORAGE_STATE_PATH,
|
||||
} from "./setup/test-helpers";
|
||||
@@ -312,7 +313,17 @@ test.describe("account sharing", () => {
|
||||
|
||||
const entry = page.locator('[data-slot="account-switcher-entry"]');
|
||||
await expect(entry).toHaveCount(1);
|
||||
await expect(entry).toContainText(E2E_OWNER.username);
|
||||
// v1.37.2 — the owner set a full name, so a delegate sees them by it
|
||||
// rather than by the greeting name or the login handle. This is the
|
||||
// record-owner's full name crossing to a read-only delegate, which is the
|
||||
// deliberate disclosure the maintainer decided on. The username stays on
|
||||
// the row as an attribute, which is the positive control for the label
|
||||
// assertion — without it, a renamed slot would make this read green.
|
||||
await expect(entry).toContainText(E2E_OWNER_FULL_NAME);
|
||||
await expect(entry).toHaveAttribute(
|
||||
"data-account-username",
|
||||
E2E_OWNER.username,
|
||||
);
|
||||
// Two lines: the name and the kind of record. The access level used to
|
||||
// take a third and was truncated more often than it was read; it is what
|
||||
// the banner says once you are inside. The row still CARRIES the level as
|
||||
@@ -351,10 +362,11 @@ test.describe("account sharing", () => {
|
||||
|
||||
// Wait for CONTENT, and for content the assertion is not about: the
|
||||
// banner naming the owner. A skeleton does not satisfy it, and it says
|
||||
// nothing about the marker either way.
|
||||
// nothing about the marker either way. The owner set a full name, so the
|
||||
// banner names them by it (v1.37.2) — the same label the switcher used.
|
||||
const banner = page.locator('[data-slot="shared-record-banner"]');
|
||||
await expect(banner).toBeVisible();
|
||||
await expect(banner).toContainText(E2E_OWNER.username);
|
||||
await expect(banner).toContainText(E2E_OWNER_FULL_NAME);
|
||||
|
||||
// The document showing the banner has never shown the delegate's own
|
||||
// number. The latch was armed by that document's own init script, so this
|
||||
|
||||
@@ -39,7 +39,7 @@ import type { BrowserContext, Page } from "@playwright/test";
|
||||
import { expect, test } from "./setup/test";
|
||||
import {
|
||||
DELEGATE_STORAGE_STATE_PATH,
|
||||
E2E_OWNER,
|
||||
E2E_OWNER_FULL_NAME,
|
||||
E2E_USER,
|
||||
OWNER_STORAGE_STATE_PATH,
|
||||
} from "./setup/test-helpers";
|
||||
@@ -149,7 +149,8 @@ test.describe("delegated writes", () => {
|
||||
|
||||
const banner = page.locator('[data-slot="shared-record-banner"]');
|
||||
await expect(banner).toBeVisible();
|
||||
await expect(banner).toContainText(E2E_OWNER.username);
|
||||
// v1.37.2 — the owner set a full name, so the banner names them by it.
|
||||
await expect(banner).toContainText(E2E_OWNER_FULL_NAME);
|
||||
// The banner drops its read-only clause on its own once the level says so.
|
||||
await expect(banner).not.toContainText("read it, not change it");
|
||||
});
|
||||
|
||||
@@ -65,6 +65,12 @@ export const E2E_OWNER = {
|
||||
role: "USER",
|
||||
} as const;
|
||||
|
||||
/**
|
||||
* v1.37.2 — the owner carries a full name, so the switcher journey can assert
|
||||
* that a delegate sees it. Generic on purpose: fixtures hold no real name.
|
||||
*/
|
||||
export const E2E_OWNER_FULL_NAME = "Test Full Name";
|
||||
|
||||
/**
|
||||
* v1.37.0 — the account that creates and administers managed profiles.
|
||||
*
|
||||
@@ -592,20 +598,22 @@ export default async function globalSetup(config: FullConfig): Promise<void> {
|
||||
`INSERT INTO users
|
||||
(id, username, email, password_hash, role,
|
||||
created_at, updated_at,
|
||||
onboarding_completed_at, onboarding_tour_completed)
|
||||
VALUES ($1, $2, $3, $4, 'USER', $5, $5, $5, true)
|
||||
onboarding_completed_at, onboarding_tour_completed, full_name)
|
||||
VALUES ($1, $2, $3, $4, 'USER', $5, $5, $5, true, $6)
|
||||
ON CONFLICT (username) DO UPDATE SET
|
||||
email = EXCLUDED.email,
|
||||
password_hash = EXCLUDED.password_hash,
|
||||
updated_at = EXCLUDED.updated_at,
|
||||
onboarding_completed_at = EXCLUDED.onboarding_completed_at,
|
||||
onboarding_tour_completed = EXCLUDED.onboarding_tour_completed`,
|
||||
onboarding_tour_completed = EXCLUDED.onboarding_tour_completed,
|
||||
full_name = EXCLUDED.full_name`,
|
||||
[
|
||||
cuid(),
|
||||
E2E_OWNER.username,
|
||||
E2E_OWNER.email,
|
||||
await hashPassword(E2E_OWNER.password),
|
||||
now,
|
||||
E2E_OWNER_FULL_NAME,
|
||||
],
|
||||
);
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@
|
||||
export {
|
||||
E2E_USER,
|
||||
E2E_OWNER,
|
||||
E2E_OWNER_FULL_NAME,
|
||||
E2E_GUARDIAN,
|
||||
E2E_SCOPE_DELEGATE,
|
||||
E2E_SCOPE_RECORDS,
|
||||
|
||||
@@ -53,6 +53,7 @@ const OWNER = {
|
||||
accountId: "acct-owner",
|
||||
username: "owner",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "read" as const,
|
||||
level: "read" as const,
|
||||
sections: null,
|
||||
|
||||
@@ -36,6 +36,7 @@ const OWNER = {
|
||||
accountId: "acct-owner",
|
||||
username: "owner",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "read" as const,
|
||||
level: "read" as const,
|
||||
sections: null,
|
||||
|
||||
@@ -7,6 +7,7 @@ const OWNER = {
|
||||
accountId: "acct-owner",
|
||||
username: "owner",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "write" as const,
|
||||
level: "write" as const,
|
||||
sections: null,
|
||||
|
||||
@@ -132,6 +132,7 @@ const OWNER_READ: AccountAccessEntry = {
|
||||
accountId: "acct-owner",
|
||||
username: "grandma",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "read" as const,
|
||||
level: "read" as const,
|
||||
sections: null,
|
||||
@@ -148,6 +149,7 @@ const MANAGED_GUARDIAN: AccountAccessEntry = {
|
||||
accountId: "managed-record",
|
||||
username: "managed-record",
|
||||
displayName: "Managed profile",
|
||||
fullName: null,
|
||||
access: "write" as const,
|
||||
level: "manage" as const,
|
||||
sections: null,
|
||||
|
||||
@@ -56,6 +56,7 @@ const OWNER = {
|
||||
accountId: "acct-owner",
|
||||
username: "grandma",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "read" as const,
|
||||
level: "read" as const,
|
||||
sections: null,
|
||||
@@ -108,6 +109,25 @@ describe("<SharedRecordBanner>", () => {
|
||||
expect(html).toContain("grandma");
|
||||
});
|
||||
|
||||
it("names the owner in full when they have a full name, to a read-only delegate", () => {
|
||||
// v1.37.2 — the banner is the account-access view that renders server-side,
|
||||
// and this OWNER is READ (`canWrite: false`). The full name leads over the
|
||||
// greeting name: a delegate about to log a reading sees whose record it is
|
||||
// as who they are, not by a nickname. A deliberate disclosure to everyone
|
||||
// shared with, read-only included (maintainer decision 2026-08-08).
|
||||
const named = {
|
||||
...OWNER,
|
||||
fullName: "Test Full Name",
|
||||
displayName: "Margarethe",
|
||||
};
|
||||
const html = render(
|
||||
{ accounts: [named], active: named, canSwitch: true },
|
||||
<SharedRecordBanner />,
|
||||
);
|
||||
expect(html).toContain('data-slot="shared-record-banner"');
|
||||
expect(html).toContain("Test Full Name");
|
||||
});
|
||||
|
||||
it("says the access is read-only, and carries the way out", () => {
|
||||
const html = render(
|
||||
{ accounts: [OWNER], active: OWNER, canSwitch: true },
|
||||
|
||||
@@ -213,6 +213,7 @@ function entry(partial: Partial<AccountAccessEntry> = {}): AccountAccessEntry {
|
||||
accountId: "p1",
|
||||
username: "managed-abc",
|
||||
displayName: "Managed record",
|
||||
fullName: null,
|
||||
access: "write",
|
||||
level: "manage",
|
||||
recordKind: "managed",
|
||||
|
||||
@@ -31,6 +31,7 @@ const canonicalEntry = {
|
||||
accountId: "record-a",
|
||||
username: "record-a",
|
||||
displayName: "Record A",
|
||||
fullName: null,
|
||||
access: "read",
|
||||
level: "read",
|
||||
recordKind: "shared",
|
||||
|
||||
@@ -27,6 +27,7 @@ const READ_ONLY: AccountAccessEntry = {
|
||||
accountId: "acct-owner",
|
||||
username: "owner",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "read",
|
||||
level: "read",
|
||||
sections: null,
|
||||
|
||||
@@ -29,6 +29,7 @@ const OWNER = {
|
||||
accountId: "acct-owner",
|
||||
username: "owner",
|
||||
displayName: "Margarethe",
|
||||
fullName: null,
|
||||
access: "write" as const,
|
||||
level: "write" as const,
|
||||
sections: null,
|
||||
|
||||
@@ -6,6 +6,7 @@ const sharedEntry = {
|
||||
accountId: "record-a",
|
||||
username: "record-a",
|
||||
displayName: "Record A",
|
||||
fullName: "Test Full Name",
|
||||
access: "read",
|
||||
level: "read",
|
||||
recordKind: "shared",
|
||||
@@ -73,6 +74,11 @@ describe("parseAccountAccess", () => {
|
||||
],
|
||||
["username", { username: "different-record" }, {}],
|
||||
["display name", { displayName: "Different record" }, {}],
|
||||
// v1.37.2 — the owner's full name is part of the entry now, so the two
|
||||
// published views of the active record must agree on it like every other
|
||||
// field. A parity matcher blind to it would let the banner and the
|
||||
// switcher name the same owner differently.
|
||||
["full name", { fullName: "Different name" }, {}],
|
||||
])(
|
||||
"fails closed when active disagrees with its canonical entry on %s",
|
||||
(_field, activeOverrides, blockOverrides) => {
|
||||
|
||||
@@ -31,6 +31,7 @@ function activeGrant({
|
||||
id: "record-owner",
|
||||
username: "record-owner",
|
||||
displayName: "Record owner",
|
||||
fullName: "Test Full Name",
|
||||
managedProfileAt,
|
||||
},
|
||||
};
|
||||
@@ -93,6 +94,31 @@ describe("resolveAccountAccess", () => {
|
||||
expect(access.recordKind).toBe("self");
|
||||
});
|
||||
|
||||
it("carries the owner's full name to a read-only delegate", async () => {
|
||||
// v1.37.2 — the record owner's full name crosses to whoever they share
|
||||
// with, read-only included (maintainer decision 2026-08-08). It is the
|
||||
// resolver that lets it cross, so it is the resolver that proves it: a
|
||||
// READ grant, and the delegate's resolved entry still names the owner in
|
||||
// full. If a future change drops `fullName` from the grantor `select`,
|
||||
// this fails rather than silently reverting the disclosure to a nickname.
|
||||
vi.mocked(prisma.accountGrant.findMany).mockResolvedValue([
|
||||
activeGrant({ access: "READ" }),
|
||||
] as never);
|
||||
|
||||
const access = await resolveAccountAccess({
|
||||
user: { id: "delegate" },
|
||||
session: {},
|
||||
});
|
||||
|
||||
expect(access.accounts[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
access: "read",
|
||||
canWrite: false,
|
||||
fullName: "Test Full Name",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps whole-record legacy payloads decodable and scoped payloads closed", () => {
|
||||
for (const fixture of LEGACY_ACCOUNT_PAYLOADS) {
|
||||
const decoded = decodeLegacyWholeRecordAccess(fixture);
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { accountLabel } from "../account-access-view";
|
||||
|
||||
/**
|
||||
* `accountLabel` is the one place the banner, the switcher and the sharing
|
||||
* panel resolve what to call a person, so the fallback order lives here and
|
||||
* is pinned here.
|
||||
*
|
||||
* v1.37.2 — the record owner's full name leads, ahead of the greeting name.
|
||||
* It is patient-identity data shown to everyone they share with, read-only
|
||||
* included (maintainer decision 2026-08-08); the switcher and the banner pass
|
||||
* the account-access entry that carries it, while the grant-view and actor
|
||||
* shapes that never had it omit the argument and keep the old behaviour.
|
||||
*/
|
||||
describe("accountLabel", () => {
|
||||
it("prefers the full name when it is set", () => {
|
||||
expect(
|
||||
accountLabel({
|
||||
fullName: "Test Full Name",
|
||||
displayName: "Testy",
|
||||
username: "test-user",
|
||||
}),
|
||||
).toBe("Test Full Name");
|
||||
});
|
||||
|
||||
it("falls back to the display name when there is no full name", () => {
|
||||
expect(
|
||||
accountLabel({
|
||||
fullName: null,
|
||||
displayName: "Testy",
|
||||
username: "test-user",
|
||||
}),
|
||||
).toBe("Testy");
|
||||
// A blank full name is not a name — trimmed to empty, it falls through.
|
||||
expect(
|
||||
accountLabel({
|
||||
fullName: " ",
|
||||
displayName: "Testy",
|
||||
username: "test-user",
|
||||
}),
|
||||
).toBe("Testy");
|
||||
});
|
||||
|
||||
it("falls back to the username when neither name is set", () => {
|
||||
expect(
|
||||
accountLabel({
|
||||
fullName: null,
|
||||
displayName: null,
|
||||
username: "test-user",
|
||||
}),
|
||||
).toBe("test-user");
|
||||
});
|
||||
|
||||
it("keeps the old two-field behaviour when no full name is passed", () => {
|
||||
// The grant-view and actor shapes never carried a full name; omitting the
|
||||
// argument must read exactly as it did before the field existed.
|
||||
expect(accountLabel({ displayName: "Testy", username: "test-user" })).toBe(
|
||||
"Testy",
|
||||
);
|
||||
expect(accountLabel({ displayName: null, username: "test-user" })).toBe(
|
||||
"test-user",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -13,6 +13,10 @@ export const accountAccessEntrySchema = z
|
||||
accountId: z.string().min(1),
|
||||
username: z.string().min(1),
|
||||
displayName: z.string().nullable(),
|
||||
// v1.37.2 — the record owner's full name, shown in front of a delegate on
|
||||
// the switcher + banner. A deliberate disclosure to everyone they share
|
||||
// with; see `accountLabel`.
|
||||
fullName: z.string().nullable(),
|
||||
access: z.enum(["read", "write"]),
|
||||
level: accountAccessLevelSchema,
|
||||
recordKind: accountRecordKindSchema,
|
||||
@@ -80,6 +84,7 @@ function matchesCanonicalAccountAccessEntry(
|
||||
active.accountId === canonical.accountId &&
|
||||
active.username === canonical.username &&
|
||||
active.displayName === canonical.displayName &&
|
||||
active.fullName === canonical.fullName &&
|
||||
active.access === canonical.access &&
|
||||
active.level === canonical.level &&
|
||||
active.recordKind === canonical.recordKind &&
|
||||
|
||||
@@ -50,6 +50,14 @@ export interface AccountAccessEntry {
|
||||
accountId: string;
|
||||
username: string;
|
||||
displayName: string | null;
|
||||
/**
|
||||
* v1.37.2 — the record owner's full name, when they have set one. Shown in
|
||||
* front of a delegate on the switcher and the banner, ahead of the greeting
|
||||
* name, so the person whose record they are entering reads as who they are
|
||||
* rather than by a nickname. See {@link accountLabel} for the deliberate
|
||||
* disclosure this carries; it is a maintainer decision, not an oversight.
|
||||
*/
|
||||
fullName: string | null;
|
||||
/**
|
||||
* The grant's level, as the owner offered it and the caller accepted it.
|
||||
*
|
||||
@@ -116,16 +124,29 @@ export interface AccountAccess {
|
||||
/**
|
||||
* The name to put in front of a person, given what the payload published.
|
||||
*
|
||||
* `displayName` when the account set one, the username otherwise. This is a
|
||||
* presentation fallback and not a derivation of anything the server decided —
|
||||
* both values are published, and which of the two reads better is the client's
|
||||
* business. Kept here so the banner, the switcher and the sharing panel cannot
|
||||
* disagree about what to call somebody.
|
||||
* `fullName` when it is set, `displayName` next, the username last. All three
|
||||
* are published; which reads best is the client's business, not a derivation
|
||||
* of anything the server decided. Kept here so the banner, the switcher and
|
||||
* the sharing panel cannot disagree about what to call somebody.
|
||||
*
|
||||
* `fullName` is optional on the argument on purpose: only the account-access
|
||||
* entry carries it, so the switcher and the banner prefer it while the
|
||||
* grant-view and actor shapes that never had it keep falling back to the
|
||||
* greeting name exactly as before.
|
||||
*
|
||||
* v1.37.2 — the record owner's full name is shown to anyone they share with,
|
||||
* read-only included; maintainer decision 2026-08-08. It is patient-identity
|
||||
* data that until now stayed behind a MANAGE-gated export, so a future reader
|
||||
* should not "tighten" this back as a leak: it is the intended behaviour, not
|
||||
* an accident of the label falling through to it.
|
||||
*/
|
||||
export function accountLabel(entry: {
|
||||
displayName: string | null;
|
||||
username: string;
|
||||
fullName?: string | null;
|
||||
}): string {
|
||||
const full = entry.fullName?.trim();
|
||||
if (full && full.length > 0) return full;
|
||||
const named = entry.displayName?.trim();
|
||||
return named && named.length > 0 ? named : entry.username;
|
||||
}
|
||||
@@ -109,6 +109,10 @@ export async function resolveAccountAccess(auth: {
|
||||
id: true,
|
||||
username: true,
|
||||
displayName: true,
|
||||
// v1.37.2 — shown in front of a delegate on the switcher + banner,
|
||||
// ahead of the greeting name. A deliberate disclosure of the owner's
|
||||
// full name to everyone they share with; see `accountLabel`.
|
||||
fullName: true,
|
||||
managedProfileAt: true,
|
||||
},
|
||||
},
|
||||
@@ -126,6 +130,7 @@ export async function resolveAccountAccess(auth: {
|
||||
accountId: grant.grantor.id,
|
||||
username: grant.grantor.username,
|
||||
displayName: grant.grantor.displayName,
|
||||
fullName: grant.grantor.fullName,
|
||||
access: level === "read" ? "read" : "write",
|
||||
level,
|
||||
recordKind:
|
||||
|
||||
Reference in new issue
Block a user