Skip to content

Commit e43d63a

Browse files
qq9340100claude
andauthored
fix(identity): API keys are minted against the minter's active organization (#8287) (#8709)
* fix(identity): mint API keys against the minter's active organization (#8287) `sys_api_key` carried no organization, so under `OS_TENANCY_POSTURE=isolated` a minted key authenticated a user with no active organization and the Layer 0 wall (`organization_id = activeOrganizationId`) could match nothing: every org-scoped read answered `200` with `total 0` while the console went on offering minting. The column was absent by an inherited rule, not by oversight — `resolveInjectedSystemColumns` skips `managedBy: 'better-auth'` objects, and `sys_api_key` carries that flag even though better-auth's `apiKey` plugin is not loaded and the table is hand-rolled ObjectStack. - declare `active_organization_id` on `sys_api_key` (+ index, list columns) - register it as an ADR-0105 D7 managed extension field, and correct that registry's long-standing drift (its comment said every column here is an extension field; the set listed one) - mint (`POST /keys`) inherits the caller's active organization, re-checks membership against `sys_member` at mint time, and refuses under a walled posture rather than handing back a key that cannot read - the verifier reads ONE spelling (PD #12), refuses an org-less key under `isolated`, and the shared resolver fails an ex-member's key closed using the membership set it had already read — zero extra queries The column is deliberately NOT named `organization_id`: that name would make `sys_api_key` itself org-walled, hiding pre-existing org-less rows from their own owners while they keep authenticating under `group`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH * test(core): resolve @objectstack/types to source in typecheck and tests (#8287) Adding the dependency made `check:test-source-alias` and `check:type-source-resolution` both fire: a unit test or typecheck that reads a sibling's built `dist/` is a verdict about a build, not about the code in this checkout, and the dangerous direction is the one that PASSES. Anchored regex aliases (array form) so the bare entry cannot swallow the `/node` subpath. Also adds the changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH * fix(identity): take the tenancy posture from the `tenancy` service, not the env (#8287) Fixes the red `Build Core` at 2b993c9 and, underneath it, a correctness bug the build failure exposed. The build break: `check:type-source-resolution` requires a cross-package type import to resolve to SOURCE, so adding `@objectstack/types` to `core` forced a `paths` rule — which collides with core's `rootDir: "./src"` under the tsup DTS build (TS6059). That gate's own header documents this exact cost. The bug it exposed is the more important half. `resolveTenancyPosture()` reads `OS_TENANCY_POSTURE`, which is what the operator ASKED for — not what is ENFORCED. Under ADR-0093 D4/D5 a deployment requesting `isolated` without the enterprise organizations runtime resolves to `single` and runs with no wall at all, so the env-reading version would have refused org-less API keys on a deployment that has no organization boundary to enforce. The posture is now an explicit input, resolved from the kernel's `tenancy` service — the same source plugin-security reconciles before handing a posture to the Layer 0 wall, so admission and the wall can never disagree. `core` drops the `@objectstack/types` dependency entirely, and both gates go quiet on their own rather than by registry widening. An ABSENT posture disables the two posture-conditional refusals, leaving behaviour exactly as before: that is a question about the deployment, not about the credential, so an unwired transport is never made worse — only less strict. Wired here: the runtime dispatcher/MCP path and the REST data API, which are the surfaces the card measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH * chore(core): drop the dead @objectstack/types source-resolution config (#8287) The tsconfig `paths` and vitest `alias` entries were added to satisfy check:type-source-resolution / check:test-source-alias when core briefly depended on @objectstack/types. That dependency is gone — the tenancy posture now arrives from the kernel's `tenancy` service — so both entries resolve nothing, and their comments describe a `resolveTenancyPosture` call that no longer exists. Left in place they would mislead the next author and re-arm the TS6059 rootDir collision the moment anyone re-added the import. Both gates stay green without them, because the predicate is the IMPORT, not the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH * test(runtime): pin the new #8287 key-mint engine double to ObjectQL's dispatch predicates (#8287) `check:engine-double-contract` went red at 339a1b6: the organization-inheritance suite added a second engine double to http-dispatcher.keys.test.ts, taking the file from 1 unguarded double to 2 on both the delete and the update slice while the shrink-only baseline records 1. The baseline is NOT raised — that is an explicit gate-weakening action and the gate's own message rules it out ("pin the new one rather than raising it"). The new `makeOrgKernel` double now routes both write verbs through assertEngineUpdateDispatch / assertEngineDeleteDispatch from @objectstack/metadata-core, so it cannot be looser than ObjectQL itself. The file's pre-existing makeKernel double keeps its measured DEBT entry untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH * test(runtime): type the new #8287 key-mint suite against HttpDispatcherResult (#8287) `check:type-check-debt --re-measure` went red at 339a1b6: @objectstack/runtime's TEST_DEBT records 227 raw tsc errors and the tree measured 235 (+8). All eight are TS18048 'res.response' is possibly 'undefined' in the organization-inheritance suite this PR added — `HttpDispatcherResult.response` is optional, and the runtime test layer is hidden from tsc at the package level, so `pnpm test` going green said nothing about them. The ledger is NOT raised — it is a shrink-only ratchet and raising it is on the maintainer's floor. A `responseOf()` helper narrows once and throws a named error when a dispatcher answers no response at all, so the failure stays distinguishable from a wrong status. Scoped to the new suite: the older suite's nine reads are the file's share of the frozen number, and pressing that down is a separate improvement to bank, not a rider on this repair. Re-measured: runtime back to exactly 227. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH * test(runtime): the new #8287 key-mint matcher refuses combinators instead of misreading them (#8287) `check:where-matcher-conformance` went red once the engine-double gate stopped aborting the ESLint job ahead of it: `makeOrgKernel`'s find matcher is an `Object.entries(where).every(...)` body with no combinator branch, so it read `$or`/`$and` as an ordinary FIELD NAME, compared `row.$or` (undefined) against the array, matched nothing, and would have handed a suite an empty result set with nothing erroring — shape (b) in that gate's header. Fixed by refusal, not by implementing the combinator: the `makeKernel` matcher 160 lines above in this same file already refuses with this exact message, so this keeps one convention in one file, and refusal is what 140 of the 233 discovered matchers already do. The baseline is NOT touched — third shrink-only ratchet on this branch, same rule. Gate now reports 233/233 conforming, 141 by refusing (+1, exactly this matcher). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7c2f386 commit e43d63a

18 files changed

Lines changed: 1037 additions & 29 deletions
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
---
2+
"@objectstack/platform-objects": minor
3+
"@objectstack/plugin-auth": minor
4+
"@objectstack/core": minor
5+
"@objectstack/runtime": minor
6+
---
7+
8+
feat(identity): API keys are minted against the minter's active organization, and carry it into the request (#8287)
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) One additive column on
11+
an `isSystem` object declaring `protection: { lock: 'full' }`, which tenants
12+
cannot author, so there is no consumer metadata to migrate and nothing
13+
authorable is renamed, retired or tombstoned — no conversion to register. The
14+
behavioural change is that a minted key now carries an organization, that a key
15+
which cannot carry one is refused under the posture where it could never read
16+
anything, and that an ex-member's key stops authenticating. -->
17+
18+
On a deployment running `OS_TENANCY_POSTURE=isolated`, a minted API key could
19+
read **nothing at all**. `sys_api_key` carried no organization column, so key
20+
authentication established a user but no active organization — and the
21+
`isolated` Layer 0 wall is `organization_id = activeOrganizationId`, which with
22+
no active organization matches no row. Every organization-scoped read answered
23+
`200` with `total 0` while the console went on offering minting, so a tenant
24+
admin could mint a valid-looking secret and discover only at call time that it
25+
read nothing. (There was no cross-tenant leak — the failure was in the other
26+
direction.)
27+
28+
**The column was absent by an inherited rule, not by oversight.**
29+
`resolveInjectedSystemColumns` injects `organization_id` into every registered
30+
object *except* `managedBy: 'better-auth'` ones, and `sys_api_key` carries that
31+
flag — even though better-auth's `apiKey` plugin is not loaded and the table is
32+
hand-rolled ObjectStack. So the fix needs the declaration *and* the ADR-0105 D7
33+
extension-field registration to stay consistent. The read side, by contrast,
34+
was **already wired**: `resolveApiKeyPrincipal` already read an organization
35+
into `tenantId` and `resolveAuthzContext` already adopted it — it was reading a
36+
column no mint path ever wrote.
37+
38+
**What changes**
39+
40+
- `sys_api_key` declares `active_organization_id` (+ index, and the column is
41+
shown in the "My Keys" and "All" list views, because the card's complaint was
42+
a credential whose reach its owner could not see).
43+
- `POST /api/v1/keys` **inherits** the caller's active organization — there is
44+
deliberately no org parameter and no cross-org key — and **re-checks the
45+
caller's `sys_member` membership at mint time**, honouring ADR-0091 validity
46+
windows. Under a walled posture it refuses (400) rather than minting a key
47+
with no organization, and refuses (403) for an organization the caller is not
48+
a member of. The mint response echoes the organization the key is pinned to.
49+
- The verifier reads **one spelling** (Prime Directive #12): the
50+
`row.organization_id ?? row.organizationId` chain it used to carry was a
51+
consumer-side tolerance for a producer that did not exist.
52+
- An **ex-member's key fails closed at verify time** — no principal, not a
53+
degrade to a user-only principal, which would resurrect the same
54+
`200 + total 0` silent-empty. Checked at verify rather than by revoking on
55+
membership loss, because membership ends through many paths (better-auth org
56+
endpoints, SCIM, a direct `sys_member` delete, a lapsing validity window) and
57+
a hook must catch every one or it silently misses. It costs **zero extra
58+
queries**: the resolver has already read `sys_member` for this user.
59+
- **Pre-existing org-less keys are never backfilled** — that would silently
60+
upgrade credentials minted under a different promise. They keep working under
61+
`single` (no wall) and under `group` (whose wall derives from the owner's
62+
memberships independently of the active organization, so they already work
63+
there), and are **refused under `isolated`**, where they are provably dead
64+
today.
65+
66+
**The column is deliberately named `active_organization_id`, not
67+
`organization_id`** — the `sys_session` spelling, for the same concept: the
68+
organization a credential makes *active*. `objectHasOrgIdField` tests for the
69+
literal `organization_id`, and Layer 0 exempts objects without it, so the other
70+
name would have made `sys_api_key` itself org-walled. Both walled postures
71+
exclude NULL, so every pre-existing org-less row would have vanished from its
72+
**own owner's** "My Keys" list while, under `group`, continuing to
73+
authenticate — a live credential nobody could see or revoke, which is a fresh
74+
instance of the very class this change removes.

packages/core/src/security/api-key.test.ts

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
parseScopes,
1010
isExpired,
1111
resolveApiKeyPrincipal,
12+
resolveApiKeyAdmission,
13+
effectiveTenancyPosture,
1214
} from './api-key.js';
1315

1416
/** In-memory sys_api_key store exposing the `find` shape the verifier uses. */
@@ -69,7 +71,11 @@ describe('resolveApiKeyPrincipal (shared verifier)', () => {
6971
it('resolves a valid key to its principal (x-api-key)', async () => {
7072
const raw = 'osk_valid';
7173
const ql = makeQl([
72-
{ key: hashApiKey(raw), revoked: false, user_id: 'u1', organization_id: 'org1', scopes: '["read"]', expires_at: FUTURE },
74+
// [#8287] Re-spelled from `organization_id`: this fixture merely USED the
75+
// alias the verifier no longer reads, and its assertion — a valid key
76+
// resolves to owner + tenant + scopes — is unchanged and still reads a
77+
// value the mint path really produces.
78+
{ key: hashApiKey(raw), revoked: false, user_id: 'u1', active_organization_id: 'org1', scopes: '["read"]', expires_at: FUTURE },
7379
]);
7480
const p = await resolveApiKeyPrincipal(ql, { 'x-api-key': raw });
7581
expect(p).toEqual({ userId: 'u1', tenantId: 'org1', scopes: ['read'] });
@@ -103,3 +109,135 @@ describe('resolveApiKeyPrincipal (shared verifier)', () => {
103109
expect(await resolveApiKeyPrincipal({}, { 'x-api-key': 'osk_x' })).toBeUndefined();
104110
});
105111
});
112+
113+
// ── [#8287] Organization on the key ────────────────────────────────────────
114+
115+
/**
116+
* The card: under `OS_TENANCY_POSTURE=isolated` a minted key read NOTHING —
117+
* `200 + total 0` on every org-scoped object — because `sys_api_key` carried no
118+
* organization at all, and the `isolated` Layer 0 wall is
119+
* `organization_id = activeOrganizationId`. With no active organization, no row
120+
* can match. These tests pin both halves of the fix: the principal now carries
121+
* the organization the key was minted against, and a key that carries none is
122+
* REFUSED under the one posture where it is provably dead, instead of
123+
* authenticating into a silent-empty.
124+
*/
125+
describe('resolveApiKeyAdmission — organization (#8287)', () => {
126+
const raw = 'osk_org_probe';
127+
// The posture is an INPUT now, resolved by the transport from the kernel's
128+
// `tenancy` service — never read from the environment here. `effectiveTenancyPosture`
129+
// is what performs that reconciliation; these tests exercise both it and the
130+
// admission behaviour it feeds.
131+
132+
it('reads the organization off the row and carries it as tenantId', async () => {
133+
const ql = makeQl([
134+
{ key: hashApiKey(raw), revoked: false, user_id: 'u1', active_organization_id: 'org_a' },
135+
]);
136+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw });
137+
expect(admission.outcome).toBe('admitted');
138+
expect(admission.outcome === 'admitted' && admission.principal.tenantId).toBe('org_a');
139+
});
140+
141+
/**
142+
* The canonical-spelling pin (PD #12). The verifier used to read
143+
* `row.organization_id ?? row.organizationId` — a consumer-side alias chain
144+
* for a producer that did not exist. The mint path now writes exactly one
145+
* spelling, so the verifier reads exactly one: a row carrying only the OLD
146+
* names resolves to NO organization, which is the honest answer for a row no
147+
* mint path ever wrote.
148+
*/
149+
it('reads ONE spelling — the retired organization_id aliases do not resolve', async () => {
150+
const ql = makeQl([
151+
{ key: hashApiKey(raw), revoked: false, user_id: 'u1', organization_id: 'org_a', organizationId: 'org_a' },
152+
]);
153+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw });
154+
// `single` (the default posture here) admits an org-less key, so this
155+
// asserts the SPELLING, not the refusal.
156+
expect(admission.outcome === 'admitted' && admission.principal.tenantId).toBeUndefined();
157+
});
158+
159+
it('admits an org-less key under `single` — there is no wall to fail', async () => {
160+
const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]);
161+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw }, Date.now(), 'single');
162+
expect(admission.outcome).toBe('admitted');
163+
});
164+
165+
/**
166+
* `group`'s wall is `organization_id IN accessible_org_ids`, and that set is
167+
* derived from the owner's `sys_member` rows INDEPENDENTLY of the active
168+
* organization — so an org-less key already reads the union of its owner's
169+
* organizations there. Refusing it would break working deployments for no
170+
* security gain, which is why the refusal is posture-conditional rather than
171+
* "no org ⇒ no key".
172+
*/
173+
it('admits an org-less key under `group` — it already works there', async () => {
174+
const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]);
175+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw }, Date.now(), 'group');
176+
expect(admission.outcome).toBe('admitted');
177+
});
178+
179+
it('REFUSES an org-less key under `isolated` — the posture where it is provably dead', async () => {
180+
const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]);
181+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw }, Date.now(), 'isolated');
182+
expect(admission.outcome).toBe('refused');
183+
expect(admission.outcome === 'refused' && admission.reason).toBe('organization_required');
184+
// The message is the operator-facing half of "loud at call time": it must
185+
// name the posture and the remedy, not merely deny.
186+
expect(admission.outcome === 'refused' && admission.message).toMatch(/isolated/);
187+
});
188+
189+
/**
190+
* The posture must be the ENFORCED one, not the requested one. ADR-0093 D4/D5:
191+
* a deployment that asks for `isolated` without the enterprise organizations
192+
* runtime runs with NO wall, and `tenancy.isolationActive` is how the service
193+
* says so. Reading `OS_TENANCY_POSTURE` instead would refuse org-less keys on
194+
* a deployment that has no wall at all.
195+
*/
196+
it('effectiveTenancyPosture reads the ENFORCED posture from the tenancy service', () => {
197+
expect(effectiveTenancyPosture({ posture: 'isolated' })).toBe('isolated');
198+
expect(effectiveTenancyPosture({ posture: 'multi' })).toBe('isolated'); // legacy alias
199+
expect(effectiveTenancyPosture({ posture: 'group' })).toBe('group');
200+
// No posture field: fall back to the boolean the service exposes.
201+
expect(effectiveTenancyPosture({ isolationActive: true })).toBe('isolated');
202+
expect(effectiveTenancyPosture({ isolationActive: false })).toBe('single');
203+
// No service at all ⇒ undefined ⇒ callers apply no posture-conditional refusal.
204+
expect(effectiveTenancyPosture(undefined)).toBeUndefined();
205+
});
206+
207+
/**
208+
* An unknown posture must NOT refuse. This is a question about the DEPLOYMENT,
209+
* not about the credential: refusing here would break every org-less key on a
210+
* `single` deployment whose transport has not been wired.
211+
*/
212+
it('admits an org-less key when the posture is unknown (no tenancy service)', async () => {
213+
const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]);
214+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw }, Date.now(), undefined);
215+
expect(admission.outcome).toBe('admitted');
216+
});
217+
218+
it('an ORG-STAMPED key is admitted under `isolated` — the fix, not just the refusal', async () => {
219+
const ql = makeQl([
220+
{ key: hashApiKey(raw), revoked: false, user_id: 'u1', active_organization_id: 'org_a' },
221+
]);
222+
const admission = await resolveApiKeyAdmission(ql, { 'x-api-key': raw }, Date.now(), 'isolated');
223+
expect(admission.outcome).toBe('admitted');
224+
expect(admission.outcome === 'admitted' && admission.principal.tenantId).toBe('org_a');
225+
});
226+
227+
/**
228+
* A refusal must stay distinguishable from "no key" — that distinction is
229+
* the whole point of the admission type. `resolveApiKeyPrincipal` collapses
230+
* both to `undefined` so every pre-existing caller keeps failing closed.
231+
*/
232+
it('resolveApiKeyPrincipal collapses a refusal to undefined (fail-closed for old callers)', async () => {
233+
const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]);
234+
const principal = await resolveApiKeyPrincipal(ql, { 'x-api-key': raw }, Date.now(), 'isolated');
235+
expect(principal).toBeUndefined();
236+
});
237+
238+
it('an absent key is `none`, never a refusal', async () => {
239+
const ql = makeQl([{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }]);
240+
const admission = await resolveApiKeyAdmission(ql, {}, Date.now(), 'isolated');
241+
expect(admission.outcome).toBe('none');
242+
});
243+
});

0 commit comments

Comments
 (0)