Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions .changeset/api-key-revoke-product-route.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
---
"@objectstack/platform-objects": patch
"@objectstack/plugin-auth": patch
---

fix(platform-objects,plugin-auth): let the API-key revoke/restore actions actually run (#7727)

`sys_api_key` contradicted itself. It declared two row actions —
`revoke_api_key` / `restore_api_key` — as `PATCH /api/v1/data/sys_api_key/{id}`
with `bodyExtra: { revoked: true|false }`, while the same object set
`enable.apiMethods = ['get', 'list']`. The declared PATCH was refused at the
ADR-0049 method gate with `405 OBJECT_API_METHOD_NOT_ALLOWED` before any
authorization ran, so **no product route revoked an API key**: the Setup →
API Keys → Revoke button produced an error toast, the row still read
`revoked = false`, and the key kept authenticating. A leaked key could only be
retired by writing the row out of band.

Enforcement of the flag was never the problem — the verifier filters
`revoked: false` and re-checks the row, so a flipped bit takes effect on the
very next `x-api-key` call. The missing piece was purely the write path, and it
had **two** gates, not one:

- **The method gate.** `enable.apiMethods` now carries `update`. `create` and
`delete` stay off: minting is `POST /api/v1/keys` (the only path that ever
returns the raw secret) and keys are retired by revoking, not deleting.
- **The affordance reconciler.** ADR-0103's `reconcileManagedApiMethods` strips
any write verb a `managedBy` object's resolved affordances do not grant —
warning, not failing. So `apiMethods` alone would still have served 405 while
the source read correctly. `userActions: { edit: true }` declares the
affordance, exactly as `sys_user` does under ADR-0092 D4.

**Opening the method does not open the columns.** `sys_api_key` stays
`managedBy: 'better-auth'`, so ADR-0092 D2's identity write guard still
fail-closed rejects user-context writes, and its per-object update whitelist
remains the only opening. `revoked` is registered there and nothing else is:
`key` stays unwritable (a rotated hash would mint a credential nobody holds),
`user_id` stays unwritable (re-owning a key is privilege transfer), and
`expires_at` stays on the mint path. A PATCH carrying only non-whitelisted
columns is refused `403 PERMISSION_DENIED` rather than degrading into a
timestamp touch, and a mixed patch applies `revoked` while stripping the rest.
The guard itself is unchanged — no general weakening, and every other identity
table keeps its default-deny.

Per ADR-0092 D4's form-rendering constraint, the columns outside the whitelist
(`name`, `prefix`, `user_id`, `scopes`, `expires_at`) are now `readonly`, so the
edit form this affordance turns on cannot offer a write the server refuses —
the declared-≠-enforced shape that caused the original defect.

Nothing pinned any of this before: the existing tests exercise key *resolution*
against a pre-revoked row and never call the route the actions declare, which is
how a declared action and a method gate cancelled out unnoticed. The new
`api-key-revoke-lifecycle` dogfood suite drives the real PATCH, asserts `200`,
and then asserts the consequence — the key stops authenticating — because a 200
that leaves the key working is the defect wearing a success code.
53 changes: 48 additions & 5 deletions packages/platform-objects/src/identity/sys-api-key.object.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,22 @@ export const SysApiKey = ObjectSchema.create({
icon: 'key-round',
isSystem: true,
managedBy: 'better-auth',
// [ADR-0092 D4 / ADR-0103] Declares the generic EDIT affordance, which is
// what lets `enable.apiMethods` keep `update` below: `managedBy` objects run
// through `reconcileManagedApiMethods`, which strips any write verb the
// resolved affordances do not grant. Without this line the declaration and
// the runtime disagree again — silently, one layer deeper than #7727's
// method gate. `create` / `delete` stay bucket-default (off): minting is
// `POST /api/v1/keys` and rows are retired by revoking, not deleting.
//
// The affordance is safe to open only because the enforcement it fronts
// already exists (D4's sequencing rule — affordance never ships ahead of
// the guard): ADR-0092 D2's guard clamps every user-context update on this
// table to the registered column whitelist, which lists `revoked` alone.
// Per D4's form-rendering constraint, every column outside that whitelist
// is marked `readonly` below, so the edit form cannot offer a write the
// server will refuse.
userActions: { edit: true },
// ADR-0010 §3.7 — managed by better-auth; tenants may not edit schema,
// but may add overlay row-level config. Use `no-overlay` if you need to
// forbid sys_metadata overlays entirely.
Expand All@@ -36,8 +52,9 @@ export const SysApiKey = ObjectSchema.create({

// Custom actions — sys_api_key is managed-by 'better-auth' but the
// `revoked` boolean is a column we control via the data API. These row
// actions use the generic PATCH /api/v1/sys_api_key/{id} endpoint with
// `bodyExtra` to set the `revoked` flag explicitly.
// actions use the generic PATCH /api/v1/data/sys_api_key/{id} endpoint with
// `bodyExtra` to set the `revoked` flag explicitly. The `target` below is
// the authority on that path; this comment used to omit `/data/`.
actions: [
{
name: 'revoke_api_key',
Expand DownExpand Up@@ -117,9 +134,16 @@ export const SysApiKey = ObjectSchema.create({

fields: {
// ── Identity ─────────────────────────────────────────────────
// The five fields below are `readonly` for one reason (ADR-0092 D4's
// form-rendering constraint): they are set on the mint path and are NOT on
// the identity write guard's column whitelist, so a user-context write to
// any of them is refused 403. With `userActions.edit` open, leaving them
// writable in the form would advertise an edit the server rejects — the
// declared-≠-enforced shape this object already paid for once (#7727).
name: Field.text({
label: 'Name',
required: true,
readonly: true,
searchable: true,
maxLength: 255,
description: 'Human-readable label for the API key',
Expand All@@ -129,6 +153,7 @@ export const SysApiKey = ObjectSchema.create({
prefix: Field.text({
label: 'Prefix',
required: false,
readonly: true,
maxLength: 16,
description: 'Visible prefix for identifying the key (e.g., "osk_")',
group: 'Identity',
Expand All@@ -137,6 +162,7 @@ export const SysApiKey = ObjectSchema.create({
user_id: Field.lookup('sys_user', {
label: 'Owner',
required: true,
readonly: true,
description: 'User who owns this API key',
group: 'Identity',
}),
Expand All@@ -145,6 +171,7 @@ export const SysApiKey = ObjectSchema.create({
scopes: Field.textarea({
label: 'Scopes',
required: false,
readonly: true,
description: 'JSON array of permission scopes',
group: 'Access',
}),
Expand All@@ -153,6 +180,7 @@ export const SysApiKey = ObjectSchema.create({
expires_at: Field.datetime({
label: 'Expires At',
required: false,
readonly: true,
group: 'Lifecycle',
}),

Expand DownExpand Up@@ -214,8 +242,23 @@ export const SysApiKey = ObjectSchema.create({
trackHistory: true,
searchable: false,
apiEnabled: true,
// #1591 — reads only: writes are refused by the identity write guard
// (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
apiMethods: ['get', 'list'],
// #1591 / #7727 — reads, plus `update` for the revoke/restore lifecycle.
//
// `create` and `delete` stay off: minting is `POST /api/v1/keys` (the only
// path that can return the raw secret once) and rows are retired by
// revoking, not deleting, so history survives.
//
// `update` is here because the two row actions above declare a PATCH
// against the data API, and a method gate that answers 405 first makes
// those actions dead on arrival — a declared affordance the runtime never
// honours (#7727). Opening the METHOD does not open the COLUMNS: this
// object is `managedBy: 'better-auth'`, so ADR-0092 D2's identity write
// guard still fail-closed rejects user-context writes, and the only
// opening is its per-object update whitelist. `revoked` is registered
// there (plugin-auth `managed-extension-fields.ts`); every other column —
// `key`, `user_id`, `expires_at`, `name`, … — is stripped, and a PATCH
// that touches nothing else is refused 403 `PERMISSION_DENIED` rather
// than degrading into a silent no-op.
apiMethods: ['get', 'list', 'update'],
},
});
20 changes: 20 additions & 0 deletions packages/plugins/plugin-auth/src/managed-extension-fields.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,6 +58,15 @@ export const MANAGED_EXTENSION_FIELDS: Readonly<Record<string, ReadonlySet<strin
'parent_organization_id',
'sort_order',
]),
sys_api_key: new Set([
// #7727 — the revoke/restore lifecycle flag. `sys_api_key` is
// `managedBy: 'better-auth'` (which is what puts it under the D2 guard),
// but the table is hand-rolled ObjectStack: `packages/core/src/security/
// api-key.ts` mints and verifies it and better-auth's `apiKey` plugin is
// not loaded, so EVERY column here is an extension field. `revoked` is
// the only one a generic write surface may touch — see the editable map.
'revoked',
]),
sys_invitation: new Set([
// ADR-0105 D8 — placement intent. NOT generically editable (absent from
// the editable map below): these decide RBAC placement, so they are set
Expand DownExpand Up@@ -86,6 +95,17 @@ export const MANAGED_EXTENSION_EDITABLE_FIELDS: Readonly<Record<string, Readonly
'parent_organization_id',
'sort_order',
]),
// #7727 — revoking a leaked API key is a product operation, and before this
// entry no product route performed it: `sys_api_key`'s own row actions
// declare a PATCH, the object's method gate answered 405, and behind that
// the guard had no whitelist to consult, so the 403 was equally certain.
// Scoping the opening to this ONE column is the point — `key` stays
// unwritable (a rotated hash would silently mint a key nobody holds),
// `user_id` stays unwritable (re-owning a key is privilege transfer), and
// `expires_at` stays on the mint path. Enforcement of the flag already
// works: the verifier filters `revoked: false` and re-checks the row, so a
// flipped bit takes effect on the very next `x-api-key` call.
sys_api_key: new Set(['revoked']),
};

/** The extension fields declared on `object`, or an empty set. */
Expand Down
Loading
Loading