From 18f3fd6b50a5a8b9720e6e4de63a239cc6afae88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 11:31:41 +0200 Subject: [PATCH 1/3] feat(sharing): carry the acting account on the session row Migration 0293 adds `sessions.acting_as_user_id`. NULL, which is what every session has carried until now, means the browser is acting as the account it signed in as. It is a selector and not an authorization. The resolver joins it against a live grant on every request, so a value stranded here by a grant that has since been revoked or expired confers nothing. Nothing sweeps the column for correctness; the sweeps that exist are housekeeping, so a delegate's browser lands back in its own account instead of on a wall of 403s. ON DELETE SET NULL, not CASCADE. The two ends mean opposite things: deleting the session's owner should take the session with it, which is the existing `user_id` cascade, while deleting the account being acted on must leave the delegate's browser session alive. A cascade on this side would sign a delegate out of their own account because somebody else deleted theirs. --- .../0293_session_acting_as/migration.sql | 36 +++++++++++++++++++ prisma/schema.prisma | 28 +++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 prisma/migrations/0293_session_acting_as/migration.sql diff --git a/prisma/migrations/0293_session_acting_as/migration.sql b/prisma/migrations/0293_session_acting_as/migration.sql new file mode 100644 index 000000000..c9c4f8e9d --- /dev/null +++ b/prisma/migrations/0293_session_acting_as/migration.sql @@ -0,0 +1,36 @@ +-- v1.36.0 — the cookie transport's acting-account carrier. +-- +-- One nullable column, and the whole reason it is a column rather than +-- something the client sends: a browser cannot address this row. The session +-- cookie carries a 32-byte secret whose hash is the lookup key, so nothing +-- reachable from the client names, reads, or writes `acting_as_user_id`. The +-- only writer is the switch endpoint, which validates the grant first. +-- +-- It is still a SELECTOR and not an authorization. The resolver joins it +-- against a live row in `account_grants` on every request, so a value stranded +-- here by a grant that has since been revoked or expired confers nothing at +-- all. Nothing sweeps this column for correctness — sweeping it (on revocation, +-- on account deletion) is housekeeping so the delegate's browser lands back in +-- its own account instead of on a wall of 403s. +-- +-- ON DELETE SET NULL, not CASCADE. The two ends mean opposite things here: +-- deleting the session's OWNER should take the session with it (that is the +-- existing `user_id` cascade), while deleting the account being ACTED ON must +-- leave the delegate's browser session alive and simply put it back into its +-- own record. A CASCADE on this side would sign a delegate out of their own +-- account because somebody else deleted theirs. +-- +-- The index is not for the resolver — that reads by primary key. It is for the +-- two sweeps that go the other way: revocation clearing every session of the +-- delegate pointing at this owner, and the SET NULL above when an account is +-- deleted. Both scan on this column. + +ALTER TABLE "sessions" ADD COLUMN "acting_as_user_id" TEXT; + +ALTER TABLE "sessions" + ADD CONSTRAINT "sessions_acting_as_user_id_fkey" + FOREIGN KEY ("acting_as_user_id") REFERENCES "users"("id") + ON DELETE SET NULL ON UPDATE CASCADE; + +CREATE INDEX "sessions_acting_as_user_id_idx" + ON "sessions" ("acting_as_user_id"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index bd0ce0168..467dc5386 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -695,7 +695,10 @@ model User { // Relations passkeys Passkey[] - sessions Session[] + sessions Session[] @relation("SessionOwner") + // v1.36.0 — browser sessions belonging to OTHER accounts that are currently + // acting on this one. Named so the delete cascade has somewhere to point. + sessionsActingOnMe Session[] @relation("SessionActingAs") challenges AuthChallenge[] // v1.23 — second-factor stores (kept separate from `passkeys`, which is the // passwordless-primary credential set; an MFA credential is meaningless @@ -1132,9 +1135,30 @@ model Session { // v1.23 — sliding "last seen" for the user-facing active-session list. lastActiveAt DateTime? @map("last_active_at") - user User @relation(fields: [userId], references: [id], onDelete: Cascade) + /// v1.36.0 — the cookie transport's acting-account carrier. NULL (the only + /// value any session has had until now) means the browser is acting as the + /// account it signed in as. A non-NULL value names the account whose record + /// this session is currently reading, and it is a SELECTOR, never an + /// authorization: the resolver joins it against a live grant on every single + /// request, so a value left here by a grant that has since ended confers + /// exactly nothing. + /// + /// Server-side by construction — the column lives on a row the client cannot + /// address (the cookie carries a secret, not this id), and the only writer is + /// the switch endpoint, which validates the grant before it stamps. + /// + /// The FK is ON DELETE SET NULL rather than CASCADE: deleting the OWNER must + /// not delete the delegate's browser session, it must put that browser back + /// into its own account. + actingAsUserId String? @map("acting_as_user_id") + + user User @relation("SessionOwner", fields: [userId], references: [id], onDelete: Cascade) + actingAs User? @relation("SessionActingAs", fields: [actingAsUserId], references: [id], onDelete: SetNull) @@index([userId]) + // Revocation clears every session pointing at the owner, and deleting an + // account nulls the same set. Both sweep on this column. + @@index([actingAsUserId]) // v1.18.7 — the sliding-30-day session reaper sweeps on `expiresAt`; without // this index the cleanup degrades to a sequential scan on a busy instance. @@index([expiresAt]) From e9029ed3c286a905690ed27afddf86cac84f9c65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 11:32:00 +0200 Subject: [PATCH 2/3] feat(sharing): resolve the acting account inside the auth layer One resolver, both transports, on every request. The cookie path reads the carrier off the session row it has already loaded; the Bearer path reads the `X-HealthLog-Account` selector header, because stamping switch state onto a long-lived token would make the credential itself ambient authority. Two carriers converge on one function for the same reason `resolveBearerToken` lives in one file for two wires. Three declared modes. `requireRecordAuth("read" | "write")` honours a valid switch and substitutes the owner as the data scope while keeping the delegate as the actor. `requireActorAuth()` always serves the caller's own rows and keeps working under a switch, which is what the switcher and the banner need. Bare `requireAuth()` refuses to run under a switch at all, with a stable `sharing.not_permitted` code. It does not fall back to the caller's own account, and that arm is the point of the whole design: a delegate who believes they are reading the owner's record and is handed their own is the same data mixing pointed the other way. A route nobody has reviewed is inaccessible while switched, never wrongly accessible. The grant is loaded from Postgres on the request that uses it. Nothing memoises the verdict, so a revocation lands on the delegate's next request rather than their next login. Refusals do not enumerate. The denial takes no constructor arguments and carries a fixed message, and neither branch looks the named account up: an account that does not exist and one that granted nothing produce the same empty row from the same query, so the two answers are identical in bytes and in timing rather than by careful wording. The reason reaches the audit row and the wide event and nothing else. `getSession()` still answers who is calling. That is what leaves `requireAdmin`, `requireCookieAuth` and `requireFreshMfa` structurally out of a switch's reach; the row's carrier rides out beside the session id as a value to be checked, never as a decision already made. The selector header is a Bearer carrier only. Sent over the cookie transport, or to an actor surface, it is refused rather than ignored. A client that names an account on a route which never reads one has a bug, and a correct response to a wrong expectation hides that bug until it arrives as a support case about data that looks wrong. Five test files gain the session shape their fixtures always claimed to have; they were building an auth context without one. --- .../__tests__/soft-delete-filter.test.ts | 5 + .../health-record/__tests__/route.test.ts | 5 + .../insights/targets/__tests__/route.test.ts | 6 +- .../api/mcp/oauth/__tests__/authorize.test.ts | 6 +- .../__tests__/route.test.ts | 5 + src/lib/api-handler.ts | 431 +++++++++++++++++- src/lib/auth/session.ts | 24 +- src/lib/logging/event-builder.ts | 12 + src/lib/logging/types.ts | 8 + 9 files changed, 497 insertions(+), 5 deletions(-) diff --git a/src/app/api/export/__tests__/soft-delete-filter.test.ts b/src/app/api/export/__tests__/soft-delete-filter.test.ts index 4544f74f9..b7b77d1f1 100644 --- a/src/app/api/export/__tests__/soft-delete-filter.test.ts +++ b/src/app/api/export/__tests__/soft-delete-filter.test.ts @@ -98,6 +98,11 @@ import { } from "@/lib/modules/gate"; const SESSION_OK = { + session: { + id: "session-1", + expiresAt: new Date(Date.now() + 3_600_000), + actingAsUserId: null, + }, user: { id: "user-1", email: "test@example.com", diff --git a/src/app/api/export/health-record/__tests__/route.test.ts b/src/app/api/export/health-record/__tests__/route.test.ts index 027c376de..905d6c272 100644 --- a/src/app/api/export/health-record/__tests__/route.test.ts +++ b/src/app/api/export/health-record/__tests__/route.test.ts @@ -98,6 +98,11 @@ import { } from "@/lib/modules/gate"; const SESSION_OK = { + session: { + id: "session-1", + expiresAt: new Date(Date.now() + 3_600_000), + actingAsUserId: null, + }, user: { id: "user-1", email: "test@example.com", role: "USER" }, } as const; diff --git a/src/app/api/insights/targets/__tests__/route.test.ts b/src/app/api/insights/targets/__tests__/route.test.ts index d3be63c57..44c48d332 100644 --- a/src/app/api/insights/targets/__tests__/route.test.ts +++ b/src/app/api/insights/targets/__tests__/route.test.ts @@ -96,7 +96,11 @@ const SESSION_USER = { }; const SESSION_OK = { - session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + session: { + id: "sess-1", + expiresAt: new Date(Date.now() + 3_600_000), + actingAsUserId: null, + }, user: SESSION_USER as never, }; diff --git a/src/app/api/mcp/oauth/__tests__/authorize.test.ts b/src/app/api/mcp/oauth/__tests__/authorize.test.ts index dce582e1c..5040f996e 100644 --- a/src/app/api/mcp/oauth/__tests__/authorize.test.ts +++ b/src/app/api/mcp/oauth/__tests__/authorize.test.ts @@ -77,7 +77,11 @@ function postReq(fields: Record): Request { function signedIn() { vi.mocked(getSession).mockResolvedValue({ - session: { id: "s1", expiresAt: new Date(Date.now() + 1e6) }, + session: { + id: "s1", + expiresAt: new Date(Date.now() + 1e6), + actingAsUserId: null, + }, user: { id: "user-1" } as never, }); } diff --git a/src/app/api/medications/intake/dose-history-import/__tests__/route.test.ts b/src/app/api/medications/intake/dose-history-import/__tests__/route.test.ts index 9fdb848b9..ef2d0708f 100644 --- a/src/app/api/medications/intake/dose-history-import/__tests__/route.test.ts +++ b/src/app/api/medications/intake/dose-history-import/__tests__/route.test.ts @@ -59,6 +59,11 @@ import { getGlobalBoss } from "@/lib/jobs/boss-instance"; import { checkRateLimit } from "@/lib/rate-limit"; const SESSION_OK = { + session: { + id: "session-1", + expiresAt: new Date(Date.now() + 3_600_000), + actingAsUserId: null, + }, user: { id: "user-1", username: "owner", role: "USER", email: null }, }; diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 48ac102a3..a1a1ad5c5 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -22,6 +22,12 @@ import { hashToken } from "./auth/hmac"; import { AssistantDisabledError } from "./feature-flags"; import { ConsentRequiredError } from "./ai/consent-guard"; import { SCOPE_HEALTH_READ, SCOPE_HEALTH_WRITE } from "./mcp/oauth/config"; +import { + findActiveGrant, + grantAllows, + touchGrantUsage, + type GrantNeed, +} from "./sharing/grants"; /** * HTTP methods a read-only credential may use on the REST surface. A request @@ -67,6 +73,63 @@ export class HttpError extends Error { } } +/** + * v1.36.0 — the two refusals the acting-account resolver can raise. + * + * Both are 403 with a stable `meta.errorCode`, the same envelope shape the + * step-up and consent gates already use, so a client branches on the code + * rather than on prose. + */ +export abstract class SharingAuthError extends HttpError { + abstract readonly errorCode: string; +} + +/** + * "You may not act as that account." + * + * Deliberately parameterless. The message is a fixed literal and the + * constructor takes no arguments, so there is no way for a caller to vary the + * response — which is what makes a selector naming an account that does not + * exist and one naming an account that granted nothing produce the SAME BYTES. + * That is not a formatting nicety: a distinguishable refusal turns this + * endpoint into an account-enumeration oracle for anyone with a login. + * + * The indistinguishability is structural rather than careful, too. Neither + * branch ever looks an account up: the resolver asks the grant table for the + * (owner, delegate) pair and a missing account and a missing grant are the + * same empty result, on the same code path, after the same one query. + * + * Why the reason is not in the response at all: the caller already knows what + * they sent, and the only party who benefits from knowing WHICH condition + * failed is someone probing. The reason goes to the audit row and the wide + * event, where the owner and the operator can see it. + */ +export class SharingAccessDeniedError extends SharingAuthError { + readonly errorCode = "sharing.access.denied"; + constructor() { + super(403, "Account access denied"); + this.name = "SharingAccessDeniedError"; + } +} + +/** + * "This endpoint does not act on another account's record." + * + * A different fact from the one above and so a different code: nothing here is + * about whether a grant exists. The route simply never declared that it can be + * used under a switch, so it refuses rather than quietly serving the caller's + * OWN record — which is the reverse data-mixing failure, and the reason this + * class exists at all. A delegate who believes they are reading the owner's + * record must never be handed their own instead. + */ +export class SharingNotPermittedError extends SharingAuthError { + readonly errorCode = "sharing.not_permitted"; + constructor() { + super(403, "This endpoint cannot be used while acting on another account"); + this.name = "SharingNotPermittedError"; + } +} + /** * Wraps an API route handler with Wide Event logging. * Creates a WideEventBuilder, runs the handler inside AsyncLocalStorage, @@ -247,6 +310,21 @@ export function apiHandler Promise>( }, { status: error.statusCode }, ); + } else if (error instanceof SharingAuthError) { + // v1.36.0 — the acting-account resolver refused. Same + // 403 + meta.errorCode envelope as the gates above, so a client + // distinguishes "you may not act as that account" from "this + // endpoint does not support acting on one" without parsing prose. + // Checked before the generic HttpError branch because it extends it. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: error.statusCode }, + ); } else if (error instanceof HttpError) { evt.setError(error); response = NextResponse.json( @@ -292,7 +370,21 @@ export function apiHandler Promise>( * to a Session row. */ export type AuthContext = { - session: { id: string; expiresAt: Date }; + session: { + id: string; + expiresAt: Date; + /** + * v1.36.0 — the cookie transport's acting-account carrier, straight off + * the session row. Absent on the Bearer path, which has no session row and + * carries its selector in a per-request header instead. + * + * Optional so that the ~180 test files which hand `requireAuth` a + * hand-built session context keep compiling; an absent field means the + * same thing as a null one — no switch. Only the acting-account resolver + * below reads it. + */ + readonly actingAsUserId?: string | null; + }; user: User; /** * Transport the caller authenticated with, derived from the SAME branch that @@ -313,6 +405,22 @@ export type AuthContext = { * 2. No cookie + `Authorization: Bearer hlk_<...>` → API token path. * 3. Neither → 401. * + * v1.36.0 — and it REFUSES to run while the request is acting on another + * account. A bare `requireAuth()` is an undeclared route: nobody has decided + * whether it is safe under a switch, and the routes that call it include the + * password change, the MFA management surface, the token mint, and the + * integration connect. So a request carrying an acting-account carrier gets a + * 403 `sharing.not_permitted` here, and never a silent fall-back to the + * caller's own record. + * + * The fall-back is the tempting wrong answer and is worth naming: it looks + * harmless, it keeps every un-migrated route working, and it is the reverse + * data-mixing failure. A delegate reading what they believe is the owner's + * record would be shown their own; a delegate writing what they believe is the + * owner's measurement would write it into theirs. An inaccessible route is an + * inconvenience. A route that answers with the wrong person's health record is + * the product being wrong about the only thing it does. + * * @param requiredPermission Optional permission scope. Only enforced for Bearer * auth — cookie sessions always pass (full user access). Omitting it is a * positive declaration: the route accepts cookie sessions and cookie- @@ -323,6 +431,25 @@ export type AuthContext = { */ export async function requireAuth( requiredPermission?: string, +): Promise { + const auth = await authenticateCaller(requiredPermission); + const carrier = await readActingCarrier(auth); + if (carrier.kind !== "none") { + throw refuseUndeclaredMode(auth, carrier); + } + return auth; +} + +/** + * Resolve the caller as themselves. The pre-v1.36.0 `requireAuth` body, + * unchanged, split out so the three declared modes above it can share it. + * + * This function knows nothing about acting accounts and must not learn: it is + * the thing every mode agrees on before they disagree about what the request + * is allowed to reach. + */ +async function authenticateCaller( + requiredPermission?: string, ): Promise { // 1. Session cookie path — unchanged. const sessionData = await getSession(); @@ -466,10 +593,312 @@ async function authenticateBearer( }; } +// ── The acting-account resolver ───────────────────────────────────────────── +// +// One function decides, for every transport, whether this request acts on +// somebody else's record — the same argument that put `resolveBearerToken` in +// one file for two wires. Two carriers converge here because they are two +// answers to one question, and two answers that live apart drift. +// +// Neither carrier authorises anything. Both merely NAME an account; the grant +// table decides, on this request, with no cached verdict anywhere. The +// difference between them is who can set them: +// +// * The cookie transport carries it on the session ROW (`actingAsUserId`). +// A browser cannot address that row — the cookie holds a secret whose hash +// is the lookup key — so the value is server state, written only by the +// switch endpoint after it has validated the grant. +// * The Bearer transport carries it in a per-request HEADER. A long-lived +// token must not accumulate switch state: stamping it on the ApiToken row +// would make the credential itself ambient authority, and the token has to +// keep meaning "this person" for as long as it exists. +// +// The header is a Bearer-only carrier. On a cookie request it is refused, not +// ignored — a client that sends a selector over a transport that does not read +// one believes it is selecting an account and is not, and the failure of that +// belief is a support case about data that looks wrong. + +/** + * The per-request account selector, on the Bearer transport only. + * + * Cross-origin JavaScript cannot set it: a custom request header forces a CORS + * preflight and this app sends no CORS headers at all, so the preflight is + * never answered. It is a selector regardless — the grant table is what + * authorises, and the header is treated as hostile input everywhere below. + */ +export const ACCOUNT_SELECTOR_HEADER = "x-healthlog-account"; + +/** + * Longest selector value worth a database round trip. Account ids are 25-char + * cuids; anything past this names no account that could exist and is refused + * as such, on the same path and with the same bytes as an unknown account. + */ +const MAX_SELECTOR_LENGTH = 64; + +/** + * What, if anything, this request says it is acting as. + * + * `misplaced-header` is its own case rather than an error thrown at read time + * so that every mode below decides for itself — the modes disagree about the + * session carrier and must not disagree about this one by accident. + */ +type ActingCarrier = + | { kind: "none" } + | { kind: "session"; accountId: string } + | { kind: "header"; accountId: string } + | { kind: "misplaced-header" }; + +/** The selector header, or null when the request did not send one. */ +async function readSelectorHeader(): Promise { + try { + const headerList = await headers(); + return headerList.get(ACCOUNT_SELECTOR_HEADER); + } catch { + // Outside a Next.js request scope (direct unit invocation of a legacy + // route) there is no header to read. Same posture as the Bearer branch of + // `authenticateCaller`. + return null; + } +} + +/** + * The acting-account carrier for this request. + * + * The session value rides out of `getSession()` on the row that call already + * loaded — a second query for a column we have just read would be waste on + * every cookie request in the app. What does NOT ride out of `getSession()` is + * a substituted user: it keeps answering "who is calling" and nothing else, + * which is exactly why `requireAdmin`, `requireCookieAuth` and + * `requireFreshMfa` are unreachable by a switch. The carrier is an id to be + * checked, not a decision that has been made. + * + * Fresh on every request, by construction: `getSession()` reads the row from + * Postgres each time, so a switch cleared by a revocation is gone from the very + * next request rather than from the next login. + */ +async function readActingCarrier(auth: AuthContext): Promise { + const header = await readSelectorHeader(); + + if (auth.authMethod === "bearer") { + // No session row exists on this transport, so the header is the only + // carrier there could be. + return header === null + ? { kind: "none" } + : { kind: "header", accountId: header }; + } + + if (header !== null) return { kind: "misplaced-header" }; + + const stamped = auth.session.actingAsUserId ?? null; + return stamped === null + ? { kind: "none" } + : { kind: "session", accountId: stamped }; +} + +/** Could this value name an account at all? */ +function selectorNamesAnAccount(value: string): boolean { + return value.length > 0 && value.length <= MAX_SELECTOR_LENGTH; +} + +/** The account a carrier names, or null when it names nothing. */ +function carrierTarget(carrier: ActingCarrier): string | null { + return "accountId" in carrier ? carrier.accountId : null; +} + +/** + * Record a refused delegation. + * + * Written for the OWNER's benefit as much as the operator's: "someone tried to + * open my record and was turned away" is the question the sharing panel exists + * to answer. The row names the actor (the caller), the account they aimed at, + * and why it failed — none of which reaches the response. + * + * Fire-and-forget: an audit write that fails must not convert a 403 into a 500, + * because the 403 is the part that protects the record. + */ +function auditRefusal( + auth: AuthContext, + carrier: ActingCarrier, + reason: string, +): void { + auditLog("sharing.access.denied", { + userId: auth.user.id, + details: { + reason, + carrier: carrier.kind, + target: carrierTarget(carrier), + method: getEvent()?.getHttpMethod() ?? null, + }, + }).catch(() => {}); +} + +/** + * This request says it is acting on another account and this route will not. + * + * Two ways to arrive: a route that never declared a mode (`undeclared_mode`), + * and a selector sent over the transport that does not carry one + * (`misplaced_selector`). One response either way — the caller learns that the + * request will not be served, and nothing about grants. + * + * The audit rule, stated once because it is a judgement call: a HEADER refusal + * always writes a row, a SESSION refusal never does. The header is a per-request + * assertion by a client — a bug or a probe, and both are worth a durable record. + * The session field is ambient browser state: while a switch is on, every + * navigation, prefetch and poll that touches a non-delegable route lands here, + * and a row per such request would bury the header rows that matter under noise + * from the feature working as designed. Both are on the wide event either way. + */ +function refuseUndeclaredMode( + auth: AuthContext, + carrier: ActingCarrier, + reason: "undeclared_mode" | "misplaced_selector" = "undeclared_mode", +): SharingNotPermittedError { + annotate({ meta: { sharing_refusal: reason } }); + if (carrier.kind !== "session") { + auditRefusal(auth, carrier, reason); + } + return new SharingNotPermittedError(); +} + +/** + * v1.36.0 — an actor surface: this route always serves the CALLER's own rows. + * + * It works under a switch. That is the whole reason the mode exists: the + * switcher itself, the banner, and the "which records may I open" list all have + * to keep answering while the browser is acting as someone else, and every one + * of them is about the delegate. So the session carrier is read and + * deliberately ignored — no query, because there is nothing the answer could + * change. + * + * A selector HEADER is refused, loudly. An actor surface is DEFINED as serving + * the caller's own rows, so a client that attaches a selector to one has a bug, + * and the bug is invisible until somebody files a support ticket about data + * that looks wrong. Ignoring the header would serve exactly the right response + * to exactly the wrong expectation. + */ +export async function requireActorAuth(): Promise { + const auth = await authenticateCaller(); + const header = await readSelectorHeader(); + if (header !== null) { + annotate({ meta: { sharing_refusal: "actor_surface" } }); + auditRefusal(auth, { kind: "header", accountId: header }, "actor_surface"); + throw new SharingNotPermittedError(); + } + return auth; +} + +/** + * A request resolved against a record that may not be the caller's own. + * + * `user` is the account whose rows the handler must read and write — so an + * existing `where: { userId: user.id }` is correct without being touched, which + * is what keeps a route migration a one-line change instead of an audit. + * `actor` is who is actually here. They are the same object when no switch is + * active. + */ +export interface RecordAuthContext extends AuthContext { + /** The authenticated caller. Equals `user` when the caller acts as themselves. */ + readonly actor: User; + /** The grant being exercised, or null when the caller acts as themselves. */ + readonly grantId: string | null; +} + +/** + * v1.36.0 — a delegable surface: this route may act on a shared record. + * + * Calling it is a DECLARATION, reviewed and frozen by a structural guard, that + * the route reads or writes health data and nothing else — no credential, no + * integration, no notification channel, no grant management, nothing that would + * let a delegate extend their own reach. The declaration is the security + * property; this function only enforces what was declared. + * + * @param need what the route does to the record. `"read"` still refuses a + * non-safe HTTP method under a read-only grant: the declaration and the + * method must BOTH be satisfiable, so a delegable GET handler that grows a + * POST export cannot quietly inherit the read grant's permission. An unknown + * method (no event context) counts as a write and is refused, the same + * fail-closed posture as the MCP audience guard above. + */ +export async function requireRecordAuth( + need: GrantNeed, +): Promise { + const auth = await authenticateCaller(); + const carrier = await readActingCarrier(auth); + + if (carrier.kind === "none") { + // Do no harm: without a carrier this is byte-for-byte the pre-v1.36.0 + // request, resolved by the same `authenticateCaller` every other mode uses. + return { ...auth, actor: auth.user, grantId: null }; + } + if (carrier.kind === "misplaced-header") { + // The route DID declare a mode; the client sent the selector over the + // cookie transport, where the carrier is the session row. + throw refuseUndeclaredMode(auth, carrier, "misplaced_selector"); + } + + /** Every refusal below is this one error, with the reason kept off the wire. */ + const denied = (reason: string): SharingAccessDeniedError => { + annotate({ meta: { sharing_refusal: reason } }); + auditRefusal(auth, carrier, reason); + return new SharingAccessDeniedError(); + }; + + if (!selectorNamesAnAccount(carrier.accountId)) { + throw denied("malformed_selector"); + } + + const method = (getEvent()?.getHttpMethod() ?? "").toUpperCase(); + const effectiveNeed: GrantNeed = + need === "write" || !READ_HTTP_METHODS.has(method) ? "write" : "read"; + + // Loaded here, on this request, every request. Not memoised on the session, + // not cached in the process, not carried on the token: a revocation has to + // land on the delegate's NEXT request, not on their next login. That is the + // one property the owner is actually promised when they press revoke. + // + // Note what is NOT looked up: the account named by the carrier. An account + // that does not exist and an account that granted nothing produce the same + // empty row from the same query, which is what makes the two refusals + // identical in bytes and in timing rather than identical by careful wording. + const grant = await findActiveGrant({ + grantorId: carrier.accountId, + granteeId: auth.user.id, + }); + if (!grant) throw denied("no_active_grant"); + if (!grantAllows(grant, effectiveNeed)) throw denied("insufficient_access"); + + const owner = await prisma.user.findUnique({ + where: { id: grant.grantorId }, + }); + // Unreachable while the FK cascade holds — a grant naming a deleted account + // is deleted with it. Fail closed anyway: an authorization we cannot resolve + // to a person is not one to act on. + if (!owner) throw denied("owner_missing"); + + getEvent()?.setActingAs(owner.id); + // Fire-and-forget, the `ApiToken.lastUsedAt` posture: the read must not wait + // on the bookkeeping, and the bookkeeping failing must not fail the read. + void touchGrantUsage(grant.id); + + return { + session: auth.session, + user: owner, + authMethod: auth.authMethod, + actor: auth.user, + grantId: grant.id, + }; +} + /** * Require an authenticated admin user. Throws HttpError(401) or HttpError(403). * Cookie-only — Bearer tokens never elevate to admin (security boundary). * Automatically annotates the Wide Event with auth context. + * + * v1.36.0 — unreachable by a switch, structurally: it resolves through + * `getSession()`, which knows only who is calling, and the role is read off + * that row. Switching into an ADMIN's record therefore confers nothing, and + * making it confer something would take a deliberate edit here rather than an + * oversight elsewhere. */ export async function requireAdmin(): Promise { const sessionData = await getSession(); diff --git a/src/lib/auth/session.ts b/src/lib/auth/session.ts index f86f7a0bd..74ef90227 100644 --- a/src/lib/auth/session.ts +++ b/src/lib/auth/session.ts @@ -246,7 +246,7 @@ export async function createSession( } export async function getSession(): Promise<{ - session: { id: string; expiresAt: Date }; + session: { id: string; expiresAt: Date; actingAsUserId: string | null }; user: User; } | null> { try { @@ -343,7 +343,27 @@ export async function getSession(): Promise<{ } return { - session: { id: session.id, expiresAt: session.expiresAt }, + session: { + id: session.id, + expiresAt: session.expiresAt, + // v1.36.0 — the acting-account CARRIER, projected because the row is + // already in hand and re-reading it per request would be waste. + // + // Read this in exactly one place: the acting-account resolver in + // `src/lib/api-handler.ts`. It is a selector and not a decision — the + // resolver still has to find a live grant for the pair before anything + // is substituted, and a value stranded here by a grant that has since + // ended confers nothing. + // + // What this function returns for `user` does NOT change and must not: + // `getSession()` answers "who is calling", and `requireAdmin`, + // `requireCookieAuth` and `requireFreshMfa` are unreachable by a switch + // precisely because that is the only thing it answers. Substituting the + // owner here would put a delegate inside the admin check. + // `src/__tests__/acting-account-boundary-guard.test.ts` freezes the set + // of files allowed to touch this field. + actingAsUserId: session.actingAsUserId, + }, user: session.user, }; } diff --git a/src/lib/logging/event-builder.ts b/src/lib/logging/event-builder.ts index 1d093a016..8852ee469 100644 --- a/src/lib/logging/event-builder.ts +++ b/src/lib/logging/event-builder.ts @@ -90,6 +90,18 @@ export class WideEventBuilder { return this; } + /** + * v1.36.0 — record that this request acted on another account's record. + * + * Merges rather than replaces, because the actor identity is already on the + * event by the time the acting account is resolved and overwriting it is the + * exact confusion the two fields exist to prevent. + */ + setActingAs(userId: string): this { + this.event.auth = { ...(this.event.auth ?? {}), acting_as: userId }; + return this; + } + setAction(action: WideEvent["action"]): this { this.event.action = action; return this; diff --git a/src/lib/logging/types.ts b/src/lib/logging/types.ts index 5d192e770..f9e77fbee 100644 --- a/src/lib/logging/types.ts +++ b/src/lib/logging/types.ts @@ -58,6 +58,14 @@ export interface WideEvent { | "cron_secret" | "telegram_webhook" | "webhook_secret"; + /** + * v1.36.0 — the account whose record this request read or wrote, when it + * is not the caller's own. `user_id` above always stays the ACTOR: the + * person who authenticated. Two fields rather than one swapped field, + * because "who did it" and "whose record" are different questions and a + * dashboard that conflates them cannot answer either. + */ + acting_as?: string; }; // Business-Daten (was wurde getan) From 3f932040ad7043bec3b04cbfd7defcfbb8c6951b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 2 Aug 2026 11:32:13 +0200 Subject: [PATCH 3/3] test(sharing): hold every arm of the acting-account resolver The unit file walks the arms with the grant module deliberately unmocked, so the resolver consuming the one "is this grant active" predicate is a property under test rather than a claim in a comment. Its Prisma fake honours its own `where` clause for the same reason: a fake that answers any query with any row turns every scoping assertion into a check that cannot fail. The integration file proves the two things a unit suite cannot, because both are about which rows come back. Substitution: the owner's measurements are returned and the delegate's own, seeded alongside, are not, so a resolver that quietly served the caller their own record shows up as the wrong values rather than as an empty list. Never cached: a revocation lands between two requests inside one test and the second one 403s. The do-no-harm arm runs against a real shipped route, and the cookie-only helpers are asserted against a real session row carrying a real switch, because the one edit that would break them is `getSession()` itself substituting the owner and no mocked session can catch that. The structural guard freezes the set of files allowed to touch the carrier column and the selector header, and asserts a non-zero match count before comparing against the allowlist. A matcher that finds nothing agrees with an emptied list and reports success; this repository has shipped several guards that did exactly that. --- .../acting-account-boundary-guard.test.ts | 135 ++++ .../__tests__/acting-account-resolver.test.ts | 616 ++++++++++++++++++ .../acting-account-resolver.test.ts | 382 +++++++++++ 3 files changed, 1133 insertions(+) create mode 100644 src/__tests__/acting-account-boundary-guard.test.ts create mode 100644 src/lib/__tests__/acting-account-resolver.test.ts create mode 100644 tests/integration/acting-account-resolver.test.ts diff --git a/src/__tests__/acting-account-boundary-guard.test.ts b/src/__tests__/acting-account-boundary-guard.test.ts new file mode 100644 index 000000000..53ca21e44 --- /dev/null +++ b/src/__tests__/acting-account-boundary-guard.test.ts @@ -0,0 +1,135 @@ +/** + * Structural guards on the acting-account boundary. + * + * The behavioural tests in `src/lib/__tests__/acting-account-resolver.test.ts` + * prove what today's code does. These prove what tomorrow's may not do, and + * they exist because the boundary has one shape that keeps it safe: the acting + * account is a value ONE function is allowed to act on, and the helpers that + * decide role and step-up never see it. + * + * Tripwires, not proofs. They cannot show the resolver is correct — only that + * the boundary has not moved without somebody editing this file. Every leg + * below asserts a non-zero match count first, because a guard whose matcher + * finds nothing reports success, and this repository has shipped several of + * those. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { globSync } from "node:fs"; +import { join, sep } from "node:path"; + +const SRC = join(process.cwd(), "src"); + +function sourceFiles(): string[] { + return globSync("**/*.{ts,tsx}", { cwd: SRC }) + .filter( + (p) => !p.startsWith(`generated${sep}`) && !p.startsWith("generated/"), + ) + .filter((p) => !p.includes("__tests__")) + .filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx")) + .map((p) => p.split(sep).join("/")) + .sort(); +} + +function read(rel: string): string { + return readFileSync(join(SRC, rel), "utf8"); +} + +function filesMatching(re: RegExp): string[] { + return sourceFiles().filter((rel) => re.test(read(rel))); +} + +/** The source text of a top-level exported function, braces included. */ +function functionBody(src: string, name: string): string { + const start = src.indexOf(`export async function ${name}(`); + if (start === -1) return ""; + const open = src.indexOf("{", src.indexOf(")", start)); + let depth = 0; + for (let i = open; i < src.length; i++) { + if (src[i] === "{") depth++; + else if (src[i] === "}") { + depth -= 1; + if (depth === 0) return src.slice(open, i + 1); + } + } + return ""; +} + +describe("the acting-account carrier is read in one place", () => { + /** + * Files allowed to touch `Session.actingAsUserId`. + * + * The column is a selector — it names an account, it does not authorise one. + * Anything that reads it and acts on the value without going through the + * resolver has invented a second answer to "whose record is this", and the + * copy that drifts is always the one nobody reads. + * + * Later items in this feature will extend this list, and that is the point: + * the switch endpoint (which writes it) and revocation cleanup (which clears + * it) arrive as a visible diff here rather than as a quiet new reader. + * + * The matcher reads source text and does not exempt comments, so a file that + * merely NAMES the column in prose trips it. Left that way deliberately: a + * guard over an authorization carrier that shouts at a mention is the right + * direction to be wrong in. + */ + const CARRIER_ALLOWLIST = [ + // Projects it off the session row it already loaded. + "lib/auth/session.ts", + // The resolver. The only thing that acts on the value. + "lib/api-handler.ts", + ].sort(); + + it("no other file reads or writes the carrier column", () => { + const readers = filesMatching(/actingAsUserId|acting_as_user_id/); + // Non-zero proof: an emptied allowlist must fail rather than agree with an + // emptied match set. + expect(readers.length).toBeGreaterThan(0); + expect(readers).toEqual(CARRIER_ALLOWLIST); + }); + + it("the selector header is named in one place", () => { + // A route that read the header itself would be deciding an authorisation + // question the resolver exists to answer once. + const namers = filesMatching(/x-healthlog-account/i); + expect(namers.length).toBeGreaterThan(0); + expect(namers).toEqual(["lib/api-handler.ts"]); + }); +}); + +describe("the cookie-only helpers cannot see the acting account", () => { + /** + * `requireAdmin`, `requireCookieAuth` and `requireFreshMfa` are safe under a + * switch for one reason: they resolve through `getSession()`, which answers + * who is CALLING, and they never consult the carrier. That is the property + * that keeps role checks and step-up gates structurally out of a delegate's + * reach, and it is one careless line away from being untrue. + */ + const COOKIE_ONLY = ["requireAdmin", "requireCookieAuth", "requireFreshMfa"]; + + it.each(COOKIE_ONLY)("%s resolves the actor and nothing else", (name) => { + const body = functionBody(read("lib/api-handler.ts"), name); + + // Non-zero proof, twice over: the slice found a function, and the slice is + // the function it claims to be. A renamed helper must fail here rather + // than silently assert against an empty string. + expect(body.length).toBeGreaterThan(0); + expect(body).toContain("getSession()"); + + expect(body).not.toContain("actingAsUserId"); + expect(body).not.toContain("requireRecordAuth"); + expect(body).not.toContain("readActingCarrier"); + }); + + it("substitutes a data-scope user in exactly one function", () => { + const src = read("lib/api-handler.ts"); + const record = functionBody(src, "requireRecordAuth"); + expect(record).toContain("readActingCarrier"); + + // `readActingCarrier` is what turns a request into "acting as somebody + // else". Two callers: the bare path, which refuses on it, and the record + // path, which acts on it. A third would be a mode nobody declared. + const callers = (src.match(/await readActingCarrier\(/g) ?? []).length; + expect(callers).toBe(2); + }); +}); diff --git a/src/lib/__tests__/acting-account-resolver.test.ts b/src/lib/__tests__/acting-account-resolver.test.ts new file mode 100644 index 000000000..fe8e624dc --- /dev/null +++ b/src/lib/__tests__/acting-account-resolver.test.ts @@ -0,0 +1,616 @@ +/** + * v1.36.0 — the acting-account resolver, arm by arm. + * + * Three things are worth more here than anywhere else in the feature, because + * everything downstream trusts them: + * + * 1. **It fails closed.** A bare `requireAuth()` reached under an active + * switch refuses. It must never fall back to the caller's own account — + * a delegate who believes they are reading the owner's record being handed + * their own is the same data-mixing failure, pointed the other way. + * 2. **Refusals do not enumerate.** A selector naming an account that does + * not exist and one naming an account that granted nothing produce the + * same bytes. Asserted as bytes, not as "both were 403". + * 3. **Nothing is cached.** The grant is loaded on the request that uses it. + * Proven against a real database in + * `tests/integration/acting-account-resolver.test.ts`, where revoking + * between two requests is a thing that can actually happen. + * + * The grant module is deliberately NOT mocked. The resolver consuming S1's + * `isGrantActive` rather than its own idea of "active" is the property that + * keeps the panel and the resolver from disagreeing, and a mocked predicate + * would report that property working while it was broken. The Prisma fake below + * honours its `where` clause for the same reason: a fake that ignores `where` + * turns every scoping assertion into a check that cannot fail. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// --- Mocks must be hoisted before importing the module under test. --- + +interface GrantRow { + id: string; + grantorId: string; + granteeId: string; + access: "READ" | "WRITE"; + acceptedAt: Date | null; + revokedAt: Date | null; + expiresAt: Date | null; +} + +const grantRows: GrantRow[] = []; +const userRows = new Map(); +const touched: string[] = []; + +vi.mock("@/lib/db", () => ({ + prisma: { + accountGrant: { + // Honours the narrowing the real query performs. `findActiveGrant` + // narrows on `revokedAt: null` in SQL and leaves accepted/expired to the + // state machine; a fake that returned any row for any `where` would make + // the cross-user arms below pass no matter what the resolver did. + findFirst: vi.fn( + async ({ + where, + }: { + where: { + grantorId: string; + granteeId: string; + revokedAt: null; + }; + }) => + grantRows.find( + (g) => + g.grantorId === where.grantorId && + g.granteeId === where.granteeId && + (where.revokedAt === null ? g.revokedAt === null : true), + ) ?? null, + ), + update: vi.fn(async ({ where }: { where: { id: string } }) => { + touched.push(where.id); + return {}; + }), + }, + user: { + findUnique: vi.fn( + async ({ where }: { where: { id: string } }) => + userRows.get(where.id) ?? null, + ), + }, + apiToken: { findUnique: vi.fn(), update: vi.fn() }, + webauthnMfaCredential: { count: vi.fn(async () => 0) }, + session: { findUnique: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/hmac", () => ({ hashToken: vi.fn(() => "hash") })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +const headerJar = new Map(); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +// --- Imports use the mocked modules above. --- + +import { NextRequest, NextResponse } from "next/server"; +import { + ACCOUNT_SELECTOR_HEADER, + apiHandler, + requireActorAuth, + requireAdmin, + requireAuth, + requireCookieAuth, + requireFreshMfa, + requireRecordAuth, + type RecordAuthContext, +} from "../api-handler"; +import { getSession } from "@/lib/auth/session"; +import { auditLog } from "@/lib/auth/audit"; +import { emitIfSampled } from "@/lib/logging/transports"; +import { prisma } from "@/lib/db"; +import type { WideEvent } from "@/lib/logging/types"; + +const OWNER = "owner-1"; +const DELEGATE = "delegate-1"; + +function user(id: string, role: "USER" | "ADMIN" = "USER") { + return { id, role, username: id, email: `${id}@example.test` }; +} + +/** Sign the delegate in over the cookie transport. */ +function signedInCookie( + actingAsUserId: string | null = null, + role: "USER" | "ADMIN" = "USER", +): void { + vi.mocked(getSession).mockResolvedValue({ + session: { + id: "session-1", + expiresAt: new Date(Date.now() + 3_600_000), + actingAsUserId, + }, + user: user(DELEGATE, role) as never, + }); +} + +/** Sign the delegate in over the Bearer transport. */ +function signedInBearer(): void { + vi.mocked(getSession).mockResolvedValue(null); + headerJar.set("authorization", "Bearer hlk_" + "a".repeat(64)); + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "token-1", + userId: DELEGATE, + permissions: ["*"], + revoked: false, + expiresAt: new Date(Date.now() + 3_600_000), + } as never); + vi.mocked(prisma.apiToken.update).mockResolvedValue({} as never); + userRows.set(DELEGATE, user(DELEGATE)); +} + +function selector(value: string): void { + headerJar.set(ACCOUNT_SELECTOR_HEADER, value); +} + +function liveGrant(overrides: Partial = {}): GrantRow { + const row: GrantRow = { + id: "grant-1", + grantorId: OWNER, + granteeId: DELEGATE, + access: "READ", + acceptedAt: new Date(Date.now() - 60_000), + revokedAt: null, + expiresAt: null, + ...overrides, + }; + grantRows.push(row); + return row; +} + +/** + * Run a resolver call the way a route runs it: inside `apiHandler`, against a + * real `NextRequest`, so the HTTP method the resolver reads is the one the + * request actually carried and the refusal is a real HTTP response. + */ +function route( + resolve: () => Promise, +): (method?: string) => Promise { + // A zero-argument handler is assignable to the one-argument shape Next.js + // calls; naming the type here keeps the call site honest without an unused + // parameter. + const handler: (request: NextRequest) => Promise = apiHandler( + async () => { + await resolve(); + return NextResponse.json({ data: null, error: null }); + }, + ); + return (method = "GET") => + handler(new NextRequest("http://localhost/api/test", { method })); +} + +/** + * Resolve inside a route and hand back what the handler body would have seen. + * Every substitution assertion runs through here rather than calling the + * resolver bare, because the HTTP method is part of the decision and a bare + * call has none. + */ +async function resolveInRoute( + resolve: () => Promise, + method = "GET", +): Promise { + let value: T | undefined; + const response = await route(async () => { + value = await resolve(); + })(method); + expect(response.status).toBe(200); + return value as T; +} + +/** The wide event the last run emitted. */ +function lastEvent(): WideEvent { + const calls = vi.mocked(emitIfSampled).mock.calls; + return calls[calls.length - 1][0] as WideEvent; +} + +function sharingAuditCalls() { + return vi + .mocked(auditLog) + .mock.calls.filter(([action]) => action === "sharing.access.denied"); +} + +beforeEach(() => { + vi.clearAllMocks(); + grantRows.length = 0; + userRows.clear(); + touched.length = 0; + headerJar.clear(); + userRows.set(OWNER, user(OWNER)); + userRows.set(DELEGATE, user(DELEGATE)); +}); + +describe("do no harm — nothing is acting as anyone", () => { + it("resolves every mode to the caller, and never asks about a grant", async () => { + signedInCookie(null); + + const bare = await resolveInRoute(() => requireAuth()); + expect(bare.user.id).toBe(DELEGATE); + + const actor = await resolveInRoute(() => requireActorAuth()); + expect(actor.user.id).toBe(DELEGATE); + + const record = await resolveInRoute(() => requireRecordAuth("read")); + expect(record.user.id).toBe(DELEGATE); + expect(record.actor.id).toBe(DELEGATE); + expect(record.grantId).toBeNull(); + + // No carrier means no authorization question was ever asked. If this + // fires, the resolver is doing work on the un-switched path — which is + // where 100% of today's traffic lives. + expect(prisma.accountGrant.findFirst).not.toHaveBeenCalled(); + expect(sharingAuditCalls()).toHaveLength(0); + }); + + it("leaves the wide event's actor identity alone", async () => { + signedInCookie(null); + const call = route(() => requireRecordAuth("read")); + await call(); + + expect(lastEvent().auth?.user_id).toBe(DELEGATE); + expect(lastEvent().auth?.acting_as).toBeUndefined(); + }); +}); + +describe("bare requireAuth refuses to run under a switch", () => { + it("refuses the session carrier without falling back to the caller", async () => { + signedInCookie(OWNER); + liveGrant(); + + const response = await route(() => requireAuth())(); + expect(response.status).toBe(403); + const body = await response.json(); + expect(body.meta.errorCode).toBe("sharing.not_permitted"); + expect(body.data).toBeNull(); + }); + + it("refuses even when the grant behind the switch is perfectly valid", async () => { + // The point of the fail-closed arm: the grant is not the question. The + // route never declared that it can be used on someone else's record. + signedInCookie(OWNER); + liveGrant(); + + await expect(requireAuth()).rejects.toMatchObject({ + errorCode: "sharing.not_permitted", + statusCode: 403, + }); + }); + + it("refuses a Bearer selector, and records it", async () => { + signedInBearer(); + selector(OWNER); + liveGrant(); + + await expect(requireAuth()).rejects.toMatchObject({ + errorCode: "sharing.not_permitted", + }); + expect(sharingAuditCalls()).toHaveLength(1); + expect(sharingAuditCalls()[0][1]).toMatchObject({ + userId: DELEGATE, + details: expect.objectContaining({ + reason: "undeclared_mode", + carrier: "header", + target: OWNER, + }), + }); + }); + + it("refuses a selector sent over the cookie transport", async () => { + // The header is a Bearer-only carrier. Ignoring it here would serve the + // caller's own record to a client that believes it asked for another's. + signedInCookie(null); + selector(OWNER); + + await expect(requireAuth()).rejects.toMatchObject({ + errorCode: "sharing.not_permitted", + }); + expect(sharingAuditCalls()[0][1]).toMatchObject({ + details: expect.objectContaining({ carrier: "misplaced-header" }), + }); + }); + + it("keeps the ambient web refusal out of the audit table", async () => { + // Every navigation and poll that touches a non-delegable route lands here + // while a switch is on. A row per request would bury the header refusals, + // which are the ones that mean something. + signedInCookie(OWNER); + await expect(requireAuth()).rejects.toThrow(); + expect(sharingAuditCalls()).toHaveLength(0); + }); +}); + +describe("requireActorAuth — always the caller's own rows", () => { + it("works under a session switch and answers as the caller", async () => { + signedInCookie(OWNER); + liveGrant(); + + const ctx = await requireActorAuth(); + expect(ctx.user.id).toBe(DELEGATE); + // The switcher, the banner and the "whose records may I open" list all run + // while switched, and every one of them is about the delegate. + expect(prisma.accountGrant.findFirst).not.toHaveBeenCalled(); + }); + + it("refuses a Bearer selector loudly", async () => { + signedInBearer(); + selector(OWNER); + liveGrant(); + + await expect(requireActorAuth()).rejects.toMatchObject({ + errorCode: "sharing.not_permitted", + statusCode: 403, + }); + expect(sharingAuditCalls()[0][1]).toMatchObject({ + details: expect.objectContaining({ reason: "actor_surface" }), + }); + }); + + it("refuses a selector on the cookie transport too", async () => { + signedInCookie(null); + selector(OWNER); + await expect(requireActorAuth()).rejects.toMatchObject({ + errorCode: "sharing.not_permitted", + }); + }); +}); + +describe("requireRecordAuth — substitution", () => { + it("substitutes the owner as the data scope and keeps the delegate as actor", async () => { + signedInCookie(OWNER); + const grant = liveGrant(); + + const resolved: RecordAuthContext = await resolveInRoute(() => + requireRecordAuth("read"), + ); + expect(resolved.user.id).toBe(OWNER); + expect(resolved.actor.id).toBe(DELEGATE); + expect(resolved.grantId).toBe(grant.id); + }); + + it("substitutes over the Bearer selector as well", async () => { + signedInBearer(); + selector(OWNER); + liveGrant(); + + const resolved = await resolveInRoute(() => requireRecordAuth("read")); + expect(resolved.user.id).toBe(OWNER); + expect(resolved.actor.id).toBe(DELEGATE); + }); + + it("names the acting account on the wide event without moving the actor", async () => { + signedInCookie(OWNER); + liveGrant(); + + await route(() => requireRecordAuth("read"))(); + expect(lastEvent().auth?.user_id).toBe(DELEGATE); + expect(lastEvent().auth?.acting_as).toBe(OWNER); + }); + + it("stamps the grant as used without making the read wait on it", async () => { + signedInCookie(OWNER); + const grant = liveGrant(); + + await resolveInRoute(() => requireRecordAuth("read")); + await new Promise((r) => setTimeout(r, 0)); + expect(touched).toEqual([grant.id]); + }); +}); + +describe("requireRecordAuth — every way a grant can fail", () => { + const refused = async (): Promise => { + const response = await route(() => requireRecordAuth("read"))(); + expect(response.status).toBe(403); + expect((await response.clone().json()).meta.errorCode).toBe( + "sharing.access.denied", + ); + return response; + }; + + it("refuses a pending invitation", async () => { + signedInCookie(OWNER); + liveGrant({ acceptedAt: null }); + await refused(); + }); + + it("refuses a revoked grant", async () => { + signedInCookie(OWNER); + liveGrant({ revokedAt: new Date(Date.now() - 1000) }); + await refused(); + }); + + it("refuses an expired grant", async () => { + signedInCookie(OWNER); + liveGrant({ expiresAt: new Date(Date.now() - 1000) }); + await refused(); + }); + + it("refuses a grant made to somebody else", async () => { + signedInCookie(OWNER); + liveGrant({ granteeId: "other-delegate" }); + await refused(); + }); + + it("refuses a write method under a read grant", async () => { + signedInCookie(OWNER); + liveGrant(); + + const response = await route(() => requireRecordAuth("read"))("POST"); + expect(response.status).toBe(403); + expect((await response.json()).meta.errorCode).toBe( + "sharing.access.denied", + ); + }); + + it("refuses a declared write under a read grant", async () => { + signedInCookie(OWNER); + liveGrant(); + + const response = await route(() => requireRecordAuth("write"))(); + expect(response.status).toBe(403); + }); + + it("admits a write method once the grant says WRITE", async () => { + // v1 never writes a WRITE row, so this arm is reached by the data rather + // than by a special case — which is what makes v2 enforcement work instead + // of a rewrite. + signedInCookie(OWNER); + liveGrant({ access: "WRITE" }); + + const response = await route(() => requireRecordAuth("write"))("POST"); + expect(response.status).toBe(200); + }); + + it("refuses when the method cannot be known", async () => { + // Outside an event context there is no method to check, so we cannot prove + // the request is a read. Same fail-closed posture as the MCP audience + // guard. + signedInCookie(OWNER); + liveGrant(); + await expect(requireRecordAuth("read")).rejects.toMatchObject({ + errorCode: "sharing.access.denied", + }); + }); + + it("refuses a selector too long to name an account, without querying", async () => { + signedInBearer(); + selector("x".repeat(65)); + await expect(requireRecordAuth("read")).rejects.toMatchObject({ + errorCode: "sharing.access.denied", + }); + expect(prisma.accountGrant.findFirst).not.toHaveBeenCalled(); + }); + + it("refuses a selector sent over the cookie transport", async () => { + signedInCookie(null); + selector(OWNER); + liveGrant(); + await expect(requireRecordAuth("read")).rejects.toMatchObject({ + errorCode: "sharing.not_permitted", + }); + }); + + it("refuses when the owner row is gone under a surviving grant", async () => { + signedInCookie(OWNER); + liveGrant(); + userRows.delete(OWNER); + await refused(); + expect(sharingAuditCalls()[0][1]).toMatchObject({ + details: expect.objectContaining({ reason: "owner_missing" }), + }); + }); +}); + +describe("refusals do not enumerate", () => { + it("answers identically for an account that does not exist and one that granted nothing", async () => { + signedInBearer(); + selector("no-such-account-at-all"); + const missing = await route(() => requireRecordAuth("read"))(); + const missingBody = await missing.text(); + + vi.clearAllMocks(); + headerJar.clear(); + signedInBearer(); + // An account that exists, has a session, has data, and simply never + // granted this caller anything. + userRows.set("real-stranger", user("real-stranger")); + selector("real-stranger"); + const present = await route(() => requireRecordAuth("read"))(); + const presentBody = await present.text(); + + expect(missing.status).toBe(present.status); + // Bytes, not statuses. A difference of one word here is an account + // enumeration oracle for anyone holding a login. + expect(missingBody).toBe(presentBody); + }); + + it("answers identically for a revoked grant and a grant that never existed", async () => { + signedInBearer(); + selector(OWNER); + liveGrant({ revokedAt: new Date(Date.now() - 1000) }); + const revoked = await route(() => requireRecordAuth("read"))(); + const revokedBody = await revoked.text(); + + vi.clearAllMocks(); + headerJar.clear(); + grantRows.length = 0; + signedInBearer(); + selector(OWNER); + const never = await route(() => requireRecordAuth("read"))(); + + expect(revokedBody).toBe(await never.text()); + }); + + it("keeps the reason off the wire and on the audit row", async () => { + signedInBearer(); + selector(OWNER); + liveGrant({ expiresAt: new Date(Date.now() - 1000) }); + + const body = await (await route(() => requireRecordAuth("read"))()).text(); + expect(body).not.toContain("expire"); + expect(body).not.toContain("grant"); + expect(sharingAuditCalls()[0][1]).toMatchObject({ + details: expect.objectContaining({ reason: "no_active_grant" }), + }); + }); +}); + +describe("the cookie-only helpers stay out of reach of a switch", () => { + it("resolves requireAdmin against the actor, never the account being acted on", async () => { + signedInCookie(OWNER, "ADMIN"); + liveGrant(); + userRows.set(OWNER, user(OWNER, "USER")); + + const ctx = await requireAdmin(); + // The role came off the caller's row. Switching into somebody's record + // must confer nothing, and switching out of an admin's must take nothing + // away. + expect(ctx.user.id).toBe(DELEGATE); + }); + + it("resolves requireCookieAuth against the actor", async () => { + signedInCookie(OWNER); + liveGrant(); + const ctx = await requireCookieAuth(); + expect(ctx.user.id).toBe(DELEGATE); + }); + + it("resolves requireFreshMfa against the actor", async () => { + signedInCookie(OWNER); + liveGrant(); + vi.mocked(getSession).mockResolvedValue({ + session: { + id: "session-1", + expiresAt: new Date(Date.now() + 3_600_000), + actingAsUserId: OWNER, + }, + user: { + ...user(DELEGATE), + totpConfirmedAt: new Date(Date.now() - 86_400_000), + } as never, + }); + vi.mocked(prisma.session.findUnique).mockResolvedValue({ + mfaVerifiedAt: new Date(), + } as never); + + const ctx = await requireFreshMfa(300); + expect(ctx.user.id).toBe(DELEGATE); + }); +}); diff --git a/tests/integration/acting-account-resolver.test.ts b/tests/integration/acting-account-resolver.test.ts new file mode 100644 index 000000000..a718a7bdd --- /dev/null +++ b/tests/integration/acting-account-resolver.test.ts @@ -0,0 +1,382 @@ +/** + * The acting-account resolver against real Postgres. + * + * The unit suite proves the arms. This file proves the two things a unit suite + * cannot, because both are about which ROWS come back: + * + * * **Substitution.** Under a valid switch the handler's `where` clause is + * built from the owner's id and returns the owner's measurements — not a + * mock's idea of them, and demonstrably not the delegate's own, which are + * seeded alongside so a resolver that quietly served the caller their own + * record would show up as the wrong ids rather than as an empty list. + * * **Nothing is cached.** A revocation lands between two requests inside one + * test and the second one 403s. Anything memoised — on the session, in the + * process, on the token — would let the first request's verdict survive + * into the second, and the owner's revoke button would mean "at some point + * later" instead of "now". + * + * The do-no-harm arm runs against a REAL shipped route (`GET /api/measurements`, + * a bare-`requireAuth` handler nobody has migrated). A mock cannot fake that, + * and the arm is worth nothing unless the route it exercises is one somebody + * actually ships. + * + * The delegable handler is assembled here rather than imported because no route + * declares `requireRecordAuth` yet — S3 lands the frozen list empty on purpose, + * and S6 migrates the first route into it. Everything under it is real: the + * real `apiHandler`, the real resolver, the real grant module, the real + * database. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { NextRequest, NextResponse } from "next/server"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +import { + ACCOUNT_SELECTOR_HEADER, + apiHandler, + requireRecordAuth, +} from "@/lib/api-handler"; +import { hashToken } from "@/lib/auth/hmac"; +import { prisma } from "@/lib/db"; +import { acceptGrant, inviteGrant, revokeGrant } from "@/lib/sharing/grants"; + +let counter = 0; + +async function makeUser(label: string) { + const suffix = `${label}-${counter++}`; + return getPrismaClient().user.create({ + data: { + username: `acting-${suffix}`, + email: `acting-${suffix}@example.test`, + role: "USER", + }, + }); +} + +async function signIn(userId: string) { + const session = await getPrismaClient().session.create({ + data: { userId, expiresAt: new Date(Date.now() + 60_000) }, + }); + cookieJar.set("healthlog_session", session.id); + return session; +} + +async function mintToken(userId: string): Promise { + const raw = `hlk_${userId}-${counter++}`.padEnd(20, "0"); + await getPrismaClient().apiToken.create({ + data: { + userId, + name: "acting-account-test", + tokenHash: hashToken(raw), + permissions: ["*"], + }, + }); + return raw; +} + +async function seedWeight(userId: string, value: number) { + return getPrismaClient().measurement.create({ + data: { + userId, + type: "WEIGHT", + value, + unit: "kg", + measuredAt: new Date(), + source: "MANUAL", + }, + }); +} + +/** + * A delegable read handler: the real wrapper, the real resolver, and a `where` + * clause built from the resolved data-scope user exactly as a migrated route + * builds one. + */ +const delegableRead: (request: NextRequest) => Promise = apiHandler( + async () => { + const { user, actor, grantId } = await requireRecordAuth("read"); + const rows = await prisma.measurement.findMany({ + where: { userId: user.id }, + orderBy: { value: "asc" }, + }); + return NextResponse.json({ + data: { + scopeUserId: user.id, + actorUserId: actor.id, + grantId, + values: rows.map((r) => r.value), + }, + error: null, + }); + }, +); + +function call(method = "GET"): Promise { + return delegableRead( + new NextRequest("http://localhost/api/test/delegable", { method }), + ); +} + +/** Owner and delegate, an accepted grant, and one measurement each. */ +async function household() { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + await seedWeight(owner.id, 81); + await seedWeight(delegate.id, 64); + const invited = await inviteGrant({ + grantorId: owner.id, + granteeId: delegate.id, + }); + const grant = await acceptGrant({ + grantId: invited.id, + granteeId: delegate.id, + ip: "203.0.113.7", + }); + return { owner, delegate, grant }; +} + +/** Point the delegate's browser session at the owner's record. */ +async function switchTo(sessionId: string, ownerId: string | null) { + await getPrismaClient().session.update({ + where: { id: sessionId }, + data: { actingAsUserId: ownerId }, + }); +} + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); +}); + +describe("acting-account resolver — substitution", () => { + it("returns the OWNER's rows to a switched delegate, not the delegate's own", async () => { + const { owner, delegate, grant } = await household(); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + const body = await (await call()).json(); + expect(body.data.scopeUserId).toBe(owner.id); + expect(body.data.actorUserId).toBe(delegate.id); + expect(body.data.grantId).toBe(grant.id); + // The delegate's own 64 kg row exists and must not be in this answer. An + // empty list would have passed a weaker assertion while hiding the exact + // failure this feature has to avoid. + expect(body.data.values).toEqual([81]); + }); + + it("substitutes over the Bearer selector too", async () => { + const { owner, delegate } = await household(); + const raw = await mintToken(delegate.id); + headerJar.set("authorization", `Bearer ${raw}`); + headerJar.set(ACCOUNT_SELECTOR_HEADER, owner.id); + + const body = await (await call()).json(); + expect(body.data.scopeUserId).toBe(owner.id); + expect(body.data.values).toEqual([81]); + }); + + it("stamps the grant as used", async () => { + const { owner, delegate, grant } = await household(); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + await call(); + // Fire-and-forget, so give the stamp a tick to land before reading it. + await new Promise((resolve) => setTimeout(resolve, 50)); + const row = await getPrismaClient().accountGrant.findUniqueOrThrow({ + where: { id: grant.id }, + }); + expect(row.lastUsedAt).not.toBeNull(); + }); + + it("serves the caller their own rows when nothing is switched", async () => { + const { delegate } = await household(); + await signIn(delegate.id); + + const body = await (await call()).json(); + expect(body.data.scopeUserId).toBe(delegate.id); + expect(body.data.grantId).toBeNull(); + expect(body.data.values).toEqual([64]); + }); +}); + +describe("acting-account resolver — the decision is never cached", () => { + it("refuses the request after the one that came before it succeeded", async () => { + const { owner, delegate, grant } = await household(); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + const first = await call(); + expect(first.status).toBe(200); + expect((await first.json()).data.values).toEqual([81]); + + await revokeGrant({ grantId: grant.id, grantorId: owner.id }); + + const second = await call(); + expect(second.status).toBe(403); + expect((await second.json()).meta.errorCode).toBe("sharing.access.denied"); + }); + + it("stops on expiry with nothing having swept anything", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + await seedWeight(owner.id, 81); + const invited = await inviteGrant({ + grantorId: owner.id, + granteeId: delegate.id, + expiresAt: new Date(Date.now() + 400), + }); + await acceptGrant({ grantId: invited.id, granteeId: delegate.id }); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + expect((await call()).status).toBe(200); + await new Promise((resolve) => setTimeout(resolve, 500)); + // The row did not change. Only the clock the resolver reads moved. + expect((await call()).status).toBe(403); + }); + + it("refuses the moment the grant is only pending", async () => { + const owner = await makeUser("owner"); + const delegate = await makeUser("delegate"); + await inviteGrant({ grantorId: owner.id, granteeId: delegate.id }); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + expect((await call()).status).toBe(403); + }); +}); + +describe("acting-account resolver — do no harm", () => { + it("leaves an un-switched request on a real shipped route exactly as it was", async () => { + const user = await makeUser("solo"); + await seedWeight(user.id, 72); + await signIn(user.id); + + const { GET } = await import("@/app/api/measurements/route"); + const response = await GET( + new NextRequest("http://localhost/api/measurements?type=WEIGHT"), + ); + expect(response.status).toBe(200); + const body = await response.json(); + expect( + body.data.measurements.map((m: { value: number }) => m.value), + ).toEqual([72]); + }); + + it("refuses that same route while a switch is active, and serves nobody's rows", async () => { + const { owner, delegate } = await household(); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + const { GET } = await import("@/app/api/measurements/route"); + const response = await GET( + new NextRequest("http://localhost/api/measurements?type=WEIGHT"), + ); + expect(response.status).toBe(403); + const body = await response.json(); + expect(body.meta.errorCode).toBe("sharing.not_permitted"); + expect(body.data).toBeNull(); + // Not the owner's rows, and — the failure that matters — not the + // delegate's own either. + expect(JSON.stringify(body)).not.toContain("64"); + }); +}); + +describe("acting-account resolver — refusals do not enumerate", () => { + it("answers a nonexistent account and a stranger's account with the same bytes", async () => { + const { delegate } = await household(); + const stranger = await makeUser("stranger"); + await seedWeight(stranger.id, 99); + const raw = await mintToken(delegate.id); + headerJar.set("authorization", `Bearer ${raw}`); + + headerJar.set(ACCOUNT_SELECTOR_HEADER, "cmnosuchaccount000000000x"); + const missing = await call(); + const missingBody = await missing.text(); + + headerJar.set(ACCOUNT_SELECTOR_HEADER, stranger.id); + const present = await call(); + const presentBody = await present.text(); + + expect(missing.status).toBe(403); + expect(present.status).toBe(403); + expect(missingBody).toBe(presentBody); + }); +}); + +describe("acting-account resolver — the cookie-only helpers", () => { + it("resolves the actor under a live switch, against the real session row", async () => { + // The unit suite asserts this with `getSession` mocked, which cannot catch + // the one edit that would break it: `getSession` itself substituting the + // owner. Here the row is real, the switch is real, and the grant behind it + // is valid — so the only reason these helpers answer with the delegate is + // that `getSession` still answers "who is calling". + const { owner, delegate } = await household(); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + const { requireAdmin, requireCookieAuth } = + await import("@/lib/api-handler"); + const cookieCtx = await requireCookieAuth(); + expect(cookieCtx.user.id).toBe(delegate.id); + + // The delegate is not an admin; the refusal proves the role was read off + // the caller's row and not the record owner's. + await getPrismaClient().user.update({ + where: { id: owner.id }, + data: { role: "ADMIN" }, + }); + await expect(requireAdmin()).rejects.toMatchObject({ statusCode: 403 }); + }); +}); + +describe("acting-account resolver — the carrier column", () => { + it("puts the delegate's browser back in its own account when the owner is deleted", async () => { + const { owner, delegate } = await household(); + const session = await signIn(delegate.id); + await switchTo(session.id, owner.id); + + await getPrismaClient().user.delete({ where: { id: owner.id } }); + + const row = await getPrismaClient().session.findUnique({ + where: { id: session.id }, + }); + // The session survives (it belongs to the delegate) with the switch + // cleared. A CASCADE here would have signed the delegate out of their own + // account because somebody else deleted theirs. + expect(row).not.toBeNull(); + expect(row!.actingAsUserId).toBeNull(); + + const body = await (await call()).json(); + expect(body.data.scopeUserId).toBe(delegate.id); + }); +});