Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/22792-explain-read-master-leg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@objectstack/plugin-security': patch
---

`security/explain`'s record-grained `read` verdict for a `controlled_by_parent` record now equals the read door's answer on the master leg (ADR-0055).

- **What changed.** The read door scopes such a record by its master as well as by its own row-level security. explain's read branch modelled the record's own row-level security only. It now asks the read door's own by-id read, with the context it is explaining, wherever every other leg of the read's row story admits the record. A record the door does not return is reported not visible, decided by the `sharing` layer. A record the door returns is reported exactly as before. A rejection of that read is reported fail-closed, as a dependency that throws already is. An `export` explanation, which streams the same find, answers the same way.
- **What is unchanged.** Every object that is not `controlled_by_parent`. Object-level reports (no `recordId`). The `allowed` field. Every write verb. The answer for an id no row carries. The `ExplainEngineDeps` type: the `recordAbsentToCaller` dependency keeps its signature and is now asked for such a read too.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@
// record's write gate), naming the leg or the reason; a rejection fails the
// request, so it is reported fail-closed; an outcome outside the vocabulary
// refuses too.
//
// A READ of such a record meets a master leg as well: the read door scopes the
// find by the master ids the principal can read. The write check does not
// answer a read, so explain asks the read door's own by-id read
// (`recordAbsentToCaller`) where every leg it models admits the record, and
// takes its answer. The last block pins that half: a record the door withholds
// is not visible on the `sharing` layer, a record the door returns leaves the
// report exactly as before, and a rejection is reported fail-closed. The REST
// answer beside the REST read door is pinned in the same dogfood suite.
import { describe, it, expect, vi } from 'vitest';
import { PermissionSetSchema } from '@objectstack/spec/security';
import { ExplainOperationSchema, type ExplainDecision, type ExplainOperation } from '@objectstack/spec/security';
Expand All @@ -39,21 +48,26 @@ 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, allowTransfer: true } },
objects: {
[OBJECT]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, allowTransfer: true, allowExport: true },
},
});

/** The principal being EXPLAINED. Kept as one object so a cell can assert the check received this very context. */
const EXPLAINED = { userId: 'u_member', tenantId: 'org1', positions: ['org_member'], permissions: ['cbx_editor'] };
const RECORD = { id: 'r1', organization_id: 'org1', owner_id: 'u_somebody' };

type Master = ExplainEngineDeps['checkControlledByParentWrite'];
type ReadDoor = ExplainEngineDeps['recordAbsentToCaller'];

function deps(opts: {
schema?: Record<string, unknown>;
master?: Master;
canEdit?: boolean;
record?: Record<string, unknown> | null;
layer1?: Record<string, unknown> | null;
/** The read door's own by-id read: does it withhold the record from the explained principal? */
readDoor?: ReadDoor;
} = {}): ExplainEngineDeps {
return {
ql: { getSchema: () => opts.schema ?? CBP_SCHEMA },
Expand All @@ -77,6 +91,7 @@ function deps(opts: {
canEditRecord: async () => opts.canEdit ?? true,
canDeleteRecord: async () => opts.canEdit ?? true,
...(opts.master ? { checkControlledByParentWrite: opts.master } : {}),
...(opts.readDoor ? { recordAbsentToCaller: opts.readDoor } : {}),
};
}

Expand All @@ -100,17 +115,23 @@ const explainAs = (d: ExplainEngineDeps, operation: ExplainOperation, recordId:
* 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.
* - `read_door_asked`: a read, and an export, which streams the same find.
* The check guards writes and is not asked; the read door's master leg is,
* through the read door's own by-id read (the last block below).
* - `not_asked`: `create`, whose master comes from the request body an
* explanation does not carry.
*/
const VERB_CLASS: Record<ExplainOperation, 'master_checked' | 'master_checked_object_gate_first' | 'not_asked'> = {
const VERB_CLASS: Record<
ExplainOperation,
'master_checked' | 'master_checked_object_gate_first' | 'read_door_asked' | 'not_asked'
> = {
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',
read: 'read_door_asked',
export: 'read_door_asked',
create: 'not_asked',
};
const MASTER_CHECKED = (Object.keys(VERB_CLASS) as ExplainOperation[]).filter((v) => VERB_CLASS[v] === 'master_checked');
Expand Down Expand Up @@ -243,7 +264,9 @@ describe('[ADR-0055] which verbs ask the check, and for whom', () => {
});
}

for (const verb of (Object.keys(VERB_CLASS) as ExplainOperation[]).filter((v) => VERB_CLASS[v] === 'not_asked')) {
for (const verb of (Object.keys(VERB_CLASS) as ExplainOperation[]).filter(
(v) => VERB_CLASS[v] === 'not_asked' || VERB_CLASS[v] === 'read_door_asked',
)) {
it(`a record-grained ${verb} does not ask it`, async () => {
const master = answering({ outcome: 'deny', leg: 'object_permission' });
await explainAs(deps({ master }), verb);
Expand All @@ -262,3 +285,83 @@ describe('[ADR-0055] which verbs ask the check, and for whom', () => {
expect(master).not.toHaveBeenCalled();
});
});

const READ_DOOR_ASKED = (Object.keys(VERB_CLASS) as ExplainOperation[]).filter((v) => VERB_CLASS[v] === 'read_door_asked');

/** The read door's answer: `true` withholds the record from the explained principal, `false` returns it. */
const readDoorAnswering = (absent: boolean): ReadDoor => vi.fn(async () => absent);

describe('[ADR-0055] a read of a controlled_by_parent record takes the read door\'s answer on the master leg', () => {
for (const verb of READ_DOOR_ASKED) {
it(`${verb}: a record the read door withholds is not visible, decided by the sharing layer`, async () => {
const d = await explainAs(deps({ readDoor: readDoorAnswering(true) }), verb);
expect(d.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' });
expect(sharingRecordOf(d)?.outcome).toBe('excluded');
});

it(`${verb}: a record the read door returns: byte-identical to the report without the question`, async () => {
const without = await explainAs(deps(), verb);
const withDoor = await explainAs(deps({ readDoor: readDoorAnswering(false) }), verb);
expect(withDoor).toEqual(without);
expect(withDoor.record).toMatchObject({ recordId: 'r1', visible: true });
});

it(`${verb}: a rejection fails the find, 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 readDoor: ReadDoor = vi.fn(async () => { throw fault; });
const d = await explainAs(deps({ readDoor }), verb);
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();
});

it(`${verb}: asked once, for that record, with the EXPLAINED context, and the write check is not asked`, async () => {
const readDoor = readDoorAnswering(false);
const master = answering({ outcome: 'deny', leg: 'object_permission' });
await explainAs(deps({ readDoor, master }), verb);
expect(readDoor).toHaveBeenCalledTimes(1);
expect(readDoor).toHaveBeenCalledWith(OBJECT, 'r1', EXPLAINED);
expect((readDoor as ReturnType<typeof vi.fn>).mock.calls[0][2]).toBe(EXPLAINED);
expect(master).not.toHaveBeenCalled();
});

it(`${verb}: a leg this report models that withholds the record decides, and the read door is not asked`, async () => {
const readDoor = readDoorAnswering(true);
const byRls = await explainAs(deps({ readDoor, layer1: { id: 'not_this_record' } }), verb);
expect(byRls.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'rls' });
const byTenant = await explainAs(deps({ readDoor, record: { ...RECORD, organization_id: 'org2' } }), verb);
expect(byTenant.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'tenant_isolation' });
expect(readDoor).not.toHaveBeenCalled();
});

for (const [label, schema] of [
['public_read_write', { name: OBJECT, sharingModel: 'public_read_write' }],
['private', { name: OBJECT, sharingModel: 'private' }],
] as const) {
it(`${verb}: not asked on a ${label} object, which derives nothing from a master: the report is unchanged`, async () => {
const readDoor = readDoorAnswering(true);
const without = await explainAs(deps({ schema }), verb);
const withDoor = await explainAs(deps({ schema, readDoor }), verb);
expect(withDoor).toEqual(without);
expect(readDoor).not.toHaveBeenCalled();
});
}

it(`${verb}: an object-level request, and a record that does not exist, are not asked about`, async () => {
const readDoor = readDoorAnswering(true);
const objectLevel = await explainAs(deps({ readDoor }), verb, null);
expect(objectLevel.record).toBeUndefined();
const missing = await explainAs(deps({ readDoor, record: null }), verb, 'r_missing');
expect(missing.record).toEqual({ recordId: 'r_missing', visible: false });
expect(readDoor).not.toHaveBeenCalled();
});
}

it('a create, which addresses no stored record, is not asked about', async () => {
const readDoor = readDoorAnswering(true);
await explainAs(deps({ readDoor }), 'create');
expect(readDoor).not.toHaveBeenCalled();
});
});
61 changes: 54 additions & 7 deletions packages/plugins/plugin-security/src/explain-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,12 @@ export interface ExplainEngineDeps {
* delete, read through {@link rlsOperationForVerb}: `update`, `delete`, and
* the lifecycle verbs that map onto them (`transfer` and `restore` onto
* update, `purge` onto delete).
*
* [ADR-0055] Asked for a record-grained read too, of a
* `controlled_by_parent` record the rest of the read's row story admits: the
* read door also scopes such a record by its master, a leg no other
* dependency here answers, so the door's own answer is what the read verdict
* takes ({@link applyRecordAttribution}).
*/
recordAbsentToCaller?: (object: string, recordId: string, context: any) => Promise<boolean>;
}
Expand Down Expand Up @@ -942,6 +948,7 @@ const MASTER_UNRESOLVED_DETAIL: Record<ControlledByParentWriteUnresolvedReason,
};

const MASTER_DERIVED = 'Write access to this record derives from its master (controlled_by_parent, ADR-0055)';
const MASTER_READ_DERIVED = 'Read access to this record derives from its master (controlled_by_parent, ADR-0055)';

/**
* [ADR-0055] The record-grained writes whose verdict on a `controlled_by_parent`
Expand Down Expand Up @@ -1516,6 +1523,41 @@ async function applyRecordAttribution(
const masterRefusal = masterOrFault === undefined || masterOrFault === DEPENDENCY_FAULT
? undefined
: masterWriteCheckRefusal(masterOrFault, engineOp);
// [ADR-0055] A record-grained read of a `controlled_by_parent` record meets
// a master leg too: the read door ANDs the record's master-derived read scope
// (the master ids the principal can read, the chain included) into the find,
// beside the row-level security above, and the sharing service imposes
// nothing of its own on such an object (its effective model is `public`).
// No other dependency answers that leg, so the read door's own by-id read is
// asked (`recordAbsentToCaller`, the question the by-id writes ask), with the
// explained context, and only where every leg this report models admits the
// record: there the master leg is the one left that withholds it. A record
// the door withholds is reported NOT visible, as the door answers, on the
// sharing layer, the layer the write verbs' master check names. A rejection
// fails the find, so it is reported fail-closed.
const readStoryAdmits = isRead && recordExists && owd.model === 'controlled_by_parent'
&& !capsDeny && crudAllowed && !layeredFault && !readFilterFault
&& tenantRecord.outcome !== 'excluded'
&& !(layer1 != null && (isDenyAll(layer1) || matches(layer1) === false));
const masterReadOrFault = readStoryAdmits && deps.recordAbsentToCaller
? await settle(deps.recordAbsentToCaller(object, recordId, context))
: undefined;
const masterReadFault = masterReadOrFault === DEPENDENCY_FAULT;
const masterReadRefusal: { outcome: ExplainRecordAttribution['outcome']; detail: string } | undefined =
masterReadOrFault === true
? {
outcome: 'excluded',
detail: `${MASTER_READ_DERIVED}: the read door, asked by this principal, does not return the record, though ` +
"every row-level layer above admits it, so its master is outside the principal's read scope" +
(context?.onBehalfOf?.userId != null
? " (on this on-behalf-of read, the delegator's read scope is composed in as well)"
: '') +
'. The record is reported NOT visible, as the read door answers.',
}
: undefined;
// One master leg per verb: the write check for a by-id write, the read
// door's master scope for a read. At most one of the two is ever asked.
const masterLegRefusal = masterRefusal ?? masterReadRefusal;
// 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
Expand All @@ -1525,7 +1567,10 @@ async function applyRecordAttribution(
? (readFilterFault
? "The sharing service's read filter could not be evaluated: it threw, and the find fails on the same " +
'call — so the record is reported NOT visible (fail closed), never admitted.'
: undefined)
: masterReadFault
? `${MASTER_READ_DERIVED}, and the read door's own by-id read of it could not be evaluated: it threw, and ` +
'the find fails on the same call — so the record is reported NOT visible (fail closed), never admitted.'
: undefined)
: (writeGateFault
? `The sharing service's per-record ${engineOp === 'delete' ? 'delete' : 'update'} gate could not be ` +
'evaluated: it threw, and the by-id write fails on the same call — so the record is reported NOT ' +
Expand Down Expand Up @@ -1560,8 +1605,8 @@ async function applyRecordAttribution(
sharingOutcome = 'not_evaluated';
} else if (sharingFaultDetail) {
sharingOutcome = 'not_evaluated';
} else if (masterRefusal) {
sharingOutcome = masterRefusal.outcome;
} else if (masterLegRefusal) {
sharingOutcome = masterLegRefusal.outcome;
} else if (owd.effect !== 'private') {
sharingOutcome = 'not_evaluated'; // baseline already grants the rows sharing would add
} else if (canEdit !== undefined) {
Expand All @@ -1585,8 +1630,8 @@ async function applyRecordAttribution(
matchesRecord: sharingFaultDetail ? undefined : sharingMatches,
rules: shareRules,
// A fault is only ever recorded against an existing record, and the
// master check [ADR-0055] is only ever asked of one.
detail: sharingFaultDetail ?? masterRefusal?.detail ?? (!recordExists
// master leg [ADR-0055] is only ever asked of one.
detail: sharingFaultDetail ?? masterLegRefusal?.detail ?? (!recordExists
? 'Record not found; sharing not evaluated.'
: owd.effect !== 'private'
? 'Baseline is not private — sharing adds nothing beyond it for this record.'
Expand Down Expand Up @@ -1778,8 +1823,10 @@ async function applyRecordAttribution(
else if (sharingFaultDetail) { visible = false; decidedBy = 'sharing'; }
// [ADR-0055] The master check refuses after the record's own row-level
// security (step 2.8 runs below the pre-image gate), and names its leg on
// the sharing layer, the record's write gate.
else if (masterRefusal) { visible = false; decidedBy = 'sharing'; }
// the sharing layer, the record's write gate. A read's master leg is asked
// only where the record's own row-level security admits it, and is named on
// the same layer.
else if (masterLegRefusal) { visible = false; decidedBy = 'sharing'; }
else if (!businessRowAdmits) { visible = false; decidedBy = owd.effect === 'private' ? 'sharing' : 'owd_baseline'; }
else {
visible = true;
Expand Down
Loading
Loading