fix: enforce explicit grant invariants
Explicit-grant management could produce states an operator could not reason about, and pushed request-shaped problems into the database. Two identical createGrant calls produced two grants; five concurrent identical calls produced five. Revoking one returned success while the equivalent kept authorizing the same user — an operator could believe they had removed access that was still live. Two issuances are now the same authorization source when user_id, permission_id, scope, source_kind, and source_ref are all equal, with NULL equal only to NULL; expiry, creation metadata, actor, and grant id do not distinguish them. Creating one while an equivalent is currently effective fails with AUTHORIZATION_GRANT_ALREADY_EXISTS (HTTP 409, no-store) naming nothing about the grant that already exists, and writing no grant and no audit row. A revoked or expired equivalent does not block a fresh issuance. Conflict rather than idempotent return: the caller asked to record a new authorization with their own actor and expiry, and answering "done" while recording neither would hide that nothing happened. Enforcement is database-backed, since nothing constrains the number of API processes: every create and revoke touching one equivalence tuple takes a transaction-scoped advisory lock keyed by a hash of that tuple, encoded as a JSON array so NULL stays distinct from the empty string and a crafted scope cannot forge a separator. A collision can only make unrelated tuples serialize, never hide a conflict, because the decision is always an exact SQL predicate. Its single-key lock space is verified disjoint from the two-key space the migration runner and bootstrap use. A time-dependent partial unique index was rejected: now() is not immutable and cannot be indexed. One revocation now clears every unrevoked equivalent — including expired-but- unrevoked legacy rows, so the history is left in one state — with one audit row per grant actually revoked, each carrying its own id, in one transaction. Differently scoped or sourced grants are untouched. Historical duplicates are not silently revoked by migration; the store prevents new effective equivalents and an operator's explicit revocation resolves the class. The request boundary no longer leaks into PostgreSQL. GrantId accepts exactly the canonical lowercase hyphenated UUID the database emits, with no trimming and no case folding, so a malformed path parameter is a 400 decided after authorization but before any store call instead of reaching the UUID comparison and returning an indistinguishable 503; a canonical unknown id still returns 404. expiresAt accepts exactly YYYY-MM-DDTHH:mm:ss.sssZ, byte-for-byte equal to the parsed date's own toISOString, and must be strictly future — bare new Date silently rewrote 2026-02-31 to March 3rd and accepted locale, date-only, offset, and millisecond-free forms, and a past expiry was stored as a grant dead on arrival. The store validates the same thing before any SQL so trusted internal callers get the guarantee too. The generic HTTP surface now rejects a scope field in every form, including null. No route middleware supplies a scope to the evaluator, so a scope accepted here produced a grant that persisted, listed, and authorized nothing. Core's scope semantics are unchanged for internal callers; no resolver was added. Listing was unbounded and returned a user's entire history. It is now keyset pagination over created_at DESC, id DESC with a strict boundary on both values, so rows sharing a timestamp are neither skipped nor repeated, at most limit + 1 rows read, explicit-source filtering server-side on every page, and an opaque unsigned position cursor that is strictly decoded and reveals nothing on rejection. Core's list method requires a validated finite bound with no unbounded fallback anywhere. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
532ce3803b
commit
5c589d8fec
@@ -6,10 +6,22 @@ import { parseEkklesiaUserId } from "./identity.js";
|
||||
/**
|
||||
* Opaque at the type level via Zod's nominal `.brand()`, exactly like `EkklesiaUserId` and
|
||||
* `PermissionId`: a plain `string` is not assignable to `GrantId` without passing through
|
||||
* `parseGrantId`/`safeParseGrantId`. Ekklesia does not assume any particular format; Core's
|
||||
* Postgres-backed store happens to generate a UUID, but callers must treat the value as opaque.
|
||||
* `parseGrantId`/`safeParseGrantId`. Callers must still treat the *value* as opaque — nothing
|
||||
* outside Core's store may construct one, derive meaning from it, or depend on its internals.
|
||||
*
|
||||
* The accepted shape is deliberately narrow: exactly the canonical lowercase hyphenated text form
|
||||
* PostgreSQL emits for the UUID-backed `id` column (`8-4-4-4-12` lowercase hexadecimal). A looser
|
||||
* "any short string" rule meant a malformed path parameter passed validation and only failed later,
|
||||
* inside PostgreSQL's own UUID comparison — turning a request-validation problem into an
|
||||
* indistinguishable infrastructure failure. There is deliberately **no** `.trim()`: whitespace
|
||||
* padding is a malformed identifier, not something to silently repair, and normalizing it here
|
||||
* would let two spellings of the same id reach the store. Uppercase is likewise rejected rather
|
||||
* than folded, so exactly one spelling of any grant is ever valid.
|
||||
*/
|
||||
export const grantIdSchema = z.string().trim().min(1).max(128).brand<"GrantId">();
|
||||
export const grantIdSchema = z
|
||||
.string()
|
||||
.regex(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/u)
|
||||
.brand<"GrantId">();
|
||||
|
||||
export type GrantId = z.infer<typeof grantIdSchema>;
|
||||
|
||||
|
||||
@@ -135,15 +135,53 @@ describe("PermissionDeclaration", () => {
|
||||
});
|
||||
|
||||
describe("GrantId", () => {
|
||||
it("parses a non-empty opaque grant id and rejects empty/oversized input", () => {
|
||||
it("parses exactly the canonical lowercase hyphenated UUID PostgreSQL emits", () => {
|
||||
const id: GrantId = parseGrantId("11111111-1111-1111-1111-111111111111");
|
||||
expect(id).toBe("11111111-1111-1111-1111-111111111111");
|
||||
expect(() => parseGrantId("")).toThrow();
|
||||
expect(() => parseGrantId("x".repeat(129))).toThrow();
|
||||
expect(parseGrantId("0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70")).toBe(
|
||||
"0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70",
|
||||
);
|
||||
});
|
||||
|
||||
it("is opaque at the type level: a plain string is not assignable without parsing", () => {
|
||||
// @ts-expect-error a bare string must not satisfy the branded GrantId type.
|
||||
const notAllowed: GrantId = "11111111-1111-1111-1111-111111111111";
|
||||
expect(notAllowed).toBe("11111111-1111-1111-1111-111111111111");
|
||||
});
|
||||
|
||||
it("rejects every non-canonical spelling rather than repairing it", () => {
|
||||
for (const value of [
|
||||
"",
|
||||
"abc",
|
||||
"not-a-uuid",
|
||||
// Uppercase is rejected, never folded, so exactly one spelling of any grant is valid.
|
||||
"0B36B1B6-9A29-4C4E-9E2D-4C0E4C5E6F70",
|
||||
// Whitespace padding is malformed input, not something to silently trim: trimming would let
|
||||
// two spellings of the same id reach the store.
|
||||
" 0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70",
|
||||
"0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70 ",
|
||||
"\t0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70\n",
|
||||
// Shortened, overlong, unhyphenated, wrong grouping, and non-hex.
|
||||
"0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f7",
|
||||
"0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f700",
|
||||
"0b36b1b69a294c4e9e2d4c0e4c5e6f70",
|
||||
"0b36b1b6-9a294-c4e-9e2d-4c0e4c5e6f70",
|
||||
"0g36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70",
|
||||
"0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70\n0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f71",
|
||||
"x".repeat(129),
|
||||
]) {
|
||||
expect(safeParseGrantId(value).success, `expected ${JSON.stringify(value)} rejected`).toBe(
|
||||
false,
|
||||
);
|
||||
expect(() => parseGrantId(value)).toThrow();
|
||||
}
|
||||
for (const value of [42, null, undefined, {}, []]) {
|
||||
expect(safeParseGrantId(value).success).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("exposes a safe-parse variant that never throws", () => {
|
||||
expect(safeParseGrantId("abc").success).toBe(true);
|
||||
expect(safeParseGrantId("11111111-1111-1111-1111-111111111111").success).toBe(true);
|
||||
expect(safeParseGrantId("").success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user