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]) 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/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/__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/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) 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); + }); +});