From 8e336ff228eb467834e077897312d408f97a2b8f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 14:27:41 +0000 Subject: [PATCH] fix(security): explain resolves the ONE authorization aggregation (#6352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildContextForUser` was a hand-written second implementation of `@objectstack/core`'s `resolveAuthzContext` aggregation, kept in step by two comments claiming it mirrored the resolver. Measured over identical rows it did not: it dropped the `sys_member` role positions, every position-bound permission set (`sys_position_permission_set`), the `everyone` anchor's bound sets, the `platform_admin` position projection, `systemPermissions`, the posture rung, `email` and the `ai_seat` synthesis. Permission sets resolve BY NAME from `positions` + `permissions`, and a position-carried set only becomes a name inside the resolver — so any user granted through a position was explained as holding nothing, and the panel reported a denial enforcement never made. It now calls `resolveUserAuthzGrants` and adds presentation only: the ADR-0091 expired / `delegated_from` row annotations the resolver correctly discards, and `hasPlatformAdminGrant`, read back off the resolver's own posture verdict. Pinned by a parity suite running both implementations over the same fixture rows with per-case expected output, so it cannot pass by both sides resolving to nothing. `check:authz-resolver`'s ALLOW entry is narrowed to what still trips its heuristic — the explain-only provenance pass — without touching its remit. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- ...xplain-context-single-authz-aggregation.md | 45 +++ .../src/explain-engine.test.ts | 359 +++++++++++++++--- .../plugin-security/src/explain-engine.ts | 241 +++++++----- scripts/check-single-authz-resolver.mjs | 19 +- 4 files changed, 499 insertions(+), 165 deletions(-) create mode 100644 .changeset/explain-context-single-authz-aggregation.md diff --git a/.changeset/explain-context-single-authz-aggregation.md b/.changeset/explain-context-single-authz-aggregation.md new file mode 100644 index 0000000000..bf9f37e7bf --- /dev/null +++ b/.changeset/explain-context-single-authz-aggregation.md @@ -0,0 +1,45 @@ +--- +'@objectstack/plugin-security': minor +--- + +Explain and enforcement now resolve ONE authorization aggregation (#6352). + +`buildContextForUser()` — the explain API's reconstruction of an arbitrary user's +context, behind `explain(request, callerContext)` and the `userId` parameter — was +a hand-written second implementation of `@objectstack/core`'s `resolveAuthzContext` +aggregation. Its agreement with enforcement was guaranteed by two comments saying +it mirrored the resolver ("mirroring the runtime resolver's semantics", "we compute +it here with the IDENTICAL rule") and by nothing else: no assertion anywhere in the +repo compared the two. + +It did not agree. Measured over identical rows, the mirror dropped: + +| input | resolver | explain mirror | +|---|---|---| +| `sys_member` role positions (ADR-0095 D3) | `org_admin`, … | — | +| position-bound permission sets (`sys_position_permission_set`) | resolved | — | +| the `everyone` anchor's bound sets (ADR-0090 D5) | resolved | — | +| `platform_admin` position projection (ADR-0068 D2) | projected | — | +| `systemPermissions` / `posture` / `email` / `ai_seat` | resolved | — | + +The user-visible consequence: permission sets are resolved BY NAME from +`context.positions ∪ context.permissions`, and a set carried by a POSITION only +becomes a name inside the resolver. So for any user whose grants arrive through a +position — the ordinary way an org grants access — the explain panel resolved fewer +sets than enforcement and reported a denial the runtime never made. A security UI +that says "you have no access" about access you have is worse than no panel. + +`buildContextForUser` now calls `resolveUserAuthzGrants` (core's userId-driven +resolver core, already the same entry point `runAs:'user'` automation runs use) and +adds presentation only: the ADR-0091 expired-grant and `delegated_from` annotations +the resolver correctly discards, and `hasPlatformAdminGrant`, which is now read +back off the resolver's own posture verdict instead of recomputed. The returned +context additionally carries `systemPermissions`, `org_user_ids`, `posture`, +`tabPermissions` and `email` — additive; no field was removed or renamed. + +Pinned by a parity suite that runs both implementations over the same fixture rows +(org role projection, position-bound sets, the `everyone` anchor, both +`platform_admin` polarities, `organization_admin` → `TENANT_ADMIN`, ADR-0091 +windows) and asserts each case's concrete expected output, so the pin cannot pass +by both sides resolving to nothing. Restoring the mirror turns 9 of those cases +red. diff --git a/packages/plugins/plugin-security/src/explain-engine.test.ts b/packages/plugins/plugin-security/src/explain-engine.test.ts index 5ed2a2f399..f903676b65 100644 --- a/packages/plugins/plugin-security/src/explain-engine.test.ts +++ b/packages/plugins/plugin-security/src/explain-engine.test.ts @@ -2,6 +2,7 @@ // ADR-0090 D6 — explain engine: layer verdicts, attribution, machine artifact. import { describe, it, expect } from 'vitest'; +import { resolveUserAuthzGrants } from '@objectstack/core'; import { PermissionSetSchema } from '@objectstack/spec/security'; import { PermissionEvaluator } from './permission-evaluator'; import { explainAccess, buildContextForUser, type ExplainEngineDeps } from './explain-engine'; @@ -420,42 +421,77 @@ describe('posture derivation aligns with enforcement (label-drift elimination)', }); }); -describe('buildContextForUser', () => { - const ql = { +// ─── buildContextForUser (#6352) ──────────────────────────────────────────── +// +// `buildContextForUser` no longer aggregates anything. It calls +// `@objectstack/core`'s `resolveUserAuthzGrants` — the SAME function every +// inbound request resolves through — and adds only presentation: the ADR-0091 +// expired / delegated row annotations, and `hasPlatformAdminGrant` read back off +// the resolver's own posture verdict. +// +// ⚠ The fixtures below use a `where`-HONOURING fake, and that is load-bearing, +// not tidiness. The deleted mirror filtered rows in memory, so it produced the +// right answer even against a fake that ignored `where` and returned every row +// for a table. The resolver delegates filtering to the engine, exactly as the +// real ObjectQL engine does, so a fake that ignores `where` now reports grants +// nobody holds — a fixture defect the old shape was blind to. + +type Rows = Record; + +/** Minimal `where`-honouring ObjectQL stand-in: scalar equality and `$in`. */ +function makeGrantQl(tables: Rows) { + return { async find(object: string, opts: any) { - if (object === 'sys_user_position') return [{ user_id: 'u2', position: 'hr_specialist' }]; - if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'ps1' }]; - if (object === 'sys_permission_set') return [{ id: 'ps1', name: 'payroll_reader' }]; - return []; + const where = opts?.where ?? {}; + return (tables[object] ?? []).filter((row) => + Object.entries(where).every(([key, cond]) => { + const cell = row[key]; + if (cond && typeof cond === 'object' && '$in' in (cond as any)) { + return ((cond as any).$in as unknown[]).includes(cell); + } + return cell === cond; + }), + ); }, }; +} + +const NOW = Date.parse('2026-07-10T12:00:00Z'); + +describe('buildContextForUser', () => { + const ql = makeGrantQl({ + sys_user_position: [{ user_id: 'u2', position: 'hr_specialist' }], + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'ps1' }], + sys_permission_set: [{ id: 'ps1', name: 'payroll_reader' }], + }); it('derives hasPlatformAdminGrant from an UNSCOPED admin_full_access user grant (matches resolveAuthzContext)', async () => { - const qlUnscoped = { - async find(object: string, _opts: any) { - if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'psAdmin' /* organization_id absent → unscoped */ }]; - if (object === 'sys_permission_set') return [{ id: 'psAdmin', name: 'admin_full_access' }]; - return []; - }, - }; + const qlUnscoped = makeGrantQl({ + // organization_id absent → unscoped + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psAdmin' }], + sys_permission_set: [{ id: 'psAdmin', name: 'admin_full_access' }], + }); const ctx = await buildContextForUser(qlUnscoped, 'u2'); expect(ctx.hasPlatformAdminGrant).toBe(true); expect(ctx.permissions).toContain('admin_full_access'); + // The resolver PROJECTS the built-in position from the same grant (ADR-0068 + // D2) and resolves the rung once — both now reach the panel unchanged. + expect(ctx.positions).toContain('platform_admin'); + expect(ctx.posture).toBe('PLATFORM_ADMIN'); }); it('a SCOPED (org-specific) admin_full_access user grant does NOT set hasPlatformAdminGrant', async () => { - const qlScoped = { - async find(object: string, _opts: any) { - if (object === 'sys_user_permission_set') return [{ user_id: 'u2', permission_set_id: 'psAdmin', organization_id: 'org1' }]; - if (object === 'sys_permission_set') return [{ id: 'psAdmin', name: 'admin_full_access' }]; - return []; - }, - }; + const qlScoped = makeGrantQl({ + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psAdmin', organization_id: 'org1' }], + sys_permission_set: [{ id: 'psAdmin', name: 'admin_full_access' }], + }); const ctx = await buildContextForUser(qlScoped, 'u2'); expect(ctx.hasPlatformAdminGrant).toBe(false); // The name is still resolved into permissions (it grants object CRUD), but it // no longer confers platform_admin posture — the drift this closes. expect(ctx.permissions).toContain('admin_full_access'); + expect(ctx.positions).not.toContain('platform_admin'); + expect(ctx.posture).toBe('MEMBER'); }); it('reconstructs positions + direct grants + the everyone anchor', async () => { @@ -464,10 +500,16 @@ describe('buildContextForUser', () => { userId: 'u2', positions: ['hr_specialist', 'everyone'], permissions: ['payroll_reader'], + // [#6352] The resolver's full envelope now reaches the panel. These four + // were MISSING from the hand-written mirror, which is why an explanation + // could disagree with enforcement — see the parity suite below. + systemPermissions: [], + org_user_ids: ['u2'], + posture: 'MEMBER', // [ADR-0105 D2] The DELEGATOR's own org access set, resolved here rather // than inherited — a delegated read is bounded by the delegator's own - // memberships. This fixture's `ql` serves no `sys_member` rows, so it is - // empty, which fails the `group` wall closed. + // memberships. This fixture serves no `sys_member` rows, so it is empty, + // which fails the `group` wall closed. accessible_org_ids: [], expiredGrants: [], delegatedPositions: [], @@ -476,18 +518,12 @@ describe('buildContextForUser', () => { }); it('surfaces delegation provenance for a position held via a delegated_from row (ADR-0091 D3)', async () => { - const NOW = Date.parse('2026-07-10T12:00:00Z'); - const qlDelegated = { - async find(object: string, _opts: any) { - if (object === 'sys_user_position') { - return [ - { user_id: 'u2', position: 'hr_specialist' }, - { user_id: 'u2', position: 'approver', delegated_from: 'u_boss', valid_until: '2026-07-20T00:00:00Z' }, - ]; - } - return []; - }, - }; + const qlDelegated = makeGrantQl({ + sys_user_position: [ + { user_id: 'u2', position: 'hr_specialist' }, + { user_id: 'u2', position: 'approver', delegated_from: 'u_boss', valid_until: '2026-07-20T00:00:00Z' }, + ], + }); const ctx = await buildContextForUser(qlDelegated, 'u2', NOW); expect(ctx.positions).toEqual(['hr_specialist', 'approver', 'everyone']); expect(ctx.delegatedPositions).toEqual([ @@ -496,32 +532,22 @@ describe('buildContextForUser', () => { }); it('filters grants outside their validity window and reports them as expired (ADR-0091 D2)', async () => { - const NOW = Date.parse('2026-07-10T12:00:00Z'); - const qlWindowed = { - async find(object: string, _opts: any) { - if (object === 'sys_user_position') { - return [ - { user_id: 'u2', position: 'hr_specialist' }, - { user_id: 'u2', position: 'payroll_approver', valid_until: '2026-07-01T00:00:00Z' }, - // Pending (future valid_from) is filtered but NOT reported as expired. - { user_id: 'u2', position: 'auditor', valid_from: '2026-08-01T00:00:00Z' }, - ]; - } - if (object === 'sys_user_permission_set') { - return [ - { user_id: 'u2', permission_set_id: 'ps1' }, - { user_id: 'u2', permission_set_id: 'ps2', valid_until: '2026-06-01T00:00:00Z' }, - ]; - } - if (object === 'sys_permission_set') { - return [ - { id: 'ps1', name: 'payroll_reader' }, - { id: 'ps2', name: 'quarter_close_admin' }, - ]; - } - return []; - }, - }; + const qlWindowed = makeGrantQl({ + sys_user_position: [ + { user_id: 'u2', position: 'hr_specialist' }, + { user_id: 'u2', position: 'payroll_approver', valid_until: '2026-07-01T00:00:00Z' }, + // Pending (future valid_from) is filtered but NOT reported as expired. + { user_id: 'u2', position: 'auditor', valid_from: '2026-08-01T00:00:00Z' }, + ], + sys_user_permission_set: [ + { user_id: 'u2', permission_set_id: 'ps1' }, + { user_id: 'u2', permission_set_id: 'ps2', valid_until: '2026-06-01T00:00:00Z' }, + ], + sys_permission_set: [ + { id: 'ps1', name: 'payroll_reader' }, + { id: 'ps2', name: 'quarter_close_admin' }, + ], + }); const ctx = await buildContextForUser(qlWindowed, 'u2', NOW); expect(ctx.positions).toEqual(['hr_specialist', 'everyone']); expect(ctx.permissions).toEqual(['payroll_reader']); @@ -532,6 +558,219 @@ describe('buildContextForUser', () => { }); }); +// ─── [#6352] The explain panel and enforcement resolve ONE aggregation ─────── +// +// Every case below runs `buildContextForUser` and `resolveUserAuthzGrants` over +// the SAME rows and asserts they agree — the assertion nothing in the repo made +// while the mirror existed. Convergence makes agreement structural rather than +// coincidental, so the suite's real job is the second half of each case: the +// `expected` block pins what the shared aggregation must actually PRODUCE, so +// the pin can never pass by both sides resolving to nothing. +// +// Each `expected` block was RED before convergence. Measured against the mirror +// on the first fixture's rows: it returned positions `['hr_specialist', +// 'everyone']` (no `sys_member` role projection) and permissions +// `['payroll_reader']` (no position-bound set, no `ai_seat`), against the +// resolver's `['org_admin', 'hr_specialist', 'everyone']` and +// `['payroll_reader', 'hr_tools', 'ai_seat']`. A user whose grants arrive +// through a POSITION was explained as holding none of them — the panel denying +// what enforcement allows, which is the exact failure an explain panel exists +// to prevent. +describe('buildContextForUser ↔ resolveUserAuthzGrants parity (#6352)', () => { + const PARITY_CASES: Array<{ + name: string; + tables: Rows; + expected: { + positions: string[]; + permissions: string[]; + systemPermissions: string[]; + accessible_org_ids: string[]; + posture: string; + hasPlatformAdminGrant: boolean; + }; + }> = [ + { + // The measured pre-change divergence, in one fixture: an org role + // (ADR-0095 D3 `sys_member` projection), a platform-RBAC position + // (ADR-0057 D4), a permission set the POSITION carries + // (`sys_position_permission_set`), and the ADR-0024 `ai_seat` synthesis. + name: 'org role + position-bound permission set + ai_seat', + tables: { + sys_user: [{ id: 'u2', email: 'u2@example.com', ai_access: true }], + sys_member: [{ user_id: 'u2', organization_id: 'org1', role: 'admin' }], + sys_user_position: [{ user_id: 'u2', position: 'hr_specialist' }], + sys_position: [{ id: 'pos_hr', name: 'hr_specialist' }], + sys_position_permission_set: [{ position_id: 'pos_hr', permission_set_id: 'ps_hr_tools' }], + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'ps1' }], + sys_permission_set: [ + { id: 'ps1', name: 'payroll_reader' }, + { id: 'ps_hr_tools', name: 'hr_tools', system_permissions: ['manage_users'] }, + ], + }, + expected: { + positions: ['org_admin', 'hr_specialist', 'everyone'], + permissions: ['payroll_reader', 'hr_tools', 'ai_seat'], + systemPermissions: ['manage_users'], + accessible_org_ids: ['org1'], + posture: 'MEMBER', + hasPlatformAdminGrant: false, + }, + }, + { + // [ADR-0090 D5] The audience anchor is not decoration: a set bound to the + // implicit `everyone` position must RESOLVE. The mirror pushed `everyone` + // onto the list and then read no position-bound sets at all, so anything + // granted this way was invisible to the panel. + name: 'everyone-anchor-bound permission set resolves', + tables: { + sys_position: [{ id: 'pos_everyone', name: 'everyone' }], + sys_position_permission_set: [{ position_id: 'pos_everyone', permission_set_id: 'ps_base' }], + sys_permission_set: [{ id: 'ps_base', name: 'company_directory' }], + }, + expected: { + positions: ['everyone'], + permissions: ['company_directory'], + systemPermissions: [], + accessible_org_ids: [], + posture: 'MEMBER', + hasPlatformAdminGrant: false, + }, + }, + { + // [ADR-0068 D2] The platform_admin derivation, both polarities. + name: 'unscoped admin_full_access derives platform_admin', + tables: { + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psAdmin' }], + sys_permission_set: [{ id: 'psAdmin', name: 'admin_full_access' }], + }, + expected: { + positions: ['platform_admin', 'everyone'], + permissions: ['admin_full_access'], + systemPermissions: [], + accessible_org_ids: [], + posture: 'PLATFORM_ADMIN', + hasPlatformAdminGrant: true, + }, + }, + { + name: 'org-scoped admin_full_access does NOT derive platform_admin', + tables: { + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psAdmin', organization_id: 'org1' }], + sys_permission_set: [{ id: 'psAdmin', name: 'admin_full_access' }], + }, + expected: { + positions: ['everyone'], + permissions: ['admin_full_access'], + systemPermissions: [], + accessible_org_ids: [], + posture: 'MEMBER', + hasPlatformAdminGrant: false, + }, + }, + { + // [ADR-0095 D3] The org-admin rung comes from the CAPABILITY grant + // `auto-org-admin-grant` writes, never from the better-auth role. + name: 'organization_admin capability grant derives TENANT_ADMIN', + tables: { + sys_member: [{ user_id: 'u2', organization_id: 'org1', role: 'admin' }], + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'psOrg', organization_id: 'org1' }], + sys_permission_set: [{ id: 'psOrg', name: 'organization_admin' }], + }, + expected: { + positions: ['org_admin', 'everyone'], + permissions: ['organization_admin'], + systemPermissions: [], + accessible_org_ids: ['org1'], + posture: 'TENANT_ADMIN', + hasPlatformAdminGrant: false, + }, + }, + { + // [ADR-0091 D2] Validity windows drop rows on BOTH sides, including the + // platform_admin derivation: an EXPIRED unscoped admin_full_access grant + // must not confer the rung. Only the explain-side annotation survives. + name: 'ADR-0091 windows: expired admin grant confers nothing', + tables: { + sys_user_position: [ + { user_id: 'u2', position: 'hr_specialist' }, + { user_id: 'u2', position: 'payroll_approver', valid_until: '2026-07-01T00:00:00Z' }, + { user_id: 'u2', position: 'auditor', valid_from: '2026-08-01T00:00:00Z' }, + ], + sys_user_permission_set: [ + { user_id: 'u2', permission_set_id: 'ps1' }, + { user_id: 'u2', permission_set_id: 'psAdmin', valid_until: '2026-06-01T00:00:00Z' }, + ], + sys_permission_set: [ + { id: 'ps1', name: 'payroll_reader' }, + { id: 'psAdmin', name: 'admin_full_access' }, + ], + }, + expected: { + positions: ['hr_specialist', 'everyone'], + permissions: ['payroll_reader'], + systemPermissions: [], + accessible_org_ids: [], + posture: 'MEMBER', + hasPlatformAdminGrant: false, + }, + }, + ]; + + for (const { name, tables, expected } of PARITY_CASES) { + it(`agrees with the enforcement resolver — ${name}`, async () => { + const grants = await resolveUserAuthzGrants(makeGrantQl(tables), 'u2', { nowMs: NOW }); + const ctx = await buildContextForUser(makeGrantQl(tables), 'u2', NOW); + + // (a) The two agree, field for field, on the whole aggregation surface. + expect(ctx.positions).toEqual(grants.positions); + expect(ctx.permissions).toEqual(grants.permissions); + expect(ctx.systemPermissions).toEqual(grants.systemPermissions); + expect(ctx.accessible_org_ids).toEqual(grants.accessible_org_ids); + expect(ctx.org_user_ids).toEqual(grants.org_user_ids); + expect(ctx.posture).toEqual(grants.posture); + expect(ctx.hasPlatformAdminGrant).toBe(grants.posture === 'PLATFORM_ADMIN'); + + // (b) Non-vacuity: agreeing on nothing is not agreement. Pin what the one + // aggregation must actually produce for these rows. + expect(ctx.positions).toEqual(expected.positions); + expect(ctx.permissions).toEqual(expected.permissions); + expect(ctx.systemPermissions).toEqual(expected.systemPermissions); + expect(ctx.accessible_org_ids).toEqual(expected.accessible_org_ids); + expect(ctx.posture).toBe(expected.posture); + expect(ctx.hasPlatformAdminGrant).toBe(expected.hasPlatformAdminGrant); + }); + } + + it('the explain-only surface is ADDITIVE — it annotates rows, it never changes the verdict', async () => { + const tables: Rows = { + sys_user_position: [ + { user_id: 'u2', position: 'hr_specialist' }, + { user_id: 'u2', position: 'approver', delegated_from: 'u_boss', valid_until: '2026-07-20T00:00:00Z' }, + { user_id: 'u2', position: 'payroll_approver', valid_until: '2026-07-01T00:00:00Z' }, + ], + sys_user_permission_set: [{ user_id: 'u2', permission_set_id: 'ps2', valid_until: '2026-06-01T00:00:00Z' }], + sys_permission_set: [{ id: 'ps2', name: 'quarter_close_admin' }], + }; + const grants = await resolveUserAuthzGrants(makeGrantQl(tables), 'u2', { nowMs: NOW }); + const ctx = await buildContextForUser(makeGrantQl(tables), 'u2', NOW); + + // The annotations are non-empty… + expect(ctx.delegatedPositions).toEqual([ + { name: 'approver', from: 'u_boss', until: '2026-07-20T00:00:00Z' }, + ]); + expect(ctx.expiredGrants).toEqual([ + { kind: 'position', name: 'payroll_approver', until: '2026-07-01T00:00:00Z' }, + { kind: 'permission_set', name: 'quarter_close_admin', until: '2026-06-01T00:00:00Z' }, + ]); + // …and the aggregation is still byte-identical to enforcement's. An expired + // grant is REPORTED, never RESOLVED. + expect(ctx.positions).toEqual(grants.positions); + expect(ctx.permissions).toEqual(grants.permissions); + expect(ctx.positions).not.toContain('payroll_approver'); + expect(ctx.permissions).not.toContain('quarter_close_admin'); + }); +}); + // ─── [#3544] the export axis is explainable ──────────────────────────────── // // Without this the axis is undiagnosable: a caller hits 403 diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 7ab7135794..ccb99012fb 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -19,9 +19,14 @@ * the SEMANTIC impact of a grant change instead of a JSON diff. */ -import { isGrantActive, isGrantExpired, derivePosture as deriveAdminPosture } from '@objectstack/core'; +import { + isGrantActive, + isGrantExpired, + derivePosture as deriveAdminPosture, + resolveUserAuthzGrants, +} from '@objectstack/core'; import { matchesFilterCondition } from '@objectstack/formula'; -import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ADMIN_FULL_ACCESS, ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec'; +import { BUILTIN_IDENTITY_PLATFORM_ADMIN, ORGANIZATION_ADMIN_GRANTS } from '@objectstack/spec'; import type { PermissionSet } from '@objectstack/spec/security'; import type { AuthzPosture, @@ -75,17 +80,21 @@ function isAuthzPosture(v: unknown): v is AuthzPosture { * 2. **Reuse `ctx.posture` verbatim when present.** A principal resolved through * the full `resolveAuthzContext` already carries the enforcement-derived * rung; consuming it directly makes drift structurally impossible. - * 3. **Fallback — re-derive from capability-grant evidence.** The explain API's - * `buildContextForUser` builds a context WITHOUT running the full - * `resolveAuthzContext`, so no `posture` is attached. We then derive from the - * SAME evidence `resolveAuthzContext` uses — NOT the previous loose - * permission-set-NAME match: + * 3. **Fallback — re-derive from capability-grant evidence.** Reached only by a + * HAND-BUILT context (tests, an internal caller assembling `{ userId, + * positions, permissions }` itself). Since #6352 the explain API's + * `buildContextForUser` resolves through `resolveUserAuthzGrants` and + * therefore carries the enforcement-derived `posture`, so it lands on (2) — + * structurally, not by agreement. This branch derives from the SAME evidence + * `resolveAuthzContext` uses — NOT the previous loose permission-set-NAME + * match: * - `PLATFORM_ADMIN` ← the **unscoped `admin_full_access` USER grant** - * (`hasPlatformAdminGrant`, computed by `buildContextForUser` byte-for- - * byte as `resolveAuthzContext` computes it), OR the `platform_admin` - * built-in position (which is itself only ever PROJECTED from that same - * grant — ADR-0068 D2). A merely-SCOPED `admin_full_access` grant (name - * present in `permissions`, not held unscoped) no longer over-labels. + * (`hasPlatformAdminGrant`, which `buildContextForUser` now READS OFF the + * resolver's own verdict rather than recomputing), OR the + * `platform_admin` built-in position (which is itself only ever + * PROJECTED from that same grant — ADR-0068 D2). A merely-SCOPED + * `admin_full_access` grant (name present in `permissions`, not held + * unscoped) no longer over-labels. * - `TENANT_ADMIN` ← the `organization_admin` **capability** grant, exactly * like enforcement (ADR-0095 D3). The better-auth `org_owner`/`org_admin` * role positions are a provisioning source only and are no longer read @@ -230,105 +239,136 @@ export interface ExplainInput { recordId?: string; } +/** The ADR-0091 `valid_until` of a grant row, as the panel prints it. */ +function untilOfGrantRow(r: any): string | undefined { + const v = r?.valid_until ?? r?.validUntil; + return v == null || v === '' ? undefined : String(v); +} + /** - * Reconstruct an evaluation context for an arbitrary user, mirroring the - * runtime resolver's semantics (`@objectstack/core` resolveAuthzContext): - * positions from `sys_user_position` (+ the implicit `everyone` anchor, - * ADR-0090 D5/D9), direct grants from `sys_user_permission_set`. Used by the - * explain API's `userId` parameter — the caller-facing authorization for - * explaining OTHERS lives in the route/service wrapper, not here. + * [#6352 / ADR-0091 D2/D3] The explain-ONLY provenance pass: the two annotations + * the panel prints that the authorization resolver, correctly, throws away. + * + * This is presentation, not aggregation. It decides nothing about who is + * authorized — `resolveUserAuthzGrants` has already decided that, and this pass + * never feeds `positions` / `permissions` / the `platform_admin` derivation. It + * only re-reads the same rows to answer two questions the resolver's output + * cannot express, because the resolver's output is by construction the set of + * grants that DID resolve: + * + * - **expired** — a row whose `valid_until` has passed, so the panel can say + * "held until … — expired" instead of silently omitting a grant the admin + * knows they granted. This is "why did access DISAPPEAR", and only a dropped + * row can answer it. + * - **delegated** — the `delegated_from` provenance of a row that DID resolve, + * so a position can be attributed "via delegation from X, until Y". + * + * Both verdicts come from the SAME shared ADR-0091 predicate module the resolver + * uses (`isGrantActive` / `isGrantExpired`, `@objectstack/core`) — one + * implementation of the window rule, consulted twice, never re-derived here. */ -export async function buildContextForUser(ql: any, userId: string, nowMs: number = Date.now()): Promise { - const positions: string[] = []; - const permissions: string[] = []; - // [ADR-0091 D2] Rows outside their validity window resolve to NOTHING (same - // predicate as resolveAuthzContext, fail-closed). Expired-but-present rows - // are collected separately so the principal layer can report the dedicated - // "held until … — expired" contributor state. +async function collectGrantProvenance( + ql: any, + userId: string, + nowMs: number, +): Promise<{ + expiredGrants: Array<{ kind: 'position' | 'permission_set'; name: string; until?: string }>; + delegatedPositions: Array<{ name: string; from: string; until?: string }>; +}> { const expiredGrants: Array<{ kind: 'position' | 'permission_set'; name: string; until?: string }> = []; - // [ADR-0091 D3] Delegation provenance: a position held via a `delegated_from` - // row is reported "via delegation from X, until Y" in the principal layer. const delegatedPositions: Array<{ name: string; from: string; until?: string }> = []; - const untilOf = (r: any): string | undefined => { - const v = r?.valid_until ?? r?.validUntil; - return v == null || v === '' ? undefined : String(v); - }; + try { const rows = await ql.find('sys_user_position', { where: { user_id: userId }, limit: 500, context: SYSTEM_CTX }); for (const r of Array.isArray(rows) ? rows : []) { const p = String((r as any)?.position ?? ''); if (!p) continue; - if (!isGrantActive(r, nowMs)) { - if (isGrantExpired(r, nowMs)) expiredGrants.push({ kind: 'position', name: p, until: untilOf(r) }); - continue; - } - if (!positions.includes(p)) positions.push(p); - const from = (r as any)?.delegated_from; - if (from != null && from !== '') { - delegatedPositions.push({ name: p, from: String(from), until: untilOf(r) }); + if (isGrantActive(r, nowMs)) { + const from = (r as any)?.delegated_from; + if (from != null && from !== '') { + delegatedPositions.push({ name: p, from: String(from), until: untilOfGrantRow(r) }); + } + } else if (isGrantExpired(r, nowMs)) { + // Pending (future `valid_from`) rows are inactive but NOT expired — they + // are not reported, because nothing was lost yet. + expiredGrants.push({ kind: 'position', name: p, until: untilOfGrantRow(r) }); } } - } catch { /* table unavailable → positions stay empty */ } - // [ADR-0095 D3 / ADR-0068 D2] platform_admin posture is DERIVED from an - // UNSCOPED (`organization_id == null`) `admin_full_access` USER grant — the - // single source of truth enforcement (`resolveAuthzContext.hasPlatformAdminGrant`) - // trusts. We compute it here with the IDENTICAL rule so the explain panel's - // posture cannot sit higher than enforcement's: a merely org-SCOPED - // admin_full_access grant must NOT confer platform_admin. - let hasPlatformAdminGrant = false; + } catch { /* table unavailable → no provenance to report */ } + try { - const grants = await ql.find('sys_user_permission_set', { where: { user_id: userId }, limit: 500, context: SYSTEM_CTX }); - const grantRows = (Array.isArray(grants) ? grants : []) as any[]; - const activeRows = grantRows.filter((g) => isGrantActive(g, nowMs)); - const expiredRows = grantRows.filter((g) => !isGrantActive(g, nowMs) && isGrantExpired(g, nowMs)); - // permission-set-ids held via an UNSCOPED active user grant (org == null). - const unscopedActiveIds = new Set( - activeRows - .filter((g) => ((g?.organization_id ?? g?.organizationId) ?? null) === null) - .map((g: any) => String(g?.permission_set_id ?? g?.permissionSetId ?? '')) - .filter(Boolean), + const rows = await ql.find('sys_user_permission_set', { where: { user_id: userId }, limit: 500, context: SYSTEM_CTX }); + const expiredRows = (Array.isArray(rows) ? rows : []).filter( + (g: any) => !isGrantActive(g, nowMs) && isGrantExpired(g, nowMs), ); - const ids = [...activeRows, ...expiredRows].map((g: any) => g?.permission_set_id).filter(Boolean); + const ids = expiredRows + .map((g: any) => g?.permission_set_id ?? g?.permissionSetId) + .filter(Boolean); if (ids.length > 0) { const sets = await ql.find('sys_permission_set', { where: { id: { $in: ids } }, limit: ids.length, context: SYSTEM_CTX }); const nameById = new Map(); for (const s of Array.isArray(sets) ? sets : []) { if ((s as any)?.id && (s as any)?.name) nameById.set(String((s as any).id), String((s as any).name)); } - for (const g of activeRows) { - const id = String(g?.permission_set_id ?? ''); - const n = nameById.get(id); - if (n && !permissions.includes(n)) permissions.push(n); - // Same predicate as resolveAuthzContext: name is admin_full_access AND - // the granting row is an unscoped user grant. - if (n === ADMIN_FULL_ACCESS && unscopedActiveIds.has(id)) hasPlatformAdminGrant = true; - } for (const g of expiredRows) { - const n = nameById.get(String(g?.permission_set_id ?? '')); - if (n) expiredGrants.push({ kind: 'permission_set', name: n, until: untilOf(g) }); + const n = nameById.get(String((g as any)?.permission_set_id ?? (g as any)?.permissionSetId ?? '')); + if (n) expiredGrants.push({ kind: 'permission_set', name: n, until: untilOfGrantRow(g) }); } } - } catch { /* ignore */ } - // [ADR-0090 D5] Authenticated principals implicitly hold the everyone anchor. - if (!positions.includes('everyone')) positions.push('everyone'); - // [ADR-0105 D2] The user's OWN org access set. Resolved here rather than - // inherited from the live principal: under the `group` posture this is the - // Layer 0 read reach, and a delegated read must be bounded by the DELEGATOR's - // memberships. Absent → empty → the group wall denies (fail-closed), which is - // the safe direction for an unresolvable delegator. - const accessible_org_ids: string[] = []; - try { - const rows = await ql.find('sys_member', { where: { user_id: userId }, limit: 200, context: SYSTEM_CTX }); - for (const m of Array.isArray(rows) ? rows : []) { - if (!isGrantActive(m, nowMs)) continue; - const org = (m as any)?.organization_id ?? (m as any)?.organizationId; - if (typeof org === 'string' && org && !accessible_org_ids.includes(org)) { - accessible_org_ids.push(org); - } - } - } catch { /* table unavailable → empty set → fails closed under `group` */ } + } catch { /* table unavailable → no provenance to report */ } + + return { expiredGrants, delegatedPositions }; +} - return { userId, positions, permissions, accessible_org_ids, expiredGrants, delegatedPositions, hasPlatformAdminGrant }; +/** + * Reconstruct an evaluation context for an arbitrary user, for the explain API's + * `userId` parameter. The caller-facing authorization for explaining OTHERS + * lives in the route/service wrapper (`explainAccessForCaller`), not here. + * + * [#6352] **The authorization aggregation is not implemented here.** It is + * `@objectstack/core`'s `resolveUserAuthzGrants` — the userId-driven core of + * `resolveAuthzContext`, i.e. the exact function every inbound request resolves + * through, called with the exact arguments (`ql`, `userId`, the ADR-0091 clock). + * So the positions (`sys_member` role projection + `sys_user_position` + the + * ADR-0090 D5 `everyone` anchor), the permission-set names (user-bound AND + * position-bound), the ADR-0091 validity windows, the ADR-0068 D2 `platform_admin` + * derivation, the ADR-0095 posture rung and the ADR-0105 D2 `accessible_org_ids` + * are all ONE implementation, not two kept in step by comment. + * + * This used to be a hand-written mirror whose only guarantee was two comments + * saying it matched. It did not: measured over identical rows it dropped the + * `sys_member` role positions, every position-bound permission set + * (`sys_position_permission_set`), the `ai_seat` synthesis, `systemPermissions` + * and the posture rung — so a user whose grants arrive through a POSITION was + * explained as holding nothing, and the panel reported a deny that enforcement + * did not make. That is the failure mode the panel exists to prevent, so the + * mirror is gone rather than pinned. + * + * What stays explain-side is presentation only, and additive: the expired / + * delegated row annotations ({@link collectGrantProvenance}), and + * `hasPlatformAdminGrant`, which is now READ OFF the resolver's own posture + * verdict instead of being recomputed from the grant rows. + */ +export async function buildContextForUser(ql: any, userId: string, nowMs: number = Date.now()): Promise { + const grants = await resolveUserAuthzGrants(ql, userId, { nowMs }); + const { expiredGrants, delegatedPositions } = await collectGrantProvenance(ql, userId, nowMs); + return { + userId, + positions: grants.positions, + permissions: grants.permissions, + systemPermissions: grants.systemPermissions, + org_user_ids: grants.org_user_ids, + accessible_org_ids: grants.accessible_org_ids, + ...(grants.tabPermissions ? { tabPermissions: grants.tabPermissions } : {}), + ...(grants.email != null ? { email: grants.email } : {}), + ...(grants.posture ? { posture: grants.posture } : {}), + expiredGrants, + delegatedPositions, + // [ADR-0068 D2] Not a second derivation: `derivePosture` returns + // PLATFORM_ADMIN if and only if the resolver saw the unscoped + // `admin_full_access` USER grant, so this reads that one verdict back. + hasPlatformAdminGrant: grants.posture === 'PLATFORM_ADMIN', + }; } /** @@ -358,19 +398,26 @@ export type DelegatorResolution = * baseline for ANY `userId`, so a deleted delegator would otherwise still * intersect against baseline-level access. The `sys_user` existence check is * the only correct fail-closed point. - * - **Tenant-scoped bags are inherited from the live principal.** The agent and - * its delegator are, by construction, in the same org, so `tenantId` / - * `org_user_ids` carry over — delegator-side RLS that substitutes them then - * compiles faithfully instead of collapsing to the deny sentinel. + * - **Tenant-scoped bags are inherited from the live principal, and the + * inheritance still WINS.** The agent and its delegator are, by construction, + * in the same org, so `tenantId` / `org_user_ids` carry over — delegator-side + * RLS that substitutes them then compiles faithfully instead of collapsing to + * the deny sentinel. Since #6352, `buildContextForUser` returns the resolver's + * own `org_user_ids`, which without a known `tenantId` is the degenerate + * `[delegatorId]` seed — the live principal's real org peer set is the better + * answer, so the assignment below overwrites it exactly as before. * `accessible_org_ids` (ADR-0105 D2) is the exception: it is resolved from * the DELEGATOR's own memberships by `buildContextForUser`, never inherited, * because inheriting it would widen a delegated read past the organizations * the delegator actually belongs to. - * - **Person-specific membership bags (`rlsMembership`) are left unresolved** - * for the first cut. Absent → the RLS compiler's fail-closed substitution - * NARROWS the delegator's row set, never widens it — safe by construction. - * Full parity (team/territory bags) is a follow-up routing the delegator - * through the shared `resolveAuthzContext`. + * - **Person-specific membership bags (`rlsMembership`) are left unresolved.** + * Absent → the RLS compiler's fail-closed substitution NARROWS the + * delegator's row set, never widens it — safe by construction. #6352 routed + * the delegator through the shared resolver (`resolveUserAuthzGrants`), which + * closed the positions / permission-set / posture half of this gap; the + * team/territory bags are not part of that envelope on either side, so they + * remain a follow-up for the `RlsMembershipResolver` contract rather than for + * this function. * - **One hop only (edge a).** The `onBehalfOf` shape carries a single delegator * id with no nested link, so a transitive agent→service→user chain is not * representable in one context. Intersecting against the immediate delegator diff --git a/scripts/check-single-authz-resolver.mjs b/scripts/check-single-authz-resolver.mjs index 15e60c7ac0..d113ee0136 100644 --- a/scripts/check-single-authz-resolver.mjs +++ b/scripts/check-single-authz-resolver.mjs @@ -198,14 +198,17 @@ const ALLOW = new Map([ ], [ 'packages/plugins/plugin-security/src/explain-engine.ts', - 'Explain/diagnostic mirror, NOT a request-context resolver. buildContextForUser() ' + - 'reconstructs an ARBITRARY user\'s grants for the explain API\'s `userId` parameter; ' + - 'its only caller is security-plugin.ts explainAccessForCaller, which authorizes the ' + - 'CALLER separately (manage_users or a delegated adminScope, ADR-0090 D6/D12) through ' + - 'the normal path. It resolves nobody\'s enforcement context, so it is not the ' + - 'drift-into-enforcement this gate guards. Its parity with the canonical resolver is a ' + - 'real but DIFFERENT invariant, unguarded today and filed as #6352 — do not fold it in ' + - 'here by widening this gate\'s remit without saying so in the header.', + 'Explain/diagnostic surface, NOT a request-context resolver, and since #6352 no longer a ' + + 'second aggregation either: buildContextForUser() now CALLS resolveUserAuthzGrants (the ' + + 'canonical resolver\'s userId-driven core) for every position / permission-set / posture / ' + + 'platform_admin verdict. What still trips this heuristic is the explain-ONLY provenance ' + + 'pass (collectGrantProvenance), which re-reads the same two tables purely to ANNOTATE rows ' + + 'the resolver dropped — expired grants ("held until … — expired") and delegated_from ' + + 'origin — and feeds no verdict. The exemption is therefore narrower than it was, not ' + + 'wider: the parity invariant it used to defer is now pinned by the tests in ' + + 'explain-engine.test.ts ("buildContextForUser ↔ resolveUserAuthzGrants parity"), never by ' + + 'this gate. Do not fold that invariant in here by widening this gate\'s remit without ' + + 'saying so in the header.', ], ]);