mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
Make the measurement-token mint cookie-only
The mint used requireAuth() with no scope, which refuses a narrow token but still admits a cookie-equivalent one. The reach is not the problem — a wildcard token can already do everything this endpoint does. The lifetimes are: a native access token lives a day, and what it mints here lives a year. So a credential that leaked for an afternoon could leave behind one that outlives revoking it, and the revocation would look like it had worked. requireCookieAuth() closes that. It is the helper the passkey registration and trusted-device routes already use for the same reason, and the sharing guard recognises it as cookie-only. The prose said more than the code did, in three places: the route header and the OpenAPI description both claimed the endpoint "cannot mint another token", which held only for narrow tokens, and the 403 arm described a scope refusal that a Bearer never reaches. All three now say what is true — no Bearer credential reaches this endpoint at any scope, and a Bearer is refused 401 before the operator switch is read. Tests follow the status change: the integration case asserting a scoped token cannot mint now expects 401 rather than 403, since the refusal is authentication and not scope, and a companion case pins the wildcard — the credential class this change is actually about — and asserts no token row is created.
This commit is contained in:
5 files changed
+57
-16
No files matched your search
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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())) {
|
||||
|
||||
@@ -1619,7 +1619,7 @@ export const authPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
"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<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
"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": {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in new issue
Block a user