diff --git a/.changeset/25106-read-scope-delegated-organization-membership.md b/.changeset/25106-read-scope-delegated-organization-membership.md new file mode 100644 index 0000000000..b25bc6135e --- /dev/null +++ b/.changeset/25106-read-scope-delegated-organization-membership.md @@ -0,0 +1,17 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security)!: the raw-SQL read scope (`getReadFilter`) answers the deny sentinel for a delegated context the operation prologue refuses — the delegator's membership must back the organization the read runs in, on a walled posture + +Clause-②: no (narrowing) +Metadata-protocol: none + + + +**BREAKING** (the security service's read scope narrows for one context shape). ADR-0021 D-C makes the raw-SQL read scope the find path's scope, and the provider's own contract is agreement with the engine middleware. Under `isolated` or `group`, the delegated-principal prologue already refuses every operation of an on-behalf-of context whose active organization the delegator's membership does not back (ADR-0090 D10, `vetOrganizationClaim`). `getReadFilter` did not ask that question, so for the same context it answered a composed read scope where the find path refuses. It now asks it, once, beside the dangling-delegator refusal it already mirrored. + +- **FROM:** under `isolated` or `group`, `getReadFilter` for an execution context carrying `onBehalfOf` and an active organization the delegator is not a member of answered a read scope composed from the two principals' layers, where the engine's find refuses the same context. +- **TO:** it answers `RLS_DENY_FILTER` itself (the spec's sentinel, by reference), so a door that bypasses the engine reads zero rows for that context, as the engine's find refuses it. +- **Unchanged:** the `single` posture, an on-behalf-of context with no active organization, an on-behalf-of context in an organization the delegator is a member of, a context without `onBehalfOf`, a dangling delegator (still the deny sentinel) and system contexts. The `/mcp` OAuth door, the one producer of an on-behalf-of context in the platform, vets the request's organization against the same membership, so the contexts it builds keep their scope. +- **The fix for an in-process producer:** code that builds an `ExecutionContext` with `onBehalfOf` by hand and hands it to the security service or to the analytics service must set its `tenantId` to an organization the delegating user is a member of, or leave it unset. diff --git a/packages/plugins/plugin-security/src/delegated-organization-membership.test.ts b/packages/plugins/plugin-security/src/delegated-organization-membership.test.ts index 2e6864b974..73cbbeecdd 100644 --- a/packages/plugins/plugin-security/src/delegated-organization-membership.test.ts +++ b/packages/plugins/plugin-security/src/delegated-organization-membership.test.ts @@ -39,12 +39,24 @@ * without the delegation link; a delegated context with no active * organization (it keeps ADR-0123 D2's refusal); a system context; and the * `single` posture, which has no wall. + * + * ## The raw-SQL read scope answers the same rule [#25106 · ADR-0021 D-C] + * + * `getReadFilter` is the read scope a door that bypasses the engine ANDs into + * its query, and its contract is agreement with the find path. So where the + * prologue refuses a delegated context, the provider answers the deny + * sentinel (`RLS_DENY_FILTER` itself, by reference), on a public, a private + * and an organization-less object alike: the rule is about the context, not + * the object. Every context the prologue admits keeps the scope it had, and + * one parity cell reads the provider beside the engine's own find over the + * same contexts. */ import { describe, it, expect, afterEach, vi } from 'vitest'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import type { PermissionSet } from '@objectstack/spec/security'; +import { RLS_DENY_FILTER } from '@objectstack/spec/data'; import { SecurityPlugin } from './security-plugin.js'; import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; import { SysPosition, SysPermissionSet, SysPositionPermissionSet } from './__tests__/legacy-catalog-objects.testkit.js'; @@ -65,6 +77,9 @@ const FIELDS = { const OBJECTS = [ { name: 'qa_account', label: 'Account', fields: FIELDS }, { name: 'qa_vault', label: 'Vault', access: { default: 'private' }, fields: FIELDS }, + // [#25106] An organization-less object: no wall applies to it, so only the + // context rule can answer the deny sentinel for it. + { name: 'qa_catalog', label: 'Catalog', tenancy: { enabled: false }, fields: FIELDS }, ]; const MEMBER: PermissionSet = { @@ -201,7 +216,7 @@ async function boot(posture: Posture) { const rows = await knex(object).select('id', 'name', 'organization_id'); return [...rows].sort((a, b) => String(a.id).localeCompare(String(b.id))); }; - return { engine, stored }; + return { engine, plugin, stored }; } interface Outcome { ok: boolean; value?: unknown; code?: string; status?: number; message?: string; developerMessage?: string } @@ -305,3 +320,137 @@ describe('[#25063] what the rule leaves alone', () => { expect((await b.stored('qa_account')).find((r) => r.id === 'r1')).toEqual({ id: 'r1', name: 'one', organization_id: ORG_D }); }); }); + +// --------------------------------------------------------------------------- +// [#25106 · ADR-0021 D-C] The raw-SQL read scope (`getReadFilter`) answers the +// same rule. Its contract is agreement with the find path: where the prologue +// refuses a delegated context, the provider answers the deny sentinel. +// --------------------------------------------------------------------------- + +const PROVIDER_OBJECTS = ['qa_account', 'qa_vault', 'qa_catalog'] as const; + +/** One row per tenant object in `org_a` and `org_c` (`org_d`'s is `boot`'s), and two catalog rows, which carry no organization. */ +async function seedForReads(engine: ObjectQL) { + for (const object of ['qa_account', 'qa_vault']) { + for (const [org, suffix] of [[ORG_A, 'a'], [ORG_C, 'c']] as const) { + await engine.insert(object, { id: `${object}_in_${suffix}`, name: 'seeded', organization_id: org }, { context: { ...SYS_CTX, tenantId: org } } as never); + } + } + for (const id of ['qa_catalog_1', 'qa_catalog_2']) { + await engine.insert('qa_catalog', { id, name: 'seeded' }, { context: SYS_CTX } as never); + } +} + +const sortedIds = (rows: unknown) => (rows as Array<{ id: string }>).map((r) => r.id).sort(); + +/** The ids a read scope admits, applied as the only predicate under a system context — as a door that bypasses the engine applies it. */ +const idsAdmittedBy = async (engine: ObjectQL, object: string, scope: Record | undefined) => + sortedIds(await engine.find(object, { where: scope ?? {}, context: { isSystem: true } } as never)); + +describe('[#25106 · ADR-0021 D-C] getReadFilter answers the deny sentinel where the prologue refuses', () => { + for (const posture of ['group', 'isolated'] as const) { + for (const object of PROVIDER_OBJECTS) { + it(`\`${posture}\` · ${object}: a delegated context in an organization the delegator is not a member of answers RLS_DENY_FILTER itself`, async () => { + const b = await boot(posture); + await seedForReads(b.engine); + expect(await b.plugin.getReadFilter(object, operator(ORG_D))).toBe(RLS_DENY_FILTER); + }); + } + } +}); + +describe('[#25106] what the rule leaves alone in the read scope', () => { + // Each answer below was read on the provider before this rule and after it, + // and is the same: the rule moves only the contexts the prologue refuses. + // A scope is pinned by the rows it admits (`idsAdmittedBy`). + const scopeRows = async (b: Awaited>, object: string, context: Record) => { + const scope = await b.plugin.getReadFilter(object, { ...context }); + expect(scope, `${object}: a scope, not the deny sentinel`).not.toBe(RLS_DENY_FILTER); + return idsAdmittedBy(b.engine, object, scope); + }; + + const IN_SET: Record<'group' | 'isolated', Record> = { + // `group` walls the delegator's leg by its own memberships (`org_a`, `org_c`); + // the operator crosses on the private object, so its rows come from both. + group: { qa_account: ['qa_account_in_a'], qa_vault: ['qa_vault_in_a', 'qa_vault_in_c'], qa_catalog: ['qa_catalog_1', 'qa_catalog_2'] }, + isolated: { qa_account: ['qa_account_in_a'], qa_vault: ['qa_vault_in_a'], qa_catalog: ['qa_catalog_1', 'qa_catalog_2'] }, + }; + + for (const posture of ['group', 'isolated'] as const) { + it(`\`${posture}\`: a delegated context in an organization the delegator belongs to keeps its scope`, async () => { + const b = await boot(posture); + await seedForReads(b.engine); + for (const object of PROVIDER_OBJECTS) { + expect(await scopeRows(b, object, operator(ORG_A)), object).toEqual(IN_SET[posture][object]); + } + }); + + it(`\`${posture}\`: the same caller in \`org_d\` WITHOUT the delegation link keeps its scope`, async () => { + const b = await boot(posture); + await seedForReads(b.engine); + expect(await scopeRows(b, 'qa_account', operator(ORG_D, false))).toEqual(['qa_account_in_d']); + expect(await scopeRows(b, 'qa_vault', operator(ORG_D, false))).toEqual(['qa_vault_in_a', 'qa_vault_in_c', 'qa_vault_in_d']); + expect(await scopeRows(b, 'qa_catalog', operator(ORG_D, false))).toEqual(['qa_catalog_1', 'qa_catalog_2']); + }); + + it(`\`${posture}\`: a delegated context with NO active organization is not this rule's — its scope is unchanged`, async () => { + // The wall decides the tenant object (no organization: no row), and the + // organization-less object keeps its rows. The private object is left + // out: the delegator holds its grant only inside an organization, so + // object admission refuses it (`canReadObject`'s half), not this rule. + const b = await boot(posture); + await seedForReads(b.engine); + expect(await scopeRows(b, 'qa_account', operator(undefined))).toEqual([]); + expect(await scopeRows(b, 'qa_catalog', operator(undefined))).toEqual(['qa_catalog_1', 'qa_catalog_2']); + }); + + it(`\`${posture}\`: a system context is unscoped`, async () => { + const b = await boot(posture); + for (const object of PROVIDER_OBJECTS) { + expect(await b.plugin.getReadFilter(object, { ...SYS_CTX, onBehalfOf: { userId: DELEGATOR } }), object).toBeUndefined(); + } + }); + } + + it('`single`: there is no wall, so a delegated context in `org_d` keeps its scope', async () => { + const b = await boot('single'); + await seedForReads(b.engine); + expect(await scopeRows(b, 'qa_account', operator(ORG_D))).toEqual(['qa_account_in_a', 'qa_account_in_c', 'qa_account_in_d']); + expect(await scopeRows(b, 'qa_vault', operator(ORG_D))).toEqual(['qa_vault_in_a', 'qa_vault_in_c', 'qa_vault_in_d']); + expect(await scopeRows(b, 'qa_catalog', operator(ORG_D))).toEqual(['qa_catalog_1', 'qa_catalog_2']); + }); +}); + +describe('[#25106 · ADR-0021 D-C] parity: the read scope agrees with the engine find on the same contexts', () => { + for (const posture of ['group', 'isolated'] as const) { + it(`\`${posture}\`: refused by find ⇔ the deny sentinel; admitted ⇒ the scope admits exactly the rows find returns`, async () => { + const b = await boot(posture); + await seedForReads(b.engine); + const contexts: Record> = { + 'delegated in org_d': operator(ORG_D), + 'delegated in org_a': operator(ORG_A), + 'undelegated in org_d': operator(ORG_D, false), + }; + const refused: string[] = []; + for (const [label, context] of Object.entries(contexts)) { + for (const object of PROVIDER_OBJECTS) { + const cell = `${label} · ${object}`; + const scope = await b.plugin.getReadFilter(object, { ...context }); + const found = await attempt(() => b.engine.find(object, { context: { ...context } } as never)); + if (!found.ok) { + refused.push(cell); + expect(found.code, cell).toBe('PERMISSION_DENIED'); + expect(found.status, cell).toBe(403); + expect(scope, cell).toBe(RLS_DENY_FILTER); + } else { + expect(scope, cell).not.toBe(RLS_DENY_FILTER); + expect(await idsAdmittedBy(b.engine, object, scope), cell).toEqual(sortedIds(found.value)); + } + } + } + // The table is not vacuous on either side: find refuses exactly the + // delegated context the delegator's membership does not back. + expect(refused).toEqual(PROVIDER_OBJECTS.map((object) => `delegated in org_d · ${object}`)); + }); + } +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 0fe4494afc..130693ab65 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -6081,7 +6081,10 @@ export class SecurityPlugin implements Plugin { * Fails CLOSED where the chain refuses, with the deny sentinel by identity: * a principal that resolves no set (ADR-0056 D2); a delegator the link names * who does not exist (the chain's `PERMISSION_DENIED`; `canReadObject`'s - * `false`); a delegator who resolves no set (object admission refuses them); + * `false`); [#25106] on a walled posture, a live active organization the + * delegator's own membership does not back (the prologue's + * `PERMISSION_DENIED`, asked of `vetOrganizationClaim` with the prologue's + * inputs); a delegator who resolves no set (object admission refuses them); * a sharing scope that cannot be resolved. A throw anywhere propagates to the * caller's catch, which denies. * @@ -6106,6 +6109,27 @@ export class SecurityPlugin implements Plugin { return RLS_DENY_FILTER; } const delegatorContext = del.context; + // [#25106 · ADR-0090 D10] The prologue's organization-membership refusal + // (`resolveOperationPrincipals`), mirrored at the same point: after the + // dangling-delegator refusal, before the delegator's sets resolve. On a + // walled posture the find path refuses every operation of a delegated + // context whose live active organization the delegator's own membership + // does not back, so the scope that agrees with it is zero rows, on every + // object. One ask of the one spelling of the rule, with the inputs the + // prologue passes; under `single` the claim stands, and a context with no + // active organization is not this rule's (ADR-0123 D2). + const liveOrganization = + context.tenantId != null && context.tenantId !== '' ? String(context.tenantId) : undefined; + if ( + liveOrganization !== undefined && + vetOrganizationClaim(liveOrganization, delegatorContext?.accessible_org_ids ?? [], this.tenancyPosture) === undefined + ) { + this.logger.warn?.( + `[security] getReadFilter received an on-behalf-of context for object '${object}' whose delegator is not a ` + + `member of the organization it runs in — denying, as the operation prologue refuses it (ADR-0090 D10, fail-closed)`, + ); + return RLS_DENY_FILTER; + } const delegatorSets = await this.resolvePermissionSetsForContext(delegatorContext); if (delegatorSets.length === 0) return RLS_DENY_FILTER;