diff --git a/.changeset/22530-explain-cbp-write-verb-parity.md b/.changeset/22530-explain-cbp-write-verb-parity.md new file mode 100644 index 00000000000..ddc77a8d614 --- /dev/null +++ b/.changeset/22530-explain-cbp-write-verb-parity.md @@ -0,0 +1,11 @@ +--- +'@objectstack/plugin-security': patch +--- + +`security/explain` answers a delete and a transfer of a `controlled_by_parent` record the way each verb's own door answers it: their record verdicts now come from the master-detail write check (ADR-0055), as the update's already did. + +- **What was wrong.** The write path runs the master-detail write check on every by-id write of such a record (an update, a delete, a transfer), and hands the record's platform ownership floor over to that check. explain asked the check for an update only. + - A delete kept its `owner_only_deletes` floor. A principal who may edit the master but did not create the record was explained as refused (`decidedBy: 'rls'`) beside a `DELETE` that answers `200`. A principal who may not edit the master was refused on that floor, with no word about the master, beside a `DELETE` whose `403` names the master. + - A transfer asked only the sharing service's per-record gate, which abstains on such a record. A principal who may not edit the master was explained as allowed to transfer it, beside the `PATCH` that writes `owner_id` and answers `403`. +- **What it does now.** For an update, a delete or a transfer of a record that exists (and a restore or a purge, which the object gate still refuses to everyone), explain asks the security service's `checkControlledByParentWrite` with the context it is explaining. The check judges edit access to the master whatever the record's own verb, so its answer is each verb's answer. A `deny` or `unresolvable` outcome makes the record not writable, decided by the `sharing` layer, whose record attribution names the verb and the refusing leg (or the reason no verdict was reached). A delete's row-level composition carries the write path's master-gate coverage vouch, as an update's does, so the ownership floor is handed over as the write path hands it, and never on behalf of a delegator. +- **What is unchanged.** Every object that is not `controlled_by_parent`. Object-level reports (no `recordId`). The `allowed` field. A `create` asks no master check: the write path reads an insert's master from the request body, which an explanation does not carry. The `ExplainEngineDeps` type: the `checkControlledByParentWrite` dependency keeps its signature and is now asked for every by-id write, not only an update. diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-write-member.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-write-member.test.ts index 9a72cc78d82..7f5093bf8f8 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-write-member.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-write-member.test.ts @@ -22,6 +22,12 @@ // derivation, so step 2.7 answers not-found first), both faces still refuse — // the member answers the master check alone, which the contract says. // +// The last block runs `security/explain` for EVERY by-id write verb step 2.8 +// runs on (update, delete, transfer) beside that verb's own door through the +// same middleware: explain asks this one member for each, because the check +// judges EDIT of the master whatever the record's verb. A verb whose door's +// master check stopped matching the member's update answer turns it red. +// // ## The fixture // // cbpm_account public_read + owner_id — read by all, edited by its owner @@ -41,7 +47,7 @@ import { describe, it, expect, vi } from 'vitest'; import { SecurityPlugin } from './security-plugin.js'; import { SharingService, type SharingEngine } from '@objectstack/plugin-sharing'; import { matchesFilterCondition } from '@objectstack/formula'; -import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { ExplainOperationSchema, PermissionSetSchema, type ExplainOperation, type PermissionSet } from '@objectstack/spec/security'; import type { ControlledByParentWriteOutcome, ISecurityService } from '@objectstack/spec/contracts'; import { assertEngineFindOnePredicate } from '@objectstack/metadata-core'; @@ -98,7 +104,9 @@ const SCHEMAS: Record = { sys_user: { name: 'sys_user', isSystem: true, fields: { id: text('id'), name: text('name') } }, }; -const CRUD = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }; +// `allowTransfer` lets a transfer through the object gate, so its record-level +// gates (step 2.7, step 2.8) are what answer it. +const CRUD = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, allowTransfer: true }; const READ = { allowRead: true }; const DETAILS = ['cbpm_contact', 'cbpm_region_note', 'cbpm_quote', 'cbpm_line', 'cbpm_orphan']; @@ -247,15 +255,37 @@ async function boot(options: { faultOn?: string } = {}) { return e; } }; + /** + * A by-id write of any verb through the engine middleware, shaped as the + * write path builds it (a delete or a purge carries no data). Resolves + * `'admitted'` or the refusal it threw. + */ + const write = async (operation: string, context: any, object: string, id: string): Promise<'admitted' | any> => { + const opCtx: any = { + object, + operation, + ...(operation === 'delete' || operation === 'purge' ? {} : { data: { id, name: 'edited' } }), + options: { where: { id } }, + context, + }; + try { + await middleware(opCtx, async () => {}); + return 'admitted'; + } catch (e) { + return e; + } + }; const member = (context: any, object: string, id: string) => serve(object, id, context); /** * [ADR-0055] `security/explain`'s record-grained update verdict through the * registered service (`explainAccessForCaller`). `userId` explains another * user, as the REST route does; the caller is the second argument. */ + const explainAs = (operation: ExplainOperation, callerContext: any, object: string, recordId: string, userId?: string) => + security.explain({ object, operation, recordId, ...(userId ? { userId } : {}) }, callerContext); const explainUpdate = (callerContext: any, object: string, recordId: string, userId?: string) => - security.explain({ object, operation: 'update', recordId, ...(userId ? { userId } : {}) }, callerContext); - return { store, ctx, patch, member, explainUpdate }; + explainAs('update', callerContext, object, recordId, userId); + return { store, ctx, patch, write, member, explainAs, explainUpdate }; } const rep = () => ({ userId: REP, tenantId: 'org-1', positions: [], permissions: ['cbpm_rep'] }); @@ -419,3 +449,80 @@ describe('[ADR-0055] security/explain answers an update as the PATCH does, from expect(sharingDetailOf(d)).toContain("refuses this update on its 'object_permission' leg"); }); }); + +/** + * Every operation `security/explain` answers, classified against step 2.8 of + * the write path. Total by construction, and checked against the spec's + * vocabulary below, so a verb added there without a row here fails. + * + * - `by_id_master_checked`: a by-id write the object gate lets through for a + * principal holding the grant, on which step 2.8 runs the master check. + * - `object_gate_refused`: step 2.8 lists it, but the object gate refuses it + * to every principal (its grant is retired until the lifecycle batch). + * - `no_by_id_write`: a read, or an insert, whose master step 2.8 reads off + * the request body an explanation does not carry. + */ +const VERB_ROWS: Record = { + update: 'by_id_master_checked', + delete: 'by_id_master_checked', + transfer: 'by_id_master_checked', + restore: 'object_gate_refused', + purge: 'object_gate_refused', + create: 'no_by_id_write', + read: 'no_by_id_write', + export: 'no_by_id_write', +}; +const verbsOf = (row: (typeof VERB_ROWS)[ExplainOperation]) => + (Object.keys(VERB_ROWS) as ExplainOperation[]).filter((v) => VERB_ROWS[v] === row); + +describe('[ADR-0055] security/explain answers every by-id write verb as its door does, from this same check', () => { + const sharingDetailOf = (d: any): string => String(d.layers.find((l: any) => l.layer === 'sharing')?.record?.detail ?? ''); + + it('the classification is total: every operation explain answers has a row', () => { + expect([...ExplainOperationSchema.options].sort()).toEqual(Object.keys(VERB_ROWS).sort()); + }); + + for (const verb of verbsOf('by_id_master_checked')) { + it(`${verb}: refused on the leg its door refuses on; admitted where its door admits`, async () => { + const h = await boot(); + for (const [context, object, id, leg] of [ + [rep(), 'cbpm_contact', 'c_other', 'record_sharing'], + [rep(), 'cbpm_region_note', 'rn_eu', 'row_level_security'], + [viewer(), 'cbpm_contact', 'c_own', 'object_permission'], + [rep(), 'cbpm_line', 'line_above_dangling', 'master_chain'], + ] as const) { + const refused = await h.write(verb, context, object, id); + expect(refused, `${verb} ${object}/${id} by ${context.userId}`).not.toBe('admitted'); + expect(refused).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + expect(String(refused.message)).toMatch(/requires edit access to its master record/); + if (LEG_REASON[leg]) expect(String(refused.message)).toMatch(LEG_REASON[leg]); + const d = await h.explainAs(verb, context, object, id); + expect(d.record, `explain ${verb} ${object}/${id} by ${context.userId}`).toEqual({ + recordId: id, + visible: false, + decidedBy: 'sharing', + }); + expect(sharingDetailOf(d)).toContain(`refuses this ${verb} on its '${leg}' leg`); + } + for (const [object, id] of [['cbpm_contact', 'c_own'], ['cbpm_contact', 'c_shared'], ['cbpm_region_note', 'rn_us'], ['cbpm_line', 'line_ok']] as const) { + const d = await h.explainAs(verb, rep(), object, id); + expect(d.record, `explain ${verb} ${object}/${id}`).toMatchObject({ recordId: id, visible: true }); + expect(sharingDetailOf(d)).not.toContain('master-detail write check'); + expect(await h.write(verb, rep(), object, id), `${verb} ${object}/${id}`).toBe('admitted'); + } + }); + } + + for (const verb of verbsOf('object_gate_refused')) { + it(`${verb}: the object gate refuses its door for every principal, and explain says the object gate decided`, async () => { + const h = await boot(); + for (const [context, object, id] of [[rep(), 'cbpm_contact', 'c_own'], [rep(), 'cbpm_contact', 'c_other']] as const) { + const refused = await h.write(verb, context, object, id); + expect(refused, `${verb} ${object}/${id}`).not.toBe('admitted'); + expect(String(refused.message)).not.toMatch(/requires edit access to its master record/); + const d = await h.explainAs(verb, context, object, id); + expect(d.record, `explain ${verb} ${object}/${id}`).toEqual({ recordId: id, visible: false, decidedBy: 'object_crud' }); + } + }); + } +}); diff --git a/packages/plugins/plugin-security/src/explain-controlled-by-parent-write.test.ts b/packages/plugins/plugin-security/src/explain-controlled-by-parent-write.test.ts index 67346e5d874..84e7c26d39f 100644 --- a/packages/plugins/plugin-security/src/explain-controlled-by-parent-write.test.ts +++ b/packages/plugins/plugin-security/src/explain-controlled-by-parent-write.test.ts @@ -1,18 +1,21 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // -// [ADR-0055] `security/explain`'s record-grained UPDATE verdict on a -// `controlled_by_parent` record comes from the master-detail write check +// [ADR-0055] `security/explain`'s record-grained verdict for every by-id WRITE +// of a `controlled_by_parent` record (an update, a delete, a transfer) comes +// from the master-detail write check // (`ISecurityService.checkControlledByParentWrite`), the composition step 2.8 -// of the write path runs, and every outcome maps the way the door maps it. +// of the write path runs on each of them, and every outcome maps the way the +// door maps it. // -// The per-record sharing gate (`canEditRecord`) cannot decide such an update: -// plugin-sharing maps `controlled_by_parent` to `public`, so it abstains, and -// its abstention reads as `true`. This file pins the engine's half — which -// outcome refuses, what the report names, when the check is asked and with -// which context — over a deps bag, so each cell isolates one outcome. The -// served member's own parity with the PATCH is pinned in +// The per-record sharing gates (`canEditRecord`, `canDeleteRecord`) cannot +// decide such a write: plugin-sharing maps `controlled_by_parent` to `public`, +// so they abstain, and an abstention reads as `true`. This file pins the +// engine's half — which outcome refuses, what the report names, which verbs +// ask the check and with which context — over a deps bag, so each cell isolates +// one outcome. The served member's own parity with each by-id write is pinned in // `controlled-by-parent-write-member.test.ts`, and the whole REST answer beside -// the REST PATCH in `packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts`. +// the REST door of each verb in +// `packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts`. // // The mapping (the spec's own consumer rule): `allow` and `not_applicable` // proceed, so the report is exactly the one a deps bag without the check @@ -22,7 +25,7 @@ // refuses too. import { describe, it, expect, vi } from 'vitest'; import { PermissionSetSchema } from '@objectstack/spec/security'; -import type { ExplainDecision, ExplainOperation } from '@objectstack/spec/security'; +import { ExplainOperationSchema, type ExplainDecision, type ExplainOperation } from '@objectstack/spec/security'; import type { ControlledByParentWriteDenialLeg, ControlledByParentWriteOutcome, @@ -36,7 +39,7 @@ const CBP_SCHEMA = { name: OBJECT, sharingModel: 'controlled_by_parent' }; const EDITOR = PermissionSetSchema.parse({ name: 'cbx_editor', - objects: { [OBJECT]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + objects: { [OBJECT]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, allowTransfer: true } }, }); /** The principal being EXPLAINED. Kept as one object so a cell can assert the check received this very context. */ @@ -80,8 +83,37 @@ function deps(opts: { const answering = (outcome: ControlledByParentWriteOutcome): Master => vi.fn(async () => outcome); /** `null` asks the object-level question (no `recordId`). */ -const explainUpdate = (d: ExplainEngineDeps, recordId: string | null = 'r1') => - explainAccess(d, { object: OBJECT, operation: 'update', context: EXPLAINED, ...(recordId !== null ? { recordId } : {}) }); +const explainAs = (d: ExplainEngineDeps, operation: ExplainOperation, recordId: string | null = 'r1') => + explainAccess(d, { object: OBJECT, operation, context: EXPLAINED, ...(recordId !== null ? { recordId } : {}) }); + +/** + * Every operation `explain` answers, classified against step 2.8 of the write + * path. The classification is total: a verb added to the explain vocabulary + * without a row here fails the completeness case below. + * + * - `master_checked`: a by-id write step 2.8 runs the master-detail write + * check on, which the EXPLAINED principal's grants let through the object + * gate (`EDITOR` holds update, delete and transfer), so the check decides + * the record. + * - `master_checked_object_gate_first`: a by-id write step 2.8 lists, which + * the object gate refuses for every principal (the restore and purge grants + * are retired until the lifecycle batch returns them). The check is still + * asked, so the report stays the door's on the day the verb is grantable, + * and the object-level CRUD layer decides first. + * - `not_asked`: a read (the check guards writes), and `create`, whose master + * comes from the request body an explanation does not carry. + */ +const VERB_CLASS: Record = { + update: 'master_checked', + delete: 'master_checked', + transfer: 'master_checked', + restore: 'master_checked_object_gate_first', + purge: 'master_checked_object_gate_first', + read: 'not_asked', + export: 'not_asked', + create: 'not_asked', +}; +const MASTER_CHECKED = (Object.keys(VERB_CLASS) as ExplainOperation[]).filter((v) => VERB_CLASS[v] === 'master_checked'); const sharingRecordOf = (d: ExplainDecision) => d.layers.find((l) => l.layer === 'sharing')?.record; @@ -92,114 +124,141 @@ const REASONS: Array<[ControlledByParentWriteUnresolvedReason, string]> = [ ['master_reference_missing', '422 MISSING_REQUIRED_FIELD'], ]; -describe('[ADR-0055] explain refuses an update the master-detail write check refuses', () => { - for (const leg of LEGS) { - it(`deny on '${leg}': the record is not writable, decided by the sharing layer, which names the leg`, async () => { - const d = await explainUpdate(deps({ master: answering({ outcome: 'deny', leg }) })); - expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); - const sharing = sharingRecordOf(d); - expect(sharing?.outcome).toBe('excluded'); - expect(sharing?.detail).toContain(`refuses this update on its '${leg}' leg`); - expect(sharing?.detail).toContain('403 PERMISSION_DENIED'); - }); - } +describe('[ADR-0055] explain refuses a by-id write the master-detail write check refuses, for every verb it decides', () => { + for (const verb of MASTER_CHECKED) { + for (const leg of LEGS) { + it(`${verb}, deny on '${leg}': the record is not writable, decided by the sharing layer, which names the leg`, async () => { + const d = await explainAs(deps({ master: answering({ outcome: 'deny', leg }) }), verb); + expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); + const sharing = sharingRecordOf(d); + expect(sharing?.outcome).toBe('excluded'); + expect(sharing?.detail).toContain(`the by-id ${verb} runs refuses this ${verb} on its '${leg}' leg`); + expect(sharing?.detail).toContain(`The ${verb} answers 403 PERMISSION_DENIED`); + }); + } + + for (const [reason, answered] of REASONS) { + it(`${verb}, unresolvable '${reason}': not writable (fail closed), the sharing layer names the reason and the ${verb}'s answer`, async () => { + const d = await explainAs(deps({ master: answering({ outcome: 'unresolvable', reason }) }), verb); + expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); + const sharing = sharingRecordOf(d); + expect(sharing?.outcome).toBe('not_evaluated'); + expect(sharing?.detail).toContain(`reaches no verdict ('${reason}')`); + expect(sharing?.detail).toContain(`the ${verb} answers ${answered}`); + }); + } - for (const [reason, answered] of REASONS) { - it(`unresolvable '${reason}': not writable (fail closed), the sharing layer names the reason and the update's answer`, async () => { - const d = await explainUpdate(deps({ master: answering({ outcome: 'unresolvable', reason }) })); + it(`${verb}: a rejection (a refused context, a store fault) fails the request, so it is reported fail-closed with no predicate`, async () => { + const fault = Object.assign(new Error('datasource unavailable'), { code: 'ERR_DATASOURCE_UNAVAILABLE', status: 503 }); + const master: Master = vi.fn(async () => { throw fault; }); + const d = await explainAs(deps({ master }), verb); expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); const sharing = sharingRecordOf(d); expect(sharing?.outcome).toBe('not_evaluated'); - expect(sharing?.detail).toContain(`reaches no verdict ('${reason}')`); - expect(sharing?.detail).toContain(answered); + expect(sharing?.rowFilter).toBeUndefined(); + expect(sharing?.matchesRecord).toBeUndefined(); + expect(sharing?.detail).toContain('master-detail write check (controlled_by_parent, ADR-0055) could not be evaluated'); + expect(sharing?.detail).toContain(`the by-id ${verb} fails on the same call`); }); - } - it('a rejection (a refused context, a store fault) fails the request, so it is reported fail-closed with no predicate', async () => { - const fault = Object.assign(new Error('datasource unavailable'), { code: 'ERR_DATASOURCE_UNAVAILABLE', status: 503 }); - const master: Master = vi.fn(async () => { throw fault; }); - const d = await explainUpdate(deps({ master })); - expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); - const sharing = sharingRecordOf(d); - expect(sharing?.outcome).toBe('not_evaluated'); - expect(sharing?.rowFilter).toBeUndefined(); - expect(sharing?.matchesRecord).toBeUndefined(); - expect(sharing?.detail).toContain('master-detail write check (controlled_by_parent, ADR-0055) could not be evaluated'); - }); - - it('an outcome outside the vocabulary refuses (fail closed), never reads as a pass', async () => { - const d = await explainUpdate(deps({ master: answering({ outcome: 'maybe' } as unknown as ControlledByParentWriteOutcome) })); - expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); - expect(sharingRecordOf(d)?.outcome).toBe('not_evaluated'); - }); + it(`${verb}: an outcome outside the vocabulary refuses (fail closed), never reads as a pass`, async () => { + const d = await explainAs(deps({ master: answering({ outcome: 'maybe' } as unknown as ControlledByParentWriteOutcome) }), verb); + expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); + expect(sharingRecordOf(d)?.outcome).toBe('not_evaluated'); + }); - it("the record's own row-level security still decides first (step 2.7 runs above step 2.8)", async () => { - const d = await explainUpdate(deps({ - master: answering({ outcome: 'deny', leg: 'record_sharing' }), - layer1: { id: 'not_this_record' }, - })); - expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'rls' }); - }); + it(`${verb}: the record's own row-level security still decides first (step 2.7 runs above step 2.8)`, async () => { + const d = await explainAs(deps({ + master: answering({ outcome: 'deny', leg: 'record_sharing' }), + layer1: { id: 'not_this_record' }, + }), verb); + expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'rls' }); + }); + } }); describe('[ADR-0055] a proceeding outcome changes nothing in the report', () => { - it('allow: the report is exactly the one a deps bag without the check gives — the master is not named', async () => { - const without = await explainUpdate(deps()); - const withAllow = await explainUpdate(deps({ master: answering({ outcome: 'allow' }) })); - expect(withAllow).toEqual(without); - expect(withAllow.record).toMatchObject({ visible: true }); - expect(sharingRecordOf(withAllow)?.detail).not.toContain('master'); - }); - - it('allow does not lift a refusal the rest of the pipeline makes (the sharing gate refuses)', async () => { - const without = await explainUpdate(deps({ canEdit: false })); - const withAllow = await explainUpdate(deps({ canEdit: false, master: answering({ outcome: 'allow' }) })); - expect(withAllow).toEqual(without); - expect(withAllow.record).toMatchObject({ visible: false }); - }); + for (const verb of MASTER_CHECKED) { + it(`${verb}, allow: the report is exactly the one a deps bag without the check gives — the master is not named`, async () => { + const without = await explainAs(deps(), verb); + const withAllow = await explainAs(deps({ master: answering({ outcome: 'allow' }) }), verb); + expect(withAllow).toEqual(without); + expect(withAllow.record).toMatchObject({ visible: true }); + expect(sharingRecordOf(withAllow)?.detail).not.toContain('master'); + }); - for (const [label, schema, canEdit] of [ - ['public_read_write', { name: OBJECT, sharingModel: 'public_read_write' }, true], - ['private, the gate admits', { name: OBJECT, sharingModel: 'private' }, true], - ['private, the gate refuses', { name: OBJECT, sharingModel: 'private' }, false], - ] as const) { - it(`not_applicable on a ${label} object: byte-identical to the report without the check`, async () => { - const without = await explainUpdate(deps({ schema, canEdit })); - const withCheck = await explainUpdate(deps({ schema, canEdit, master: answering({ outcome: 'not_applicable' }) })); - expect(withCheck).toEqual(without); + it(`${verb}, allow does not lift a refusal the rest of the pipeline makes (the sharing gate refuses)`, async () => { + const without = await explainAs(deps({ canEdit: false }), verb); + const withAllow = await explainAs(deps({ canEdit: false, master: answering({ outcome: 'allow' }) }), verb); + expect(withAllow).toEqual(without); + expect(withAllow.record).toMatchObject({ visible: false }); }); + + for (const [label, schema, canEdit] of [ + ['public_read_write', { name: OBJECT, sharingModel: 'public_read_write' }, true], + ['private, the gate admits', { name: OBJECT, sharingModel: 'private' }, true], + ['private, the gate refuses', { name: OBJECT, sharingModel: 'private' }, false], + ] as const) { + it(`${verb}, not_applicable on a ${label} object: byte-identical to the report without the check`, async () => { + const without = await explainAs(deps({ schema, canEdit }), verb); + const withCheck = await explainAs(deps({ schema, canEdit, master: answering({ outcome: 'not_applicable' }) }), verb); + expect(withCheck).toEqual(without); + }); + } } - it('absent check (a kernel without the member): the update keeps the sharing gate\'s answer and claims nothing about a master', async () => { - const d = await explainUpdate(deps()); - expect(d.record).toMatchObject({ recordId: 'r1', visible: true }); - expect(sharingRecordOf(d)?.detail).not.toContain('master'); + it('absent check (a kernel without the member): each write keeps the sharing gate\'s answer and claims nothing about a master', async () => { + for (const verb of MASTER_CHECKED) { + const d = await explainAs(deps(), verb); + expect(d.record, verb).toMatchObject({ recordId: 'r1', visible: true }); + expect(sharingRecordOf(d)?.detail, verb).not.toContain('master'); + } }); }); -describe('[ADR-0055] when the check is asked, and for whom', () => { - it('an update of a record that exists asks it once, for that record, with the EXPLAINED context', async () => { - const master = answering({ outcome: 'allow' }); - await explainUpdate(deps({ master })); - expect(master).toHaveBeenCalledTimes(1); - expect(master).toHaveBeenCalledWith(OBJECT, 'r1', EXPLAINED); - expect((master as ReturnType).mock.calls[0][2]).toBe(EXPLAINED); +describe('[ADR-0055] which verbs ask the check, and for whom', () => { + it('the classification is total: every operation explain answers is classified against step 2.8', () => { + // A verb added to the explain vocabulary lands here unclassified and turns + // this red, so its parity with the door is decided rather than inherited. + expect([...ExplainOperationSchema.options].sort()).toEqual(Object.keys(VERB_CLASS).sort()); }); - for (const operation of ['read', 'create', 'delete', 'transfer', 'export'] as ExplainOperation[]) { - it(`a record-grained ${operation} does not ask it`, async () => { + for (const verb of MASTER_CHECKED) { + it(`a ${verb} of a record that exists asks it once, for that record, with the EXPLAINED context`, async () => { + const master = answering({ outcome: 'allow' }); + await explainAs(deps({ master }), verb); + expect(master).toHaveBeenCalledTimes(1); + expect(master).toHaveBeenCalledWith(OBJECT, 'r1', EXPLAINED); + expect((master as ReturnType).mock.calls[0][2]).toBe(EXPLAINED); + }); + } + + for (const verb of (Object.keys(VERB_CLASS) as ExplainOperation[]).filter((v) => VERB_CLASS[v] === 'master_checked_object_gate_first')) { + it(`a ${verb} asks it too, and the object gate, which refuses ${verb} to every principal, decides first`, async () => { + const master = answering({ outcome: 'deny', leg: 'object_permission' }); + const d = await explainAs(deps({ master }), verb); + expect(master).toHaveBeenCalledTimes(1); + expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'object_crud' }); + expect(sharingRecordOf(d)?.detail).toContain(`refuses this ${verb} on its 'object_permission' leg`); + }); + } + + for (const verb of (Object.keys(VERB_CLASS) as ExplainOperation[]).filter((v) => VERB_CLASS[v] === 'not_asked')) { + it(`a record-grained ${verb} does not ask it`, async () => { const master = answering({ outcome: 'deny', leg: 'object_permission' }); - await explainAccess(deps({ master }), { object: OBJECT, operation, context: EXPLAINED, recordId: 'r1' }); + await explainAs(deps({ master }), verb); expect(master).not.toHaveBeenCalled(); }); } - it('an object-level update (no recordId) does not ask it, and a record that does not exist is not asked about', async () => { + it('an object-level write (no recordId) does not ask it, and a record that does not exist is not asked about', async () => { const master = answering({ outcome: 'deny', leg: 'object_permission' }); - const objectLevel = await explainUpdate(deps({ master }), null); - expect(objectLevel.record).toBeUndefined(); - const missing = await explainUpdate(deps({ master, record: null }), 'r_missing'); - expect(missing.record).toEqual({ recordId: 'r_missing', visible: false }); + for (const verb of MASTER_CHECKED) { + const objectLevel = await explainAs(deps({ master }), verb, null); + expect(objectLevel.record, verb).toBeUndefined(); + const missing = await explainAs(deps({ master, record: null }), verb, 'r_missing'); + expect(missing.record, verb).toEqual({ recordId: 'r_missing', visible: false }); + } expect(master).not.toHaveBeenCalled(); }); }); diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index eadebfe59c3..0990c5fc41d 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -374,19 +374,24 @@ export interface ExplainEngineDeps { * [ADR-0055] The master-detail write check's own answer for a by-id UPDATE * of `recordId` by `context`: `ISecurityService.checkControlledByParentWrite`, * the composition step 2.8 of the write path runs. On a - * `controlled_by_parent` record the per-record sharing gate above abstains - * (the object's effective model is `public`), so it cannot be the update's + * `controlled_by_parent` record the per-record sharing gates above abstain + * (the object's effective model is `public`), so they cannot be a write's * verdict there: this check is, and the door refuses on every outcome but * `allow` and `not_applicable`. * - * Asked for every `update` of a record that exists, with the context being - * EXPLAINED. The member holds the `controlled_by_parent` predicate, so the - * engine keeps no second copy of it: `not_applicable` leaves the report - * exactly as it was. A rejection (a context the write path refuses, or a - * store fault) is a dependency fault, reported fail-closed. + * Asked for every by-id write step 2.8 runs the check on + * ({@link MASTER_CHECKED_BY_ID_WRITES}: `update`, `delete`, `transfer`, + * `restore`, `purge`) of a record that exists, with the context being + * EXPLAINED. The update's answer is each of those writes' answer: the check + * judges EDIT access to the master whatever the record's own verb, and reads + * the verb only to pick its `insert` branch. The member holds the + * `controlled_by_parent` predicate, so the engine keeps no second copy of it: + * `not_applicable` leaves the report exactly as it was. A rejection (a + * context the write path refuses, or a store fault) is a dependency fault, + * reported fail-closed. * - * Optional: a deps bag wired without it explains the update from the sharing - * gate alone, as before, and claims nothing about a master. + * Optional: a deps bag wired without it explains each write from the sharing + * gates alone, as before, and claims nothing about a master. */ checkControlledByParentWrite?: ( object: string, @@ -907,23 +912,56 @@ const MASTER_DENIAL_LEG_DETAIL: Record master_chain: 'the controlled_by_parent chain cannot be walked to a master that governs its own rows', }; -/** [ADR-0055] Why the master-detail write check reached no verdict, and what the update answers instead. */ -const MASTER_UNRESOLVED_DETAIL: Record = { - master_detail_relation_missing: - 'the object declares controlled_by_parent with no master_detail relation to derive access from, and the ' + - 'update answers 422 INVALID_METADATA', - record_not_found: 'the record does not exist, and the update answers 404 RECORD_NOT_FOUND', - master_reference_missing: "the record's master reference is empty, and the update answers 422 MISSING_REQUIRED_FIELD", +/** + * [ADR-0055] Why the master-detail write check reached no verdict, and what the + * write answers instead. The answer is the same for every by-id write the check + * runs on: each condition throws one error class whatever the verb. + */ +const MASTER_UNRESOLVED_DETAIL: Record = { + master_detail_relation_missing: { + condition: 'the object declares controlled_by_parent with no master_detail relation to derive access from', + answer: '422 INVALID_METADATA', + }, + record_not_found: { condition: 'the record does not exist', answer: '404 RECORD_NOT_FOUND' }, + master_reference_missing: { condition: "the record's master reference is empty", answer: '422 MISSING_REQUIRED_FIELD' }, }; const MASTER_DERIVED = 'Write access to this record derives from its master (controlled_by_parent, ADR-0055)'; /** - * [ADR-0055] The sharing layer's record attribution for an update the - * master-detail write check REFUSES — the check that decides an update of a + * [ADR-0055] The record-grained writes whose verdict on a `controlled_by_parent` + * record the master-detail write check decides: every write step 2.8 of the + * write path runs that check on (the guard of step 2.8 in `security-plugin.ts`) + * that addresses one existing record by id. + * + * `insert` is the one verb step 2.8 runs on that is not here. Its master is read + * off the request BODY, which an explanation does not carry, and it addresses + * no existing record, so a record-grained `create` has no master check to ask. + * + * One answer serves every verb here. Whatever the record's own verb, the check + * judges EDIT access to the master (the object-level `update` grant, the + * master's write row-level security and record sharing, on every hop up the + * chain) and reads the verb only to pick its `insert` branch. + * `masterGateCoversOperation` states the same fact for the ownership floor's + * hand-over: deleting a child does not delete its master. So the answer the + * served member computes for an update is the answer step 2.8 gives each of + * these writes, and the engine asks it for each. + * + * `restore` and `purge` are refused at the object gate for every principal + * until their grants return, so the object-level CRUD layer decides their + * record verdict first. They are listed because step 2.8 lists them, so the + * report stays the door's on the day they can be granted. + */ +const MASTER_CHECKED_BY_ID_WRITES: ReadonlySet = new Set(['update', 'delete', 'transfer', 'restore', 'purge']); + +/** + * [ADR-0055] The sharing layer's record attribution for a by-id write the + * master-detail write check REFUSES — the check that decides such a write of a * `controlled_by_parent` record — or `undefined` where it does not refuse. + * `verb` is the write being explained (one of + * {@link MASTER_CHECKED_BY_ID_WRITES}), named in the prose. * - * The mapping is the door's: the update proceeds on `allow` and on + * The mapping is the door's: the write proceeds on `allow` and on * `not_applicable`, and is refused on every other outcome, one outside the * vocabulary included (fail closed). A proceeding outcome leaves the report * exactly as the rest of the pipeline computes it. ⛔ It does not name the @@ -933,6 +971,7 @@ const MASTER_DERIVED = 'Write access to this record derives from its master (con */ function masterWriteCheckRefusal( outcome: ControlledByParentWriteOutcome, + verb: string, ): { outcome: ExplainRecordAttribution['outcome']; detail: string } | undefined { switch (outcome?.outcome) { case 'allow': @@ -941,17 +980,19 @@ function masterWriteCheckRefusal( case 'deny': return { outcome: 'excluded', - detail: `${MASTER_DERIVED}: the master-detail write check the by-id update runs refuses this update on its ` + + detail: `${MASTER_DERIVED}: the master-detail write check the by-id ${verb} runs refuses this ${verb} on its ` + `'${outcome.leg}' leg — ${MASTER_DENIAL_LEG_DETAIL[outcome.leg] ?? 'a leg this report does not describe'}. ` + - 'The update answers 403 PERMISSION_DENIED.', + `The ${verb} answers 403 PERMISSION_DENIED.`, }; - case 'unresolvable': + case 'unresolvable': { + const unresolved = MASTER_UNRESOLVED_DETAIL[outcome.reason]; return { outcome: 'not_evaluated', detail: `${MASTER_DERIVED}, and the master-detail write check reaches no verdict ('${outcome.reason}'): ` + - `${MASTER_UNRESOLVED_DETAIL[outcome.reason] ?? 'a reason this report does not describe'}. The record is ` + - 'reported NOT writable (fail closed), never admitted.', + `${unresolved ? `${unresolved.condition}, and the ${verb} answers ${unresolved.answer}` : 'a reason this report does not describe'}. ` + + 'The record is reported NOT writable (fail closed), never admitted.', }; + } default: return { outcome: 'not_evaluated', @@ -1431,21 +1472,22 @@ async function applyRecordAttribution( : undefined; const writeGateFault = canEditOrFault === DEPENDENCY_FAULT; const canEdit = canEditOrFault === DEPENDENCY_FAULT ? undefined : canEditOrFault; - // [ADR-0055] An update of a `controlled_by_parent` record is judged by the + // [ADR-0055] A by-id write of a `controlled_by_parent` record (an update, a + // delete, a transfer: {@link MASTER_CHECKED_BY_ID_WRITES}) is judged by the // master-detail write check (step 2.8), which the sharing gate above cannot // stand in for: it abstains on such a record, and its abstention reads as - // `true`. So the check is asked for every update of a record that exists, + // `true`. So the check is asked for every such write of a record that exists, // with the explained context, and it decides whether it applies: `allow` and // `not_applicable` change nothing, every other outcome refuses // ({@link masterWriteCheckRefusal}). A rejection is the request failing, // exactly as for the gate. - const masterOrFault = engineOp === 'update' && recordExists && deps.checkControlledByParentWrite + const masterOrFault = MASTER_CHECKED_BY_ID_WRITES.has(engineOp) && recordExists && deps.checkControlledByParentWrite ? await settle(deps.checkControlledByParentWrite(object, recordId, context)) : undefined; const masterFault = masterOrFault === DEPENDENCY_FAULT; const masterRefusal = masterOrFault === undefined || masterOrFault === DEPENDENCY_FAULT ? undefined - : masterWriteCheckRefusal(masterOrFault); + : masterWriteCheckRefusal(masterOrFault, engineOp); // The sharing call THIS operation's row verdict rests on: the read filter for // a read (the sharing middleware ANDs it into every find), the per-record gate // for a write (the middleware gates every by-id write on it). Either one @@ -1462,7 +1504,7 @@ async function applyRecordAttribution( 'writable (fail closed), never admitted.' : masterFault ? 'The master-detail write check (controlled_by_parent, ADR-0055) could not be evaluated: it rejected — ' + - 'a refusal of the context or a store fault — and the by-id update fails on the same call, so the ' + + `a refusal of the context or a store fault — and the by-id ${engineOp} fails on the same call, so the ` + 'record is reported NOT writable (fail closed), never admitted.' : undefined); const anyShareAdmits = shareRules.some((r) => r.effect === 'admits'); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index c9f353de6ca..597bd3407f7 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -469,11 +469,11 @@ interface RlsFilterOptions { * • `getReadFilter` and `checkAuthoredRowWrite` do not, and need not: the * floor is `update`/`delete`-only so the read path never carries it, and * the authored-write probe already removes it by provenance; - * • `security/explain`'s record-grained UPDATE verdict sets it on step - * 2.7's condition, because explain asks the master gate for that same - * update ({@link SecurityPlugin.checkControlledByParentWrite}) and - * refuses on its refusal; its `delete` verdict asks no master gate and - * does not. + * • `security/explain`'s record-grained UPDATE and DELETE verdicts set it + * on step 2.7's condition, because explain asks the master gate for + * every by-id write it explains + * ({@link SecurityPlugin.checkControlledByParentWrite}) and refuses on + * its refusal. * * `false`/absent leaves the floor exactly where it is. Never affects Layer 0 * (the tenant wall) or any app-authored policy. @@ -5431,14 +5431,14 @@ export class SecurityPlugin implements Plugin { // answer the by-id PATCH / DELETE gives: // - Layer 1 carries the pre-image gate's floor decision (step 2.7, // {@link resolvePreImageFloorDrop}). - // - [ADR-0055] For an `update` it also carries step 2.7's coverage vouch, - // `masterGateCoversThisWrite`, on the same condition (not on behalf of - // anyone): explain asks the master gate for every update of a record - // that exists ({@link checkControlledByParentWrite}, wired below), so a - // `controlled_by_parent` record's ownership floor is handed over to that - // check here exactly as the write path hands it over. `delete` keeps - // the floor: explain asks no master gate for it, so it has nothing to - // vouch with. + // - [ADR-0055] It also carries step 2.7's coverage vouch, + // `masterGateCoversThisWrite`, on step 2.7's own condition (not on behalf + // of anyone), for an `update` and a `delete` alike: explain asks the + // master gate for every by-id write of a record that exists + // ({@link checkControlledByParentWrite}, wired below), so a + // `controlled_by_parent` record's ownership floor (`owner_only_writes`, + // `owner_only_deletes`) is handed over to that check here exactly as the + // write path hands it over. // - plugin-sharing's per-record gate is asked with `__writeScope` stamped // as the middleware stamps it (step 2.6), always overwritten, so an // `org` / unit writer is not judged owner-only. @@ -5462,7 +5462,10 @@ export class SecurityPlugin implements Plugin { const dropPlatformOwnershipFloor = await this.resolvePreImageFloorDrop( engineOp, o, recordId, c, sets, actsOnBehalfOf(c), ).catch(() => false); - const masterGateCoversThisWrite = engineOp === 'update' && !actsOnBehalfOf(c); + // Step 2.7's vouch is `!delegatorSets` for every verb it runs on; the + // floor itself comes off only where `masterGateCoversOperation` says the + // master gate covers the verb (`update`, `delete`). + const masterGateCoversThisWrite = !actsOnBehalfOf(c); return { dropPlatformOwnershipFloor, masterGateCoversThisWrite }; }; const withWriteScope = async (o: string, c: any): Promise => @@ -5546,10 +5549,13 @@ export class SecurityPlugin implements Plugin { // the caller-context by-id read, every data middleware included. recordAbsentToCaller: (o: string, rid: string, c: any) => absentUnderCallerRead(() => this.readRowById(o, rid, c)), - // [ADR-0055] The master-detail write check an update meets at step - // 2.8, served as `ISecurityService.checkControlledByParentWrite` and - // asked with the EXPLAINED context. Its presence is what licenses the - // update's `masterGateCoversThisWrite` vouch above: the two go together. + // [ADR-0055] The master-detail write check a by-id update, delete or + // transfer meets at step 2.8, served as + // `ISecurityService.checkControlledByParentWrite` and asked with the + // EXPLAINED context. The update's answer is each of those verbs' answer + // (the check judges EDIT of the master whatever the record's verb). Its + // presence is what licenses the `masterGateCoversThisWrite` vouch + // above: the two go together. checkControlledByParentWrite: (o: string, rid: string, c: any) => this.checkControlledByParentWrite(o, rid, c), }, diff --git a/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts b/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts index a2bb5a3915d..5bae6f9e4e1 100644 --- a/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts +++ b/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts @@ -29,6 +29,17 @@ // question (may this principal update `cpg_contract` at all), which is `true` // for every member here; this suite does not measure it. // +// ## Every write verb, not only update +// +// The data door runs the same master-detail write check (step 2.8) on a by-id +// DELETE, and on a transfer (the `PATCH` that writes `owner_id`, step 3.5's +// transfer door) too, and hands each one's ownership floor over to it the same +// way. The second block below enumerates every operation explain answers, +// classified against that door, and pins explain's verdict beside the door's +// answer for the master's editor and for a non-editor, per verb. Its +// classification is total, so a verb added to the explain vocabulary without a +// door parity row fails it. +// // ## Why the boot is org-bound // // The ownership floor binds `org_member` principals. An org-less boot measures @@ -37,7 +48,9 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { bootStack, type VerifyStack } from '@objectstack/verify'; -import { cpgStack, cpgSecurity } from './fixtures/cbp-parent-gates-fixture.js'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { ExplainOperationSchema, PermissionSetSchema, type ExplainOperation, type PermissionSet } from '@objectstack/spec/security'; +import { cpgStack, cpgBaselineSet, cpgFilesAndThreadsSet } from './fixtures/cbp-parent-gates-fixture.js'; import { assertArmed, principalArmed } from './armed.js'; const SYS = { isSystem: true } as const; @@ -49,6 +62,31 @@ const OWNERSHIP_FLOOR_DISARM = "an org-less harness: a fresh sign-up then holds only ['everyone'], so no floor applies and the editor " + 'control below passes whether or not explain hands the floor over. `orgContext: true` arms it.'; +/** + * The steward's grant: every write verb on the detail, so the object gate lets + * each verb through and the record-level gates answer it. `allowTransfer` is the + * grant the transfer door (a `PATCH` that writes `owner_id`) asks for. + */ +const contractStewardSet: PermissionSet = PermissionSetSchema.parse({ + name: 'cpe_contract_steward', + label: 'explain parity fixture — every write verb on the detail', + objects: { + cpg_contract: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, allowTransfer: true }, + }, +}); + +/** The fixture's own security composition, plus the steward's grant. */ +function cpeSecurity(): SecurityPlugin { + return new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, cpgBaselineSet, cpgFilesAndThreadsSet, contractStewardSet], + fallbackPermissionSet: cpgBaselineSet.name, + }); +} + +const OWNERSHIP_WRITE_AND_DELETE_FLOORS = + "the platform's wildcard row-level floors (`owner_only_writes` and `owner_only_deletes`, positions " + + "['org_member']), which a by-id write of a controlled_by_parent record hands over to the master-detail write check"; + /** Status plus the error code, for an assertion message that shows the whole answer. */ async function answer(res: Response): Promise<{ status: number; code: string | undefined; body: string }> { const body = await res.text(); @@ -74,6 +112,54 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do let memberContractId: string; /** The control: a `public_read_write` record. */ let boardId: string; + let adminId: string; + /** The steward: every write verb on `cpg_contract` (see `contractStewardSet`), the master's editor or not. */ + let stewardTok: string; + let stewardId: string; + /** Per verb with a by-id door: a child of the admin's master, which the steward may not edit. */ + const underAdmin: Partial> = {}; + /** Per verb with a by-id door: a child of the steward's own master, which the steward did not create. */ + const underSteward: Partial> = {}; + + /** + * Every operation explain answers, classified against the data door. Total + * by construction (`Record`) and checked against the + * spec's vocabulary below, so a verb added there without a row here fails. + * + * - `by_id_door`: the door a principal reaches by id, which runs the + * master-detail write check (step 2.8) and hands the record's ownership + * floor over to it (step 2.7). `transfer`'s door is the `PATCH` that + * writes `owner_id`: the engine dispatches no `transfer` operation, and + * that write is where the transfer grant is enforced (step 3.5). + * - `object_gate_refused`: step 2.8 lists the verb, but the object gate + * refuses it to every principal (its grant is retired until the lifecycle + * batch returns it) and no route dispatches it. + * - `no_by_id_write`: not a write the master check judges by id — a read, + * or an insert, whose master step 2.8 reads off the request body. + */ + type DoorRow = + | { kind: 'by_id_door'; door: (token: string, id: string) => Promise } + | { kind: 'object_gate_refused' } + | { kind: 'no_by_id_write' }; + const DOORS: Record = { + update: { kind: 'by_id_door', door: (t, id) => stack.apiAs(t, 'PATCH', `/data/cpg_contract/${id}`, { name: 'stewarded' }) }, + delete: { kind: 'by_id_door', door: (t, id) => stack.apiAs(t, 'DELETE', `/data/cpg_contract/${id}`) }, + transfer: { + kind: 'by_id_door', + door: (t, id) => stack.apiAs(t, 'PATCH', `/data/cpg_contract/${id}`, { owner_id: adminId }), + }, + restore: { kind: 'object_gate_refused' }, + purge: { kind: 'object_gate_refused' }, + create: { kind: 'no_by_id_write' }, + read: { kind: 'no_by_id_write' }, + export: { kind: 'no_by_id_write' }, + }; + const verbsOf = (kind: DoorRow['kind']) => (Object.keys(DOORS) as ExplainOperation[]).filter((v) => DOORS[v].kind === kind); + const doorOf = (verb: ExplainOperation) => { + const row = DOORS[verb]; + if (row.kind !== 'by_id_door') throw new Error(`${verb} has no by-id door`); + return row.door; + }; const uid = async (email: string) => (await ql.findOne('sys_user', { where: { email }, context: SYS }))?.id; @@ -91,16 +177,31 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do return { record: decision.record, sharing: sharing?.record as { outcome?: string; detail?: string } | undefined }; }; + /** explain for any verb, as the REST route answers it; the sharing layer's record attribution beside the record verdict. */ + const explainAs = async (token: string, operation: ExplainOperation, object: string, recordId: string) => { + const res = await stack.apiAs(token, 'POST', '/security/explain', { object, operation, recordId }); + const text = await res.text(); + expect(res.status, `explain ${operation} ${object}/${recordId}: ${text}`).toBe(200); + const decision = JSON.parse(text) as any; + const sharing = (decision.layers ?? []).find((l: any) => l.layer === 'sharing'); + return { record: decision.record, sharing: sharing?.record as { outcome?: string; detail?: string } | undefined }; + }; + const patch = async (token: string, object: string, id: string, name: string) => answer(await stack.apiAs(token, 'PATCH', `/data/${object}/${id}`, { name })); beforeAll(async () => { - stack = await bootStack(cpgStack as never, { security: cpgSecurity(), orgContext: true }); + stack = await bootStack(cpgStack as never, { security: cpeSecurity(), orgContext: true }); adminTok = await stack.signIn(); memberTok = await stack.signUp('cpe-member@verify.test'); ql = await stack.kernel.getServiceAsync('objectql'); - const adminId = await uid('admin@objectos.ai'); + adminId = await uid('admin@objectos.ai'); memberId = await uid('cpe-member@verify.test'); + stewardTok = await stack.signUp('cpe-steward@verify.test'); + stewardId = await uid('cpe-steward@verify.test'); + const stewardSet = await ql.findOne('sys_permission_set', { where: { name: contractStewardSet.name }, context: SYS }); + expect(stewardSet?.id, 'fixture permission set seeded').toBeTruthy(); + await ql.insert('sys_user_permission_set', { user_id: stewardId, permission_set_id: stewardSet.id }, { context: { ...SYS } }); await assertArmed([ principalArmed({ @@ -111,6 +212,17 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do control: OWNERSHIP_FLOOR, disarmedBy: OWNERSHIP_FLOOR_DISARM, }), + principalArmed({ + stack, + token: stewardTok, + who: 'the steward (every write verb on the detail; edits their own master, not the admin\'s)', + positions: ['org_member'], + permissions: [contractStewardSet.name], + control: OWNERSHIP_WRITE_AND_DELETE_FLOORS, + disarmedBy: + OWNERSHIP_FLOOR_DISARM + + ' And without the steward grant every delete and transfer below is refused at the object gate, before any record-level gate answers.', + }), ]); const adminAccountId = (await ql.insert('cpg_account', { name: 'admin account', owner_id: adminId }, { context: { ...SYS } })).id; @@ -119,6 +231,20 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do memberContractId = ( await ql.insert('cpg_contract', { name: 'member contract', account: memberAccountId }, { context: { ...SYS } }) ).id; + // One pair of children per verb with a by-id door, inserted as the system, so + // the steward created neither: the ownership floor binds both until a door + // hands it over to the master check. + const stewardAccountId = ( + await ql.insert('cpg_account', { name: 'steward account', owner_id: stewardId }, { context: { ...SYS } }) + ).id; + for (const verb of verbsOf('by_id_door')) { + underAdmin[verb] = ( + await ql.insert('cpg_contract', { name: `${verb}: admin's master`, account: adminAccountId }, { context: { ...SYS } }) + ).id; + underSteward[verb] = ( + await ql.insert('cpg_contract', { name: `${verb}: steward's master`, account: stewardAccountId }, { context: { ...SYS } }) + ).id; + } const boardRes = await stack.apiAs(adminTok, 'POST', '/data/cpg_board', { name: 'open board' }); expect(boardRes.status, 'board create').toBeLessThan(300); const board = (await boardRes.json()) as any; @@ -169,4 +295,44 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do const admitted = await patch(memberTok, 'cpg_board', boardId, 'open board'); expect(admitted.status, `member PATCH of the board: ${admitted.body}`).toBe(200); }); + describe('every write verb explain answers: its verdict is the door\'s, for the master\'s editor and for a non-editor', () => { + it('the classification is total: every operation explain answers has a door row', () => { + expect([...ExplainOperationSchema.options].sort()).toEqual(Object.keys(DOORS).sort()); + }); + + for (const verb of verbsOf('by_id_door')) { + it(`${verb}: the master's non-editor is explained refused on the master leg the door refuses on`, async () => { + const id = underAdmin[verb]!; + const explained = await explainAs(stewardTok, verb, 'cpg_contract', id); + const door = await answer(await doorOf(verb)(stewardTok, id)); + expect(door.status, `${verb} door, the master's non-editor: ${door.body}`).toBe(403); + expect(door.code).toBe('PERMISSION_DENIED'); + expect(door.body).toContain('requires edit access to its master record'); + expect(door.body).toContain('(row-level security)'); + + expect(explained.record, `explain ${verb}`).toEqual({ recordId: id, visible: false, decidedBy: 'sharing' }); + expect(explained.sharing?.outcome).toBe('excluded'); + expect(explained.sharing?.detail).toContain(`refuses this ${verb} on its 'row_level_security' leg`); + }); + + it(`${verb}: the master's editor, who did not create the child, is explained writable and the door answers 200`, async () => { + const id = underSteward[verb]!; + const explained = await explainAs(stewardTok, verb, 'cpg_contract', id); + const door = await answer(await doorOf(verb)(stewardTok, id)); + expect(door.status, `${verb} door, the master's editor: ${door.body}`).toBe(200); + + expect(explained.record, `explain ${verb}`).toMatchObject({ recordId: id, visible: true }); + expect(explained.sharing?.detail ?? '').not.toContain('master-detail write check'); + }); + } + + for (const verb of verbsOf('object_gate_refused')) { + it(`${verb}: refused at the object gate, for the master's editor and for a non-editor alike`, async () => { + for (const id of [underAdmin.update!, underSteward.update!]) { + const explained = await explainAs(stewardTok, verb, 'cpg_contract', id); + expect(explained.record, `explain ${verb} ${id}`).toEqual({ recordId: id, visible: false, decidedBy: 'object_crud' }); + } + }); + } + }); });