fix: enforce registered authorization permissions
Two related fail-closed gaps let a well-formed but never-declared permission id slip through: - POST /api/v1/grants created explicit grants for any well-formed PermissionId without checking the injected permission registry, allowing dormant grants for invented identifiers that could become effective the moment those identifiers were registered later. - createAuthorizationEvaluator validated the permission id only inside the restricted-baseline branch, so a malformed permission combined with the public or authenticated baseline could still return allowed. Both now fail closed: the grants route checks the target permission against the trusted registry before calling store.createGrant (sanitized 403, no store or audit write), and the evaluator validates permission shape before any baseline is allowed to return an allowed decision, without adding a registry dependency to the evaluator itself. The live integration lifecycle test switched from an unregistered calendar.events.read to the registered core.authorization.grants.manage permission, since module permission registration does not exist yet.
This commit is contained in:
@@ -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`.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user