diff --git a/apps/api/src/grants-routes.ts b/apps/api/src/grants-routes.ts index 78fdd4d..2f3d0c0 100644 --- a/apps/api/src/grants-routes.ts +++ b/apps/api/src/grants-routes.ts @@ -164,6 +164,16 @@ export function createGrantsRoutes(options: CreateGrantsRoutesOptions): Hono { return c.json({ code: "INVALID_REQUEST" }, 400); } + // A well-formed PermissionId is not necessarily a *registered* one. Only the trusted registry + // (never the request) decides what may be granted — the same registry `requireManagePermission` + // already consults for the caller's own `core.authorization.grants.manage` check — so a caller + // cannot pre-create a dormant grant for an invented identifier that later becomes effective the + // moment someone registers it. Sanitized identically to any other denial: this must never reveal + // whether the identifier was malformed (400, above) or simply unregistered (403, here). + if (!options.registry.has(parsed.permission)) { + return c.json({ code: "AUTHORIZATION_DENIED" }, 403); + } + try { const grant = await options.store.createGrant({ actor: createUserActor(principal.userId), diff --git a/apps/api/tests/api-http.integration.ts b/apps/api/tests/api-http.integration.ts index 652e291..c997385 100644 --- a/apps/api/tests/api-http.integration.ts +++ b/apps/api/tests/api-http.integration.ts @@ -832,7 +832,13 @@ describe.sequential("live API — platform owner and delegated grant management" const ownerEmail = "authz-owner@example.test"; const managerEmail = "authz-manager@example.test"; const password = "correct horse battery staple"; - const testPermission = "calendar.events.read"; + // Module permission registration does not exist yet, so `core.authorization.grants.manage` — + // the one permission `CORE_PERMISSION_REGISTRY` actually declares — stands in for "some + // registered permission" throughout this scenario's explicit-grant lifecycle. + const testPermission = "core.authorization.grants.manage"; + // Well-formed (passes `PermissionId` shape validation) but never registered — used to prove the + // live route rejects it even for a caller who otherwise has full grant-management authority. + const unregisteredPermission = "calendar.events.read"; it("starts a freshly migrated API and signs up the eventual owner and eventual delegated manager", async () => { database = await createDisposableDatabase("authz_http"); @@ -913,6 +919,54 @@ describe.sequential("live API — platform owner and delegated grant management" expect(response.status).toBe(200); }); + it("rejects a well-formed but unregistered permission even from a caller with full grant-management authority, and writes no grant or audit row", async () => { + const admin = await openAdministrativeClient(database.url); + try { + const grantCountBefore = await safeQuery<{ count: string }>( + admin, + `select count(*)::text as count from "ekklesia_core_grant" where permission_id = $1`, + [unregisteredPermission], + ); + const auditCountBefore = await safeQuery<{ count: string }>( + admin, + `select count(*)::text as count from "ekklesia_core_authorization_audit" where permission_id = $1`, + [unregisteredPermission], + ); + expect(grantCountBefore[0]?.count).toBe("0"); + expect(auditCountBefore[0]?.count).toBe("0"); + + const response = await fetch(`${origin}/api/v1/grants`, { + body: JSON.stringify({ + permission: unregisteredPermission, + subjectUserId: managerUserId, + }), + headers: { + "content-type": "application/json", + cookie: managerCookie, + origin: dashboardOrigin, + }, + method: "POST", + }); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ code: "AUTHORIZATION_DENIED" }); + + const grantCountAfter = await safeQuery<{ count: string }>( + admin, + `select count(*)::text as count from "ekklesia_core_grant" where permission_id = $1`, + [unregisteredPermission], + ); + const auditCountAfter = await safeQuery<{ count: string }>( + admin, + `select count(*)::text as count from "ekklesia_core_authorization_audit" where permission_id = $1`, + [unregisteredPermission], + ); + expect(grantCountAfter[0]?.count).toBe("0"); + expect(auditCountAfter[0]?.count).toBe("0"); + } finally { + await safeEnd(admin); + } + }); + 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`, { body: JSON.stringify({ diff --git a/apps/api/tests/grants-routes.test.ts b/apps/api/tests/grants-routes.test.ts index 0cd368f..0b1b37c 100644 --- a/apps/api/tests/grants-routes.test.ts +++ b/apps/api/tests/grants-routes.test.ts @@ -9,6 +9,7 @@ import { type AuthorizationEvaluator, type AuthorizationStore, AuthorizationStoreError, + CORE_AUTHORIZATION_GRANTS_MANAGE, CORE_PERMISSION_REGISTRY, } from "@ekklesia/core/authorization"; import type { EkklesiaIdentityRuntime } from "@ekklesia/core/identity"; @@ -20,7 +21,13 @@ const trustedOrigin = "https://dashboard.example.test"; const untrustedOrigin = "https://evil.example.test"; const manager = createAuthenticatedPrincipal("manager_1"); const subjectUserId = parseEkklesiaUserId("subject_1"); +// Well-formed but never declared in `CORE_PERMISSION_REGISTRY` — used to prove the routes reject +// (or, for listing, still return) an unregistered permission id rather than assuming registration. const permission = parsePermissionId("calendar.events.read"); +// A permission `CORE_PERMISSION_REGISTRY` actually declares, so tests that must reach +// `store.createGrant`/`store.revokeGrant` can do so without also depending on module permission +// registration, which does not exist yet. +const registeredPermission = CORE_AUTHORIZATION_GRANTS_MANAGE; const grantId = parseGrantId("11111111-1111-1111-1111-111111111111"); function fakeRuntime(overrides: Partial = {}): EkklesiaIdentityRuntime { @@ -170,7 +177,7 @@ describe("POST /api/v1/grants", () => { const createdGrant = { createdAt: new Date("2026-01-01T00:00:00.000Z"), id: grantId, - permission, + permission: registeredPermission, scope: parseAuthorizationScope("calendar-event:123"), sourceKind: "explicit" as const, userId: subjectUserId, @@ -179,7 +186,11 @@ describe("POST /api/v1/grants", () => { const { app } = buildApp({ store }); const response = await app.request("/api/v1/grants", { - body: JSON.stringify({ permission, scope: "calendar-event:123", subjectUserId }), + body: JSON.stringify({ + permission: registeredPermission, + scope: "calendar-event:123", + subjectUserId, + }), headers: { "content-type": "application/json", origin: trustedOrigin }, method: "POST", }); @@ -190,7 +201,7 @@ describe("POST /api/v1/grants", () => { expect(store.createGrant).toHaveBeenCalledWith( expect.objectContaining({ actor: { type: "user", userId: manager.userId }, - permission, + permission: registeredPermission, scope: "calendar-event:123", sourceKind: "explicit", userId: subjectUserId, @@ -198,6 +209,20 @@ describe("POST /api/v1/grants", () => { ); }); + it("rejects a well-formed but unregistered permission with the same sanitized denial as an unauthorized caller, and never calls the store", async () => { + const { app, store } = buildApp({}); + + const response = await app.request("/api/v1/grants", { + body: JSON.stringify({ permission, subjectUserId }), + headers: { "content-type": "application/json", origin: trustedOrigin }, + method: "POST", + }); + + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ code: "AUTHORIZATION_DENIED" }); + expect(store.createGrant).not.toHaveBeenCalled(); + }); + it.each([ "actor", "owner", @@ -256,7 +281,7 @@ describe("POST /api/v1/grants", () => { const { app } = buildApp({ store }); const response = await app.request("/api/v1/grants", { - body: JSON.stringify({ permission, subjectUserId }), + body: JSON.stringify({ permission: registeredPermission, subjectUserId }), headers: { "content-type": "application/json", origin: trustedOrigin }, method: "POST", }); diff --git a/core/src/authorization/evaluator.ts b/core/src/authorization/evaluator.ts index ed8a95e..8b8f43a 100644 --- a/core/src/authorization/evaluator.ts +++ b/core/src/authorization/evaluator.ts @@ -32,8 +32,13 @@ export class AuthorizationEvaluationError extends Error { export interface AuthorizeRequest { readonly principal: EkklesiaPrincipal; - /** Validated internally via the branded `PermissionId` schema; a malformed or unrecognizable - * value denies rather than throwing — "unknown or malformed permissions fail closed." */ + /** Validated internally via the branded `PermissionId` schema *before any baseline can return an + * allowed decision* — a malformed or unrecognizable value denies rather than throwing, for + * `public` and `authenticated` baselines exactly as much as `restricted`. This is shape + * validation only: the evaluator has no registry and cannot tell a well-formed-but-unregistered + * permission from one that is actually registered. Only a trusted caller consulting a + * `PermissionRegistry` (`core/src/authorization/permissions.ts`) before calling `authorize` can + * make that distinction — see that module and `apps/api/src/authorization-middleware.ts`. */ readonly permission: unknown; /** Validated internally against the known `public`/`authenticated`/`restricted` set; anything * else denies rather than throwing, for the same fail-closed reason. */ @@ -80,6 +85,16 @@ export function createAuthorizationEvaluator( return AUTHORIZATION_DENIED; } + // Shape-validated before any baseline is allowed to return `allowed` — a malformed + // permission id must deny under `public` and `authenticated` exactly as much as under + // `restricted`, even though neither of the first two ever reads `parsedPermission.data`. + // This is a pure, synchronous check: it never touches the grant or platform-owner ports, so + // `public`/`authenticated` still resolve with no database lookup at all. + const parsedPermission = safeParsePermissionId(request.permission); + if (!parsedPermission.success) { + return AUTHORIZATION_DENIED; + } + if (parsedBaseline.data === "public") { return AUTHORIZATION_ALLOWED; } @@ -96,11 +111,6 @@ export function createAuthorizationEvaluator( return AUTHORIZATION_DENIED; } - const parsedPermission = safeParsePermissionId(request.permission); - if (!parsedPermission.success) { - return AUTHORIZATION_DENIED; - } - let scope: AuthorizationScope | undefined; if (request.scope !== undefined) { const parsedScope = safeParseAuthorizationScope(request.scope); diff --git a/core/tests/authorization-evaluator.test.ts b/core/tests/authorization-evaluator.test.ts index 8c9ba86..bbc41ca 100644 --- a/core/tests/authorization-evaluator.test.ts +++ b/core/tests/authorization-evaluator.test.ts @@ -115,6 +115,28 @@ describe("Core authorization evaluator", () => { expect(grants.calls).toHaveLength(0); expect(owners.calls).toHaveLength(0); }); + + it("denies an anonymous principal when the permission id is malformed, without throwing or querying grants or ownership", async () => { + const decision = await evaluator().authorize({ + baseline: "public", + permission: "not a valid permission id", + principal: ANONYMOUS_PRINCIPAL, + }); + expect(decision).toEqual({ allowed: false }); + expect(grants.calls).toHaveLength(0); + expect(owners.calls).toHaveLength(0); + }); + + it("denies an authenticated principal when the permission id is malformed", async () => { + const decision = await evaluator().authorize({ + baseline: "public", + permission: "not a valid permission id", + principal: createAuthenticatedPrincipal(userId), + }); + expect(decision).toEqual({ allowed: false }); + expect(grants.calls).toHaveLength(0); + expect(owners.calls).toHaveLength(0); + }); }); describe("authenticated baseline", () => { @@ -145,6 +167,28 @@ describe("Core authorization evaluator", () => { expect(grants.calls).toHaveLength(0); expect(owners.calls).toHaveLength(0); }); + + it("denies an authenticated principal when the permission id is malformed, without throwing or querying grants or ownership", async () => { + const decision = await evaluator().authorize({ + baseline: "authenticated", + permission: "not a valid permission id", + principal: createAuthenticatedPrincipal(userId), + }); + expect(decision).toEqual({ allowed: false }); + expect(grants.calls).toHaveLength(0); + expect(owners.calls).toHaveLength(0); + }); + + it("still denies an anonymous principal when the permission id is malformed too", async () => { + const decision = await evaluator().authorize({ + baseline: "authenticated", + permission: "not a valid permission id", + principal: ANONYMOUS_PRINCIPAL, + }); + expect(decision).toEqual({ allowed: false }); + expect(grants.calls).toHaveLength(0); + expect(owners.calls).toHaveLength(0); + }); }); describe("restricted baseline", () => { 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 afcdad4..5f84daa 100644 --- a/docs/architecture/decisions/0013-core-authorization-and-owner-bootstrap.md +++ b/docs/architecture/decisions/0013-core-authorization-and-owner-bootstrap.md @@ -46,6 +46,18 @@ or malformed permission id is treated identically to a denied decision, never a would work if declared" response — deny-by-default holds even for permissions nobody has declared yet. +Registration is enforced everywhere a permission id can originate from outside Core's own trusted +route wiring, not only for the caller's own required permission. `requirePermission` +(`apps/api/src/authorization-middleware.ts`) checks the registry for the permission a *route* +requires; `POST /api/v1/grants` (`apps/api/src/grants-routes.ts`) separately checks the registry +for the *target* permission a caller is requesting be granted to someone else, since that value +comes from the request body rather than route wiring. Without the second check, a caller with +`core.authorization.grants.manage` could create a stored grant for a permission id that is merely +well-formed, not registered — a dormant grant that would silently become effective the moment +that identifier was registered later, with no further action or review at registration time. Both +checks reject with the identical sanitized `403 {"code":"AUTHORIZATION_DENIED"}` an ordinary denial +would return, and neither ever calls into the store on rejection. + ### Permissions declare one trusted baseline: public, authenticated, or restricted Every registered permission declares exactly one `PermissionBaseline` at registration time. The @@ -60,14 +72,23 @@ restricted check. ### Unknown permissions and client-supplied baselines fail closed -Two independent fail-closed layers exist on purpose, not one relied on twice. The registry itself -(`PermissionRegistry.get`/`.has`) returns `undefined`/`false` for anything malformed or -unregistered. `requirePermission` treats a registry miss as an immediate sanitized `403`, before -principal resolution or evaluation ever runs. Separately, `createAuthorizationEvaluator` itself -denies (without throwing) any malformed or unrecognized `permission`/`baseline`/`scope` it -receives, so a future caller that reaches the evaluator directly — bypassing the registry, for -example a different transport Core does not yet have — inherits the same deny-by-default guarantee -rather than depending on the registry check having already run. +Two independent fail-closed layers exist on purpose, not one relied on twice, and they check +different things. The registry itself (`PermissionRegistry.get`/`.has`) returns `undefined`/`false` +for anything malformed or unregistered — this is the only layer that knows what is *registered*. +`requirePermission` treats a registry miss as an immediate sanitized `403`, before principal +resolution or evaluation ever runs; `POST /api/v1/grants` performs the equivalent registry check +for the target permission named in its body (see above). Separately, `createAuthorizationEvaluator` +independently re-validates permission *shape* — it holds no registry and cannot recognize +registration at all — denying (without throwing) any malformed or unrecognized +`permission`/`baseline`/`scope` it receives. This shape check runs, and can deny, **before any +baseline is allowed to return an allowed decision** — `public` and `authenticated` are not +exempt, even though neither reads the permission value for any other reason — so a future caller +that reaches the evaluator directly, bypassing the registry entirely (for example a different +transport Core does not yet have), inherits the same fail-closed guarantee for malformed input +rather than depending on a registry check having already run upstream. The evaluator's shape check +and the registry's registration check are deliberately not merged into one: only the registry can +answer "is this permission registered," and the evaluator must remain usable — and safe against +malformed input — by a caller that has no registry to consult at all. ### Explicit and relationship-derived grants share persistence but have distinct source kinds diff --git a/docs/development/authorization.md b/docs/development/authorization.md index 01102ec..177eda3 100644 --- a/docs/development/authorization.md +++ b/docs/development/authorization.md @@ -53,6 +53,33 @@ matter: Module-owned permission declarations and a discovery/composition path for them are out of scope for this slice (deferred to the Module SDK work). +### Two layers, not one: shape versus registration + +`createAuthorizationEvaluator` (`core/src/authorization/evaluator.ts`) has no registry and is +never given one — it independently re-validates `permission`, `baseline`, and `scope` as a pure +shape check (`safeParsePermissionId`/`permissionBaselineSchema`/`safeParseAuthorizationScope`) and +denies a malformed value under **every** baseline, including `public` and `authenticated`, before +any baseline is allowed to return an allowed decision. This is defense in depth for a future +caller that reaches the evaluator directly, not a substitute for the registry: the evaluator has +no way to tell a well-formed-but-unregistered permission id (for example a plausible-looking +`calendar.events.read` that nothing has ever declared) from a genuinely registered one — it can +only reject what is not even shaped like a `PermissionId` at all. + +*Registration* — "does this permission actually exist" — is exclusively the registry's +responsibility, consulted by every trusted caller before the evaluator ever runs: +`requirePermission` (`apps/api/src/authorization-middleware.ts`) looks up the *caller's own* +required permission (for example `core.authorization.grants.manage`, always a hardcoded route +constant, never client-supplied) in the registry before resolving a principal or evaluating +anything; `POST /api/v1/grants` (`apps/api/src/grants-routes.ts`) separately looks up the *target* +permission being granted — the one named in the request body — in the same registry, after body +shape-validation and before ever calling `store.createGrant`, so a caller cannot create a dormant +grant for an invented permission id that might later become effective the moment someone actually +registers it. A malformed target permission (fails `PermissionId` shape validation) still resolves +to the body-level `400 {"code":"INVALID_REQUEST"}` documented below, unchanged; a well-formed but +unregistered target permission instead receives the identical sanitized +`403 {"code":"AUTHORIZATION_DENIED"}` a genuinely denied caller would, so that response never +reveals whether the caller was denied or the permission was simply never registered. + ## Platform-owner bootstrap There is no "first user becomes owner" inference anywhere in this repository, and no HTTP @@ -191,7 +218,7 @@ cannot smuggle a server-derived field through a request that otherwise looks wel | Field | Required | Notes | | --- | --- | --- | | `subjectUserId` | Yes | Must parse as a valid `EkklesiaUserId`. | -| `permission` | Yes | Must parse as a valid `PermissionId`. | +| `permission` | Yes | Must parse as a valid `PermissionId` **and** be registered in `CORE_PERMISSION_REGISTRY` — see [Two layers, not one](#two-layers-not-one-shape-versus-registration). A malformed value is a `400`; a well-formed but unregistered one is a `403`. | | `scope` | No | Must parse as a valid, non-empty `AuthorizationScope` if present. | | `expiresAt` | No | An ISO date string if present. | @@ -225,7 +252,7 @@ A grant, wherever returned: | --- | --- | | `200` / `201` | Success. | | `400 {"code":"INVALID_REQUEST"}` | Malformed query/body/param (including a disallowed extra field). | -| `403 {"code":"AUTHORIZATION_DENIED"}` | Caller lacks `core.authorization.grants.manage`, or the permission id was unregistered. | +| `403 {"code":"AUTHORIZATION_DENIED"}` | Caller lacks `core.authorization.grants.manage`; the caller's own required permission id was unregistered; or, for `POST /api/v1/grants`, the well-formed *target* permission named in the body is not registered. | | `403 {"code":"UNTRUSTED_ORIGIN"}` | Missing or untrusted `Origin` on a mutation route. | | `404 {"code":"AUTHORIZATION_GRANT_NOT_FOUND"}` | The targeted grant id does not exist as an *explicit* grant — this is also what revoking a real relationship-derived grant's id looks like, indistinguishably. | | `503` | Principal resolution, evaluation, or store failure (`PRINCIPAL_RESOLUTION_FAILED`, `AUTHORIZATION_EVALUATION_FAILED`, or an `AuthorizationStoreError` code). | @@ -273,15 +300,23 @@ there are or whether the failure happened during startup. - `apps/api/tests/authorization-middleware.test.ts` — allow/deny/unregistered/infrastructure- failure paths for `requirePermission`, and `requireTrustedOrigin`'s allow/reject behavior. - `apps/api/tests/grants-routes.test.ts` — all three routes' allow/deny paths, disallowed-field - and malformed-field rejection, exact server-derived-argument assertions, sourceKind-filtered - revoke returning 404 for a non-explicit grant, origin enforcement, and store-error mapping. + and malformed-field rejection, a well-formed but unregistered target permission rejected with + `403` and `store.createGrant` never called, exact server-derived-argument assertions, + sourceKind-filtered revoke returning 404 for a non-explicit grant, origin enforcement, and + store-error mapping. - `apps/api/tests/lifecycle.test.ts` — shared-pool-closes-exactly-once coverage described above. +- `core/tests/authorization-evaluator.test.ts` — includes a malformed permission id denying under + every baseline (`public`, `authenticated`, `restricted`), never just `restricted`, and never + consulting the grant or platform-owner ports for `public`/`authenticated`. - `apps/api/tests/api-http.integration.ts` (live PostgreSQL) — end-to-end: anonymous/non-owner denial, real bootstrap against the running API's own database, owner authority over the HTTP grants API, the owner delegating `core.authorization.grants.manage` to another account and that - account then managing grants as a non-owner, a create/list/revoke cycle with audit-trail - verification, disallowed-field rejection, missing/untrusted-origin rejection, a real - relationship-derived grant proven invisible to LIST and unreachable by REVOKE, and an + account then managing grants as a non-owner, a well-formed but unregistered target permission + rejected with `403` (with no grant or audit row written) even for a caller with full + grant-management authority, a create/list/revoke cycle with audit-trail verification (using + `core.authorization.grants.manage` itself as the granted permission, since module permission + registration does not exist yet), disallowed-field rejection, missing/untrusted-origin rejection, + a real relationship-derived grant proven invisible to LIST and unreachable by REVOKE, and an infrastructure-failure scenario (database dropped out from under a live session) asserting a `503` rather than a false `403` or `200`.