diff --git a/README.md b/README.md index a5c0a88..f3579f3 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,30 @@ remains the deployment's own responsibility. Phase 5D is still not complete** (authorization integrity, module/runtime/migration correctness, and deployment/supply-chain closure plus a final independent re-audit) remain outstanding. +Phase 5D pass 3 of 5 preserves authorization security history and makes explicit-grant management +unambiguous and bounded. Authorization **audit history now survives user deletion**: the audit +subject is a retained identifier snapshot with no cascading foreign key, so deleting a user still +removes their live grants and platform-owner row but no longer destroys the record of what they had +been granted and by whom. Platform-owner bootstrap's single use moved to a dedicated singleton with +no user foreign key — previously deleting the bootstrapped owner's user row silently reopened the +one operation that establishes a platform's first administrator; the operational consequence is that +**deleting the last owner does not reactivate bootstrap**, and existing installations are backfilled +as consumed by the new migration. Two equivalent grants can no longer both be effective: creation +conflicts with a sanitized `409`, serialized under real PostgreSQL concurrency, and one revocation +now clears every unrevoked equivalent with a per-row audit entry, so revoking a grant can no longer +report success while a duplicate keeps authorizing the user. Grant ids must be canonical UUIDs and +expirations one exact future-only format, both decided before any database call; the generic HTTP +surface creates unscoped grants only; and listing is bounded keyset pagination with an opaque, +unsigned position cursor. One forward Core migration (`0002_authorization_integrity`, +`{"mode":"none"}`) must be applied before the new code starts. See +[Authorization](docs/development/authorization.md) and +[ADR 0019](docs/architecture/decisions/0019-durable-authorization-history-and-grant-management.md). +**This pass adds no roles, teams, tenants, service accounts, scope resolver, user-deletion endpoint, +or owner-recovery workflow; audit append-only behavior remains an application convention, not a +database or cryptographic guarantee. Phase 5D is still not complete** — passes 4 (module/runtime/ +migration correctness) and 5 (deployment/supply chain plus a final independent re-audit) remain +outstanding, and Phase 6 remains blocked. + ## Quick start Prerequisites: Node.js 24 LTS, pnpm 11.12, and Docker with Compose. diff --git a/apps/api/src/grants-routes.ts b/apps/api/src/grants-routes.ts index 45ee0aa..3622374 100644 --- a/apps/api/src/grants-routes.ts +++ b/apps/api/src/grants-routes.ts @@ -1,16 +1,17 @@ -import type { AuthorizationScope, EkklesiaUserId, PermissionId } from "@ekklesia/contracts"; +import type { EkklesiaUserId, GrantId, PermissionId } from "@ekklesia/contracts"; import { createUserActor, isAuthenticatedPrincipal, - safeParseAuthorizationScope, safeParseEkklesiaUserId, safeParseGrantId, safeParsePermissionId, } from "@ekklesia/contracts"; import { + AUTHORIZATION_GRANT_LIST_MAX_LIMIT, type AuthorizationStore, AuthorizationStoreError, CORE_AUTHORIZATION_GRANTS_MANAGE, + type GrantPageBoundary, type GrantRecord, } from "@ekklesia/core/authorization"; import { Hono } from "hono"; @@ -26,13 +27,36 @@ import { CORE_MUTATION_MAX_BODY_BYTES, platformBodyLimit } from "./body-limits.j export interface CreateGrantsRoutesOptions extends AuthorizationMiddlewareOptions { readonly store: AuthorizationStore; readonly trustedOrigins: ReadonlySet; + /** Injected only so expiration validation is deterministic under test. Production composition + * leaves this unset and the routes read real time. */ + readonly now?: () => Date; } +const DEFAULT_LIST_LIMIT = 50; + function sanitizedStoreErrorResponse(c: Context, error: unknown): Response { c.header("Cache-Control", "no-store"); if (error instanceof AuthorizationStoreError) { - const status = error.code === "AUTHORIZATION_GRANT_NOT_FOUND" ? 404 : 503; - return c.json({ code: error.code }, status); + if (error.code === "AUTHORIZATION_GRANT_NOT_FOUND") { + return c.json({ code: error.code }, 404); + } + // The one conflict the caller can act on. It deliberately reveals nothing about the grant that + // already exists — not its id, actor, scope, source reference, or expiry — only that an + // equivalent authorization is already effective for this subject. + if (error.code === "AUTHORIZATION_GRANT_ALREADY_EXISTS") { + return c.json({ code: error.code }, 409); + } + // Defense in depth: the routes below reject both of these before ever reaching the store, so + // arriving here means a request-shaped problem slipped through. It is still a client input + // failure, never an infrastructure one, and it reports exactly like any other malformed + // request rather than leaking the store's internal distinction. + if ( + error.code === "AUTHORIZATION_GRANT_EXPIRATION_INVALID" || + error.code === "AUTHORIZATION_GRANT_LIMIT_INVALID" + ) { + return c.json({ code: "INVALID_REQUEST" }, 400); + } + return c.json({ code: error.code }, 503); } return c.json({ code: "INTERNAL_ERROR" }, 503); } @@ -40,20 +64,55 @@ function sanitizedStoreErrorResponse(c: Context, error: unknown): Response { interface ParsedCreateGrantBody { readonly expiresAt?: Date; readonly permission: PermissionId; - readonly scope?: AuthorizationScope; readonly subjectUserId: EkklesiaUserId; } -const CREATE_GRANT_ALLOWED_KEYS = new Set(["subjectUserId", "permission", "scope", "expiresAt"]); +const CREATE_GRANT_ALLOWED_KEYS = new Set(["subjectUserId", "permission", "expiresAt"]); /** - * Accepts only `subjectUserId`, `permission`, an optional `scope`, and an optional `expiresAt`. - * Any other key present — `actor`, `sourceKind`, `sourceRef`, any audit field, `createdAt`, - * `revokedAt`, a grant id, or anything claiming owner/effective-permission state — makes the - * whole body invalid rather than being silently dropped, so a caller cannot smuggle - * server-derived fields through a request that otherwise looks well-formed. + * The one accepted expiration representation: `YYYY-MM-DDTHH:mm:ss.sssZ`, byte-for-byte equal to + * the parsed `Date`'s own `toISOString()`. + * + * `new Date(value)` alone was far too permissive. It silently *normalized* impossible dates + * (`2026-02-31T00:00:00.000Z` became March 3rd) and accepted locale-shaped input + * (`"March 5, 2027"`), date-only values, offsets, and missing milliseconds — so what an operator + * typed and what the platform stored could differ without any error. The regular expression pins + * the lexical form and the round-trip comparison rejects every value the parser would otherwise + * quietly rewrite. */ -function parseCreateGrantBody(raw: unknown): ParsedCreateGrantBody | undefined { +const CANONICAL_INSTANT_PATTERN = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/u; + +function parseCanonicalFutureInstant(value: unknown, now: Date): Date | undefined { + if (typeof value !== "string" || !CANONICAL_INSTANT_PATTERN.test(value)) { + return undefined; + } + const parsed = new Date(value); + if (Number.isNaN(parsed.getTime()) || parsed.toISOString() !== value) { + return undefined; + } + // Strictly future: an expiration equal to or before the acceptance clock produces a grant that + // is already dead, which is never what the caller meant. + if (parsed.getTime() <= now.getTime()) { + return undefined; + } + return parsed; +} + +/** + * Accepts only `subjectUserId`, `permission`, and an optional `expiresAt`. Any other key present — + * `actor`, `sourceKind`, `sourceRef`, any audit field, `createdAt`, `revokedAt`, a grant id, or + * anything claiming owner/effective-permission state — makes the whole body invalid rather than + * being silently dropped, so a caller cannot smuggle server-derived fields through a request that + * otherwise looks well-formed. + * + * **`scope` is deliberately among the rejected keys**, including when supplied as `null`, empty, or + * an otherwise valid scope string. Core's scoped-grant semantics remain intact for trusted internal + * callers, but no HTTP route middleware supplies a scope to the evaluator today, so a scope + * accepted here would produce a grant that looks meaningful, persists, appears in listings, and can + * authorize nothing at all. Accepting it would be offering a switch that is not connected to + * anything. When a server-owned resolver exists, this is the one place that changes. + */ +function parseCreateGrantBody(raw: unknown, now: Date): ParsedCreateGrantBody | undefined { if (typeof raw !== "object" || raw === null || Array.isArray(raw)) { return undefined; } @@ -71,31 +130,138 @@ function parseCreateGrantBody(raw: unknown): ParsedCreateGrantBody | undefined { return undefined; } - let scope: AuthorizationScope | undefined; - if (body.scope !== undefined) { - const parsedScope = safeParseAuthorizationScope(body.scope); - if (!parsedScope.success) { - return undefined; - } - scope = parsedScope.data; - } - let expiresAt: Date | undefined; if (body.expiresAt !== undefined) { - if (typeof body.expiresAt !== "string") { + expiresAt = parseCanonicalFutureInstant(body.expiresAt, now); + if (expiresAt === undefined) { return undefined; } - const parsedDate = new Date(body.expiresAt); - if (Number.isNaN(parsedDate.getTime())) { - return undefined; - } - expiresAt = parsedDate; } return Object.freeze({ ...(expiresAt === undefined ? {} : { expiresAt }), permission: permission.data, - ...(scope === undefined ? {} : { scope }), + subjectUserId: subjectUserId.data, + }); +} + +const CURSOR_VERSION = 1; +const BASE64URL_PATTERN = /^[A-Za-z0-9_-]+$/u; + +/** + * An opaque pagination *position*, not an authorization token. It carries only the subject user id, + * the boundary row's creation instant, and the boundary grant id — never a permission, scope, + * source reference, actor, credential, or secret — so there is nothing in it worth protecting. + * + * It is deliberately unsigned. Signing would imply the cursor carries authority it does not have: + * the permission check and the server-side subject filter remain authoritative on every page, and + * a forged cursor can at most name a different position within grants the caller is already + * entitled to list. The strict decode below still refuses anything that is not exactly what this + * function would emit. + */ +function encodeCursor(userId: EkklesiaUserId, boundary: GrantPageBoundary): string { + return Buffer.from( + JSON.stringify({ + g: boundary.id, + t: boundary.createdAt.toISOString(), + u: userId, + v: CURSOR_VERSION, + }), + "utf8", + ).toString("base64url"); +} + +/** + * Strict in every dimension: canonical unpadded base64url, valid UTF-8 JSON, an object with exactly + * the four expected keys, the exact version, a subject matching the *requested* one, a canonical + * `toISOString()` timestamp, a canonical `GrantId`, and — the catch-all — a re-encoding that + * reproduces the supplied string byte-for-byte, which rejects padding, trailing data, and any + * alternative encoding of the same payload. A rejected cursor reveals nothing it decoded. + */ +function decodeCursor(raw: string, userId: EkklesiaUserId): GrantPageBoundary | undefined { + if (!BASE64URL_PATTERN.test(raw)) { + return undefined; + } + let parsed: unknown; + try { + parsed = JSON.parse(Buffer.from(raw, "base64url").toString("utf8")); + } catch { + return undefined; + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + return undefined; + } + const payload = parsed as Record; + if (Object.keys(payload).sort().join(",") !== "g,t,u,v") { + return undefined; + } + if (payload.v !== CURSOR_VERSION || payload.u !== userId || typeof payload.t !== "string") { + return undefined; + } + const createdAt = new Date(payload.t); + if (Number.isNaN(createdAt.getTime()) || createdAt.toISOString() !== payload.t) { + return undefined; + } + const id = safeParseGrantId(payload.g); + if (!id.success) { + return undefined; + } + const boundary: GrantPageBoundary = Object.freeze({ createdAt, id: id.data }); + if (encodeCursor(userId, boundary) !== raw) { + return undefined; + } + return boundary; +} + +const LIST_ALLOWED_QUERY_KEYS = new Set(["userId", "limit", "cursor"]); +/** Canonical base-10, no sign, no leading zero, no decimal point, no whitespace, no exponent. */ +const CANONICAL_POSITIVE_INTEGER_PATTERN = /^[1-9][0-9]*$/u; + +interface ParsedListQuery { + readonly after?: GrantPageBoundary; + readonly limit: number; + readonly subjectUserId: EkklesiaUserId; +} + +/** Fully validated before any store or database work happens, so an invalid page request never + * becomes a query, and never becomes a way to probe which subjects or grants exist. */ +function parseListQuery(url: string): ParsedListQuery | undefined { + const searchParams = new URL(url).searchParams; + for (const key of searchParams.keys()) { + if (!LIST_ALLOWED_QUERY_KEYS.has(key) || searchParams.getAll(key).length > 1) { + return undefined; + } + } + + const subjectUserId = safeParseEkklesiaUserId(searchParams.get("userId")); + if (!subjectUserId.success) { + return undefined; + } + + let limit = DEFAULT_LIST_LIMIT; + const rawLimit = searchParams.get("limit"); + if (rawLimit !== null) { + if (!CANONICAL_POSITIVE_INTEGER_PATTERN.test(rawLimit)) { + return undefined; + } + limit = Number(rawLimit); + if (!Number.isSafeInteger(limit) || limit < 1 || limit > AUTHORIZATION_GRANT_LIST_MAX_LIMIT) { + return undefined; + } + } + + let after: GrantPageBoundary | undefined; + const rawCursor = searchParams.get("cursor"); + if (rawCursor !== null) { + after = decodeCursor(rawCursor, subjectUserId.data); + if (after === undefined) { + return undefined; + } + } + + return Object.freeze({ + ...(after === undefined ? {} : { after }), + limit, subjectUserId: subjectUserId.data, }); } @@ -125,9 +291,12 @@ function serializeGrant(grant: GrantRecord) { * `sourceKind` at all), and revocation is constrained to `sourceKind: "explicit"`, so attempting * to revoke a relationship-derived grant by id behaves exactly like the grant does not exist — * those grants remain owned by the module operation that established the relationship. + * + * Creation is unscoped-only; see `parseCreateGrantBody`. */ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { const routes = new Hono(); + const now = options.now ?? ((): Date => new Date()); const requireManagePermission = requirePermission(options, CORE_AUTHORIZATION_GRANTS_MANAGE); const requireOrigin = requireTrustedOrigin(options.trustedOrigins); // Origin is checked first (a rejected cross-origin mutation must never have its body read), then @@ -137,15 +306,29 @@ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { routes.get("/api/v1/grants", requireManagePermission, async (c) => { c.header("Cache-Control", "no-store"); - const subjectUserId = safeParseEkklesiaUserId(c.req.query("userId")); - if (!subjectUserId.success) { + // Reached only after `requireManagePermission` allowed the caller, so query validation can + // never disclose a validation distinction — or the existence of any subject or grant — to an + // unauthorized caller. + const query = parseListQuery(c.req.url); + if (!query) { return c.json({ code: "INVALID_REQUEST" }, 400); } try { - const grants = await options.store.listGrantsForUser(subjectUserId.data, { + const page = await options.store.listGrantsForUser(query.subjectUserId, { + ...(query.after === undefined ? {} : { after: query.after }), + limit: query.limit, sourceKind: "explicit", }); - return c.json({ grants: grants.map(serializeGrant) }, 200); + return c.json( + { + grants: page.grants.map(serializeGrant), + nextCursor: + page.nextBoundary === undefined + ? null + : encodeCursor(query.subjectUserId, page.nextBoundary), + }, + 200, + ); } catch (error) { return sanitizedStoreErrorResponse(c, error); } @@ -169,7 +352,7 @@ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { } catch { return c.json({ code: "INVALID_REQUEST" }, 400); } - const parsed = parseCreateGrantBody(rawBody); + const parsed = parseCreateGrantBody(rawBody, now()); if (!parsed) { return c.json({ code: "INVALID_REQUEST" }, 400); } @@ -189,7 +372,6 @@ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { actor: createUserActor(principal.userId), ...(parsed.expiresAt === undefined ? {} : { expiresAt: parsed.expiresAt }), permission: parsed.permission, - ...(parsed.scope === undefined ? {} : { scope: parsed.scope }), sourceKind: "explicit", userId: parsed.subjectUserId, }); @@ -212,6 +394,9 @@ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { return c.json({ code: "AUTHORIZATION_DENIED" }, 403); } + // Canonical-UUID validation happens after authorization but strictly before any store or + // database call: a malformed id previously reached PostgreSQL's own UUID comparison and came + // back as an indistinguishable infrastructure failure (503) rather than a request problem. const grantId = safeParseGrantId(c.req.param("grantId")); if (!grantId.success) { return c.json({ code: "INVALID_REQUEST" }, 400); @@ -220,7 +405,7 @@ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { try { await options.store.revokeGrant({ actor: createUserActor(principal.userId), - grantId: grantId.data, + grantId: grantId.data satisfies GrantId, sourceKind: "explicit", }); return c.json({ success: true }, 200); diff --git a/apps/api/tests/api-http.integration.ts b/apps/api/tests/api-http.integration.ts index da3838f..12754dc 100644 --- a/apps/api/tests/api-http.integration.ts +++ b/apps/api/tests/api-http.integration.ts @@ -1396,7 +1396,7 @@ describe.sequential("live API — platform owner and delegated grant management" headers: { cookie: ownerCookie }, }); expect(response.status).toBe(200); - expect(await response.json()).toEqual({ grants: [] }); + expect(await response.json()).toEqual({ grants: [], nextCursor: null }); }); it("lets the owner delegate grants.manage to another account through the explicit-grant HTTP API", async () => { @@ -1480,10 +1480,13 @@ describe.sequential("live API — platform owner and delegated grant management" it("creates an explicit grant via HTTP, deriving sourceKind and actor server-side, and records an atomic audit entry", async () => { const response = await fetch(`${origin}/api/v1/grants`, { + // A subject with no equivalent grant yet — the manager already holds an equivalent + // `grants.manage` grant from the delegation scenario above, which the new duplicate + // invariant would (correctly) refuse. Unscoped: the generic HTTP surface creates only + // unscoped explicit grants, since a scope it accepted could never authorize anything. body: JSON.stringify({ permission: testPermission, - scope: "calendar-event:1", - subjectUserId: managerUserId, + subjectUserId: ownerUserId, }), headers: { "content-type": "application/json", @@ -1498,7 +1501,7 @@ describe.sequential("live API — platform owner and delegated grant management" expect(response.status).toBe(201); expect(body.grant.sourceKind).toBe("explicit"); - expect(body.grant.userId).toBe(managerUserId); + expect(body.grant.userId).toBe(ownerUserId); createdGrantId = body.grant.id; const admin = await openAdministrativeClient(database.url); @@ -1514,14 +1517,21 @@ describe.sequential("live API — platform owner and delegated grant management" } }); - it("lists the explicit grant via HTTP", async () => { - const response = await fetch(`${origin}/api/v1/grants?userId=${managerUserId}`, { + it("lists the explicit grant via HTTP, bounded and with an explicit next-page cursor field", async () => { + const response = await fetch(`${origin}/api/v1/grants?userId=${ownerUserId}`, { headers: { cookie: ownerCookie }, }); - const body = (await response.json()) as { grants: Array<{ id: string }> }; + const body = (await response.json()) as { + grants: Array<{ id: string; scope: string | null }>; + nextCursor: string | null; + }; expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("no-store"); expect(body.grants.some((grant) => grant.id === createdGrantId)).toBe(true); + // Created through the generic HTTP surface, so it is unscoped. + expect(body.grants.find((grant) => grant.id === createdGrantId)?.scope).toBeNull(); + expect(body.nextCursor).toBeNull(); }); it("rejects a create-grant request carrying disallowed fields such as actor or sourceKind, even alongside otherwise-valid fields", async () => { @@ -1659,6 +1669,302 @@ describe.sequential("live API — authorization infrastructure failure is never }); }); +/** + * Pass 3's explicit-grant invariants, exercised end-to-end through a real authorized + * grants-management session against real PostgreSQL — the layer where a validation decision that + * "should" happen before the database either does or does not. + */ +describe.sequential("live API — explicit-grant invariants over real HTTP", () => { + let database: { name: string; url: string }; + let running: RunningApi; + let origin: string; + let ownerUserId: string; + let subjectUserId: string; + let ownerCookie: string; + const password = "correct horse battery staple"; + const managePermission = "core.authorization.grants.manage"; + + async function post(path: string, body?: unknown): Promise { + return await fetch(`${origin}${path}`, { + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + headers: { + ...(body === undefined ? {} : { "content-type": "application/json" }), + cookie: ownerCookie, + origin: dashboardOrigin, + }, + method: "POST", + }); + } + + async function list(query: string): Promise<{ body: unknown; response: Response }> { + const response = await fetch(`${origin}/api/v1/grants?${query}`, { + headers: { cookie: ownerCookie }, + }); + return { body: await response.json(), response }; + } + + it("starts a freshly migrated API with a bootstrapped owner and a separate grant subject", async () => { + database = await createDisposableDatabase("grant_invariants"); + await applyCoreMigrations(database.url); + running = await startMigratedApi(database.url); + origin = apiOrigin(running); + + ownerUserId = (await signUp(origin, "invariants-owner@example.test", password)).body.user.id; + subjectUserId = (await signUp(origin, "invariants-subject@example.test", password)).body.user + .id; + + const { close, db } = createAuthorizationDrizzleDatabase(database.url); + try { + await bootstrapPlatformOwner(db, { userId: parseEkklesiaUserId(ownerUserId) }); + } finally { + await close(); + } + const ownerSignIn = await signIn(origin, "invariants-owner@example.test", password); + expect(ownerSignIn.cookiePair).toBeDefined(); + ownerCookie = ownerSignIn.cookiePair as string; + }); + + it("rejects a malformed non-UUID grant id with a sanitized 400, never a database failure", async () => { + for (const malformed of [ + "not-a-uuid", + "0B36B1B6-9A29-4C4E-9E2D-4C0E4C5E6F70", + "0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f7", + ]) { + const response = await post(`/api/v1/grants/${malformed}/revoke`); + const raw = await response.text(); + + // Before Pass 3 this reached PostgreSQL's own UUID comparison and returned 503. + expect(response.status, malformed).toBe(400); + expect(JSON.parse(raw)).toEqual({ code: "INVALID_REQUEST" }); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(raw).not.toContain(malformed); + } + }); + + it("returns the sanitized 404 for a canonical but unknown grant id", async () => { + const response = await post("/api/v1/grants/00000000-0000-4000-8000-000000000000/revoke"); + expect(response.status).toBe(404); + expect(await response.json()).toEqual({ code: "AUTHORIZATION_GRANT_NOT_FOUND" }); + }); + + it("rejects a scoped create body with a sanitized 400 and writes nothing", async () => { + const response = await post("/api/v1/grants", { + permission: managePermission, + scope: "calendar-event:1", + subjectUserId, + }); + expect(response.status).toBe(400); + expect(await response.json()).toEqual({ code: "INVALID_REQUEST" }); + + const admin = await openAdministrativeClient(database.url); + try { + expect( + await safeQuery(admin, `select id from "ekklesia_core_grant" where user_id = $1`, [ + subjectUserId, + ]), + ).toHaveLength(0); + } finally { + await safeEnd(admin); + } + }); + + it("rejects normalized-impossible, non-canonical, and past expirations with a sanitized 400", async () => { + for (const expiresAt of [ + "2026-02-31T00:00:00.000Z", + "March 5, 2027", + "2027-01-01", + "2027-01-01T00:00:00Z", + "2027-01-01T00:00:00.000+02:00", + "2020-01-01T00:00:00.000Z", + ]) { + const response = await post("/api/v1/grants", { + expiresAt, + permission: managePermission, + subjectUserId, + }); + expect(response.status, expiresAt).toBe(400); + expect(await response.json()).toEqual({ code: "INVALID_REQUEST" }); + } + }); + + it("creates one grant, then answers an equivalent creation with a sanitized 409 and no second row", async () => { + const created = await post("/api/v1/grants", { + permission: managePermission, + subjectUserId, + }); + expect(created.status).toBe(201); + + const duplicate = await post("/api/v1/grants", { + permission: managePermission, + subjectUserId, + }); + const raw = await duplicate.text(); + expect(duplicate.status).toBe(409); + expect(JSON.parse(raw)).toEqual({ code: "AUTHORIZATION_GRANT_ALREADY_EXISTS" }); + expect(duplicate.headers.get("cache-control")).toBe("no-store"); + // No existing grant id, actor, scope, source reference, SQL, or driver text leaks. + expect(raw).not.toContain(ownerUserId); + expect(raw).not.toContain(subjectUserId); + expect(raw).not.toContain("ekklesia_core_grant"); + + // A differing expiration is not a different grant either. + const withExpiry = await post("/api/v1/grants", { + expiresAt: "2099-01-01T00:00:00.000Z", + permission: managePermission, + subjectUserId, + }); + expect(withExpiry.status).toBe(409); + + const admin = await openAdministrativeClient(database.url); + try { + expect( + await safeQuery( + admin, + `select id from "ekklesia_core_grant" where user_id = $1 and revoked_at is null`, + [subjectUserId], + ), + ).toHaveLength(1); + } finally { + await safeEnd(admin); + } + }); + + it("paginates deterministically across at least three pages and ends with a null cursor", async () => { + // Seeded through the test-only administrative client so the page count is deterministic and + // independent of how many grants the scenarios above happened to create. + const admin = await openAdministrativeClient(database.url); + try { + await safeQuery( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id, created_at) + select $1, 'calendar.events.page' || generation, null, 'explicit', null, 'user', $2, + timestamptz '2026-01-01 00:00:00Z' + from generate_series(1, 6) as generation`, + [subjectUserId, ownerUserId], + ); + await safeQuery( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id) + values ($1, 'roster.volunteer.read', null, 'relationship', 'roster:1', 'user', $2)`, + [subjectUserId, ownerUserId], + ); + } finally { + await safeEnd(admin); + } + + const seen: string[] = []; + let cursor: string | null = null; + let pages = 0; + for (;;) { + const query = `userId=${subjectUserId}&limit=3${ + cursor === null ? "" : `&cursor=${encodeURIComponent(cursor)}` + }`; + const { body, response } = await list(query); + const page = body as { + grants: Array<{ id: string; sourceKind: string }>; + nextCursor: string | null; + }; + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(page.grants.length).toBeLessThanOrEqual(3); + // Relationship grants never appear on any page. + expect(page.grants.every((grant) => grant.sourceKind === "explicit")).toBe(true); + seen.push(...page.grants.map((grant) => grant.id)); + pages += 1; + if (page.nextCursor === null) { + break; + } + cursor = page.nextCursor; + expect(pages).toBeLessThan(10); + } + + expect(pages).toBeGreaterThanOrEqual(3); + expect(seen).toHaveLength(7); + expect(new Set(seen).size).toBe(seen.length); + }); + + it("rejects an out-of-range limit and a foreign or malformed cursor with a sanitized 400", async () => { + for (const query of [ + `userId=${subjectUserId}&limit=0`, + `userId=${subjectUserId}&limit=101`, + `userId=${subjectUserId}&limit=01`, + `userId=${subjectUserId}&limit=1&limit=2`, + `userId=${subjectUserId}&unknown=1`, + `userId=${subjectUserId}&cursor=!!!`, + // A cursor minted for a different subject must not be replayable against this one. + `userId=${subjectUserId}&cursor=${encodeURIComponent( + Buffer.from( + JSON.stringify({ + g: "00000000-0000-4000-8000-000000000000", + t: "2026-01-01T00:00:00.000Z", + u: ownerUserId, + v: 1, + }), + "utf8", + ).toString("base64url"), + )}`, + ]) { + const { body, response } = await list(query); + expect(response.status, query).toBe(400); + expect(body).toEqual({ code: "INVALID_REQUEST" }); + } + }); + + it("revokes every unrevoked equivalent through one HTTP revocation, leaving nothing authorizing", async () => { + const permission = "calendar.events.legacydupe"; + const admin = await openAdministrativeClient(database.url); + let ids: string[] = []; + try { + // Legacy duplicates the pre-Pass-3 store could produce, planted directly because the + // production store now refuses to create them. + const rows = await safeQuery<{ id: string }>( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id) + values ($1, $2, null, 'explicit', null, 'user', $3), + ($1, $2, null, 'explicit', null, 'user', $3), + ($1, $2, null, 'explicit', null, 'user', $3) + returning id`, + [subjectUserId, permission, ownerUserId], + ); + ids = rows.map((row) => row.id); + } finally { + await safeEnd(admin); + } + + const response = await post(`/api/v1/grants/${ids[0]}/revoke`); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ success: true }); + + const inspector = await openAdministrativeClient(database.url); + try { + const remaining = await safeQuery( + inspector, + `select id from "ekklesia_core_grant" + where user_id = $1 and permission_id = $2 and revoked_at is null`, + [subjectUserId, permission], + ); + expect(remaining).toHaveLength(0); + const audit = await safeQuery<{ grant_id: string }>( + inspector, + `select grant_id from "ekklesia_core_authorization_audit" + where action = 'grant.revoke' and grant_id = any($1::uuid[])`, + [ids], + ); + expect([...new Set(audit.map((row) => row.grant_id))].sort()).toEqual([...ids].sort()); + } finally { + await safeEnd(inspector); + } + }); + + it("shuts down cleanly", async () => { + await running.shutdown(); + await dropDatabase(database.name); + }); +}); + describe.sequential("live API — installed module permission end-to-end", () => { let database: { name: string; url: string }; let modulesDirectory: string; diff --git a/apps/api/tests/app.test.ts b/apps/api/tests/app.test.ts index b9d1be1..7a5ea2a 100644 --- a/apps/api/tests/app.test.ts +++ b/apps/api/tests/app.test.ts @@ -62,7 +62,7 @@ function fakeAuthorization(overrides: { grantPlatformOwner: vi.fn(), hasActiveGrant: vi.fn().mockResolvedValue(false), isPlatformOwner: vi.fn().mockResolvedValue(false), - listGrantsForUser: vi.fn().mockResolvedValue([]), + listGrantsForUser: vi.fn().mockResolvedValue({ grants: [] }), revokeGrant: vi.fn(), revokePlatformOwner: vi.fn(), ...overrides.store, diff --git a/apps/api/tests/grants-routes.test.ts b/apps/api/tests/grants-routes.test.ts index 5b4b976..5d685c5 100644 --- a/apps/api/tests/grants-routes.test.ts +++ b/apps/api/tests/grants-routes.test.ts @@ -1,6 +1,5 @@ import { createAuthenticatedPrincipal, - parseAuthorizationScope, parseEkklesiaUserId, parseGrantId, parsePermissionId, @@ -50,7 +49,7 @@ function fakeStore(overrides: Partial = {}): AuthorizationSt grantPlatformOwner: vi.fn(), hasActiveGrant: vi.fn().mockResolvedValue(false), isPlatformOwner: vi.fn().mockResolvedValue(false), - listGrantsForUser: vi.fn().mockResolvedValue([]), + listGrantsForUser: vi.fn().mockResolvedValue({ grants: [] }), revokeGrant: vi.fn(), revokePlatformOwner: vi.fn(), ...overrides, @@ -59,6 +58,7 @@ function fakeStore(overrides: Partial = {}): AuthorizationSt function buildApp(options: { readonly allowed?: boolean; + readonly now?: () => Date; readonly runtime?: EkklesiaIdentityRuntime; readonly store?: AuthorizationStore; }) { @@ -72,6 +72,7 @@ function buildApp(options: { "/", createGrantsRoutes({ evaluator, + ...(options.now === undefined ? {} : { now: options.now }), registry: CORE_PERMISSION_REGISTRY, runtime, store, @@ -103,15 +104,20 @@ describe("GET /api/v1/grants", () => { userId: subjectUserId, }, ]; - const store = fakeStore({ listGrantsForUser: vi.fn().mockResolvedValue(grants) }); + const store = fakeStore({ listGrantsForUser: vi.fn().mockResolvedValue({ grants }) }); const { app } = buildApp({ store }); const response = await app.request(`/api/v1/grants?userId=${subjectUserId}`); - const body = (await response.json()) as { grants: unknown[] }; + const body = (await response.json()) as { grants: unknown[]; nextCursor: string | null }; expect(response.status).toBe(200); expect(body.grants).toHaveLength(1); - expect(store.listGrantsForUser).toHaveBeenCalledWith(subjectUserId, { sourceKind: "explicit" }); + expect(body.nextCursor).toBeNull(); + // The default page size is applied server-side; `sourceKind` filtering stays server-side too. + expect(store.listGrantsForUser).toHaveBeenCalledWith(subjectUserId, { + limit: 50, + sourceKind: "explicit", + }); }); it("rejects a missing or malformed userId query parameter", async () => { @@ -184,7 +190,6 @@ describe("POST /api/v1/grants", () => { createdAt: new Date("2026-01-01T00:00:00.000Z"), id: grantId, permission: registeredPermission, - scope: parseAuthorizationScope("calendar-event:123"), sourceKind: "explicit" as const, userId: subjectUserId, }; @@ -192,11 +197,7 @@ describe("POST /api/v1/grants", () => { const { app } = buildApp({ store }); const response = await app.request("/api/v1/grants", { - body: JSON.stringify({ - permission: registeredPermission, - scope: "calendar-event:123", - subjectUserId, - }), + body: JSON.stringify({ permission: registeredPermission, subjectUserId }), headers: { "content-type": "application/json", origin: trustedOrigin }, method: "POST", }); @@ -208,11 +209,14 @@ describe("POST /api/v1/grants", () => { expect.objectContaining({ actor: { type: "user", userId: manager.userId }, permission: registeredPermission, - scope: "calendar-event:123", sourceKind: "explicit", userId: subjectUserId, }), ); + // The generic HTTP surface creates unscoped explicit grants only. + expect(store.createGrant).toHaveBeenCalledWith( + expect.not.objectContaining({ scope: expect.anything() }), + ); }); it("rejects a well-formed but unregistered permission with the same sanitized denial as an unauthorized caller, and never calls the store", async () => { @@ -512,3 +516,366 @@ describe("Core grant mutation request body limit", () => { expect(store.revokeGrant).not.toHaveBeenCalled(); }); }); + +/** + * Pass 3 request-contract regressions. Every case here asserts both the sanitized public response + * *and* that the store was never reached, because the point of each fix is that a malformed or + * unsupported request is decided at the request boundary rather than turning into database work or + * an indistinguishable infrastructure failure. + */ +describe("explicit-grant request invariants", () => { + const validExpiry = "2099-01-01T00:00:00.000Z"; + + async function post(app: Hono, body: unknown) { + return await app.request("/api/v1/grants", { + body: JSON.stringify(body), + headers: { "content-type": "application/json", origin: trustedOrigin }, + method: "POST", + }); + } + + it("rejects a scope field in any form, before any store mutation", async () => { + // Core keeps scoped-grant semantics for trusted internal callers, but no HTTP middleware + // supplies a scope to the evaluator, so a scope accepted here could never authorize anything. + for (const scope of ["calendar-event:123", "", " ", null, 42, {}, []]) { + const { app, store } = buildApp({}); + const response = await post(app, { permission: registeredPermission, scope, subjectUserId }); + + expect(response.status, `scope ${JSON.stringify(scope)}`).toBe(400); + expect(await response.json()).toEqual({ code: "INVALID_REQUEST" }); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(store.createGrant).not.toHaveBeenCalled(); + } + }); + + it("accepts exactly one canonical expiration representation", async () => { + const createdGrant = { + createdAt: new Date("2026-01-01T00:00:00.000Z"), + expiresAt: new Date(validExpiry), + id: grantId, + permission: registeredPermission, + sourceKind: "explicit" as const, + userId: subjectUserId, + }; + const store = fakeStore({ createGrant: vi.fn().mockResolvedValue(createdGrant) }); + const { app } = buildApp({ store }); + + const response = await post(app, { + expiresAt: validExpiry, + permission: registeredPermission, + subjectUserId, + }); + + expect(response.status).toBe(201); + expect(store.createGrant).toHaveBeenCalledWith( + expect.objectContaining({ expiresAt: new Date(validExpiry) }), + ); + }); + + it("rejects normalized-impossible, locale, offset, imprecise, padded, and past expirations", async () => { + const now = new Date("2026-07-26T12:00:00.000Z"); + for (const expiresAt of [ + // Normalized impossible date: `new Date` silently rewrote this to March 3rd. + "2026-02-31T00:00:00.000Z", + "2026-13-01T00:00:00.000Z", + // Locale-shaped and date-only forms. + "March 5, 2027", + "2027-01-01", + // Missing milliseconds, excess fractional precision, offsets, lowercase separators. + "2027-01-01T00:00:00Z", + "2027-01-01T00:00:00.123456Z", + "2027-01-01T00:00:00.000+02:00", + "2027-01-01t00:00:00.000z", + "2027-01-01T00:00:00.000+00:00", + // Whitespace padding and trailing text. + " 2027-01-01T00:00:00.000Z", + "2027-01-01T00:00:00.000Z ", + "2027-01-01T00:00:00.000Z extra", + // Past and exactly-now are both dead on arrival. + "2020-01-01T00:00:00.000Z", + "2026-07-26T12:00:00.000Z", + // Non-strings. + 42, + null, + true, + ]) { + const { app, store } = buildApp({ now: () => now }); + const response = await post(app, { + expiresAt, + permission: registeredPermission, + subjectUserId, + }); + + expect(response.status, `expiresAt ${JSON.stringify(expiresAt)}`).toBe(400); + expect(await response.json()).toEqual({ code: "INVALID_REQUEST" }); + expect(store.createGrant).not.toHaveBeenCalled(); + } + }); + + it("returns a sanitized 409 for an equivalent grant that is already effective", async () => { + const store = fakeStore({ + createGrant: vi + .fn() + .mockRejectedValue(new AuthorizationStoreError("AUTHORIZATION_GRANT_ALREADY_EXISTS")), + }); + const { app } = buildApp({ store }); + + const response = await post(app, { permission: registeredPermission, subjectUserId }); + const raw = await response.text(); + + expect(response.status).toBe(409); + expect(JSON.parse(raw)).toEqual({ code: "AUTHORIZATION_GRANT_ALREADY_EXISTS" }); + expect(response.headers.get("cache-control")).toBe("no-store"); + // The body is the stable code and nothing else — no existing grant id, actor, scope, source + // reference, expiry, SQL, constraint name, or driver text. + expect(Object.keys(JSON.parse(raw))).toEqual(["code"]); + for (const leaked of [ + grantId, + manager.userId, + subjectUserId, + "calendar-event", + "constraint", + "select ", + "insert ", + "ekklesia_core_grant", + "Error", + "at ", + ]) { + expect(raw, `must not leak ${leaked}`).not.toContain(leaked); + } + }); + + it("rejects a non-canonical grant id on revoke without ever calling the store", async () => { + for (const raw of [ + "not-a-uuid", + "0B36B1B6-9A29-4C4E-9E2D-4C0E4C5E6F70", + "%200b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f70", + "0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f7", + "0b36b1b6-9a29-4c4e-9e2d-4c0e4c5e6f700", + "0b36b1b69a294c4e9e2d4c0e4c5e6f70", + ]) { + const { app, store } = buildApp({}); + const response = await app.request(`/api/v1/grants/${raw}/revoke`, { + headers: { origin: trustedOrigin }, + method: "POST", + }); + + expect(response.status, `grantId ${raw}`).toBe(400); + expect(await response.json()).toEqual({ code: "INVALID_REQUEST" }); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(store.revokeGrant).not.toHaveBeenCalled(); + } + }); + + it("still returns the sanitized 404 for a canonical but unknown grant id", async () => { + const store = fakeStore({ + revokeGrant: vi + .fn() + .mockRejectedValue(new AuthorizationStoreError("AUTHORIZATION_GRANT_NOT_FOUND")), + }); + const { app } = buildApp({ store }); + + const response = await app.request(`/api/v1/grants/${grantId}/revoke`, { + headers: { origin: trustedOrigin }, + method: "POST", + }); + + expect(response.status).toBe(404); + expect(await response.json()).toEqual({ code: "AUTHORIZATION_GRANT_NOT_FOUND" }); + expect(store.revokeGrant).toHaveBeenCalled(); + }); +}); + +describe("explicit-grant listing pagination", () => { + function grantAt(index: number, iso: string) { + return { + createdAt: new Date(iso), + id: parseGrantId(`00000000-0000-4000-8000-${String(index).padStart(12, "0")}`), + permission: registeredPermission, + sourceKind: "explicit" as const, + userId: subjectUserId, + }; + } + + it("applies the default limit of 50 and accepts the 1 and 100 boundaries", async () => { + for (const [query, expected] of [ + ["", 50], + ["&limit=1", 1], + ["&limit=50", 50], + ["&limit=100", 100], + ] as const) { + const { app, store } = buildApp({}); + const response = await app.request(`/api/v1/grants?userId=${subjectUserId}${query}`); + + expect(response.status, query).toBe(200); + expect(store.listGrantsForUser).toHaveBeenCalledWith(subjectUserId, { + limit: expected, + sourceKind: "explicit", + }); + } + }); + + it("rejects every non-canonical or out-of-range limit before any store work", async () => { + for (const limit of [ + "0", + "101", + "-1", + "+5", + "1.0", + "1.5", + "01", + "005", + " 5", + "5 ", + "1e2", + "", + "abc", + "0x10", + "Infinity", + "NaN", + ]) { + const { app, store } = buildApp({}); + const response = await app.request( + `/api/v1/grants?userId=${subjectUserId}&limit=${encodeURIComponent(limit)}`, + ); + + expect(response.status, `limit ${JSON.stringify(limit)}`).toBe(400); + expect(await response.json()).toEqual({ code: "INVALID_REQUEST" }); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(store.listGrantsForUser).not.toHaveBeenCalled(); + } + }); + + it("rejects unknown and duplicated query keys", async () => { + for (const query of [ + `userId=${subjectUserId}&sort=asc`, + `userId=${subjectUserId}&offset=10`, + `userId=${subjectUserId}&sourceKind=relationship`, + `userId=${subjectUserId}&userId=${subjectUserId}`, + `userId=${subjectUserId}&limit=5&limit=6`, + `userId=${subjectUserId}&cursor=a&cursor=b`, + `userId=${subjectUserId}&cursor=`, + ]) { + const { app, store } = buildApp({}); + const response = await app.request(`/api/v1/grants?${query}`); + + expect(response.status, query).toBe(400); + expect(store.listGrantsForUser).not.toHaveBeenCalled(); + } + }); + + it("emits a cursor only when another page exists, and traverses without duplicates or omissions", async () => { + // Three rows sharing one `created_at`, which is exactly the case a timestamp-only keyset + // boundary would skip or repeat. + const sameInstant = "2026-01-01T00:00:00.000Z"; + const all = [grantAt(3, sameInstant), grantAt(2, sameInstant), grantAt(1, sameInstant)]; + + const seen: string[] = []; + let cursor: string | null = null; + let pages = 0; + do { + const store = fakeStore({ + listGrantsForUser: vi.fn().mockImplementation((_userId, listOptions) => { + const startIndex = + listOptions.after === undefined + ? 0 + : all.findIndex((grant) => grant.id === listOptions.after.id) + 1; + const slice = all.slice(startIndex, startIndex + listOptions.limit); + const more = startIndex + listOptions.limit < all.length; + const last = slice.at(-1); + return Promise.resolve({ + grants: slice, + ...(more && last ? { nextBoundary: { createdAt: last.createdAt, id: last.id } } : {}), + }); + }), + }); + const { app } = buildApp({ store }); + const url: string = `/api/v1/grants?userId=${subjectUserId}&limit=1${ + cursor === null ? "" : `&cursor=${encodeURIComponent(cursor)}` + }`; + const response = await app.request(url); + const body = (await response.json()) as { + grants: { id: string }[]; + nextCursor: string | null; + }; + + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(body.grants).toHaveLength(1); + seen.push(body.grants[0]?.id as string); + cursor = body.nextCursor; + pages += 1; + } while (cursor !== null && pages < 10); + + expect(pages).toBe(3); + expect(seen).toEqual(all.map((grant) => grant.id)); + expect(new Set(seen).size).toBe(3); + // The final page ends the traversal explicitly. + expect(cursor).toBeNull(); + }); + + it("refuses a cursor issued for a different subject, and every malformed encoding", async () => { + const boundary = { createdAt: new Date("2026-01-01T00:00:00.000Z"), id: grantId }; + const store = fakeStore({ + listGrantsForUser: vi.fn().mockResolvedValue({ + grants: [grantAt(1, boundary.createdAt.toISOString())], + nextBoundary: boundary, + }), + }); + const { app: issuingApp } = buildApp({ store }); + const issued = (await ( + await issuingApp.request(`/api/v1/grants?userId=${subjectUserId}&limit=1`) + ).json()) as { nextCursor: string }; + expect(issued.nextCursor).toMatch(/^[A-Za-z0-9_-]+$/u); + + const encode = (payload: unknown) => + Buffer.from(JSON.stringify(payload), "utf8").toString("base64url"); + const canonical = { + g: grantId, + t: "2026-01-01T00:00:00.000Z", + u: subjectUserId, + v: 1, + }; + + for (const [label, cursor] of [ + // A cursor minted for a different subject must not be replayable against this one. + ["other subject", encode({ ...canonical, u: "someone_else_1" })], + ["wrong version", encode({ ...canonical, v: 2 })], + ["extra field", encode({ ...canonical, extra: "x" })], + ["missing field", encode({ g: canonical.g, t: canonical.t, u: canonical.u })], + ["invalid timestamp", encode({ ...canonical, t: "2026-01-01" })], + ["non-canonical timestamp", encode({ ...canonical, t: "2026-01-01T00:00:00Z" })], + ["invalid grant id", encode({ ...canonical, g: "not-a-uuid" })], + ["array payload", encode([canonical])], + ["not json", Buffer.from("nonsense", "utf8").toString("base64url")], + ["padded base64", `${Buffer.from(JSON.stringify(canonical), "utf8").toString("base64")}`], + ["malformed base64url", "!!!not-base64!!!"], + ["trailing data", `${encode(canonical)}AA`], + ] as const) { + const { app, store: pagedStore } = buildApp({}); + const response = await app.request( + `/api/v1/grants?userId=${subjectUserId}&cursor=${encodeURIComponent(cursor)}`, + ); + const raw = await response.text(); + + expect(response.status, label).toBe(400); + expect(JSON.parse(raw)).toEqual({ code: "INVALID_REQUEST" }); + // A rejected cursor reveals nothing it decoded, and does zero store/database work. + expect(raw).not.toContain(grantId); + expect(raw).not.toContain("someone_else_1"); + expect(pagedStore.listGrantsForUser).not.toHaveBeenCalled(); + } + + // The cursor the API itself issued is accepted. + const { app: acceptingApp, store: acceptingStore } = buildApp({}); + const accepted = await acceptingApp.request( + `/api/v1/grants?userId=${subjectUserId}&cursor=${encodeURIComponent(issued.nextCursor)}`, + ); + expect(accepted.status).toBe(200); + expect(acceptingStore.listGrantsForUser).toHaveBeenCalledWith(subjectUserId, { + after: boundary, + limit: 50, + sourceKind: "explicit", + }); + }); +}); diff --git a/apps/api/tests/module-http-routes.test.ts b/apps/api/tests/module-http-routes.test.ts index ffe4f5c..2b95461 100644 --- a/apps/api/tests/module-http-routes.test.ts +++ b/apps/api/tests/module-http-routes.test.ts @@ -43,7 +43,7 @@ function fakeStore(): AuthorizationStore { grantPlatformOwner: vi.fn(), hasActiveGrant: vi.fn().mockResolvedValue(false), isPlatformOwner: vi.fn().mockResolvedValue(false), - listGrantsForUser: vi.fn().mockResolvedValue([]), + listGrantsForUser: vi.fn().mockResolvedValue({ grants: [] }), revokeGrant: vi.fn(), revokePlatformOwner: vi.fn(), }; diff --git a/core/src/authorization/store.ts b/core/src/authorization/store.ts index 0946539..5e30d0c 100644 --- a/core/src/authorization/store.ts +++ b/core/src/authorization/store.ts @@ -28,7 +28,10 @@ import { } from "./schema.js"; export type AuthorizationStoreErrorCode = + | "AUTHORIZATION_GRANT_ALREADY_EXISTS" | "AUTHORIZATION_GRANT_CREATE_FAILED" + | "AUTHORIZATION_GRANT_EXPIRATION_INVALID" + | "AUTHORIZATION_GRANT_LIMIT_INVALID" | "AUTHORIZATION_GRANT_LIST_FAILED" | "AUTHORIZATION_GRANT_LOOKUP_FAILED" | "AUTHORIZATION_GRANT_NOT_FOUND" @@ -38,7 +41,12 @@ export type AuthorizationStoreErrorCode = | "AUTHORIZATION_PLATFORM_OWNER_NOT_FOUND"; const storeErrorMessages: Readonly> = Object.freeze({ + AUTHORIZATION_GRANT_ALREADY_EXISTS: + "An equivalent authorization grant is already effective for this user.", AUTHORIZATION_GRANT_CREATE_FAILED: "The authorization grant could not be created.", + AUTHORIZATION_GRANT_EXPIRATION_INVALID: + "The authorization grant expiration must be a valid instant in the future.", + AUTHORIZATION_GRANT_LIMIT_INVALID: "The authorization grant page size is invalid.", AUTHORIZATION_GRANT_LIST_FAILED: "The authorization grant list could not be retrieved.", AUTHORIZATION_GRANT_LOOKUP_FAILED: "The authorization grant lookup could not be completed.", AUTHORIZATION_GRANT_NOT_FOUND: "The authorization grant was not found or is already revoked.", @@ -49,6 +57,10 @@ const storeErrorMessages: Readonly> "The platform-owner record was not found or is already revoked.", }); +/** The largest page any caller may request from `listGrantsForUser`, enforced identically by the + * store and by the HTTP surface so there is exactly one bound, not two that could drift. */ +export const AUTHORIZATION_GRANT_LIST_MAX_LIMIT = 100; + /** Never carries the underlying database error, SQL text, host, port, path, or stack. */ export class AuthorizationStoreError extends Error { readonly code: AuthorizationStoreErrorCode; @@ -101,11 +113,38 @@ export interface RevokeGrantInput { readonly sourceKind?: GrantSourceKind; } +/** One row's position in the `created_at DESC, id DESC` ordering, used as a strict keyset + * boundary. Both values are required: comparing on the timestamp alone would skip or repeat rows + * whenever two grants share a `created_at`, which concurrent creation makes routine. */ +export interface GrantPageBoundary { + readonly createdAt: Date; + readonly id: GrantId; +} + export interface ListGrantsForUserOptions { + /** Required. The maximum number of rows to return, 1 through + * `AUTHORIZATION_GRANT_LIST_MAX_LIMIT`. There is deliberately no default and no unbounded + * fallback anywhere: an unbounded listing returned a user's entire authorization history in one + * response, and a bound a caller may simply omit is not a bound. */ + readonly limit: number; + /** When provided, only grants strictly *after* this position in the ordering are returned. */ + readonly after?: GrantPageBoundary; /** When provided, only grants with this exact `sourceKind` are returned. */ readonly sourceKind?: GrantSourceKind; } +export interface GrantPage { + readonly grants: readonly GrantRecord[]; + /** The last returned row's position when more rows exist, otherwise `undefined`. */ + readonly nextBoundary?: GrantPageBoundary; +} + +export interface CreateAuthorizationStoreOptions { + /** Injected only so expiration validation is deterministic under test. Production composition + * leaves this unset and the store reads real time. */ + readonly now?: () => Date; +} + /** * Core's Postgres-backed authorization persistence: grants, platform-owner status, and the * append-only audit log, all behind one explicit factory — no environment reads, no connection @@ -117,12 +156,9 @@ export interface ListGrantsForUserOptions { export interface AuthorizationStore extends AuthorizationGrantPort, AuthorizationPlatformOwnerPort { createGrant(input: CreateGrantInput): Promise; revokeGrant(input: RevokeGrantInput): Promise; - /** Every grant ever recorded for the user, active or revoked, newest first — the caller decides - * what to display. Never includes another user's grants. */ - listGrantsForUser( - userId: EkklesiaUserId, - options?: ListGrantsForUserOptions, - ): Promise; + /** One bounded page of the user's grants, active or revoked, ordered `created_at DESC, id DESC`. + * Never includes another user's grants. */ + listGrantsForUser(userId: EkklesiaUserId, options: ListGrantsForUserOptions): Promise; grantPlatformOwner(userId: EkklesiaUserId, actor: AuthorizationActor): Promise; revokePlatformOwner(userId: EkklesiaUserId, actor: AuthorizationActor): Promise; } @@ -160,11 +196,141 @@ function toGrantRecord(row: GrantRow): GrantRecord { }); } -export function createAuthorizationStore(db: AuthorizationDrizzleDatabase): AuthorizationStore { +/** + * The five values that make two grant issuances the *same authorization source*: subject, + * permission, scope, source kind, and source reference — with `NULL` equal only to `NULL`. + * `expires_at`, creation metadata, actor metadata, and the grant id deliberately do **not** + * distinguish them: re-issuing the identical authority with a different expiry or by a different + * administrator is the same grant, not a second one. + */ +interface GrantEquivalence { + readonly userId: string; + readonly permissionId: string; + readonly scope: string | null; + readonly sourceKind: string; + readonly sourceRef: string | null; +} + +function equivalenceOf(input: CreateGrantInput): GrantEquivalence { + return { + permissionId: input.permission, + scope: input.scope ?? null, + sourceKind: input.sourceKind, + sourceRef: input.sourceRef ?? null, + userId: input.userId, + }; +} + +/** + * Serializes every create/revoke touching one equivalence tuple against a transaction-scoped + * PostgreSQL advisory lock, so concurrent callers cannot both observe "no effective equivalent" + * and both insert. A process-local mutex would not do this — nothing constrains the number of API + * processes — and neither would an unlocked check-then-insert. + * + * The key is a 64-bit hash of the tuple encoded as a JSON array, which keeps `NULL` and the empty + * string distinct and makes separator forgery through a crafted scope or source reference + * impossible. A hash collision is harmless by construction: it can only make two unrelated tuples + * serialize against each other, never make a real conflict go unnoticed, because the authoritative + * decision is always the exact SQL predicate below and never the hash. + * + * `pg_advisory_xact_lock(bigint)` uses PostgreSQL's *single-key* advisory-lock space, which is + * verified to be disjoint from the two-key space used by the migration runner + * (`core/src/migrations/postgres-database.ts`) and by platform-owner bootstrap. Grant serialization + * therefore can never block, or be blocked by, migration or bootstrap locking. Being + * transaction-scoped, it is released on COMMIT or ROLLBACK with no manual unlock. + */ +async function lockEquivalence( + tx: Parameters[0]>[0], + equivalence: GrantEquivalence, +): Promise { + await tx.execute( + sql`select pg_advisory_xact_lock(hashtextextended(jsonb_build_array( + ${equivalence.userId}::text, + ${equivalence.permissionId}::text, + ${equivalence.scope}::text, + ${equivalence.sourceKind}::text, + ${equivalence.sourceRef}::text + )::text, 0))`, + ); +} + +/** Matches exactly the rows sharing an equivalence tuple. `is not distinct from` is what gives + * `NULL = NULL` for the two nullable members; plain `=` would silently match nothing. */ +function equivalenceCondition(equivalence: GrantEquivalence) { + return and( + eq(ekklesiaCoreGrant.userId, equivalence.userId), + eq(ekklesiaCoreGrant.permissionId, equivalence.permissionId), + sql`${ekklesiaCoreGrant.scope} is not distinct from ${equivalence.scope}::text`, + eq(ekklesiaCoreGrant.sourceKind, equivalence.sourceKind), + sql`${ekklesiaCoreGrant.sourceRef} is not distinct from ${equivalence.sourceRef}::text`, + ); +} + +/** A supplied expiration must be a real instant strictly later than the acceptance clock. A past + * or equal-to-now expiration produces a grant that is dead the moment it is written, which is + * never what a caller meant; rejecting it here means trusted internal callers get the same + * guarantee the HTTP surface enforces, rather than each validating separately. */ +function assertValidExpiration(expiresAt: Date | undefined, now: Date): void { + if (expiresAt === undefined) { + return; + } + if ( + !(expiresAt instanceof Date) || + Number.isNaN(expiresAt.getTime()) || + expiresAt.getTime() <= now.getTime() + ) { + throw new AuthorizationStoreError("AUTHORIZATION_GRANT_EXPIRATION_INVALID"); + } +} + +function assertValidLimit(limit: number): number { + if ( + typeof limit !== "number" || + !Number.isSafeInteger(limit) || + limit < 1 || + limit > AUTHORIZATION_GRANT_LIST_MAX_LIMIT + ) { + throw new AuthorizationStoreError("AUTHORIZATION_GRANT_LIMIT_INVALID"); + } + return limit; +} + +export function createAuthorizationStore( + db: AuthorizationDrizzleDatabase, + options: CreateAuthorizationStoreOptions = {}, +): AuthorizationStore { + const now = options.now ?? ((): Date => new Date()); + return Object.freeze({ createGrant: async (input: CreateGrantInput): Promise => { + // Validated before any transaction opens, so a rejected expiration performs no database work + // at all and can never leave a grant or an audit row behind. + assertValidExpiration(input.expiresAt, now()); + const equivalence = equivalenceOf(input); try { return await db.transaction(async (tx) => { + await lockEquivalence(tx, equivalence); + + // Under the lock, "currently effective" is authoritative: an equivalent that is revoked, + // or whose expiry has already passed, does not block a fresh issuance. + const effective = await tx + .select({ id: ekklesiaCoreGrant.id }) + .from(ekklesiaCoreGrant) + .where( + and( + equivalenceCondition(equivalence), + isNull(ekklesiaCoreGrant.revokedAt), + or( + isNull(ekklesiaCoreGrant.expiresAt), + gt(ekklesiaCoreGrant.expiresAt, sql`now()`), + ), + ), + ) + .limit(1); + if (effective.length > 0) { + throw new AuthorizationStoreError("AUTHORIZATION_GRANT_ALREADY_EXISTS"); + } + const [row] = await tx .insert(ekklesiaCoreGrant) .values({ @@ -202,16 +368,32 @@ export function createAuthorizationStore(db: AuthorizationDrizzleDatabase): Auth } }, + /** + * Revoking one grant revokes *every* currently unrevoked issuance sharing its equivalence + * tuple, in one transaction, with one `grant.revoke` audit row per grant actually changed. + * + * Addressing a single id and revoking only that row was actively misleading: an operator who + * revoked a grant received `success` while an equivalent duplicate kept authorizing the same + * user. Expired-but-unrevoked equivalents are included deliberately — they are already + * ineffective, but leaving them unrevoked would keep the history in two states for what is one + * authorization source. Grants with a different subject, permission, scope, source kind, or + * source reference are untouched. + */ revokeGrant: async (input: RevokeGrantInput): Promise => { try { await db.transaction(async (tx) => { - const [row] = await tx - .update(ekklesiaCoreGrant) - .set({ - revokedAt: sql`now()`, - revokedByActorId: actorId(input.actor), - revokedByActorType: input.actor.type, + // The tuple columns are immutable once written, so reading them before taking the lock + // is safe; whether the target is still unrevoked is re-decided under the lock by the + // update predicate below. + const [target] = await tx + .select({ + permissionId: ekklesiaCoreGrant.permissionId, + scope: ekklesiaCoreGrant.scope, + sourceKind: ekklesiaCoreGrant.sourceKind, + sourceRef: ekklesiaCoreGrant.sourceRef, + userId: ekklesiaCoreGrant.userId, }) + .from(ekklesiaCoreGrant) .where( and( eq(ekklesiaCoreGrant.id, input.grantId), @@ -221,20 +403,38 @@ export function createAuthorizationStore(db: AuthorizationDrizzleDatabase): Auth : [eq(ekklesiaCoreGrant.sourceKind, input.sourceKind)]), ), ) - .returning(); - if (!row) { + .limit(1); + if (!target) { throw new AuthorizationStoreError("AUTHORIZATION_GRANT_NOT_FOUND"); } - await tx.insert(ekklesiaCoreAuthorizationAudit).values({ - action: "grant.revoke", - actorId: actorId(input.actor), - actorType: input.actor.type, - grantId: row.id, - permissionId: row.permissionId, - scope: row.scope, - subjectUserId: row.userId, - }); + await lockEquivalence(tx, target); + + const rows = await tx + .update(ekklesiaCoreGrant) + .set({ + revokedAt: sql`now()`, + revokedByActorId: actorId(input.actor), + revokedByActorType: input.actor.type, + }) + .where(and(equivalenceCondition(target), isNull(ekklesiaCoreGrant.revokedAt))) + .returning(); + if (rows.length === 0) { + // A concurrent revocation committed between the lookup and the lock. + throw new AuthorizationStoreError("AUTHORIZATION_GRANT_NOT_FOUND"); + } + + await tx.insert(ekklesiaCoreAuthorizationAudit).values( + rows.map((row) => ({ + action: "grant.revoke", + actorId: actorId(input.actor), + actorType: input.actor.type, + grantId: row.id, + permissionId: row.permissionId, + scope: row.scope, + subjectUserId: row.userId, + })), + ); }); } catch (error) { if (isAuthorizationStoreError(error)) { @@ -269,10 +469,21 @@ export function createAuthorizationStore(db: AuthorizationDrizzleDatabase): Auth } }, + /** + * Deterministic keyset pagination over `created_at DESC, id DESC`. The boundary is a strict + * row-value comparison on *both* columns, so grants sharing a `created_at` — which concurrent + * creation makes routine — can neither be skipped nor repeated across pages. Offset pagination + * would do exactly that under concurrent writes, which is why it is not used. + * + * At most `limit + 1` rows are read: the extra row only answers "is there another page" and is + * never returned. + */ listGrantsForUser: async ( userId: EkklesiaUserId, - options: ListGrantsForUserOptions = {}, - ): Promise => { + listOptions: ListGrantsForUserOptions, + ): Promise => { + const limit = assertValidLimit(listOptions.limit); + const after = listOptions.after; try { const rows = await db .select() @@ -280,14 +491,31 @@ export function createAuthorizationStore(db: AuthorizationDrizzleDatabase): Auth .where( and( eq(ekklesiaCoreGrant.userId, userId), - ...(options.sourceKind === undefined + ...(listOptions.sourceKind === undefined ? [] - : [eq(ekklesiaCoreGrant.sourceKind, options.sourceKind)]), + : [eq(ekklesiaCoreGrant.sourceKind, listOptions.sourceKind)]), + ...(after === undefined + ? [] + : [ + sql`(${ekklesiaCoreGrant.createdAt}, ${ekklesiaCoreGrant.id}) < (${after.createdAt}::timestamptz, ${after.id}::uuid)`, + ]), ), ) - .orderBy(desc(ekklesiaCoreGrant.createdAt)); - return Object.freeze(rows.map(toGrantRecord)); - } catch { + .orderBy(desc(ekklesiaCoreGrant.createdAt), desc(ekklesiaCoreGrant.id)) + .limit(limit + 1); + + const page = rows.slice(0, limit).map(toGrantRecord); + const last = page.at(-1); + return Object.freeze({ + grants: Object.freeze(page), + ...(rows.length > limit && last !== undefined + ? { nextBoundary: Object.freeze({ createdAt: last.createdAt, id: last.id }) } + : {}), + }); + } catch (error) { + if (isAuthorizationStoreError(error)) { + throw error; + } throw new AuthorizationStoreError("AUTHORIZATION_GRANT_LIST_FAILED"); } }, diff --git a/core/tests/authorization-store.integration.ts b/core/tests/authorization-store.integration.ts index fd8d31b..3bf02d1 100644 --- a/core/tests/authorization-store.integration.ts +++ b/core/tests/authorization-store.integration.ts @@ -9,6 +9,7 @@ import { type GrantId, parseAuthorizationScope, parseEkklesiaUserId, + parseGrantId, parsePermissionId, SYSTEM_ACTOR, } from "@ekklesia/contracts"; @@ -341,23 +342,56 @@ describe.sequential("live Core authorization store", () => { }); }); - it("denies a grant once it expires, evaluated against a real elapsed instant", async () => { - const expiringPermission = parsePermissionId("calendar.events.export"); - const past = new Date(Date.now() - 1_000); + it("refuses to create a grant that is already expired on arrival", async () => { + // The store no longer accepts a past expiration at all: such a grant is dead the moment it is + // written, which is never what a caller meant. This is validated before any transaction opens. + let captured: unknown; + try { + await store.createGrant({ + actor, + expiresAt: new Date(Date.now() - 1_000), + permission: parsePermissionId("calendar.events.export"), + sourceKind: "explicit", + userId: userOneId, + }); + } catch (error) { + captured = error; + } + expect(captured).toMatchObject({ code: "AUTHORIZATION_GRANT_EXPIRATION_INVALID" }); + expect( + await safeQuery(admin, `select id from "ekklesia_core_grant" where permission_id = $1`, [ + "calendar.events.export", + ]), + ).toHaveLength(0); + }); + + it("denies a grant once its expiry has passed, evaluated against a real elapsed instant", async () => { + const expiringPermission = parsePermissionId("calendar.events.expiring"); + const expiresAt = new Date(Date.now() + 60_000); await store.createGrant({ actor, - expiresAt: past, + expiresAt, permission: expiringPermission, sourceKind: "explicit", userId: userOneId, }); - const active = await store.hasActiveGrant({ - now: new Date(), - permission: expiringPermission, - userId: userOneId, - }); - expect(active).toBe(false); + // Deterministic rather than sleeping: the read port evaluates expiration against the instant + // it is given, so advancing that instant past the stored expiry is a real elapsed-time check. + expect( + await store.hasActiveGrant({ + now: new Date(expiresAt.getTime() - 1_000), + permission: expiringPermission, + userId: userOneId, + }), + ).toBe(true); + expect( + await store.hasActiveGrant({ + now: new Date(expiresAt.getTime() + 1_000), + permission: expiringPermission, + userId: userOneId, + }), + ).toBe(false); }); it("allows a grant that has not yet expired", async () => { @@ -539,3 +573,515 @@ describe.sequential("live Core authorization store — no automatic migration", } }); }); + +/** + * Pass 3 grant-integrity scenarios, all against real PostgreSQL because every property under test + * is a database-concurrency or database-constraint property that a fake cannot establish. + * + * The administrative client each scenario uses (`openAdministrativeClient`) is **test-only + * provisioning and inspection infrastructure**: it seeds users, plants legacy duplicate rows that + * the production store would now refuse to create, and reads back the resulting state. It is never + * an API pool or a runtime connection, and it is closed in `afterAll` along with its disposable + * database. + */ +describe.sequential("live Core authorization store — grant equivalence and revocation", () => { + let database: { name: string; url: string }; + let admin: Client; + let store: ReturnType; + let close: () => Promise; + + const subject = parseEkklesiaUserId("equiv_subject"); + const other = parseEkklesiaUserId("equiv_other"); + const actor = createUserActor("equiv_other"); + const permission = parsePermissionId("calendar.events.read"); + + async function effectiveCount(userId: string, permissionId: string): Promise { + const rows = await safeQuery<{ n: string }>( + admin, + `select count(*)::int as n from "ekklesia_core_grant" + where user_id = $1 and permission_id = $2 and revoked_at is null + and (expires_at is null or expires_at > now())`, + [userId, permissionId], + ); + return Number(rows[0]?.n ?? 0); + } + + it("prepares a disposable database with the three Core migrations applied", async () => { + database = await createDisposableDatabase("equivalence"); + await applyCoreMigrations(database.url); + admin = await openAdministrativeClient(database.url); + await seedUser(admin, subject, "equiv-subject@example.test"); + await seedUser(admin, other, "equiv-other@example.test"); + const handle = createAuthorizationDrizzleDatabase(database.url); + close = handle.close; + store = createAuthorizationStore(handle.db); + expect(await effectiveCount(subject, permission)).toBe(0); + }); + + it("creates the first unscoped explicit grant and writes exactly one create audit row", async () => { + const grant = await store.createGrant({ + actor, + permission, + sourceKind: "explicit", + userId: subject, + }); + + expect(grant.scope).toBeUndefined(); + const audit = await safeQuery<{ action: string }>( + admin, + `select action from "ekklesia_core_authorization_audit" where grant_id = $1`, + [grant.id], + ); + expect(audit.map((row) => row.action)).toEqual(["grant.create"]); + }); + + it("refuses a sequential equivalent creation, leaving no second grant and no second audit row", async () => { + const auditBefore = await safeQuery( + admin, + `select id from "ekklesia_core_authorization_audit" where subject_user_id = $1 and permission_id = $2`, + [subject, permission], + ); + + let captured: unknown; + try { + await store.createGrant({ actor, permission, sourceKind: "explicit", userId: subject }); + } catch (error) { + captured = error; + } + + expect(captured).toBeInstanceOf(AuthorizationStoreError); + expect(captured).toMatchObject({ code: "AUTHORIZATION_GRANT_ALREADY_EXISTS" }); + expect(await effectiveCount(subject, permission)).toBe(1); + const auditAfter = await safeQuery( + admin, + `select id from "ekklesia_core_authorization_audit" where subject_user_id = $1 and permission_id = $2`, + [subject, permission], + ); + expect(auditAfter).toHaveLength(auditBefore.length); + }); + + it("a differing expiration does not make an otherwise identical issuance distinct", async () => { + let captured: unknown; + try { + await store.createGrant({ + actor, + expiresAt: new Date(Date.now() + 3_600_000), + permission, + sourceKind: "explicit", + userId: subject, + }); + } catch (error) { + captured = error; + } + expect(captured).toMatchObject({ code: "AUTHORIZATION_GRANT_ALREADY_EXISTS" }); + expect(await effectiveCount(subject, permission)).toBe(1); + }); + + it("a different subject, permission, scope, source kind, or source reference is not equivalent", async () => { + const distinct = [ + { permission, sourceKind: "explicit" as const, userId: other }, + { + permission: parsePermissionId("calendar.events.write"), + sourceKind: "explicit" as const, + userId: subject, + }, + { + permission, + scope: parseAuthorizationScope("calendar-event:1"), + sourceKind: "explicit" as const, + userId: subject, + }, + { permission, sourceKind: "relationship" as const, userId: subject }, + { + permission, + sourceKind: "relationship" as const, + sourceRef: "roster:7", + userId: subject, + }, + ]; + for (const input of distinct) { + const grant = await store.createGrant({ actor, ...input }); + expect(grant.id).toBeDefined(); + } + // A second issuance of one of those distinct tuples is itself a duplicate. + let captured: unknown; + try { + await store.createGrant({ + actor, + permission, + scope: parseAuthorizationScope("calendar-event:1"), + sourceKind: "explicit", + userId: subject, + }); + } catch (error) { + captured = error; + } + expect(captured).toMatchObject({ code: "AUTHORIZATION_GRANT_ALREADY_EXISTS" }); + }); + + it("serializes concurrent equivalent creations to exactly one grant and one conflict", async () => { + const concurrentPermission = parsePermissionId("calendar.events.concurrent"); + // Six genuinely concurrent transactions against the same tuple. The transaction-scoped + // advisory lock is what makes this deterministic: no sleeps, no retries, no process-local + // mutex — a second API process would be serialized identically. + const results = await Promise.allSettled( + Array.from({ length: 6 }, () => + store.createGrant({ + actor, + permission: concurrentPermission, + sourceKind: "explicit", + userId: subject, + }), + ), + ); + + const fulfilled = results.filter((result) => result.status === "fulfilled"); + const rejected = results.filter((result) => result.status === "rejected"); + expect(fulfilled).toHaveLength(1); + expect(rejected).toHaveLength(5); + for (const result of rejected) { + expect((result as PromiseRejectedResult).reason).toMatchObject({ + code: "AUTHORIZATION_GRANT_ALREADY_EXISTS", + }); + } + expect(await effectiveCount(subject, concurrentPermission)).toBe(1); + const audit = await safeQuery( + admin, + `select id from "ekklesia_core_authorization_audit" + where subject_user_id = $1 and permission_id = $2 and action = 'grant.create'`, + [subject, concurrentPermission], + ); + expect(audit).toHaveLength(1); + }); + + it("allows a fresh issuance once the previous equivalent is revoked, and again once one expires", async () => { + const cyclePermission = parsePermissionId("calendar.events.cycle"); + const first = await store.createGrant({ + actor, + permission: cyclePermission, + sourceKind: "explicit", + userId: subject, + }); + await store.revokeGrant({ actor, grantId: first.id, sourceKind: "explicit" }); + + // Revoked equivalents do not block a new issuance. + const second = await store.createGrant({ + actor, + expiresAt: new Date(Date.now() + 60_000), + permission: cyclePermission, + sourceKind: "explicit", + userId: subject, + }); + expect(second.id).not.toBe(first.id); + + // Force the second grant past its expiry through the administrative client, which is the only + // deterministic way to observe "expired equivalent" without sleeping. + await safeQuery( + admin, + `update "ekklesia_core_grant" set expires_at = now() - interval '1 second' where id = $1`, + [second.id], + ); + const third = await store.createGrant({ + actor, + permission: cyclePermission, + sourceKind: "explicit", + userId: subject, + }); + expect(third.id).not.toBe(second.id); + expect(await effectiveCount(subject, cyclePermission)).toBe(1); + }); + + it("one revocation revokes every unrevoked equivalent, audits each by its own id, and leaves nothing authorizing", async () => { + const legacyPermission = parsePermissionId("calendar.events.legacy"); + // Plant the exact state the old store could produce: several equivalent issuances, including + // one already expired but never revoked, plus two deliberately non-equivalent neighbours. + const legacyIds: string[] = []; + for (const expires of ["null", "now() + interval '1 hour'", "now() - interval '1 hour'"]) { + const rows = await safeQuery<{ id: string }>( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id, expires_at) + values ($1, $2, null, 'explicit', null, 'user', $3, ${expires}) returning id`, + [subject, legacyPermission, other], + ); + legacyIds.push(rows[0]?.id as string); + } + const scopedNeighbour = await safeQuery<{ id: string }>( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id) + values ($1, $2, 'calendar-event:9', 'explicit', null, 'user', $3) returning id`, + [subject, legacyPermission, other], + ); + // Deliberately on its own permission: an unscoped relationship grant for `legacyPermission` + // would legitimately keep authorizing after this revocation (it is a different equivalence + // tuple), which would make the "nothing authorizes" assertion below meaningless rather than + // wrong. The scoped neighbour above stays on the same permission because a scoped grant never + // satisfies an unscoped check. + const relationshipNeighbour = await safeQuery<{ id: string }>( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id) + values ($1, 'calendar.events.legacyrel', null, 'relationship', 'roster:9', 'user', $2) returning id`, + [subject, other], + ); + expect(await effectiveCount(subject, legacyPermission)).toBe(3); + + await store.revokeGrant({ + actor, + grantId: parseGrantId(legacyIds[0] as string), + sourceKind: "explicit", + }); + + const revoked = await safeQuery<{ id: string; revoked_at: Date | null }>( + admin, + `select id, revoked_at from "ekklesia_core_grant" where id = any($1::uuid[])`, + [legacyIds], + ); + // Every unrevoked equivalent, including the expired-but-unrevoked one, is now revoked. + expect(revoked.every((row) => row.revoked_at !== null)).toBe(true); + + const auditRows = await safeQuery<{ grant_id: string }>( + admin, + `select grant_id from "ekklesia_core_authorization_audit" + where action = 'grant.revoke' and grant_id = any($1::uuid[])`, + [legacyIds], + ); + // One audit row per grant actually revoked, each carrying that row's own id. + expect(auditRows).toHaveLength(legacyIds.length); + expect([...new Set(auditRows.map((row) => row.grant_id))].sort()).toEqual( + [...legacyIds].sort(), + ); + + // Differently scoped and differently sourced grants are untouched. + const untouched = await safeQuery<{ revoked_at: Date | null }>( + admin, + `select revoked_at from "ekklesia_core_grant" where id = any($1::uuid[])`, + [[scopedNeighbour[0]?.id, relationshipNeighbour[0]?.id]], + ); + expect(untouched.every((row) => row.revoked_at === null)).toBe(true); + + // No equivalent issuance continues authorizing the user. + expect( + await store.hasActiveGrant({ + now: new Date(), + permission: legacyPermission, + userId: subject, + }), + ).toBe(false); + }); + + it("serializes a concurrent create and revoke on the same tuple without leaving an unexplained duplicate", async () => { + const racePermission = parsePermissionId("calendar.events.race"); + const seed = await store.createGrant({ + actor, + permission: racePermission, + sourceKind: "explicit", + userId: subject, + }); + + const [revokeResult, createResult] = await Promise.allSettled([ + store.revokeGrant({ actor, grantId: seed.id, sourceKind: "explicit" }), + store.createGrant({ + actor, + permission: racePermission, + sourceKind: "explicit", + userId: subject, + }), + ]); + + // Whichever order the two transactions serialize in, the end state is consistent with that + // order: either the create lost to an effective equivalent (0 effective), or it ran after the + // revoke (1 effective). Two effective equivalents is the state this pass makes impossible. + expect(revokeResult.status).toBe("fulfilled"); + const effective = await effectiveCount(subject, racePermission); + expect(effective).toBeLessThanOrEqual(1); + if (createResult.status === "rejected") { + expect(createResult.reason).toMatchObject({ code: "AUTHORIZATION_GRANT_ALREADY_EXISTS" }); + expect(effective).toBe(0); + } else { + expect(effective).toBe(1); + } + }); + + it("rolls back every grant and audit change when the multi-row revocation transaction fails", async () => { + const doomedPermission = parsePermissionId("calendar.events.doomed"); + const rows = await safeQuery<{ id: string }>( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id) + values ($1, $2, null, 'explicit', null, 'user', $3), ($1, $2, null, 'explicit', null, 'user', $3) + returning id`, + [subject, doomedPermission, other], + ); + // Force the audit insert inside the revoke transaction to violate a real constraint: the audit + // action CHECK only permits the four known actions, so a temporary trigger that rewrites the + // action makes the whole multi-row revocation fail after the grant updates were staged. + await safeQuery( + admin, + `create function ekklesia_test_break_audit() returns trigger as $$ + begin new.action := 'not.a.real.action'; return new; end; + $$ language plpgsql`, + ); + await safeQuery( + admin, + `create trigger ekklesia_test_break_audit_trigger before insert on "ekklesia_core_authorization_audit" + for each row execute function ekklesia_test_break_audit()`, + ); + try { + let captured: unknown; + try { + await store.revokeGrant({ + actor, + grantId: parseGrantId(rows[0]?.id as string), + sourceKind: "explicit", + }); + } catch (error) { + captured = error; + } + expect(captured).toMatchObject({ code: "AUTHORIZATION_GRANT_REVOKE_FAILED" }); + + const after = await safeQuery<{ revoked_at: Date | null }>( + admin, + `select revoked_at from "ekklesia_core_grant" where id = any($1::uuid[])`, + [rows.map((row) => row.id)], + ); + expect(after.every((row) => row.revoked_at === null)).toBe(true); + const audit = await safeQuery( + admin, + `select id from "ekklesia_core_authorization_audit" where permission_id = $1`, + [doomedPermission], + ); + expect(audit).toHaveLength(0); + } finally { + // Always restored, even when an assertion above fails. + await safeQuery( + admin, + `drop trigger if exists ekklesia_test_break_audit_trigger on "ekklesia_core_authorization_audit"`, + ); + await safeQuery(admin, `drop function if exists ekklesia_test_break_audit()`); + } + }); + + it("reports duplicate conflict, not-found, and infrastructure failure as three distinct sanitized outcomes", async () => { + const codes: string[] = []; + try { + await store.createGrant({ actor, permission, sourceKind: "explicit", userId: subject }); + } catch (error) { + codes.push((error as AuthorizationStoreError).code); + } + try { + await store.revokeGrant({ + actor, + grantId: parseGrantId("00000000-0000-4000-8000-000000000000"), + sourceKind: "explicit", + }); + } catch (error) { + codes.push((error as AuthorizationStoreError).code); + } + await close(); + try { + await store.hasActiveGrant({ now: new Date(), permission, userId: subject }); + } catch (error) { + codes.push((error as AuthorizationStoreError).code); + } + + expect(codes).toEqual([ + "AUTHORIZATION_GRANT_ALREADY_EXISTS", + "AUTHORIZATION_GRANT_NOT_FOUND", + "AUTHORIZATION_GRANT_LOOKUP_FAILED", + ]); + await safeEnd(admin); + }); +}); + +describe.sequential("live Core authorization store — bounded keyset pagination", () => { + let database: { name: string; url: string }; + let admin: Client; + let store: ReturnType; + let close: () => Promise; + + const subject = parseEkklesiaUserId("page_subject"); + + it("seeds explicit and relationship grants, including several sharing one created_at", async () => { + database = await createDisposableDatabase("pagination"); + await applyCoreMigrations(database.url); + admin = await openAdministrativeClient(database.url); + await seedUser(admin, subject, "page-subject@example.test"); + const handle = createAuthorizationDrizzleDatabase(database.url); + close = handle.close; + store = createAuthorizationStore(handle.db); + + // Seven explicit grants all sharing one identical `created_at`, which is exactly the case a + // timestamp-only keyset boundary would skip or repeat, plus relationship grants that must + // never appear on any page. + await safeQuery( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id, created_at) + select $1, 'calendar.events.p' || generation, null, 'explicit', null, 'user', $1, + timestamptz '2026-01-01 00:00:00Z' + from generate_series(1, 7) as generation`, + [subject], + ); + await safeQuery( + admin, + `insert into "ekklesia_core_grant" + (user_id, permission_id, scope, source_kind, source_ref, created_by_actor_type, created_by_actor_id, created_at) + select $1, 'calendar.events.r' || generation, null, 'relationship', 'roster:' || generation, 'user', $1, + timestamptz '2026-01-01 00:00:00Z' + from generate_series(1, 3) as generation`, + [subject], + ); + expect( + await safeQuery(admin, `select id from "ekklesia_core_grant" where user_id = $1`, [subject]), + ).toHaveLength(10); + }); + + it("traverses every explicit grant exactly once across pages, excluding relationship grants", async () => { + const seen: string[] = []; + let after: { createdAt: Date; id: GrantId } | undefined; + let pages = 0; + for (;;) { + const page = await store.listGrantsForUser(subject, { + ...(after === undefined ? {} : { after }), + limit: 3, + sourceKind: "explicit", + }); + pages += 1; + seen.push(...page.grants.map((grant) => grant.id)); + expect(page.grants.every((grant) => grant.sourceKind === "explicit")).toBe(true); + if (page.nextBoundary === undefined) { + expect(page.grants.length).toBeLessThanOrEqual(3); + break; + } + after = page.nextBoundary; + expect(pages).toBeLessThan(10); + } + + // 7 explicit rows at limit 3 → pages of 3, 3, 1. No duplicates, no omissions, deterministic + // despite every row sharing one created_at. + expect(pages).toBe(3); + expect(seen).toHaveLength(7); + expect(new Set(seen).size).toBe(7); + // Ordering is created_at DESC, id DESC — with equal timestamps that is a pure descending id + // sort, which is exactly what makes traversal total and repeatable. + expect([...seen].sort().reverse()).toEqual(seen); + }); + + it("honours the 1 and 100 boundaries and rejects anything outside them", async () => { + expect( + (await store.listGrantsForUser(subject, { limit: 1, sourceKind: "explicit" })).grants, + ).toHaveLength(1); + const all = await store.listGrantsForUser(subject, { limit: 100, sourceKind: "explicit" }); + expect(all.grants).toHaveLength(7); + expect(all.nextBoundary).toBeUndefined(); + for (const limit of [0, 101, -1, 1.5]) { + await expect(store.listGrantsForUser(subject, { limit })).rejects.toMatchObject({ + code: "AUTHORIZATION_GRANT_LIMIT_INVALID", + }); + } + await close(); + await safeEnd(admin); + }); +}); diff --git a/core/tests/authorization-store.test.ts b/core/tests/authorization-store.test.ts index 865067d..a9f3f2b 100644 --- a/core/tests/authorization-store.test.ts +++ b/core/tests/authorization-store.test.ts @@ -4,8 +4,13 @@ const poolConstructor = vi.hoisted(() => vi.fn()); vi.mock("pg", () => ({ Pool: poolConstructor })); +import { parseEkklesiaUserId, parsePermissionId, SYSTEM_ACTOR } from "@ekklesia/contracts"; import { createAuthorizationDrizzleDatabase } from "../src/authorization/database.js"; -import { AuthorizationStoreError, createAuthorizationStore } from "../src/authorization/store.js"; +import { + AUTHORIZATION_GRANT_LIST_MAX_LIMIT, + AuthorizationStoreError, + createAuthorizationStore, +} from "../src/authorization/store.js"; describe("Core authorization store — import and construction safety", () => { it("does not construct a pool or open a connection merely because the module is imported", () => { @@ -33,7 +38,11 @@ describe("Core authorization store — import and construction safety", () => { describe("AuthorizationStoreError", () => { it.each([ + "AUTHORIZATION_GRANT_ALREADY_EXISTS", "AUTHORIZATION_GRANT_CREATE_FAILED", + "AUTHORIZATION_GRANT_EXPIRATION_INVALID", + "AUTHORIZATION_GRANT_LIMIT_INVALID", + "AUTHORIZATION_GRANT_LIST_FAILED", "AUTHORIZATION_GRANT_LOOKUP_FAILED", "AUTHORIZATION_GRANT_NOT_FOUND", "AUTHORIZATION_GRANT_REVOKE_FAILED", @@ -56,3 +65,71 @@ describe("AuthorizationStoreError", () => { ); }); }); + +/** + * Input validation the store performs *before* any transaction opens, so a rejected value performs + * no database work at all. The fake database below records every call it receives; asserting it + * stayed empty is what proves "no grant and no audit row" rather than merely "an error was + * thrown". `createAuthorizationStore`'s injectable clock keeps this deterministic without + * arbitrary sleeps or real-time flakiness. + */ +describe("Core authorization store — pre-database input validation", () => { + const fixedNow = new Date("2026-07-26T12:00:00.000Z"); + + function recordingStore() { + const calls: string[] = []; + poolConstructor.mockImplementation(function fakePool() { + return { end: async () => undefined, on: vi.fn(), query: vi.fn() }; + }); + const { db } = createAuthorizationDrizzleDatabase("postgresql://example.test/ekklesia"); + const instrumented = new Proxy(db, { + get(target, property, receiver) { + if (property === "transaction" || property === "select" || property === "execute") { + calls.push(String(property)); + } + return Reflect.get(target, property, receiver) as unknown; + }, + }); + return { calls, store: createAuthorizationStore(instrumented, { now: () => fixedNow }) }; + } + + it.each([ + ["a past instant", new Date("2020-01-01T00:00:00.000Z")], + ["exactly the acceptance clock", new Date("2026-07-26T12:00:00.000Z")], + ["an invalid Date", new Date("nonsense")], + ])("rejects %s expiration with no database work at all", async (_label, expiresAt) => { + const { calls, store } = recordingStore(); + + await expect( + store.createGrant({ + actor: SYSTEM_ACTOR, + expiresAt, + permission: parsePermissionId("calendar.events.read"), + sourceKind: "explicit", + userId: parseEkklesiaUserId("subject_1"), + }), + ).rejects.toMatchObject({ code: "AUTHORIZATION_GRANT_EXPIRATION_INVALID" }); + expect(calls).toEqual([]); + }); + + it.each([ + 0, + -1, + 101, + 1.5, + Number.NaN, + Number.POSITIVE_INFINITY, + Number.MAX_SAFE_INTEGER + 2, + ])("rejects the unusable page size %s with no database work at all", async (limit) => { + const { calls, store } = recordingStore(); + + await expect( + store.listGrantsForUser(parseEkklesiaUserId("subject_1"), { limit }), + ).rejects.toMatchObject({ code: "AUTHORIZATION_GRANT_LIMIT_INVALID" }); + expect(calls).toEqual([]); + }); + + it("exposes one shared maximum page size rather than two that could drift", () => { + expect(AUTHORIZATION_GRANT_LIST_MAX_LIMIT).toBe(100); + }); +}); diff --git a/docs/architecture/decisions/0013-core-authorization-and-owner-bootstrap.md b/docs/architecture/decisions/0013-core-authorization-and-owner-bootstrap.md index 8634fee..ccd32bd 100644 --- a/docs/architecture/decisions/0013-core-authorization-and-owner-bootstrap.md +++ b/docs/architecture/decisions/0013-core-authorization-and-owner-bootstrap.md @@ -129,6 +129,17 @@ touching grant semantics. ### Platform-owner bootstrap is explicit, CLI-only, globally single-use, permanently consumed even after revocation, concurrency-safe, and audited atomically +> **Partial correction — see [ADR 0019](0019-durable-authorization-history-and-grant-management.md).** +> Defining "bootstrap has already been consumed" as *any row ever created in +> `ekklesia_core_platform_owner`*, described immediately below, was not durable: that table's +> `user_id` cascades on user deletion, so deleting the bootstrapped owner's user row removed the +> owner row and reopened the one-time operation. ADR 0019 moves that authority to a dedicated +> `ekklesia_core_platform_owner_bootstrap` singleton with no user foreign key, backfilled for +> existing installations. The intent recorded here — explicit, CLI-only, globally single-use, +> permanently consumed even after revocation, concurrency-safe, and audited atomically — is +> unchanged and still accepted; only the marker it is read from moved. This section is left as +> originally accepted, as the historical record. + There is no signup-order or first-user inference anywhere in this repository. The only way to establish a platform owner is `pnpm authorization:bootstrap-owner -- --user-id --confirm-user-id ` (`core/src/cli/bootstrap-platform-owner.ts`, diff --git a/docs/architecture/decisions/0019-durable-authorization-history-and-grant-management.md b/docs/architecture/decisions/0019-durable-authorization-history-and-grant-management.md new file mode 100644 index 0000000..65d55ec --- /dev/null +++ b/docs/architecture/decisions/0019-durable-authorization-history-and-grant-management.md @@ -0,0 +1,270 @@ +# 0019: Durable authorization history and grant management + +- Status: Accepted +- Date: 2026-07-26 +- Amends: [0013](0013-core-authorization-and-owner-bootstrap.md) +- Extends: [0017](0017-public-request-perimeter-hardening.md), [0018](0018-identity-session-resolution-and-database-transport.md) + +## Context + +Phase 5D passes 1 and 2 closed the public request perimeter and the identity/database transport +behind it. Pass 3 addresses the authorization layer those protect: whether security history +survives, whether the one-time bootstrap that creates a platform's first administrator is really +one-time, and whether explicit-grant creation, revocation, validation, and listing are unambiguous +and bounded. Ten defects were reproduced against the accepted pass-2 tree before anything changed. + +**Security history did not survive.** `ekklesia_core_authorization_audit.subject_user_id` carried a +cascading foreign key to the live Better Auth user row. Deleting a user with two audit rows left +zero: the record of what that user had been granted, by whom, and when it was revoked disappeared +at precisely the moment it mattered most. + +**Bootstrap's single use was not durable.** "Already consumed" meant "a row exists in +`ekklesia_core_platform_owner`" — and that table also cascades on user deletion. Deleting the +bootstrapped owner's user row removed the owner row *and* its audit evidence, after which +`bootstrapPlatformOwner` succeeded again for a different user. There was no state independent of +user lifecycle recording that bootstrap had ever run. + +**Equivalent grants multiplied silently.** Two identical `createGrant` calls produced two grants; +five concurrent identical calls produced five. Revoking one returned `success` while an equivalent +issuance kept authorizing the same user — an operator could believe they had removed access that +was still live. + +**The request boundary leaked into the database.** The generic HTTP API accepted a `scope` even +though no route middleware supplies one to the evaluator, so a scoped grant persisted, appeared in +listings, and could authorize nothing. `GrantId` accepted any 1–128-character string, so a +malformed path parameter reached PostgreSQL's own UUID comparison and returned an indistinguishable +`503`. `expiresAt` went through bare `new Date()`, which silently normalized `2026-02-31` to March +3rd and accepted `"March 5, 2027"`, date-only, offset, and millisecond-free forms; a past +expiration was persisted as a grant that was dead on arrival. Listing had no bound at all and +returned a user's entire authorization history in one response. + +## Decision + +### Security history is retained, not referential + +`ekklesia_core_authorization_audit.subject_user_id` stays a non-null `text` column and loses its +foreign key to `ekklesia_core_user`. It is now a **historical snapshot**: an identifier that may +name a user row which no longer exists. No name, email, or other profile data is copied into +authorization tables — a deleted user's audit history says what happened, not who they were. The +subject index is preserved, and the Drizzle relation to the user table is deliberately removed +because modelling a retained snapshot as a live relation would misdescribe the data. + +Live rows still cascade on user deletion, which is intended: deleting a user removes their grants +and their platform-owner row, so nothing continues authorizing a principal that no longer exists. +Only the *history* is exempt. + +**Append-only remains an application/store convention, not a database guarantee.** Core's +authorization store exposes no update or delete path for audit rows. Nothing in PostgreSQL enforces +that: no role, trigger, or rule prevents a database administrator, a future migration, or anything +else holding the same credentials from modifying them. This ADR claims exactly the property that +exists. + +### Bootstrap consumption is its own durable record + +`ekklesia_core_platform_owner_bootstrap` is a dedicated single-row table meaning "platform-owner +bootstrap was successfully consumed." Its `id` is pinned by a check constraint to the literal +`'singleton'`, so the primary key makes a second row structurally impossible rather than merely +unwritten by convention. It has **no foreign key to any user**, and Core never deletes from it, so +it survives owner revocation and user deletion alike. An optional `consumed_for_user_id` records +which user the original bootstrap made an owner, in the same retained-identifier style as the audit +subject. + +`bootstrapPlatformOwner` consults and claims that record — never the continued existence of a +platform-owner row. The claim, the target-user check, the owner insert, and the audit entry are one +transaction, serialized by the existing transaction-scoped advisory lock, so exactly one of +concurrent first attempts succeeds and the other receives +`PLATFORM_OWNER_BOOTSTRAP_ALREADY_CONSUMED`. Consumption is tied to *success*: a missing target +user (`PLATFORM_OWNER_BOOTSTRAP_USER_NOT_FOUND`), a constraint violation, or any other failure +rolls the sentinel back with everything else, leaving the one legitimate first use available. + +There is deliberately no reset command, deletion path, HTTP endpoint, environment override, force +flag, or second bootstrap mechanism. The operational consequence is explicit: **deleting the last +platform owner does not reactivate bootstrap.** Re-establishing an owner after that is a +separately designed owner-management flow that does not exist yet. + +Core migration `0002_authorization_integrity` backfills the sentinel for any installation that +already holds a platform-owner row — active or revoked — so an existing deployment cannot gain a +fresh first use by upgrading. It deliberately leaves `consumed_for_user_id` null, because a backfill +cannot know which of several historical owner rows the original single use targeted and guessing +would fabricate history. An installation that never had an owner is left untouched, because a +legitimately never-bootstrapped deployment must retain its one first use. + +**One upgrade limitation is irrecoverable and is stated rather than papered over.** If an operator +manually deleted the bootstrapped owner's user row *before* this migration, the old cascade already +erased both the owner row and its audit evidence, and the migration has nothing left to detect. Such +an installation backfills as never-bootstrapped and retains one bootstrap use. At the accepted +checkpoint no supported Better Auth or API user-deletion path exists that could produce this state; +it requires direct database access. The migration cannot reconstruct an event for which no durable +evidence remains, and does not pretend to. + +### Grant equivalence is exactly five values + +Two grant issuances are the same authorization source when all five are equal, with `NULL` equal +only to `NULL`: + +1. `user_id` +2. `permission_id` +3. `scope` +4. `source_kind` +5. `source_ref` + +`expires_at`, creation metadata, actor metadata, and the grant id deliberately do **not** +distinguish them. Re-issuing the identical authority with a different expiry, or by a different +administrator, is the same grant — not a second one. + +Creation fails with the stable `AUTHORIZATION_GRANT_ALREADY_EXISTS` when an equivalent grant is +currently effective (`revoked_at IS NULL` and `expires_at IS NULL OR expires_at > now()`). A revoked +or already-expired equivalent does not block a fresh issuance. A rejected duplicate creates no grant +and no audit row. + +**Conflict, not idempotent return.** Returning the existing grant would silently answer a different +question than the caller asked — the caller wanted a new authorization recorded with their own +actor and expiry, and telling them "done" while recording neither is the ambiguity this pass exists +to remove. An explicit 409 lets an operator see that the authority already exists and decide. + +### Serialization is database-backed, not process-local + +Every create and revoke touching one equivalence tuple takes a transaction-scoped +`pg_advisory_xact_lock` keyed by a 64-bit hash of that tuple encoded as a JSON array — which keeps +`NULL` distinct from the empty string and makes separator forgery through a crafted scope or source +reference impossible. A hash collision is harmless by construction: it can only make two unrelated +tuples serialize against each other, never make a real conflict go unnoticed, because the +authoritative decision is always an exact SQL predicate and never the hash. + +A process-local mutex would not do this — nothing constrains the number of API processes — and +neither would an unlocked check-then-insert. `pg_advisory_xact_lock(bigint)` uses PostgreSQL's +single-key advisory-lock space, verified disjoint from the two-key space the migration runner and +platform-owner bootstrap use, so grant serialization can never block or be blocked by either. + +A **time-dependent partial unique index** (`... WHERE revoked_at IS NULL AND expires_at > now()`) +was rejected outright: `now()` is not immutable, so PostgreSQL cannot index on it, and any +approximation would either over-constrain legitimate re-issuance after expiry or silently stop +enforcing. + +### Revocation resolves the whole equivalence class + +Revocation is still addressed by one grant id and retains the explicit-versus-relationship source +guard. Once the target is found, one successful revocation revokes **every** currently unrevoked +issuance sharing its equivalence tuple — including expired-but-unrevoked legacy equivalents, so the +history is left in one unambiguous state — and inserts one `grant.revoke` audit row per grant +actually revoked, each carrying that row's own grant id. All updates and audit inserts are one +transaction. Unknown, already-revoked, and wrong-source ids keep the existing not-found behavior, +and the public success response remains `200 {"success":true}`. + +Historical duplicates are **not** silently revoked by migration. The store now prevents creating +another effective equivalent, and revocation removes all equivalents when an operator explicitly +revokes one. Choosing a historical "winner" or changing live authorization inside a data migration, +with no operator action, is not something this pass will do. + +### The generic HTTP surface creates unscoped explicit grants only + +`POST /api/v1/grants` rejects the presence of a `scope` field — including `null`, empty, malformed, +and otherwise valid scope strings — with `400 {"code":"INVALID_REQUEST"}` before any store mutation. +Core's `AuthorizationScope`, the evaluator's exact-scope semantics, and the store's scope column are +all unchanged and remain available to trusted internal callers and to future server-owned scope +work; listing and serialization still show a scope on historical or internally created grants. + +No route-parameter-to-scope resolver is added in this pass, and no module route declaration or +handler context changes. Accepting a scope that nothing can evaluate would be offering a switch that +is not connected to anything. + +### Canonical identifiers and future-only expiration + +`GrantId` accepts exactly the canonical lowercase hyphenated UUID PostgreSQL emits (`8-4-4-4-12` +lowercase hexadecimal), with no trimming. Uppercase, whitespace-padded, shortened, overlong, and +non-UUID values are rejected rather than repaired, so exactly one spelling of any grant is valid and +a malformed id is a `400` decided after authorization but strictly before any store or database +call. A canonical id that does not exist still returns the sanitized `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 later than the authoritative acceptance clock — equal-to- +now and past are invalid. The round-trip comparison is what rejects normalized impossible dates. +The Core store independently rejects an invalid or non-future `Date` before any SQL, using the +stable internal `AUTHORIZATION_GRANT_EXPIRATION_INVALID`, so trusted internal callers get the same +guarantee; the public HTTP response stays exactly `400 {"code":"INVALID_REQUEST"}` with +`Cache-Control: no-store`. A rejected expiration creates no grant and no audit row. A deterministic +clock is injectable only at the narrow Core store and API route factory seams needed for tests; +production leaves it unset. No maximum lifetime, renewal workflow, scheduler, or background +expiration job is introduced. + +### Listing is bounded keyset pagination with an opaque, unsigned cursor + +`GET /api/v1/grants?userId=&limit=<1..100>&cursor=` returns +`{"grants": [...], "nextCursor": string | null}`. `limit` defaults to 50 and must be a canonical +base-10 integer; unknown query keys, duplicated keys, empty values, non-canonical numbers, and +malformed cursors are rejected with `400 {"code":"INVALID_REQUEST"}` before any store or database +work. Ordering is `created_at DESC, id DESC` with a strict row-value keyset boundary on **both** +columns, so rows sharing a `created_at` — which concurrent creation makes routine — can neither be +skipped nor repeated. At most `limit + 1` rows are read and at most `limit` returned. +`sourceKind: "explicit"` filtering stays server-side on every page. Offset pagination was rejected +because it does exactly the skipping and repeating this ordering avoids. + +The cursor is a small versioned payload containing only the subject user id, boundary creation +timestamp, and boundary grant id, encoded as canonical unpadded base64url. Decoding is strict in +every dimension — exact version, exact keys, matching requested subject, canonical `toISOString()` +timestamp, canonical `GrantId`, and a re-encoding that reproduces the supplied string byte-for-byte +— and a rejected cursor reveals nothing it decoded. + +**The cursor is a pagination position, not an authorization token.** It is deliberately unsigned: +the permission check and the server-side subject filter remain authoritative on every page, so a +forged cursor can at most name a different position within grants the caller is already entitled to +list. It carries no permission, scope, source reference, actor data, credential, or secret, and it +is not a consistency snapshot — rows created or revoked between pages are simply seen or not seen +according to the ordering. + +Core's list method requires and validates a finite bound; no internal call site retains an unbounded +fallback. No total count, offset pagination, search, sorting options, or filtering beyond the +existing subject and explicit-source restriction is added. + +## Alternatives + +- **Keeping the audit foreign key and adding `ON DELETE SET NULL`** was rejected: a null subject is + not history, and the column's whole purpose is naming who the security event concerned. +- **Copying the user's name or email into the audit row so history stays readable** was rejected: it + converts an authorization table into a shadow profile store and would survive a deletion the user + may have requested. +- **Treating "an owner row exists" as consumed, but blocking the cascade instead** was rejected: an + owner row is live authorization state that an operator may legitimately want removed, while the + fact that bootstrap ran is a historical event. Conflating the two is what produced the defect. +- **A `--force`/reset path for bootstrap** was rejected as directly reintroducing the vulnerability + under a different name. +- **Guessing "consumed" for every upgraded database** was rejected: it would burn the one legitimate + first use of a never-bootstrapped installation. +- **Idempotently returning the existing grant on duplicate creation** — see above. +- **A partial unique index over the equivalence tuple** was rejected for the `now()` immutability + reason above; the advisory lock covers the complete tuple with no time dependence. +- **Revoking only the addressed grant and reporting how many equivalents remain** was rejected: it + puts the burden of resolving an ambiguous state on the operator, in a response that would have to + disclose other grants' existence. +- **Silently revoking historical duplicates in the migration** was rejected as changing live + authorization without operator action. +- **Adding a route-parameter-to-scope resolver in this pass** was rejected as scope creep with + security consequences; rejecting scoped creation is the honest interim state. +- **Signing the pagination cursor** was rejected: it would imply the cursor carries authority it does + not have, and invite treating it as a capability rather than a position. + +## Consequences + +Authorization audit history survives user deletion; live grants and platform-owner rows still +cascade, as intended. Bootstrap's single use is durable across owner revocation and user deletion, +backfilled for existing installations with one documented, irrecoverable exception, and permanently +unavailable once consumed — including after the last owner is deleted. Two equivalent grants can no +longer exist effectively, sequentially or concurrently, and one revocation leaves no equivalent +issuance authorizing the user. Malformed grant ids and expirations are request-validation results +decided before any database call, never infrastructure failures. Explicit-grant listing is bounded +and deterministically traversable. + +Core gains one table and one forward migration (`0002_authorization_integrity`), classified +`{"mode":"none"}`: it creates a table, drops one referential constraint specifically to *retain* +data, and inserts a backfill row. It drops no table, column, or row. The `DROP CONSTRAINT` text is +not itself a reason to classify it approval-required — the production CLI has no trusted external +approval provider, so an approval-blocked security fix would be undeployable. Migration 0002 must be +applied before the new code starts; migrations 0000 and 0001 are byte-identical with unchanged +checksums. + +No roles, groups, teams, tenants, service accounts, API tokens, scope resolver, user-deletion +endpoint, owner-recovery workflow, or module-facing change is introduced, and the trusted compiled- +backend module model is unchanged. Audit append-only behavior remains an application convention, not +a cryptographic or database-role guarantee. **Phase 5D is not complete** — passes 4 (module/runtime/ +migration correctness) and 5 (deployment/supply chain plus an independent whole-repository re-audit) +remain outstanding, and Phase 6 remains blocked. diff --git a/docs/architecture/decisions/README.md b/docs/architecture/decisions/README.md index f4fe311..0b0a636 100644 --- a/docs/architecture/decisions/README.md +++ b/docs/architecture/decisions/README.md @@ -27,3 +27,4 @@ ADRs capture durable choices that affect more than one package or phase. - [0016: Module database access](0016-module-database-access.md) — Accepted - [0017: Public request perimeter hardening — client address, signup gating, body limits, timeouts](0017-public-request-perimeter-hardening.md) — Accepted - [0018: Identity session resolution, canonical public origins, and PostgreSQL transport policy](0018-identity-session-resolution-and-database-transport.md) — Accepted +- [0019: Durable authorization history and grant management](0019-durable-authorization-history-and-grant-management.md) — Accepted diff --git a/docs/development/api.md b/docs/development/api.md index 6f4e328..32da520 100644 --- a/docs/development/api.md +++ b/docs/development/api.md @@ -15,7 +15,12 @@ Phase 5D pass 1 hardened this same public perimeter against an independent secur server-owned client-address resolution boundary for the rate limiter, public sign-up disabled by default, an explicit request-body-size policy for every public route (including a module route's own declared contract), and finite Node header/request timeouts — see -[ADR 0017](../architecture/decisions/0017-public-request-perimeter-hardening.md). +[ADR 0017](../architecture/decisions/0017-public-request-perimeter-hardening.md). Pass 3 made the +explicit-grant API's own request contracts exact: canonical UUID grant ids, one canonical +future-only expiration format, unscoped-only creation, a `409` duplicate conflict, and bounded +keyset listing — see +[Authorization](authorization.md#explicit-grant-http-api) and +[ADR 0019](../architecture/decisions/0019-durable-authorization-history-and-grant-management.md). ## What this is, and is not @@ -69,7 +74,7 @@ No real values are shown here — configure per environment and never commit the | --- | --- | --- | | `POST\|GET /api/auth/*` | Better Auth (mounted directly) | Better Auth's own request/response shapes pass through unmodified — see [Better Auth's email/password docs](https://www.better-auth.com/docs/authentication/email-password) for the exact endpoints (`/sign-up/email`, `/sign-in/email`, `/sign-out`, `/get-session`, ...). | | `GET /api/v1/principal` | Ekklesia | Returns the minimal `EkklesiaPrincipal` contract as JSON. `Cache-Control: no-store`. | -| `GET\|POST /api/v1/grants*` | Ekklesia | Explicit authorization grant management, gated by `core.authorization.grants.manage`. See [Authorization](authorization.md#explicit-grant-http-api). | +| `GET\|POST /api/v1/grants*` | Ekklesia | Explicit authorization grant management, gated by `core.authorization.grants.manage`. Listing is bounded keyset pagination; creation is unscoped-only and conflicts (`409`) on an equivalent effective grant; revocation clears every unrevoked equivalent. See [Authorization](authorization.md#explicit-grant-http-api). | | `/api/v1/modules//*` | Installed modules | Every route an installed module registered during its `register` hook, gated by that route's own declared permission. See [Module HTTP routes](#module-http-routes). | Better Auth's handler is unreachable outside `/api/auth/*`; every other unmatched path returns a diff --git a/packages/contracts/src/authorization.ts b/packages/contracts/src/authorization.ts index a1d8e03..3162e4e 100644 --- a/packages/contracts/src/authorization.ts +++ b/packages/contracts/src/authorization.ts @@ -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; diff --git a/packages/contracts/tests/authorization.test.ts b/packages/contracts/tests/authorization.test.ts index 65a5a16..6a2dce2 100644 --- a/packages/contracts/tests/authorization.test.ts +++ b/packages/contracts/tests/authorization.test.ts @@ -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); }); });