mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
Merge pull request #707 from MBombeck/work/dw-a
Scope the idempotency replay cell to the record a request claims
This commit is contained in:
8 files changed
+690
-90
No files matched your search
@@ -74,6 +74,9 @@ describe("the acting-account carrier is read in one place", () => {
|
||||
const CARRIER_ALLOWLIST = [
|
||||
// Projects it off the session row it already loaded.
|
||||
"lib/auth/session.ts",
|
||||
// Reads it. The one statement of "what does this request say it is acting
|
||||
// as", for both transports and for every caller that has to ask.
|
||||
"lib/auth/acting-carrier.ts",
|
||||
// The resolver. The only thing that acts on the value.
|
||||
"lib/api-handler.ts",
|
||||
// v1.36.0 — the only thing that WRITES it: the switch endpoint sets it
|
||||
@@ -113,10 +116,32 @@ describe("the acting-account carrier is read in one place", () => {
|
||||
const namers = filesMatching(/x-healthlog-account/i);
|
||||
expect(namers.length).toBeGreaterThan(0);
|
||||
expect(namers).toEqual([
|
||||
"lib/api-handler.ts",
|
||||
"lib/auth/acting-carrier.ts",
|
||||
"lib/openapi/routes/account-sharing.ts",
|
||||
]);
|
||||
});
|
||||
|
||||
/**
|
||||
* Who may ask "which record does this request claim" without an auth
|
||||
* context in hand.
|
||||
*
|
||||
* `readClaimedActingAccount()` answers before any grant check has run, so a
|
||||
* caller that treats its answer as permission has skipped the only thing
|
||||
* that grants any. Exactly one caller exists, beside the file that declares
|
||||
* it: the idempotency wrapper, which uses the claim to pick a cache cell and
|
||||
* leaves the check to the handler it wraps. Everything else that needs the
|
||||
* acting account has an auth context by then and goes through
|
||||
* `requireRecordAuth`, which checks a live grant before returning. A second
|
||||
* caller here is a design question, not a patch.
|
||||
*/
|
||||
it("the unchecked claim has exactly two consumers", () => {
|
||||
const consumers = filesMatching(/readClaimedActingAccount/);
|
||||
expect(consumers.length).toBeGreaterThan(0);
|
||||
expect(consumers).toEqual([
|
||||
"lib/auth/acting-carrier.ts",
|
||||
"lib/idempotency.ts",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the cookie-only helpers cannot see the acting account", () => {
|
||||
|
||||
@@ -65,9 +65,13 @@ describe("T1 — the Bearer resolution set is frozen", () => {
|
||||
// `defaultUserIdResolver` — resolves the Bearer token to a user id so an
|
||||
// idempotency key files under its owner. It authorises nothing: the
|
||||
// handler still runs its own `requireAuth()`, and this resolver only
|
||||
// decides which user's cache cell a replay lands in. Narrow in reach and
|
||||
// deliberate, but it is a raw Bearer value becoming a user id, so it
|
||||
// belongs on this list rather than beside it.
|
||||
// decides which cache cell a replay lands in. Since delegated writes it
|
||||
// decides that from two values — the caller resolved here, and the record
|
||||
// the request claims, read through the one acting-account carrier reader
|
||||
// and folded into the key. Both are unchecked selectors; neither widens
|
||||
// what the handler will do. Narrow in reach and deliberate, but it is a
|
||||
// raw Bearer value becoming a user id, so it belongs on this list rather
|
||||
// than beside it.
|
||||
"lib/idempotency.ts",
|
||||
].sort();
|
||||
|
||||
|
||||
@@ -75,7 +75,16 @@ function request(): NextRequest {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
// The whole shape `getSession()` returns. The session half carries the
|
||||
// acting-account carrier, which the idempotency wrapper reads to decide
|
||||
// whose record a cell belongs to — a fake that omitted it made the wrapper
|
||||
// fail on a field production always has.
|
||||
mocks.getSession.mockResolvedValue({
|
||||
session: {
|
||||
id: "session-1",
|
||||
expiresAt: new Date(Date.now() + 60_000),
|
||||
actingAsUserId: null,
|
||||
},
|
||||
user: { id: "user-1", role: "USER" },
|
||||
});
|
||||
mocks.findUnique.mockResolvedValue(null);
|
||||
|
||||
@@ -219,7 +219,11 @@ describe("withIdempotency", () => {
|
||||
);
|
||||
const wrapped = withIdempotency<[NextRequest]>(handler);
|
||||
await wrapped(makeRequest("POST", { "idempotency-key": "abc-12345678" }));
|
||||
expect(getSession).toHaveBeenCalledTimes(1);
|
||||
// Twice: once to resolve who is calling, once to read which record they
|
||||
// say they are acting on. The second read is what a delegated write costs
|
||||
// here, and it is a single indexed lookup on a request that is about to
|
||||
// write anyway.
|
||||
expect(getSession).toHaveBeenCalledTimes(2);
|
||||
expect(prisma.idempotencyKey.create).toHaveBeenCalledTimes(1);
|
||||
const persisted = vi.mocked(prisma.idempotencyKey.create).mock
|
||||
.calls[0][0] as { data: { userId: string } };
|
||||
@@ -536,3 +540,136 @@ describe("isCachableStatus do-not-cache rules (V3 audit)", () => {
|
||||
expect(stored.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The cell is scoped to the record the request claims, not only to the caller.
|
||||
*
|
||||
* `tests/integration/idempotency-record-scoped-cell.test.ts` proves the rows.
|
||||
* These prove the key the wrapper composes, which is what decides whether two
|
||||
* requests meet in one cell — including the two cases a row-level test cannot
|
||||
* show, because both end in no row at all.
|
||||
*/
|
||||
describe("withIdempotency — record-scoped cells", () => {
|
||||
const CLIENT_KEY = "abc-12345678";
|
||||
|
||||
/** Present these headers to the wrapper for the next call. */
|
||||
function present(values: Record<string, string>): void {
|
||||
vi.mocked(headers).mockResolvedValue({
|
||||
get: (name: string) => values[name.toLowerCase()] ?? null,
|
||||
} as unknown as ReturnType<typeof headers> extends Promise<infer T>
|
||||
? T
|
||||
: never);
|
||||
}
|
||||
|
||||
/** Sign the caller in over the cookie transport, optionally switched. */
|
||||
function cookieSession(actingAsUserId: string | null): void {
|
||||
vi.mocked(getSession).mockResolvedValue({
|
||||
session: {
|
||||
id: "s-1",
|
||||
expiresAt: new Date(Date.now() + 60_000),
|
||||
actingAsUserId,
|
||||
},
|
||||
user: { id: "u-1", role: "USER" },
|
||||
} as Awaited<ReturnType<typeof getSession>>);
|
||||
}
|
||||
|
||||
async function run(key = CLIENT_KEY): Promise<void> {
|
||||
const handler = vi.fn(async () =>
|
||||
NextResponse.json({ data: { ok: true }, error: null }, { status: 201 }),
|
||||
);
|
||||
const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1");
|
||||
await wrapped(makeRequest("POST", { "idempotency-key": key }));
|
||||
}
|
||||
|
||||
/** The key the wrapper looked the cell up under. */
|
||||
function lookedUpKey(): string {
|
||||
const call = vi.mocked(prisma.idempotencyKey.findUnique).mock
|
||||
.calls[0][0] as {
|
||||
where: { userId_key_method_path: { userId: string; key: string } };
|
||||
};
|
||||
return call.where.userId_key_method_path.key;
|
||||
}
|
||||
|
||||
/** The row the wrapper claimed. */
|
||||
function claimed(): { userId: string; key: string } {
|
||||
return (
|
||||
vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as {
|
||||
data: { userId: string; key: string };
|
||||
}
|
||||
).data;
|
||||
}
|
||||
|
||||
it("folds the account named by the Bearer selector into the key", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
present({ "x-healthlog-account": "owner-9" });
|
||||
|
||||
await run();
|
||||
|
||||
expect(lookedUpKey()).toBe(`owner-9|${CLIENT_KEY}`);
|
||||
// The owner column stays the ACTOR. It is a foreign key to `users` with a
|
||||
// cascade behind it, so it can only ever hold a real account id — which is
|
||||
// why the record moved into the key and not into here.
|
||||
expect(claimed()).toMatchObject({
|
||||
userId: "u-1",
|
||||
key: `owner-9|${CLIENT_KEY}`,
|
||||
});
|
||||
});
|
||||
|
||||
it("folds the account the cookie session is switched to", async () => {
|
||||
cookieSession("owner-7");
|
||||
present({});
|
||||
|
||||
await run();
|
||||
|
||||
expect(claimed().key).toBe(`owner-7|${CLIENT_KEY}`);
|
||||
});
|
||||
|
||||
it("keys a request with no acting account byte-for-byte as the client sent it", async () => {
|
||||
cookieSession(null);
|
||||
present({});
|
||||
|
||||
await run();
|
||||
|
||||
expect(lookedUpKey()).toBe(CLIENT_KEY);
|
||||
expect(claimed()).toMatchObject({ userId: "u-1", key: CLIENT_KEY });
|
||||
});
|
||||
|
||||
it("ignores a selector sent over the cookie transport", async () => {
|
||||
// The resolver refuses that request outright (`misplaced_selector`). The
|
||||
// cell must not pretend it named a record either — a claim the request was
|
||||
// never going to be served must not move where its answer is filed.
|
||||
cookieSession(null);
|
||||
present({ "x-healthlog-account": "owner-9" });
|
||||
|
||||
await run();
|
||||
|
||||
expect(claimed().key).toBe(CLIENT_KEY);
|
||||
});
|
||||
|
||||
it("refuses a client key that carries the separator", async () => {
|
||||
// The one way a caller could aim at somebody else's cell: send
|
||||
// `owner-9|abc-12345678` as the key from their own un-switched session. The
|
||||
// key validator does not admit the byte, so the header is malformed and the
|
||||
// wrapper does not cache at all.
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
present({});
|
||||
|
||||
await run(`owner-9|${CLIENT_KEY}`);
|
||||
|
||||
expect(prisma.idempotencyKey.findUnique).not.toHaveBeenCalled();
|
||||
expect(prisma.idempotencyKey.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips the cache when the claimed account names nothing that could exist", async () => {
|
||||
// Longer than any account id. The request is refused downstream; caching it
|
||||
// would let a caller write an arbitrarily long key into the table on the
|
||||
// way to a 403.
|
||||
vi.mocked(getSession).mockResolvedValue(null);
|
||||
present({ "x-healthlog-account": "x".repeat(65) });
|
||||
|
||||
await run();
|
||||
|
||||
expect(prisma.idempotencyKey.findUnique).not.toHaveBeenCalled();
|
||||
expect(prisma.idempotencyKey.create).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
+24
-84
@@ -19,6 +19,13 @@ import {
|
||||
STEP_UP_ELEVATION_TTL_SECONDS,
|
||||
} from "./auth/step-up";
|
||||
import { hashToken } from "./auth/hmac";
|
||||
import {
|
||||
carrierTarget,
|
||||
decideActingCarrier,
|
||||
readSelectorHeader,
|
||||
selectorNamesAnAccount,
|
||||
type ActingCarrier,
|
||||
} from "./auth/acting-carrier";
|
||||
import { AssistantDisabledError } from "./feature-flags";
|
||||
import { ConsentRequiredError } from "./ai/consent-guard";
|
||||
import { SCOPE_HEALTH_READ, SCOPE_HEALTH_WRITE } from "./mcp/oauth/config";
|
||||
@@ -597,69 +604,23 @@ async function authenticateBearer(
|
||||
//
|
||||
// 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.
|
||||
// one file for two wires. Two carriers converge on one decision 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.
|
||||
// That decision now lives in `./auth/acting-carrier`, because a second caller
|
||||
// needs it: the idempotency wrapper files its replay cell under the record a
|
||||
// request claims, and it runs before any auth context exists. What stays here
|
||||
// is what only this file may do — turning a claim into a data scope after a
|
||||
// live grant says so.
|
||||
|
||||
/**
|
||||
* The per-request account selector, on the Bearer transport only.
|
||||
* The selector header's name, re-exported.
|
||||
*
|
||||
* 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.
|
||||
* It is declared beside the carrier reader it belongs to; every route-facing
|
||||
* helper is imported from this module, and the OpenAPI table and the suites
|
||||
* that drive a switched request were written against that path.
|
||||
*/
|
||||
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<string | null> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
export { ACCOUNT_SELECTOR_HEADER } from "./auth/acting-carrier";
|
||||
|
||||
/**
|
||||
* The acting-account carrier for this request.
|
||||
@@ -677,32 +638,11 @@ async function readSelectorHeader(): Promise<string | null> {
|
||||
* next request rather than from the next login.
|
||||
*/
|
||||
async function readActingCarrier(auth: AuthContext): Promise<ActingCarrier> {
|
||||
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;
|
||||
return decideActingCarrier({
|
||||
transport: auth.authMethod === "bearer" ? "bearer" : "cookie",
|
||||
stamped: auth.session.actingAsUserId ?? null,
|
||||
header: await readSelectorHeader(),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/**
|
||||
* What a request says it is acting as.
|
||||
*
|
||||
* Two things in the app have to answer that question, and they have to answer
|
||||
* it the same way:
|
||||
*
|
||||
* * `requireRecordAuth` (`src/lib/api-handler.ts`), which turns the claim
|
||||
* into a data scope after checking a live grant.
|
||||
* * `withIdempotency` (`src/lib/idempotency.ts`), which files the replay cell
|
||||
* under the record the request claims — before any handler runs, so it
|
||||
* cannot wait for the grant check the handler performs.
|
||||
*
|
||||
* The claim is a SELECTOR and never an authorisation. Nothing in this file
|
||||
* looks at a grant, and no caller may treat a carrier as permission: the
|
||||
* account named here is an id to be checked. What it decides is only which
|
||||
* account is being named, and by which transport — the question `grants.ts`
|
||||
* warns must have exactly one answer, because a second implementation of it
|
||||
* drifts and the copy that drifts is the one nobody reads.
|
||||
*
|
||||
* The transport rules, unchanged from where they were first written:
|
||||
*
|
||||
* * The cookie transport carries the claim on the session ROW. A browser
|
||||
* cannot address that row, so the value is server state written only by
|
||||
* the switch endpoint after it validated a grant.
|
||||
* * The Bearer transport carries it in a per-request HEADER. A long-lived
|
||||
* token must not accumulate switch state.
|
||||
* * The header on a cookie request is `misplaced`, not ignored — a client
|
||||
* that believes it is selecting an account and is not produces a support
|
||||
* case about data that looks wrong.
|
||||
*/
|
||||
import { headers } from "next/headers";
|
||||
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
|
||||
/**
|
||||
* 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 it is read.
|
||||
*/
|
||||
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.
|
||||
*/
|
||||
export 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 caller decides for itself — the callers disagree about what to
|
||||
* do with a misplaced claim and must not disagree about detecting one.
|
||||
*/
|
||||
export 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. */
|
||||
export async function readSelectorHeader(): Promise<string | null> {
|
||||
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 carrier decision itself, as a pure function of what the transport
|
||||
* carried. Pure on purpose: it is the one statement of the rule, and a rule
|
||||
* that needs a request context to be read is a rule that gets re-implemented.
|
||||
*/
|
||||
export function decideActingCarrier(input: {
|
||||
transport: "cookie" | "bearer";
|
||||
/** The session row's stamp. Ignored on the Bearer transport, which has none. */
|
||||
stamped: string | null;
|
||||
header: string | null;
|
||||
}): ActingCarrier {
|
||||
if (input.transport === "bearer") {
|
||||
// No session row exists on this transport, so the header is the only
|
||||
// carrier there could be.
|
||||
return input.header === null
|
||||
? { kind: "none" }
|
||||
: { kind: "header", accountId: input.header };
|
||||
}
|
||||
|
||||
if (input.header !== null) return { kind: "misplaced-header" };
|
||||
|
||||
return input.stamped === null
|
||||
? { kind: "none" }
|
||||
: { kind: "session", accountId: input.stamped };
|
||||
}
|
||||
|
||||
/** The account a carrier names, or null when it names nothing. */
|
||||
export function carrierTarget(carrier: ActingCarrier): string | null {
|
||||
return "accountId" in carrier ? carrier.accountId : null;
|
||||
}
|
||||
|
||||
/** Could this value name an account at all? */
|
||||
export function selectorNamesAnAccount(value: string): boolean {
|
||||
return value.length > 0 && value.length <= MAX_SELECTOR_LENGTH;
|
||||
}
|
||||
|
||||
/**
|
||||
* The account this request CLAIMS, resolved without an auth context in hand.
|
||||
*
|
||||
* For callers that run BEFORE authentication has been resolved into a context —
|
||||
* today that is the idempotency wrapper, which has to know which record a
|
||||
* request is aimed at before it decides whether the answer is already cached.
|
||||
* It resolves the session itself for exactly one reason: the transport decides
|
||||
* which carrier counts, and "there is a valid session" is what the transport
|
||||
* is. Guessing it from the presence of a cookie or an `Authorization` header
|
||||
* would be a second, weaker answer to a question this file exists to answer
|
||||
* once — and the case it gets wrong (a stale session cookie beside a live
|
||||
* Bearer token) is precisely a delegated write landing in the wrong cell.
|
||||
*
|
||||
* The cost is one indexed session lookup on a request that is already about to
|
||||
* write. It is not shared with the caller's own auth resolution because the
|
||||
* resolver a route passes to `withIdempotency` is pluggable, and threading a
|
||||
* session through that seam would make every custom resolver responsible for a
|
||||
* question none of them asks.
|
||||
*
|
||||
* Returns the CLAIMED account, unverified and unchecked. The grant check
|
||||
* happens later, inside the handler, on every request.
|
||||
*/
|
||||
export async function readClaimedActingAccount(): Promise<string | null> {
|
||||
const header = await readSelectorHeader();
|
||||
let session: Awaited<ReturnType<typeof getSession>> = null;
|
||||
try {
|
||||
session = await getSession();
|
||||
} catch {
|
||||
// A session that cannot be resolved is not a switched one. The request
|
||||
// still has to be authenticated by whatever wraps this call, and an
|
||||
// unauthenticated request claims nothing.
|
||||
session = null;
|
||||
}
|
||||
return carrierTarget(
|
||||
decideActingCarrier({
|
||||
transport: session ? "cookie" : "bearer",
|
||||
stamped: session?.session.actingAsUserId ?? null,
|
||||
header,
|
||||
}),
|
||||
);
|
||||
}
|
||||
+65
-1
@@ -6,12 +6,21 @@
|
||||
* retry within the TTL (24h) for the same `(userId, key, method, path)`
|
||||
* tuple returns the cached envelope as a 200 (carrying the original
|
||||
* status inside the JSON if needed) — no second side-effect.
|
||||
*
|
||||
* `userId` is the CALLER. Since a caller can act on somebody else's record,
|
||||
* the record the request claims is folded into `key` (see `cellKey`), so the
|
||||
* tuple identifies a request to one person's record rather than a request by
|
||||
* one person. A caller acting only as themselves is unaffected, byte for byte.
|
||||
*/
|
||||
import type { NextRequest } from "next/server";
|
||||
import { headers } from "next/headers";
|
||||
import { NextResponse } from "next/server";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import {
|
||||
readClaimedActingAccount,
|
||||
selectorNamesAnAccount,
|
||||
} from "@/lib/auth/acting-carrier";
|
||||
import { hashToken } from "@/lib/auth/hmac";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { isP2002 } from "@/lib/prisma-errors";
|
||||
@@ -37,6 +46,34 @@ const PENDING_STATUS = 0;
|
||||
|
||||
const KEY_REGEX = /^[A-Za-z0-9_\-:.]{8,128}$/;
|
||||
|
||||
/**
|
||||
* Separates the record from the client's key inside a cell's `key` column.
|
||||
*
|
||||
* `KEY_REGEX` above does not admit it, which is the whole reason it was
|
||||
* chosen: a caller cannot hand-craft a key that lands in the cell of a request
|
||||
* they are not making. Changing either one without the other reopens that.
|
||||
*/
|
||||
const RECORD_SEPARATOR = "|";
|
||||
|
||||
/**
|
||||
* The `key` column for a request, given the record it claims to act on.
|
||||
*
|
||||
* Why the key and not the owner column: `IdempotencyKey.userId` is a foreign
|
||||
* key to `users` with a cascade behind it (`prisma/schema.prisma`), so it can
|
||||
* hold one real account id and nothing composite. It keeps holding the ACTOR —
|
||||
* which is what makes "two delegates, one owner, one key" two cells without
|
||||
* anything extra — and the record joins the other half of the composite unique
|
||||
* index instead.
|
||||
*
|
||||
* No carrier means the key the client sent, byte for byte. Every request that
|
||||
* is nobody's delegate therefore files exactly where it filed before, and the
|
||||
* fold cannot change the behaviour of a client that never heard of sharing.
|
||||
*/
|
||||
function cellKey(clientKey: string, actingAccountId: string | null): string {
|
||||
if (actingAccountId === null) return clientKey;
|
||||
return `${actingAccountId}${RECORD_SEPARATOR}${clientKey}`;
|
||||
}
|
||||
|
||||
export interface IdempotencyContext {
|
||||
userId: string;
|
||||
key: string;
|
||||
@@ -288,6 +325,11 @@ export function isCachableStatus(status: number): boolean {
|
||||
* /external clients — without it, every Bearer-authed retry was running
|
||||
* the handler again and creating duplicate measurements (audit C-4).
|
||||
*
|
||||
* It resolves the CALLER and only the caller — which record they are aiming at
|
||||
* is a separate question, answered by `readClaimedActingAccount()` above and
|
||||
* folded into the key. Neither answer authorises anything: the handler runs
|
||||
* its own `requireAuth()` / `requireRecordAuth()` either way.
|
||||
*
|
||||
* Exported for unit testing; production callers should let
|
||||
* `withIdempotency()` pick this up automatically via its default arg.
|
||||
*/
|
||||
@@ -374,10 +416,32 @@ export function withIdempotency<
|
||||
const userId = await userIdResolver(...args);
|
||||
if (!userId) return handler(...args);
|
||||
|
||||
// Which RECORD this request is aimed at, as claimed by the transport and
|
||||
// not yet checked against a grant — the check belongs to the handler, and
|
||||
// this runs before it. One delegate posting the same key first for one
|
||||
// person and then for another must not meet themselves in a single cell:
|
||||
// that replays the first person's response, writes nothing to the second
|
||||
// person's record, and reports success. Nothing errors and nothing logs a
|
||||
// conflict, which is why it is folded here rather than left to call sites.
|
||||
//
|
||||
// The consequence of using the CLAIM: inside the 24h window a delegate
|
||||
// whose grant has since ended can still replay a key they already
|
||||
// completed, without the grant check running. They receive bytes they
|
||||
// already had and nothing new is written — a fresh key from them is
|
||||
// refused, because that request reaches the handler.
|
||||
const claimedRecord = await readClaimedActingAccount();
|
||||
if (claimedRecord !== null && !selectorNamesAnAccount(claimedRecord)) {
|
||||
// A claim that names no account that could exist. The request is refused
|
||||
// downstream; skipping the cache keeps a caller from writing an
|
||||
// arbitrarily long key into the table on the way to that refusal, and
|
||||
// never lets one fall back onto the un-delegated cell.
|
||||
return handler(...args);
|
||||
}
|
||||
|
||||
const url = new URL(request.url);
|
||||
const ctx: IdempotencyContext = {
|
||||
userId,
|
||||
key,
|
||||
key: cellKey(key, claimedRecord),
|
||||
method: request.method,
|
||||
path: url.pathname,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,267 @@
|
||||
/**
|
||||
* The replay cache is scoped to the RECORD, not only to the caller.
|
||||
*
|
||||
* One delegate, two people's records, one `Idempotency-Key`. Before this was
|
||||
* fixed the cell was `(caller, key, method, path)`, so the second record's
|
||||
* request replayed the first record's cached 201: no handler, no row, and a
|
||||
* success response carrying the other person's id. Nothing errored and nothing
|
||||
* logged a conflict, which is why it needed a test rather than a bug report.
|
||||
*
|
||||
* Everything under the test is real — the shipped wrapper, the shipped
|
||||
* resolver, the shipped grant module's tables, the composite unique index and
|
||||
* the `user_id` foreign key. The handler is assembled here rather than imported
|
||||
* because what is under test is the cell, not any one domain's write path; a
|
||||
* shipped route would bind this file to that route's schema and its migration
|
||||
* to a delegable mode.
|
||||
*/
|
||||
import { NextRequest, NextResponse } from "next/server";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
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 { withIdempotency } from "@/lib/idempotency";
|
||||
|
||||
const KEY = "retry-0f3a1c9d";
|
||||
|
||||
let counter = 0;
|
||||
|
||||
async function makeUser(label: string) {
|
||||
const suffix = `${label}-${counter++}`;
|
||||
return getPrismaClient().user.create({
|
||||
data: {
|
||||
username: `cell-${suffix}`,
|
||||
email: `cell-${suffix}@example.test`,
|
||||
role: "USER",
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async function mintToken(userId: string): Promise<string> {
|
||||
const raw = `hlk_${userId}-${counter++}`.padEnd(20, "0");
|
||||
await getPrismaClient().apiToken.create({
|
||||
data: {
|
||||
userId,
|
||||
name: "idempotency-cell-test",
|
||||
tokenHash: hashToken(raw),
|
||||
permissions: ["*"],
|
||||
},
|
||||
});
|
||||
return raw;
|
||||
}
|
||||
|
||||
/**
|
||||
* An accepted WRITE grant, written directly.
|
||||
*
|
||||
* `inviteGrant()` mints READ today; the level is Chunk B's to open. What this
|
||||
* file needs is a row in the state the resolver reads, and that state is the
|
||||
* two columns below.
|
||||
*/
|
||||
async function writeGrant(grantorId: string, granteeId: string) {
|
||||
return getPrismaClient().accountGrant.create({
|
||||
data: {
|
||||
grantorId,
|
||||
granteeId,
|
||||
access: "WRITE",
|
||||
acceptedAt: new Date(Date.now() - 60_000),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
let handlerRuns = 0;
|
||||
|
||||
/**
|
||||
* A delegated write, wrapped exactly as the shipped routes wrap one:
|
||||
* `apiHandler(withIdempotency(handler))`.
|
||||
*/
|
||||
const delegableWrite: (request: NextRequest) => Promise<Response> = apiHandler(
|
||||
withIdempotency<[NextRequest]>(async () => {
|
||||
handlerRuns += 1;
|
||||
const { user } = await requireRecordAuth("write");
|
||||
const row = await prisma.measurement.create({
|
||||
data: {
|
||||
userId: user.id,
|
||||
type: "WEIGHT",
|
||||
value: 80 + handlerRuns,
|
||||
unit: "kg",
|
||||
measuredAt: new Date(),
|
||||
source: "MANUAL",
|
||||
},
|
||||
});
|
||||
return NextResponse.json(
|
||||
{ data: { id: row.id, userId: row.userId }, error: null },
|
||||
{ status: 201 },
|
||||
);
|
||||
}),
|
||||
);
|
||||
|
||||
function post(key: string | null = KEY): Promise<Response> {
|
||||
const headers: Record<string, string> = {
|
||||
"content-type": "application/json",
|
||||
};
|
||||
if (key) headers["idempotency-key"] = key;
|
||||
return delegableWrite(
|
||||
new NextRequest("http://localhost/api/test/delegable-write", {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: JSON.stringify({ ok: true }),
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
/** Point the next request at a record. */
|
||||
function actAs(ownerId: string | null): void {
|
||||
if (ownerId === null) headerJar.delete(ACCOUNT_SELECTOR_HEADER);
|
||||
else headerJar.set(ACCOUNT_SELECTOR_HEADER, ownerId);
|
||||
}
|
||||
|
||||
async function measurementsOf(userId: string) {
|
||||
return getPrismaClient().measurement.findMany({ where: { userId } });
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
await truncateAllTables(getPrismaClient());
|
||||
cookieJar.clear();
|
||||
headerJar.clear();
|
||||
handlerRuns = 0;
|
||||
});
|
||||
|
||||
describe("the replay cell is per record", () => {
|
||||
it("writes to BOTH records when one key is reused across two owners", async () => {
|
||||
const delegate = await makeUser("delegate");
|
||||
const ownerA = await makeUser("owner-a");
|
||||
const ownerB = await makeUser("owner-b");
|
||||
await writeGrant(ownerA.id, delegate.id);
|
||||
await writeGrant(ownerB.id, delegate.id);
|
||||
headerJar.set("authorization", `Bearer ${await mintToken(delegate.id)}`);
|
||||
|
||||
actAs(ownerA.id);
|
||||
const first = await post();
|
||||
expect(first.status).toBe(201);
|
||||
expect((await first.json()).data.userId).toBe(ownerA.id);
|
||||
|
||||
// Same caller, same key, same path — a different person's record. The
|
||||
// handler must run again, because nothing about this request was answered
|
||||
// by the previous one.
|
||||
actAs(ownerB.id);
|
||||
const second = await post();
|
||||
expect(second.status).toBe(201);
|
||||
const secondBody = await second.json();
|
||||
|
||||
// Asserted first because it is the damage: without a per-record cell this
|
||||
// is an empty list, and the caller was told 201.
|
||||
expect(await measurementsOf(ownerB.id)).toHaveLength(1);
|
||||
expect(secondBody.data.userId).toBe(ownerB.id);
|
||||
expect(second.headers.get("X-Idempotent-Replay")).not.toBe("true");
|
||||
expect(handlerRuns).toBe(2);
|
||||
expect(await measurementsOf(ownerA.id)).toHaveLength(1);
|
||||
// Two cells under the one caller: the actor's id still owns both rows, so
|
||||
// the foreign key and its cascade are untouched by the fold.
|
||||
const cells = await getPrismaClient().idempotencyKey.findMany({
|
||||
where: { userId: delegate.id },
|
||||
});
|
||||
expect(cells).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("still replays a genuine retry on the SAME record", async () => {
|
||||
const delegate = await makeUser("delegate");
|
||||
const owner = await makeUser("owner");
|
||||
await writeGrant(owner.id, delegate.id);
|
||||
headerJar.set("authorization", `Bearer ${await mintToken(delegate.id)}`);
|
||||
actAs(owner.id);
|
||||
|
||||
const first = await post();
|
||||
const firstBody = await first.json();
|
||||
const replay = await post();
|
||||
|
||||
expect(replay.status).toBe(201);
|
||||
expect(replay.headers.get("X-Idempotent-Replay")).toBe("true");
|
||||
expect(await replay.json()).toEqual(firstBody);
|
||||
expect(handlerRuns).toBe(1);
|
||||
expect(await measurementsOf(owner.id)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("keys a request with no acting account exactly as the client sent it", async () => {
|
||||
const caller = await makeUser("self");
|
||||
headerJar.set("authorization", `Bearer ${await mintToken(caller.id)}`);
|
||||
|
||||
const response = await post();
|
||||
expect(response.status).toBe(201);
|
||||
|
||||
// The self path is the pre-existing path and must stay byte-identical:
|
||||
// same owner column, same key column, no separator, no prefix.
|
||||
const cells = await getPrismaClient().idempotencyKey.findMany();
|
||||
expect(cells).toHaveLength(1);
|
||||
expect(cells[0]).toMatchObject({ userId: caller.id, key: KEY });
|
||||
expect(await measurementsOf(caller.id)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("replays inside the TTL after the grant is revoked, and writes nothing", async () => {
|
||||
// The accepted consequence of folding the CLAIMED account rather than a
|
||||
// verified one: within the 24h window a revoked delegate's retry is
|
||||
// answered from the cache without the grant check running again. Pinned
|
||||
// here so it stays a decision. What matters is the second half — the
|
||||
// response is one they already received, and no new row appears.
|
||||
const delegate = await makeUser("delegate");
|
||||
const owner = await makeUser("owner");
|
||||
const grant = await writeGrant(owner.id, delegate.id);
|
||||
headerJar.set("authorization", `Bearer ${await mintToken(delegate.id)}`);
|
||||
actAs(owner.id);
|
||||
|
||||
const first = await post();
|
||||
const firstBody = await first.json();
|
||||
expect(first.status).toBe(201);
|
||||
|
||||
// The owner withdraws access. Both revocation columns move together —
|
||||
// the database refuses one without the other.
|
||||
await getPrismaClient().accountGrant.update({
|
||||
where: { id: grant.id },
|
||||
data: { revokedAt: new Date(), revokedBy: "GRANTOR" },
|
||||
});
|
||||
|
||||
const afterRevoke = await post();
|
||||
expect(afterRevoke.status).toBe(201);
|
||||
expect(afterRevoke.headers.get("X-Idempotent-Replay")).toBe("true");
|
||||
expect(await afterRevoke.json()).toEqual(firstBody);
|
||||
expect(handlerRuns).toBe(1);
|
||||
expect(await measurementsOf(owner.id)).toHaveLength(1);
|
||||
|
||||
// A request the cache does NOT answer is refused, which is what keeps the
|
||||
// window above bounded to keys the delegate had already completed.
|
||||
const fresh = await post("retry-11119999");
|
||||
expect(fresh.status).toBe(403);
|
||||
expect(await measurementsOf(owner.id)).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user