diff --git a/CHANGELOG.md b/CHANGELOG.md index f829d842d..7e1a77033 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,11 @@ There is now a token for exactly that job, and for nothing else. somebody has shared with you is refused, and refused before the sharing grant is even consulted — so no future change to what sharing permits can widen what one of these tokens reaches. +- Minting one requires being signed in on the web. No API token can mint + another, whatever it is allowed to do — the sign-in a phone holds lasts a + day and what it could mint here lasts a year, so allowing it would let a + credential that leaked for an afternoon leave behind one that outlives + cancelling it. - Closed a narrower gap found while building this. A repeated request carrying an `Idempotency-Key` could be answered from the cache after a check that asked only whether a sharing grant was live, and not which diff --git a/src/app/api/tokens/measurements/__tests__/route.test.ts b/src/app/api/tokens/measurements/__tests__/route.test.ts index 2626d8050..a5bad79c9 100644 --- a/src/app/api/tokens/measurements/__tests__/route.test.ts +++ b/src/app/api/tokens/measurements/__tests__/route.test.ts @@ -13,7 +13,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/api-handler", () => ({ apiHandler: (fn: unknown) => fn, - requireAuth: vi.fn(), + requireCookieAuth: vi.fn(), })); vi.mock("@/lib/auth/issue-token", () => ({ @@ -38,7 +38,7 @@ vi.mock("@/lib/logging/context", () => ({ })); import { POST } from "../route"; -import { requireAuth } from "@/lib/api-handler"; +import { requireCookieAuth } from "@/lib/api-handler"; import { issueApiToken } from "@/lib/auth/issue-token"; import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkRateLimit } from "@/lib/rate-limit"; @@ -58,7 +58,7 @@ function req(body: unknown): Request { beforeEach(() => { vi.resetAllMocks(); - vi.mocked(requireAuth).mockResolvedValue({ user: USER } as never); + vi.mocked(requireCookieAuth).mockResolvedValue({ user: USER } as never); vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); vi.mocked(auditLog).mockResolvedValue(undefined as never); diff --git a/src/app/api/tokens/measurements/route.ts b/src/app/api/tokens/measurements/route.ts index b101c7c95..ccbe80e87 100644 --- a/src/app/api/tokens/measurements/route.ts +++ b/src/app/api/tokens/measurements/route.ts @@ -15,9 +15,8 @@ * request shape reaches it; the scope it names is accepted by exactly two * routes, both writes, both on the holder's own record. It cannot read a * reading back, cannot edit or delete one, cannot reach the export, and cannot - * mint another token — the `requireAuth()` below names no scope, so a - * measurements token presenting itself here is refused like any other narrow - * credential. That last property is why the mint is worth its own file: the + * mint another token — no Bearer credential reaches this endpoint at all, at + * any scope. That last property is why the mint is worth its own file: the * blast radius of a credential pasted into a container the user does not * operate should not include making more credentials. * @@ -31,7 +30,7 @@ */ import { NextRequest } from "next/server"; -import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiHandler, requireCookieAuth } from "@/lib/api-handler"; import { apiError, apiSuccess, @@ -64,12 +63,20 @@ const MINT_RATE_LIMIT_MAX = 10; const MINT_RATE_LIMIT_WINDOW_MS = 60 * 1000; export const POST = apiHandler(async (request: NextRequest) => { - // No scope argument, and that is load-bearing rather than incidental: it - // keeps the fail-closed default, so the only credentials that reach the mint - // are a cookie session and a cookie-equivalent token. It also refuses any - // acting-account carrier, so a delegate cannot mint a credential against the - // record they are helping with. - const { user } = await requireAuth(); + // Cookie-only, and the reason is lifetime rather than reach. + // + // `requireAuth()` would refuse a narrow token — a measurements credential + // cannot mint its own successor — but it still admits a cookie-EQUIVALENT + // one, and that is the case worth closing. A native access token lives a + // day; what it could mint here lives a year. So a credential that leaked for + // an afternoon would leave behind one that outlives revoking it, and the + // revocation would look like it had worked. + // + // A session cannot do that: it is held by a browser the person is sitting in + // front of, and it is the surface this endpoint is reached from anyway. The + // same argument the passkey-registration and trusted-device routes make, and + // `requireCookieAuth` is the helper they share. + const { user } = await requireCookieAuth(); annotate({ action: { name: "tokens.measurements.create" } }); if (!(await isApiGloballyEnabled())) { diff --git a/src/lib/openapi/routes/auth.ts b/src/lib/openapi/routes/auth.ts index efcb7a878..265690f28 100644 --- a/src/lib/openapi/routes/auth.ts +++ b/src/lib/openapi/routes/auth.ts @@ -1619,7 +1619,7 @@ export const authPaths: NonNullable = { "Mints a Bearer scoped to exactly `measurements:write` and audits the mint. THE RESPONSE CARRIES THE RAW TOKEN — the only place it ever exists; it is stored as an HMAC and no path re-reveals it.\n\n" + "**What it can do.** `POST /api/measurements` and `POST /api/measurements/batch`, on its owner's own record, attributing `source: MANUAL`. An entry naming `APPLE_HEALTH` is refused 422 rather than relabelled: that source is half a dedup key the phone also writes into, it participates in the cross-source merge, and it is what decides the Apple Health card may claim a sync happened. For the same reason a write through this credential does not move the native client's sync checkpoint.\n\n" + "**What it cannot do.** Everything else, including the measurement reads on the same paths, the edit and delete legs, and the export — a scope grants what it names and nothing adjacent. It cannot be pointed at a shared record: a request carrying the account selector is refused 403 before any grant is read, whatever grants its holder actually has. And it cannot mint another token, this endpoint included.\n\n" + - "Minting needs a cookie session or a cookie-equivalent token. Gated by the operator's instance-wide API switch. Body capped at 16 KiB; 10 mints per user per minute. Tokens appear in `GET /api/tokens` and are revoked at `DELETE /api/tokens/{id}` like any other.", + "Minting requires a COOKIE SESSION. No Bearer credential reaches this endpoint at any scope, wildcard included — not because a wildcard lacks the reach, but because of the lifetimes involved: a native access token lives a day and what it could mint here lives a year, so admitting one would let a short-lived compromise leave behind a credential that outlives revoking it. Gated by the operator's instance-wide API switch. Body capped at 16 KiB; 10 mints per user per minute. Tokens appear in `GET /api/tokens` and are revoked at `DELETE /api/tokens/{id}` like any other.", requestBody: { required: true, content: { @@ -1645,7 +1645,7 @@ export const authPaths: NonNullable = { }, "403": { description: - "The operator has switched the API off instance-wide (`AppSettings.apiGlobal`), or the caller presented a narrow-scope token rather than a session — a scoped credential cannot mint credentials. Nothing was minted either way.", + "The operator has switched the API off instance-wide (`AppSettings.apiGlobal`). Nothing was minted. A Bearer caller does not reach this arm — it is refused 401 before the switch is read.", content: { "application/json": { schema: errorEnvelope } }, }, "413": { diff --git a/tests/integration/bearer-scope-enforcement.test.ts b/tests/integration/bearer-scope-enforcement.test.ts index 37acbdc27..3677027f0 100644 --- a/tests/integration/bearer-scope-enforcement.test.ts +++ b/tests/integration/bearer-scope-enforcement.test.ts @@ -626,6 +626,10 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe }); it("cannot mint another token", async () => { + // 401, not 403, and the difference is the point: the mint is cookie-only, + // so a Bearer is turned away as unauthenticated before any scope is read. + // The refusal is therefore not "this token's scope is too narrow" — no + // token is wide enough, a wildcard included. await armToken(["measurements:write"], "mwrite7"); const { POST } = await import("@/app/api/tokens/measurements/route"); const res = await POST( @@ -638,7 +642,32 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe body: JSON.stringify({ name: "second" }), } as never), ); - expect(res.status).toBe(403); + expect(res.status).toBe(401); + }); + + it("and neither can a wildcard token", async () => { + // The case the cookie-only resolver exists for. A native access token + // lives a day; what this endpoint mints lives a year, so admitting one + // would let a short-lived compromise leave behind a credential that + // survives revoking it. + await armToken(["*"], "wildmint"); + const { POST } = await import("@/app/api/tokens/measurements/route"); + const res = await POST( + new NextRequest("https://health.example/api/tokens/measurements", { + method: "POST", + headers: { + authorization: headerJar.get("authorization")!, + "content-type": "application/json", + }, + body: JSON.stringify({ name: "minted by a wildcard" }), + } as never), + ); + expect(res.status).toBe(401); + expect( + await getPrismaClient().apiToken.count({ + where: { permissions: { has: "measurements:write" } }, + }), + ).toBe(0); }); it("cannot reach the medication ingest surface", async () => {