Skip to content

Commit 52200b4

Browse files
claude[bot]claude
andauthored
fix(platform-objects,plugin-auth): give API-key revoke/restore a working product route (#7727) (#7769)
`sys_api_key` declared two row actions — `revoke_api_key` / `restore_api_key` — as `PATCH /api/v1/data/sys_api_key/{id}` with `bodyExtra: {revoked}`, while the same object set `enable.apiMethods = ['get','list']`. The declared PATCH died at the ADR-0049 method gate with 405 before any authorization ran, so no product route revoked an API key: the Setup UI's Revoke button errored, the row kept `revoked = false`, and the key kept authenticating. The write path had two gates, not one: - the method gate — `enable.apiMethods` now carries `update` (`create` / `delete` stay off: minting is `POST /api/v1/keys`, and keys are retired by revoking rather than deleting); - ADR-0103's `reconcileManagedApiMethods`, which strips any write verb a `managedBy` object's affordances do not grant and only warns. `apiMethods` alone would still have served 405 while the source read correctly, so `userActions: { edit: true }` declares the affordance — the ADR-0092 D4 pattern `sys_user` already uses. Opening the method does not open the columns. The object 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 stays the only opening; `revoked` is registered there and nothing else is. The guard itself is untouched — no general weakening, and every other identity table keeps its default-deny. Per D4's form-rendering constraint the columns outside the whitelist are now `readonly`, so the edit form this affordance turns on cannot offer a write the server refuses. Nothing pinned any of this: the existing tests exercise key resolution against a pre-revoked row and never call the route the actions declare. The new dogfood suite drives the real PATCH, asserts 200, then asserts the consequence — the key stops authenticating — and pins the refusals with code AND status. Claude-Session: https://claude.ai/code/session_01SqARTSYRvgutYHaLXbx6f7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2701e95 commit 52200b4

5 files changed

Lines changed: 356 additions & 5 deletions

File tree

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/platform-objects": patch
3+
"@objectstack/plugin-auth": patch
4+
---
5+
6+
fix(platform-objects,plugin-auth): let the API-key revoke/restore actions actually run (#7727)
7+
8+
`sys_api_key` contradicted itself. It declared two row actions —
9+
`revoke_api_key` / `restore_api_key` — as `PATCH /api/v1/data/sys_api_key/{id}`
10+
with `bodyExtra: { revoked: true|false }`, while the same object set
11+
`enable.apiMethods = ['get', 'list']`. The declared PATCH was refused at the
12+
ADR-0049 method gate with `405 OBJECT_API_METHOD_NOT_ALLOWED` before any
13+
authorization ran, so **no product route revoked an API key**: the Setup →
14+
API Keys → Revoke button produced an error toast, the row still read
15+
`revoked = false`, and the key kept authenticating. A leaked key could only be
16+
retired by writing the row out of band.
17+
18+
Enforcement of the flag was never the problem — the verifier filters
19+
`revoked: false` and re-checks the row, so a flipped bit takes effect on the
20+
very next `x-api-key` call. The missing piece was purely the write path, and it
21+
had **two** gates, not one:
22+
23+
- **The method gate.** `enable.apiMethods` now carries `update`. `create` and
24+
`delete` stay off: minting is `POST /api/v1/keys` (the only path that ever
25+
returns the raw secret) and keys are retired by revoking, not deleting.
26+
- **The affordance reconciler.** ADR-0103's `reconcileManagedApiMethods` strips
27+
any write verb a `managedBy` object's resolved affordances do not grant —
28+
warning, not failing. So `apiMethods` alone would still have served 405 while
29+
the source read correctly. `userActions: { edit: true }` declares the
30+
affordance, exactly as `sys_user` does under ADR-0092 D4.
31+
32+
**Opening the method does not open the columns.** `sys_api_key` stays
33+
`managedBy: 'better-auth'`, so ADR-0092 D2's identity write guard still
34+
fail-closed rejects user-context writes, and its per-object update whitelist
35+
remains the only opening. `revoked` is registered there and nothing else is:
36+
`key` stays unwritable (a rotated hash would mint a credential nobody holds),
37+
`user_id` stays unwritable (re-owning a key is privilege transfer), and
38+
`expires_at` stays on the mint path. A PATCH carrying only non-whitelisted
39+
columns is refused `403 PERMISSION_DENIED` rather than degrading into a
40+
timestamp touch, and a mixed patch applies `revoked` while stripping the rest.
41+
The guard itself is unchanged — no general weakening, and every other identity
42+
table keeps its default-deny.
43+
44+
Per ADR-0092 D4's form-rendering constraint, the columns outside the whitelist
45+
(`name`, `prefix`, `user_id`, `scopes`, `expires_at`) are now `readonly`, so the
46+
edit form this affordance turns on cannot offer a write the server refuses —
47+
the declared-≠-enforced shape that caused the original defect.
48+
49+
Nothing pinned any of this before: the existing tests exercise key *resolution*
50+
against a pre-revoked row and never call the route the actions declare, which is
51+
how a declared action and a method gate cancelled out unnoticed. The new
52+
`api-key-revoke-lifecycle` dogfood suite drives the real PATCH, asserts `200`,
53+
and then asserts the consequence — the key stops authenticating — because a 200
54+
that leaves the key working is the defect wearing a success code.

packages/platform-objects/src/identity/sys-api-key.object.ts

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ export const SysApiKey = ObjectSchema.create({
2020
icon: 'key-round',
2121
isSystem: true,
2222
managedBy: 'better-auth',
23+
// [ADR-0092 D4 / ADR-0103] Declares the generic EDIT affordance, which is
24+
// what lets `enable.apiMethods` keep `update` below: `managedBy` objects run
25+
// through `reconcileManagedApiMethods`, which strips any write verb the
26+
// resolved affordances do not grant. Without this line the declaration and
27+
// the runtime disagree again — silently, one layer deeper than #7727's
28+
// method gate. `create` / `delete` stay bucket-default (off): minting is
29+
// `POST /api/v1/keys` and rows are retired by revoking, not deleting.
30+
//
31+
// The affordance is safe to open only because the enforcement it fronts
32+
// already exists (D4's sequencing rule — affordance never ships ahead of
33+
// the guard): ADR-0092 D2's guard clamps every user-context update on this
34+
// table to the registered column whitelist, which lists `revoked` alone.
35+
// Per D4's form-rendering constraint, every column outside that whitelist
36+
// is marked `readonly` below, so the edit form cannot offer a write the
37+
// server will refuse.
38+
userActions: { edit: true },
2339
// ADR-0010 §3.7 — managed by better-auth; tenants may not edit schema,
2440
// but may add overlay row-level config. Use `no-overlay` if you need to
2541
// forbid sys_metadata overlays entirely.
@@ -36,8 +52,9 @@ export const SysApiKey = ObjectSchema.create({
3652

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

118135
fields: {
119136
// ── Identity ─────────────────────────────────────────────────
137+
// The five fields below are `readonly` for one reason (ADR-0092 D4's
138+
// form-rendering constraint): they are set on the mint path and are NOT on
139+
// the identity write guard's column whitelist, so a user-context write to
140+
// any of them is refused 403. With `userActions.edit` open, leaving them
141+
// writable in the form would advertise an edit the server rejects — the
142+
// declared-≠-enforced shape this object already paid for once (#7727).
120143
name: Field.text({
121144
label: 'Name',
122145
required: true,
146+
readonly: true,
123147
searchable: true,
124148
maxLength: 255,
125149
description: 'Human-readable label for the API key',
@@ -129,6 +153,7 @@ export const SysApiKey = ObjectSchema.create({
129153
prefix: Field.text({
130154
label: 'Prefix',
131155
required: false,
156+
readonly: true,
132157
maxLength: 16,
133158
description: 'Visible prefix for identifying the key (e.g., "osk_")',
134159
group: 'Identity',
@@ -137,6 +162,7 @@ export const SysApiKey = ObjectSchema.create({
137162
user_id: Field.lookup('sys_user', {
138163
label: 'Owner',
139164
required: true,
165+
readonly: true,
140166
description: 'User who owns this API key',
141167
group: 'Identity',
142168
}),
@@ -145,6 +171,7 @@ export const SysApiKey = ObjectSchema.create({
145171
scopes: Field.textarea({
146172
label: 'Scopes',
147173
required: false,
174+
readonly: true,
148175
description: 'JSON array of permission scopes',
149176
group: 'Access',
150177
}),
@@ -153,6 +180,7 @@ export const SysApiKey = ObjectSchema.create({
153180
expires_at: Field.datetime({
154181
label: 'Expires At',
155182
required: false,
183+
readonly: true,
156184
group: 'Lifecycle',
157185
}),
158186

@@ -214,8 +242,23 @@ export const SysApiKey = ObjectSchema.create({
214242
trackHistory: true,
215243
searchable: false,
216244
apiEnabled: true,
217-
// #1591 — reads only: writes are refused by the identity write guard
218-
// (ADR-0092 D2) and owned by better-auth. HTTP answers 405 before the 403.
219-
apiMethods: ['get', 'list'],
245+
// #1591 / #7727 — reads, plus `update` for the revoke/restore lifecycle.
246+
//
247+
// `create` and `delete` stay off: minting is `POST /api/v1/keys` (the only
248+
// path that can return the raw secret once) and rows are retired by
249+
// revoking, not deleting, so history survives.
250+
//
251+
// `update` is here because the two row actions above declare a PATCH
252+
// against the data API, and a method gate that answers 405 first makes
253+
// those actions dead on arrival — a declared affordance the runtime never
254+
// honours (#7727). Opening the METHOD does not open the COLUMNS: this
255+
// object is `managedBy: 'better-auth'`, so ADR-0092 D2's identity write
256+
// guard still fail-closed rejects user-context writes, and the only
257+
// opening is its per-object update whitelist. `revoked` is registered
258+
// there (plugin-auth `managed-extension-fields.ts`); every other column —
259+
// `key`, `user_id`, `expires_at`, `name`, … — is stripped, and a PATCH
260+
// that touches nothing else is refused 403 `PERMISSION_DENIED` rather
261+
// than degrading into a silent no-op.
262+
apiMethods: ['get', 'list', 'update'],
220263
},
221264
});

packages/plugins/plugin-auth/src/managed-extension-fields.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,15 @@ export const MANAGED_EXTENSION_FIELDS: Readonly<Record<string, ReadonlySet<strin
5858
'parent_organization_id',
5959
'sort_order',
6060
]),
61+
sys_api_key: new Set([
62+
// #7727 — the revoke/restore lifecycle flag. `sys_api_key` is
63+
// `managedBy: 'better-auth'` (which is what puts it under the D2 guard),
64+
// but the table is hand-rolled ObjectStack: `packages/core/src/security/
65+
// api-key.ts` mints and verifies it and better-auth's `apiKey` plugin is
66+
// not loaded, so EVERY column here is an extension field. `revoked` is
67+
// the only one a generic write surface may touch — see the editable map.
68+
'revoked',
69+
]),
6170
sys_invitation: new Set([
6271
// ADR-0105 D8 — placement intent. NOT generically editable (absent from
6372
// the editable map below): these decide RBAC placement, so they are set
@@ -86,6 +95,17 @@ export const MANAGED_EXTENSION_EDITABLE_FIELDS: Readonly<Record<string, Readonly
8695
'parent_organization_id',
8796
'sort_order',
8897
]),
98+
// #7727 — revoking a leaked API key is a product operation, and before this
99+
// entry no product route performed it: `sys_api_key`'s own row actions
100+
// declare a PATCH, the object's method gate answered 405, and behind that
101+
// the guard had no whitelist to consult, so the 403 was equally certain.
102+
// Scoping the opening to this ONE column is the point — `key` stays
103+
// unwritable (a rotated hash would silently mint a key nobody holds),
104+
// `user_id` stays unwritable (re-owning a key is privilege transfer), and
105+
// `expires_at` stays on the mint path. Enforcement of the flag already
106+
// works: the verifier filters `revoked: false` and re-checks the row, so a
107+
// flipped bit takes effect on the very next `x-api-key` call.
108+
sys_api_key: new Set(['revoked']),
89109
};
90110

91111
/** The extension fields declared on `object`, or an empty set. */

0 commit comments

Comments
 (0)