diff --git a/.changeset/core-authrefusal-field-removed.md b/.changeset/core-authrefusal-field-removed.md new file mode 100644 index 0000000000..595cffbc26 --- /dev/null +++ b/.changeset/core-authrefusal-field-removed.md @@ -0,0 +1,45 @@ +--- +"@objectstack/core": minor +--- + +fix(core): `ResolvedAuthzContext.authRefusal` is removed — a published member nothing ever read (#14273) + +**BREAKING** published-type narrowing, shipped as `minor` under the repo's +launch-window convention for breaking changes. `ResolvedAuthzContext` — the +envelope `resolveAuthzContext` answers, exported from `@objectstack/core`'s +root entry — loses its optional `authRefusal?: { reason; message }` member. +Maintainer ruling 2026-09-02 (option A, ADR-0049 enforce-or-remove), +re-affirmed 2026-09-03 as A1 with the carriers a published narrowing owes +once the type was measured as public API: the member was written by the two +posture-conditional API-key refusals (`organization_required` at admission, +`organization_membership_ended` after grants) since #8287 and read by nothing +— zero runtime readers across every transport and consumer in the repo for +its whole life; only test assertions ever looked at it. + +What changes: + +- `ResolvedAuthzContext` no longer declares `authRefusal`. Code that reads + `ctx.authRefusal` stops compiling (`TS2339`); at runtime the property was + already absent from every resolved context except the two refused ones. +- The two refusals themselves are UNCHANGED: they still fire, still fail + closed (no `userId`, empty grants), and every transport still answers the + generic anonymous `401 UNAUTHENTICATED`. No status code, body or header + moves — a holder of someone else's key learns nothing, exactly as before. +- The refusal REASON is observable on exactly one surface, and it is not the + envelope: the server-side `[security] API key refused (reason) ...` `warn` + line at the decision point (#15256 / 2A), which names the key row id, + principal and organization for the operator. The pins that kept the two + reasons distinguishable through the field now read that line. +- `ApiKeyRefusalReason` and `ApiKeyAdmission` are unchanged — the reason + vocabulary still exists; it just no longer has a copy on the resolved + context. + +**Migration.** A consumer that read `ctx.authRefusal` deletes the read; there +is no replacement on the envelope, by design — disclosing the reason to a +caller (option B) was ruled out as a security-boundary question, and the +recorded fallback if a reader ever appears is an audit-side outlet (option C), +never the wire. Fail-closed handling keys on the absent `userId`, as every +in-repo transport already did. An operator who needs the reason reads the +server log line. + + diff --git a/.changeset/membership-ended-session-revoke.md b/.changeset/membership-ended-session-revoke.md index c6bb0a2580..d6725426ab 100644 --- a/.changeset/membership-ended-session-revoke.md +++ b/.changeset/membership-ended-session-revoke.md @@ -18,7 +18,7 @@ an evaluation cannot. cause. There is no Zod enum behind the column — it is free `text` — so the field's own description is the published vocabulary, and that is where the value is declared. The string deliberately matches the one the API-key arm of the same ruling family already - mints for this event (`authRefusal.reason` in `resolve-authz-context.ts`), so one grep + mints for this event (`ApiKeyRefusalReason` in `resolve-authz-context.ts`), so one grep finds every place the platform acts on a membership ending. - **The trigger acts on the ORGANIZATION'S CLAIM, never on the user** (maintainer ruling, decision batch #49 item 4, option B). A user who still holds another membership is diff --git a/packages/core/src/security/authz-store-unavailable.ts b/packages/core/src/security/authz-store-unavailable.ts index ecbd9b6228..89638bbf30 100644 --- a/packages/core/src/security/authz-store-unavailable.ts +++ b/packages/core/src/security/authz-store-unavailable.ts @@ -46,13 +46,13 @@ * ## Why a THROW, and not a field on the envelope * * The alternative was a discriminator field on `ResolvedAuthzContext` — the - * shape `authRefusal` already has. That was rejected on a MEASUREMENT, not a - * preference: `authRefusal` has existed since #8287 and, outside this module - * and its own unit test, has **zero** consumers anywhere in the repo. A - * diagnostic field on this envelope is demonstrably not read by any door. Every - * transport reads `userId` and `systemPermissions`; a new sibling field would - * have to be taught to eight separate call sites before it made a single door - * louder, and would answer the old quiet 403 at every site that was missed. + * shape `authRefusal` had (#8287). That was rejected on a MEASUREMENT, not a + * preference: from #8287 until #14273 removed it, `authRefusal` had **zero** + * consumers anywhere in the repo outside this module and test assertions — a + * reading #14273 acted on by deleting the field. Every transport reads `userId` + * and `systemPermissions`; a new sibling field would have to be taught to eight + * separate call sites before it made a single door louder, and would answer + * the old quiet 403 at every site that was missed. * * A field is quiet by default and must be deliberately made loud. A throw is * loud by default and must be deliberately silenced. On a security surface diff --git a/packages/core/src/security/resolve-authz-context.test.ts b/packages/core/src/security/resolve-authz-context.test.ts index 50186f8f67..b8d3ef2460 100644 --- a/packages/core/src/security/resolve-authz-context.test.ts +++ b/packages/core/src/security/resolve-authz-context.test.ts @@ -6,6 +6,21 @@ import { POSTURE_RANK } from './posture-ladder.js'; import { hashApiKey } from './api-key.js'; import type { AuthzPosture } from '@objectstack/spec/security'; +/** + * [#14273 A1] The refusal REASON is observable on exactly ONE surface: the + * server-side `warnApiKeyRefusal` line (#15256 / 2A). `ResolvedAuthzContext. + * authRefusal` was the envelope's copy of it and, from #8287 on, had zero + * readers outside test assertions — so the field is gone, and the pins that + * kept the two reasons DISTINGUISHABLE read the line the operator reads. + * ⛔ Not the wire: a caller still gets the generic anonymous 401. + */ +const apiKeyRefusalLines = (spy: ReturnType) => + spy.mock.calls + .map((c: unknown[]) => c.map(String).join(' ')) + .filter((l: string) => l.includes('API key refused')); +const apiKeyRefusalReasons = (spy: ReturnType) => + apiKeyRefusalLines(spy).map((l: string) => /API key refused \(([a-z_]+)\)/.exec(l)?.[1]); + /** * Contract test for the SINGLE authorization resolver. Every authorization * source MUST be honored here — this is the regression net that would have @@ -1162,12 +1177,16 @@ describe('resolveAuthzContext — API-key organization (#8287)', () => { sys_user_permission_set: [], }); + let warnSpy: ReturnType; + beforeEach(() => { warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); }); + afterEach(() => { warnSpy.mockRestore(); }); + it('adopts the key organization as the request tenant when membership holds', async () => { const ql = makeQl(tables([{ user_id: 'u1', organization_id: 'org_a', role: 'member' }])); const ctx = await resolveAuthzContext({ ql, headers: keyHeaders(), tenancyPosture: 'isolated' }); expect(ctx.userId).toBe('u1'); expect(ctx.tenantId).toBe('org_a'); - expect(ctx.authRefusal).toBeUndefined(); + expect(apiKeyRefusalLines(warnSpy)).toHaveLength(0); }); /** @@ -1186,7 +1205,7 @@ describe('resolveAuthzContext — API-key organization (#8287)', () => { expect(ctx.userId).toBeUndefined(); expect(ctx.tenantId).toBeUndefined(); expect(ctx.permissions).toEqual([]); - expect(ctx.authRefusal?.reason).toBe('organization_membership_ended'); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_membership_ended']); }); it('refuses when the membership row exists but its ADR-0091 window has lapsed', async () => { @@ -1195,14 +1214,14 @@ describe('resolveAuthzContext — API-key organization (#8287)', () => { ])); const ctx = await resolveAuthzContext({ ql, headers: keyHeaders(), tenancyPosture: 'isolated' }); expect(ctx.userId).toBeUndefined(); - expect(ctx.authRefusal?.reason).toBe('organization_membership_ended'); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_membership_ended']); }); it('the same key under `group` is refused too — the wall is membership-derived there as well', async () => { const ql = makeQl(tables([{ user_id: 'u1', organization_id: 'org_other', role: 'member' }])); const ctx = await resolveAuthzContext({ ql, headers: keyHeaders(), tenancyPosture: 'group' }); expect(ctx.userId).toBeUndefined(); - expect(ctx.authRefusal?.reason).toBe('organization_membership_ended'); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_membership_ended']); }); /** @@ -1214,7 +1233,7 @@ describe('resolveAuthzContext — API-key organization (#8287)', () => { const ql = makeQl(tables([])); const ctx = await resolveAuthzContext({ ql, headers: keyHeaders(), tenancyPosture: 'single' }); expect(ctx.userId).toBe('u1'); - expect(ctx.authRefusal).toBeUndefined(); + expect(apiKeyRefusalLines(warnSpy)).toHaveLength(0); }); /** @@ -1238,7 +1257,7 @@ describe('resolveAuthzContext — API-key organization (#8287)', () => { tenancyPosture: 'isolated', }); expect(ctx.userId).toBeUndefined(); - expect(ctx.authRefusal?.reason).toBe('organization_required'); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_required']); }); /** @@ -1247,6 +1266,35 @@ describe('resolveAuthzContext — API-key organization (#8287)', () => { * how that stays true: a later refactor that re-reads `sys_member` for this * check turns a free assertion into a per-request cost, silently. */ + /** + * [#14273 A1] The envelope carries NO refusal field. Pinned by own-property + * on BOTH refusal paths, so a writer that re-adds the member under any name + * or type reddens here — a typed read cannot pin an absence the compiler + * already refuses. The reason lives on the warn line (above) and nowhere on + * the context; the wire never carried it and still does not. + */ + it('[#14273] a post-grant refusal answers an envelope with no `authRefusal` — the reason is server-side only', async () => { + const ql = makeQl(tables([{ user_id: 'u1', organization_id: 'org_other', role: 'member' }])); + const ctx = await resolveAuthzContext({ ql, headers: keyHeaders(), tenancyPosture: 'isolated' }); + expect(ctx.userId).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(ctx, 'authRefusal')).toBe(false); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_membership_ended']); + }); + + it('[#14273] an admission refusal answers an envelope with no `authRefusal` either', async () => { + const ql = makeQl({ + sys_api_key: [{ key: hashApiKey(raw), revoked: false, user_id: 'u1' }], + sys_user: [{ id: 'u1' }], + sys_member: [{ user_id: 'u1', organization_id: 'org_a', role: 'owner' }], + sys_user_position: [], + sys_user_permission_set: [], + }); + const ctx = await resolveAuthzContext({ ql, headers: keyHeaders(), tenancyPosture: 'isolated' }); + expect(ctx.userId).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(ctx, 'authRefusal')).toBe(false); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_required']); + }); + it('costs zero additional queries (sys_member is read once)', async () => { let memberReads = 0; const inner = makeQl(tables([{ user_id: 'u1', organization_id: 'org_a', role: 'member' }])); @@ -1663,7 +1711,7 @@ describe('[#15409] a session organization claim that no membership backs', () => // switch to it instead of being signed out of everything. expect(ctx.accessible_org_ids).toEqual(['org_beta']); // ⛔ And it is NOT the API-key refusal: no principal was refused. - expect(ctx.authRefusal).toBeUndefined(); + expect(apiKeyRefusalLines(warnSpy)).toHaveLength(0); }); /** @@ -1778,7 +1826,7 @@ describe('[#15409] a session organization claim that no membership backs', () => ql, headers: { 'x-api-key': raw }, tenancyPosture: 'isolated', }); expect(ctx.userId).toBeUndefined(); - expect(ctx.authRefusal?.reason).toBe('organization_membership_ended'); + expect(apiKeyRefusalReasons(warnSpy)).toEqual(['organization_membership_ended']); // ⛔ Not degraded into the session's drop: the key is REFUSED, not trimmed. expect(dropLines()).toHaveLength(0); }); diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 9928f2cfc4..e4f015df70 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -114,26 +114,6 @@ export interface ResolvedAuthzContext { * anonymous requests carry no rung. */ posture?: AuthzPosture; - /** - * [#8287] Set when an inbound API key was REFUSED — a real, intact - * credential this deployment's tenancy posture cannot admit. The context is - * otherwise EMPTY (no `userId`), so every transport already fails it closed - * to 401 with no change; this field only lets a transport that wants to say - * WHY do so, instead of answering the operator with a bare "unauthenticated" - * for a key they can see is neither revoked nor expired. - * - * ⚠️ `reason` is NOT an `error.code`. The wire vocabulary is closed - * (ADR-0112: `StandardErrorCode ∪ ERROR_CODE_LEDGER`, both in `packages/spec`) - * and a refused credential's standard member is `UNAUTHENTICATED`. This is a - * diagnostic discriminator for the message, deliberately lowercase so it can - * never be mistaken for one. - * - * ⚠️ [#14273 A1] This field has ZERO consumers outside test assertions and is - * REMOVED by that card, in its own PR. The operator exit it was meant to be - * is now {@link warnApiKeyRefusal}'s server-side `warn` line (#15256 / 2A), - * which is why removing it costs nothing. ⛔ Not removed here. - */ - authRefusal?: { reason: ApiKeyRefusalReason; message: string }; } export interface ResolveAuthzInput { @@ -182,9 +162,9 @@ function safeJsonParse(s: string, fallback: T): T { * is neither revoked nor expired, and a 401 that says only "unauthenticated". * * `ResolvedAuthzContext.authRefusal` was that exit and never got a consumer - * (zero readers outside two test assertions); #14273's A1 ruling REMOVES the - * field in its own PR. ⛔ Not removed here — cross-referenced only. This log - * line is the operator exit that field never delivered. + * (zero readers outside test assertions, #8287 through #14273); #14273's A1 + * ruling removed the field. This log line is the operator exit that field + * never delivered, and the one place the refusal REASON is observable. * * ## What may appear here * @@ -407,7 +387,6 @@ export async function resolveAuthzContext(input: ResolveAuthzInput): Promise { @@ -297,7 +297,7 @@ describe('the payload agrees with the ONE authorization authority, set for set', expect(activeOrg, `expected no active org, got ${activeOrg}`).toBeFalsy(); // The resolver projects EVERY membership when no tenant scopes it - // (`resolve-authz-context.ts:815`), so these names are ADDED to a payload + // (`resolve-authz-context.ts:788`), so these names are ADDED to a payload // that carried none of them before this card. That is the behaviour change // the changeset's carve-out names. const positions = (envelope as any)?.user?.positions ?? []; diff --git a/packages/rest/src/execctx-authz-input-seam-reachability.test.ts b/packages/rest/src/execctx-authz-input-seam-reachability.test.ts index 0d628851bd..2c289b5a80 100644 --- a/packages/rest/src/execctx-authz-input-seam-reachability.test.ts +++ b/packages/rest/src/execctx-authz-input-seam-reachability.test.ts @@ -88,7 +88,7 @@ * outside vitest (it needs git history) and is recorded on the card. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname, resolve } from 'node:path'; @@ -515,15 +515,28 @@ describe('[#13906] §2 — the Layer 0 ex-member refusal, and what a failed post // Same fixtures, same real resolver, posture present: the refusal fires // and carries its reason. This is what distinguishes "the refusal was // skipped" (§ next) from "the refusal never applied to this fixture". - const headers = new Headers({ 'x-api-key': RAW_EXMEMBER_KEY }); - const authz = await resolveAuthzContext({ - ql: qlWith({ memberships: MEMBER_ROWS }), - headers, - getSession: async () => undefined, - tenancyPosture: 'isolated', - } as any); - expect(authz.authRefusal?.reason).toBe('organization_membership_ended'); - expect(authz.userId).toBeUndefined(); + // [#14273 A1] `ResolvedAuthzContext` carries no refusal field any more + // (zero readers; removed). The ONE surface that names the refusal is the + // server-side `warnApiKeyRefusal` line (#15256 / 2A), so the mechanism + // control reads that — and the wire above still answers the anonymous floor. + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const headers = new Headers({ 'x-api-key': RAW_EXMEMBER_KEY }); + const authz = await resolveAuthzContext({ + ql: qlWith({ memberships: MEMBER_ROWS }), + headers, + getSession: async () => undefined, + tenancyPosture: 'isolated', + } as any); + expect(authz.userId).toBeUndefined(); + const refused = warnSpy.mock.calls + .map((c: unknown[]) => c.map(String).join(' ')) + .filter((l: string) => l.includes('API key refused')); + expect(refused).toHaveLength(1); + expect(refused[0]).toContain('API key refused (organization_membership_ended)'); + } finally { + warnSpy.mockRestore(); + } }); it('REPAIRED [decision 1 A]: tenancy REGISTERED AND FAILING (factory throws) → 503 outage, no longer a served 200', async () => { @@ -544,19 +557,28 @@ describe('[#13906] §2 — the Layer 0 ex-member refusal, and what a failed post }); it('⚠️ MEASURED PERMISSIVE (mechanism): with the posture absent the resolver ADMITS the ex-member as a full principal', async () => { - const headers = new Headers({ 'x-api-key': RAW_EXMEMBER_KEY }); - const authz = await resolveAuthzContext({ - ql: qlWith({ memberships: MEMBER_ROWS }), - headers, - getSession: async () => undefined, - tenancyPosture: undefined, - } as any); - expect(authz.authRefusal).toBeUndefined(); - expect(authz.userId).toBe('u_exmember'); - // The membership fact is IN HAND and says "not a member of org_A" — the - // refusal was gated off by the missing posture, not by missing data. - expect(authz.accessible_org_ids).not.toContain('org_A'); - expect(authz.tenantId).toBe('org_A'); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const headers = new Headers({ 'x-api-key': RAW_EXMEMBER_KEY }); + const authz = await resolveAuthzContext({ + ql: qlWith({ memberships: MEMBER_ROWS }), + headers, + getSession: async () => undefined, + tenancyPosture: undefined, + } as any); + // [#14273 A1] No refusal fired — the warn line is the refusal's only + // surface and it is silent: an ADMISSION, not a quiet refusal. + expect(warnSpy.mock.calls + .map((c: unknown[]) => c.map(String).join(' ')) + .filter((l: string) => l.includes('API key refused'))).toHaveLength(0); + expect(authz.userId).toBe('u_exmember'); + // The membership fact is IN HAND and says "not a member of org_A" — the + // refusal was gated off by the missing posture, not by missing data. + expect(authz.accessible_org_ids).not.toContain('org_A'); + expect(authz.tenantId).toBe('org_A'); + } finally { + warnSpy.mockRestore(); + } }); it('THE COLLAPSE IS ENDED: "registered and failed" and "never registered" no longer answer alike', async () => {