diff --git a/.changeset/adr-0111-depth-share-management.md b/.changeset/adr-0111-depth-share-management.md new file mode 100644 index 0000000000..1524ccde82 --- /dev/null +++ b/.changeset/adr-0111-depth-share-management.md @@ -0,0 +1,27 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-sharing": minor +"@objectstack/plugin-security": minor +--- + +feat(sharing): hierarchy managers may manage shares within their write DEPTH (ADR-0111 D1 DEPTH) + +`canManageShares` gains its named DEPTH extension: a caller whose effective +WRITE scope on the object is a hierarchy scope (`unit` / `unit_and_below` / +`own_and_reports`) may now manage shares on a record whose owner falls within +that scope's owner set — the same set the write filter and `canEdit` already +honour, resolved by the enterprise `hierarchy-scope-resolver`. This lets a +manager grant/revoke/list shares on a subordinate's record, matching +Salesforce (roles above the owner) and Dataverse (the `Share` privilege's BU +depth), without expanding the MVP owner + Modify-All authority. + +- New `ISecurityService.resolveWriteScope(object, context)` — the effective + write scope, resolved by the same evaluator the CRUD middleware uses; fails + closed to `own`. Mirrored on the sharing plugin's structural probe. +- The gate honours only the three hierarchy scopes. `org` from the probe is + deliberately ignored: it means both a genuine Modify-All holder (already + granted via `hasWriteBypass`) AND the fail-OPEN "no permission set mentions + this object" default, so honouring it here would reopen the hole + `hasWriteBypass` was chosen to avoid. +- Fails closed with no security service or no enterprise resolver — the open + edition stays owner + Modify-All, exactly as before. diff --git a/content/docs/kernel/runtime-services/sharing-service.mdx b/content/docs/kernel/runtime-services/sharing-service.mdx index 04b4d24580..b2d07157ed 100644 --- a/content/docs/kernel/runtime-services/sharing-service.mdx +++ b/content/docs/kernel/runtime-services/sharing-service.mdx @@ -24,9 +24,13 @@ services.sharing.listShares(object: string, recordId: string, context: SharingEx `grant` / `revoke` / `listShares` are **management operations**, enforced in the service for every non-system caller: the caller must hold `canManageShares` on -the record — its owner, a holder of Modify All Data on the object, or system -context. A deployment without `@objectstack/plugin-security` fails closed to -owner-only. Pass `{ isSystem: true }` only from platform-internal machinery. +the record — its **owner**, a holder of **Modify All Data**, a **hierarchy +manager** whose effective write DEPTH (`unit` / `unit_and_below` / +`own_and_reports`) covers the record's owner (via the enterprise +`hierarchy-scope-resolver`), or **system** context. A deployment without +`@objectstack/plugin-security` fails closed to owner-only; without the +enterprise resolver the DEPTH path is inert and authority stays owner + +Modify-All. Pass `{ isSystem: true }` only from platform-internal machinery. ## The verb boundary (ADR-0111 D3) diff --git a/docs/adr/0111-record-share-management-authority-and-verb-boundary.md b/docs/adr/0111-record-share-management-authority-and-verb-boundary.md index ab687d64a7..1540e35498 100644 --- a/docs/adr/0111-record-share-management-authority-and-verb-boundary.md +++ b/docs/adr/0111-record-share-management-authority-and-verb-boundary.md @@ -1,6 +1,6 @@ # ADR-0111: Record-share management authority and the verb boundary — sharing needs "who may manage a share" and "which verbs a level grants" -**Status**: Accepted (2026-07-30) — **P0 + P1 + D8 (framework) implemented**. P0 (D1/D2/D4/D5/D6/D7/D9): `canManageShares` + `hasWriteBypass`, verified by the #3902 Mallory reproduction. P1 (D3, the verb boundary): `canDelete` + verb-split `buildWriteFilter` in `plugin-sharing/src/sharing-service.ts`, routed by the middleware and `/security/explain`, verified by the "edit share cannot delete" suite. D8 (share-link re-share): `ShareLinkService.revokeLink` now admits a record share-manager via the late-bound `canManageShares` probe, and the mint-authority ruling (publicSharing opt-in + visibility) is enforced as before — verified in `share-link-service.test.ts`. **Still open**: the cloud-side D8 wiring (`HttpDispatcher.handleShareLinks` + a `.objectstack-sha` bump, in `objectstack-ai/cloud`) and the DEPTH management extension (D1 D-future) — the follow-ups this ADR's rollout section names. +**Status**: Accepted (2026-07-30) — **P0 + P1 + D8 (framework) implemented**. P0 (D1/D2/D4/D5/D6/D7/D9): `canManageShares` + `hasWriteBypass`, verified by the #3902 Mallory reproduction. P1 (D3, the verb boundary): `canDelete` + verb-split `buildWriteFilter` in `plugin-sharing/src/sharing-service.ts`, routed by the middleware and `/security/explain`, verified by the "edit share cannot delete" suite. D8 (share-link re-share): `ShareLinkService.revokeLink` now admits a record share-manager via the late-bound `canManageShares` probe, and the mint-authority ruling (publicSharing opt-in + visibility) is enforced as before — verified in `share-link-service.test.ts`. D1 DEPTH extension: `canManageShares` now admits a hierarchy manager whose effective write scope covers the record's owner, via the new `ISecurityService.resolveWriteScope` probe + the enterprise `hierarchy-scope-resolver` (framework side, fails closed to owner+Modify-All without the resolver). **Still open**: the cloud-side wiring for both D8 and the DEPTH extension (`HttpDispatcher.handleShareLinks` + a `.objectstack-sha` bump + `security-enterprise` integration tests, in `objectstack-ai/cloud`) — batched into one framework-SHA bump per this ADR's rollout. **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove — a security property that parses but enforces nothing is worse than absent), [ADR-0057](./0057-erp-authorization-core-business-units-and-scope-depth.md) (DEPTH scopes + the `sys_record_share` / `sys_sharing_rule` split), [ADR-0066](./0066-unified-authorization-model.md) (unified capability model; `modifyAllRecords` super-user bit), [ADR-0078](./0078-no-silently-inert-metadata.md) (no silently inert metadata — a persisted share level or recipient type that no gate consults is exactly this), [ADR-0090](./0090-permission-model-v2-concept-convergence.md) (D1 secure-default OWD, D4 retired aliases, D10 delegated identity intersection), [ADR-0091](./0091-grant-lifecycle-and-recertification.md) (time-boxed grants — the lifecycle axis this ADR deliberately does not re-open) **Consumers**: `@objectstack/plugin-sharing` (`sharing-service.ts`, `sharing-rule-service.ts`, `share-link-service.ts`, `sharing-plugin.ts`), `@objectstack/plugin-security` (`ISecurityService` — a write-bypass probe), `@objectstack/rest` (`rest-server.ts` sharing / sharing-rule / share-link routes), `@objectstack/spec` (`contracts/sharing-service.ts`, `security/capabilities.ts`) @@ -90,7 +90,7 @@ Introduce `canManageShares(object, recordId, context): Promise` on `ISh **Explicitly not** `getEffectiveScope(...) === 'org'`: that helper returns `'org'` for the "no permission set even mentions this object" case (a compatibility fail-*open* baked into the read path). Reusing it as a management gate would be a fresh hole. Management authority keys off the **explicit** `modifyAllRecords` bit only. -**DEPTH is a named direction, not in the MVP.** Salesforce (roles above the owner) and Dataverse (the `Share` privilege's BU depth) both let hierarchy managers share subordinates' records. ObjectStack's `__writeScope` (`unit` / `unit_and_below` / `own_and_reports`) is computed today only inside the engine middleware, not on the REST management path; plumbing it here is real work behind the enterprise `hierarchy-scope-resolver`. The MVP is owner + `modifyAllRecords`; D-future records the DEPTH extension so the model has a place for it. +**DEPTH — landed after the MVP.** Salesforce (roles above the owner) and Dataverse (the `Share` privilege's BU depth) both let hierarchy managers share subordinates' records. The MVP shipped as owner + `modifyAllRecords`; the DEPTH extension then landed on the framework side: rather than reuse the middleware-only `__writeScope` (absent on the REST management path), `canManageShares` resolves the caller's effective write scope through a new `ISecurityService.resolveWriteScope` probe and, for a hierarchy scope only, expands it via the enterprise `hierarchy-scope-resolver` (the same owner set the write filter uses). `org` from that probe is deliberately NOT honoured as manager authority — it doubles as the fail-open "no permission set matches" default — so org-wide management stays gated by the explicit `modifyAllRecords` bypass. Fails closed to owner + Modify-All when the resolver (open edition) or security service is absent. **Grant may never exceed the granter's own level** (Dataverse's rule). A caller who may manage sharing but is not owner/super-user may only grant a level they themselves hold on the record. In the MVP this is trivially satisfied (only owner/super-user pass the gate, and both hold every level), but the constraint is stated now so the DEPTH extension inherits it rather than bolting it on later. diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 584bd3d3c7..acdaa35c43 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -682,6 +682,27 @@ export class SecurityPlugin implements Plugin { return false; } }, + // [ADR-0111 D1 DEPTH] Effective WRITE scope — the DEPTH primitive behind + // canManageShares' hierarchy-manager branch. Same evaluator the CRUD + // write path uses; fails CLOSED to 'own' on error / principal-less / + // on-behalf-of. Returns 'org' for system and for Modify-All holders — + // AND for the fail-open unmatched-object case, which is why the sharing + // gate treats 'org' from here as non-authoritative on its own. + resolveWriteScope: async (object: string, context?: any): Promise<'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'> => { + if (context?.isSystem) return 'org'; + if (!context?.userId || context?.onBehalfOf?.userId) return 'own'; + try { + const meta = await this.getObjectSecurityMeta(object); + const sets = await this.resolvePermissionSetsForContext(context); + return this.permissionEvaluator.getEffectiveScope('write', object, sets, { isPrivate: meta.isPrivate }); + } catch (e) { + this.logger.warn?.( + `[security] resolveWriteScope failed for object '${object}' (user ${context?.userId ?? 'unknown'}) — narrowing to 'own' (fail-closed)`, + e instanceof Error ? e : new Error(String(e)), + ); + return 'own'; + } + }, // [ADR-0046 §6.7] Effective permission-set NAMES for a caller — the // primitive the REST read layer needs to evaluate a permission-set- // gated book/doc audience ({ permissionSet: '…' }). Same resolution diff --git a/packages/plugins/plugin-sharing/src/sharing-service.test.ts b/packages/plugins/plugin-sharing/src/sharing-service.test.ts index ce4cd87a40..6ddfe4a6d1 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.test.ts @@ -748,6 +748,76 @@ describe('[ADR-0111 D1] SharingService.canManageShares', () => { expect(await svc.canManageShares('account', 'nope', { userId: 'alice' })).toBe(false); expect(await svc.canManageShares('account', 'a1', {})).toBe(false); }); + + // ── [ADR-0111 D1 DEPTH] hierarchy-manager authority ────────────────── + // a1 is owned by alice. bob is alice's manager (own_and_reports covers alice); + // carol is an unrelated peer. The enterprise resolver is stubbed. + it('a hierarchy manager whose write DEPTH covers the owner may manage the record', async () => { + const svc = new SharingService({ + engine, + securityService: () => ({ + hasWriteBypass: async () => false, + resolveWriteScope: async () => 'own_and_reports', + }), + hierarchyResolver: () => ({ + // bob manages alice; nobody else in bob's reports. + resolveOwnerIds: async (ctx: any) => ctx.userId === 'bob' ? ['bob', 'alice'] : [ctx.userId], + }), + }); + expect(await svc.canManageShares('account', 'a1', { userId: 'bob' })).toBe(true); + }); + + it('a peer with the same DEPTH scope but NOT covering the owner is denied', async () => { + const svc = new SharingService({ + engine, + securityService: () => ({ + hasWriteBypass: async () => false, + resolveWriteScope: async () => 'unit', + }), + hierarchyResolver: () => ({ + resolveOwnerIds: async (ctx: any) => [ctx.userId], // carol's unit owner-set excludes alice + }), + }); + expect(await svc.canManageShares('account', 'a1', { userId: 'carol' })).toBe(false); + }); + + it("a probe reporting 'org' does NOT widen management (fail-open guard — only Modify All via hasWriteBypass grants org)", async () => { + const svc = new SharingService({ + engine, + securityService: () => ({ + hasWriteBypass: async () => false, // NOT a real Modify-All holder + resolveWriteScope: async () => 'org', // the fail-open unmatched-object default + }), + // A resolver that would (wrongly) return everyone if consulted. + hierarchyResolver: () => ({ resolveOwnerIds: async () => ['alice', 'bob', 'carol'] }), + }); + expect(await svc.canManageShares('account', 'a1', { userId: 'stranger' })).toBe(false); + }); + + it("'own' scope adds nothing beyond the ownership check (non-owner denied)", async () => { + const svc = new SharingService({ + engine, + securityService: () => ({ + hasWriteBypass: async () => false, + resolveWriteScope: async () => 'own', + }), + hierarchyResolver: () => ({ resolveOwnerIds: async (ctx: any) => [ctx.userId] }), + }); + expect(await svc.canManageShares('account', 'a1', { userId: 'mallory' })).toBe(false); + }); + + it('DEPTH branch needs the enterprise resolver — a manager scope with no resolver falls back to owner-only', async () => { + const svc = new SharingService({ + engine, + securityService: () => ({ + hasWriteBypass: async () => false, + resolveWriteScope: async () => 'unit', + }), + // No hierarchyResolver → resolveOwnerScopeIds fails closed to [me], which + // excludes alice, so bob cannot manage alice's record. + }); + expect(await svc.canManageShares('account', 'a1', { userId: 'bob' })).toBe(false); + }); }); describe('[ADR-0111 D1/D4/D5] the #3902 Mallory reproduction', () => { diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 191d2c976e..5daffcc5e9 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -89,6 +89,17 @@ function hasOwnerField(schema: any): boolean { */ export interface SharingSecurityProbe { hasWriteBypass?(object: string, context: unknown): Promise; + /** + * [ADR-0111 D1 DEPTH] The caller's effective WRITE scope on `object` + * (`own` / `own_and_reports` / `unit` / `unit_and_below` / `org`), resolved + * from their permission sets exactly as the CRUD middleware resolves it. + * Used by {@link SharingService.canManageShares} to let a hierarchy manager + * manage shares on records they can write by DEPTH. Fails closed to `own`. + */ + resolveWriteScope?( + object: string, + context: unknown, + ): Promise<'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'>; } export interface SharingServiceOptions { @@ -356,10 +367,11 @@ export class SharingService implements ISharingService { * [ADR-0111 D1] May `context` MANAGE shares (grant / revoke / list) on * `(object, recordId)`? System → yes. Record owner → yes. Super-user write * bypass (`modifyAllRecords`, probed via the late-bound security service) → - * yes. Everything else — a missing record, a principal-less context, a - * probe failure, a deployment without plugin-security — fails CLOSED to - * `false`. The DEPTH extension (hierarchy managers may share subordinates' - * records) is a named ADR-0111 direction, deliberately not implemented here. + * yes. [ADR-0111 D1 DEPTH] A HIERARCHY MANAGER whose write DEPTH + * (`unit` / `unit_and_below` / `own_and_reports`) covers the record's owner → + * yes. Everything else — a missing record, a principal-less context, a probe + * failure, a deployment without plugin-security or the enterprise resolver — + * fails CLOSED to `false`. */ async canManageShares( object: string, @@ -370,7 +382,9 @@ export class SharingService implements ISharingService { if (!object || !recordId || !context?.userId) return false; // Ownership — read under system context so field-level masking cannot - // hide the owner column from the decision itself. + // hide the owner column from the decision itself. Keep the owner value: + // the DEPTH branch below reuses it rather than re-reading the row. + let owner: unknown; try { const rows = await this.engine.find(object, { where: { id: recordId }, @@ -380,21 +394,43 @@ export class SharingService implements ISharingService { }); const row: any = Array.isArray(rows) ? rows[0] : undefined; if (!row) return false; - const owner = row[OWNER_FIELD]; + owner = row[OWNER_FIELD]; if (owner != null && String(owner) === String(context.userId)) return true; } catch { return false; } + const probe = this.securityService?.(); + // Modify All Data — the EXPLICIT bypass only (ADR-0111 D1/D2; never the // effective write scope, whose unmatched-object case fails open to 'org'). try { - const probe = this.securityService?.(); if (probe && typeof probe.hasWriteBypass === 'function') { - return (await probe.hasWriteBypass(object, context)) === true; + if ((await probe.hasWriteBypass(object, context)) === true) return true; } } catch { - /* fall through to deny */ + /* fall through */ + } + + // [ADR-0111 D1 DEPTH] Hierarchy-manager authority: a caller whose effective + // WRITE scope on this object is a HIERARCHY scope may manage shares on a + // record whose owner falls within that scope's owner set (the same set the + // write filter/canEdit use). Only the three hierarchy scopes widen here — + // `own` adds nothing beyond the ownership check above, and `org` is + // DELIBERATELY ignored: `resolveWriteScope` returns `org` both for a genuine + // Modify-All holder (already handled) AND for the fail-OPEN + // "no permission set mentions this object" case, so honouring `org` here + // would reopen exactly the hole `hasWriteBypass` was chosen to avoid. + if (owner != null && probe && typeof probe.resolveWriteScope === 'function') { + try { + const scope = await probe.resolveWriteScope(object, context); + if (scope === 'unit' || scope === 'unit_and_below' || scope === 'own_and_reports') { + const ownerIds = await this.resolveOwnerScopeIds(context, scope); + if (ownerIds.includes(String(owner))) return true; + } + } catch { + /* fall through to deny */ + } } return false; } diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index c38ad9032a..7ef8a494fe 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -236,6 +236,28 @@ export interface ISecurityService { */ hasWriteBypass(object: string, context?: SecurityContext): Promise; + /** + * [ADR-0111 D1 DEPTH] The caller's effective WRITE scope on `object` — + * `own` / `own_and_reports` / `unit` / `unit_and_below` / `org` — resolved + * from their permission sets exactly as the CRUD middleware's write path + * resolves it (`getEffectiveScope('write', …)`). + * + * The sharing layer's management gate (`ISharingService.canManageShares`) + * uses this to let a HIERARCHY MANAGER manage shares on records within their + * DEPTH. Note the two meanings of `org`: a genuine `modifyAllRecords` holder, + * AND the fail-OPEN "no permission set mentions this object" default — so a + * caller of this method must treat `org` as authoritative ONLY when paired + * with an explicit {@link hasWriteBypass} check, never on its own. + * + * **Fails CLOSED** to `own` (the narrowest scope) on a resolution error, a + * principal-less context, or an on-behalf-of context (no D10 delegator + * intersection on this path). A system context resolves to `org`. + */ + resolveWriteScope( + object: string, + context?: SecurityContext, + ): Promise<'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org'>; + /** * Explain WHY access is granted or denied — the decision plus the layers that * produced it (permission sets, object permissions, RLS, sharing, field mask).