diff --git a/.changeset/sharing-rule-recipient-reconcile.md b/.changeset/sharing-rule-recipient-reconcile.md new file mode 100644 index 0000000000..b823b0d49f --- /dev/null +++ b/.changeset/sharing-rule-recipient-reconcile.md @@ -0,0 +1,40 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-sharing": minor +--- + +feat(security)!: reconcile the SharingRule authoring surface with the enforced runtime — rename `group` → `team`, add `business_unit`, prune `guest` + owner-type rules (#1878) + +The authoring `ShareRecipientType` enum had drifted behind the ADR-0090 D3 +rename and the enforced runtime: the runtime expands `team` (via +`sys_team`/`sys_team_member`) and `business_unit`, but the authoring enum +still offered the pre-rename `group` (silently skipped at seed time) and +omitted the two live recipients. After this change **every authorable +recipient and rule type is enforced** — nothing on the SharingRule surface +validates and then silently does nothing (ADR-0078). + +- **`sharedWith.type: 'group'` → `'team'`** (wire-rename): the enum member is + renamed to match the runtime vocabulary and now maps through the seed + bootstrap to the live `TeamGraphService` expansion. Flat `sys_team` + membership; enforced. +- **`business_unit` added** to the authoring enum — exactly one business + unit's members (no subtree; use `unit_and_subordinates` for the subtree). + The runtime + bootstrap already enforced it; only the enum omitted it. +- **`guest` removed** — it had no runtime recipient mapping. Anonymous access + is served by the public-form grant and share links, not sharing rules. +- **Owner-type rules removed** (`type: 'owner'`, `ownedBy`, + `OwnerSharingRuleSchema` + its type export): they depend on live + team/position membership, which the static materialiser cannot track, so + they validated but never materialised a share. They return as an enforced + form if membership-reactive re-materialisation is designed. + `SharingRuleSchema` is now the criteria form; the `queue` recipient stays + runtime-reserved (no `sys_queue` yet) and deliberately non-authorable. + +**Migration** (stale definitions now fail parse with the valid options listed): +- `sharedWith: { type: 'group', … }` → `sharedWith: { type: 'team', … }`. +- `sharedWith: { type: 'guest', … }` → delete the rule; expose the records + via a public form or share link instead. +- `type: 'owner'` rules → rewrite as a `type: 'criteria'` rule scoping the + rows by field values (see the migrated examples: + `share_open_tasks_with_manager` in app-showcase, + `share_active_leads_with_manager` in app-crm), or use a scope-depth grant. diff --git a/content/docs/permissions/sharing-rules.mdx b/content/docs/permissions/sharing-rules.mdx index f62e6921ed..7bfae11705 100644 --- a/content/docs/permissions/sharing-rules.mdx +++ b/content/docs/permissions/sharing-rules.mdx @@ -132,27 +132,32 @@ export const AccountTeamSharingRule = defineSharingRule({ ### Recipient types -`sharedWith` (and `ownedBy`) accept a `{ type, value }` recipient: +`sharedWith` accepts a `{ type, value }` recipient. **Every authorable +recipient is enforced** — each expands to concrete users at seed time and +materializes `sys_record_share` grants: | `type` | Shares with | |:--|:--| | `user` | A single user | -| `group` | All members of a public group — **declared but not enforced**: skipped at seed time with a warning (ADR-0049) | +| `team` | All members of a `sys_team` (flat collaboration grouping) | | `position` | Everyone assigned that position (flat expansion — positions have no tree) | | `unit_and_subordinates` | Everyone in that **business unit and every unit beneath it** (the BU tree is the one hierarchy — ADR-0090 D3) | -| `guest` | Public / anonymous (`guest`) principals — **declared but not enforced**: no runtime recipient mapping, skipped at seed time with a warning (ADR-0049) | +| `business_unit` | Everyone in exactly that business unit (no subtree) | A criteria `condition` must be compilable by the CEL → filter pushdown compiler. A condition the compiler cannot lower is **skipped and logged — never seeded as a permissive match-all** (ADR-0049): a bad condition under-shares rather than over-shares. -## Owner-Based Sharing Rules - -> **[Experimental — not enforced.]** Owner-based (`type: 'owner'`) rules -> depend on live position membership and have no static `criteria_json` -> equivalent, so the seeder **skips them with a warning** — they materialize -> no record shares today. Use a criteria rule or a scope-depth grant. +> **Retired shapes.** The pre-ADR-0090 `group` recipient was renamed to +> `team`, and the `guest` recipient was removed — anonymous access is served +> by the [public-form grant](/docs/permissions/authorization) and share +> links, not sharing rules. Owner-based rules (`type: 'owner'`, `ownedBy`) +> were also removed: they depended on live membership the static seeder +> cannot track, so they validated but never materialized a share. Use a +> criteria rule or a scope-depth grant instead; none of these shapes parses +> anymore, so a stale definition fails loudly at authoring time instead of +> silently doing nothing (ADR-0078). ## Analytics and Dataset Read Scope diff --git a/content/docs/references/security/sharing.mdx b/content/docs/references/security/sharing.mdx index 0a646e9ab8..b8bd38e65d 100644 --- a/content/docs/references/security/sharing.mdx +++ b/content/docs/references/security/sharing.mdx @@ -16,8 +16,8 @@ The baseline security posture for an object. ## TypeScript Usage ```typescript -import { CriteriaSharingRule, OWDModel, OwnerSharingRule, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security'; -import type { CriteriaSharingRule, OWDModel, OwnerSharingRule, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security'; +import { CriteriaSharingRule, OWDModel, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security'; +import type { CriteriaSharingRule, OWDModel, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security'; // Validate data const result = CriteriaSharingRule.parse(data); @@ -37,7 +37,7 @@ const result = CriteriaSharingRule.parse(data); | **object** | `string` | ✅ | Target Object Name | | **active** | `boolean` | optional | | | **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | | -| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` | ✅ | The recipient of the shared access | +| **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` | ✅ | The recipient of the shared access | | **type** | `'criteria'` | ✅ | | | **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | ✅ | Predicate (CEL). e.g. P`record.department == "Sales"` | @@ -54,25 +54,6 @@ const result = CriteriaSharingRule.parse(data); * `controlled_by_parent` ---- - -## OwnerSharingRule - -### Properties - -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Unique rule name (snake_case) | -| **label** | `string` | optional | Human-readable label | -| **description** | `string` | optional | Administrative notes | -| **object** | `string` | ✅ | Target Object Name | -| **active** | `boolean` | ✅ | | -| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | ✅ | | -| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` | ✅ | The recipient of the shared access | -| **type** | `'owner'` | ✅ | | -| **ownedBy** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` | ✅ | Source group/position whose records are being shared | - - --- ## ShareRecipientType @@ -80,10 +61,10 @@ const result = CriteriaSharingRule.parse(data); ### Allowed Values * `user` -* `group` +* `team` * `position` * `unit_and_subordinates` -* `guest` +* `business_unit` --- @@ -101,14 +82,6 @@ const result = CriteriaSharingRule.parse(data); ## SharingRule -### Union Options - -This schema accepts one of the following structures: - -#### Option 1 - -**Type:** `criteria` - ### Properties | Property | Type | Required | Description | @@ -119,32 +92,10 @@ This schema accepts one of the following structures: | **object** | `string` | ✅ | Target Object Name | | **active** | `boolean` | optional | | | **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | | -| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` | ✅ | The recipient of the shared access | +| **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` | ✅ | The recipient of the shared access | | **type** | `'criteria'` | ✅ | | | **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | ✅ | Predicate (CEL). e.g. P`record.department == "Sales"` | ---- - -#### Option 2 - -**Type:** `owner` - -### Properties - -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **name** | `string` | ✅ | Unique rule name (snake_case) | -| **label** | `string` | optional | Human-readable label | -| **description** | `string` | optional | Administrative notes | -| **object** | `string` | ✅ | Target Object Name | -| **active** | `boolean` | optional | | -| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | | -| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` | ✅ | The recipient of the shared access | -| **type** | `'owner'` | ✅ | | -| **ownedBy** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` | ✅ | Source group/position whose records are being shared | - ---- - --- @@ -152,7 +103,6 @@ This schema accepts one of the following structures: ### Allowed Values -* `owner` * `criteria` diff --git a/examples/app-crm/src/security/sharing-rules.ts b/examples/app-crm/src/security/sharing-rules.ts index 96425cc92f..876ae54490 100644 --- a/examples/app-crm/src/security/sharing-rules.ts +++ b/examples/app-crm/src/security/sharing-rules.ts @@ -22,19 +22,21 @@ export const HighValueOpportunitySharingRule = defineSharingRule({ }); /** - * Owner-based sharing: leads owned by a Sales Rep are shared read-only - * with their manager so managers can coach on individual pipelines. + * Criteria-based sharing: in-flight leads (not yet converted or disqualified) + * are shared read-only with Sales Managers for coaching visibility. + * + * Replaces the retired owner-based `share_rep_leads_with_manager` rule: + * `type: 'owner'` (`ownedBy`) no longer parses — it depended on live position + * membership and was silently skipped at seed time (ADR-0078). The enforced + * criteria form scopes the same coaching set by pipeline state instead. */ export const RepLeadSharingRule = defineSharingRule({ - type: 'owner', - name: 'share_rep_leads_with_manager', - label: "Rep's Leads → Manager (read-only)", - description: "Share each rep's leads with the Sales Manager role for coaching visibility.", + type: 'criteria', + name: 'share_active_leads_with_manager', + label: 'Active Leads → Manager (read-only)', + description: 'Share in-flight (not converted/disqualified) leads with Sales Managers for coaching visibility.', object: 'crm_lead', - ownedBy: { - type: 'position', - value: 'sales_rep', - }, + condition: "record.status != 'converted' && record.status != 'disqualified'", accessLevel: 'read', sharedWith: { type: 'position', diff --git a/examples/app-crm/test/smoke.test.ts b/examples/app-crm/test/smoke.test.ts index ac35724410..1b2d1bed7a 100644 --- a/examples/app-crm/test/smoke.test.ts +++ b/examples/app-crm/test/smoke.test.ts @@ -88,11 +88,12 @@ describe('app-crm minimal metadata bundle', () => { expect(stack.i18n!.supportedLocales).toContain('zh-CN'); }); - it('has sharing rules (criteria + owner types)', () => { + it('has criteria sharing rules (the enforced form — owner-type was retired)', () => { const rules = stack.sharingRules ?? []; expect(rules.length).toBeGreaterThanOrEqual(2); - expect(rules.some((r) => r.type === 'criteria')).toBe(true); - expect(rules.some((r) => r.type === 'owner')).toBe(true); + // `type: 'owner'` no longer parses (never enforced; ADR-0078): every + // declared rule is the enforced criteria form. + expect(rules.every((r) => r.type === 'criteria')).toBe(true); }); // #3420 — official examples must boot warning-free. A generic (non-better-auth) diff --git a/examples/app-showcase/src/security/sharing-rules.ts b/examples/app-showcase/src/security/sharing-rules.ts index e6c14cf7b0..701e434c30 100644 --- a/examples/app-showcase/src/security/sharing-rules.ts +++ b/examples/app-showcase/src/security/sharing-rules.ts @@ -72,22 +72,23 @@ export const NewInquiryFieldOpsRule = defineSharingRule({ }); /** - * owner-based: a contributor's tasks are shared read-only with managers. + * criteria-based: open (not-done) tasks are shared read-only with managers + * for oversight. * - * [experimental — not enforced] Owner-type rules depend on live position - * membership and have no static `criteria_json` equivalent, so the seed - * bootstrap SKIPS them (logged) rather than seeding a permissive match-all - * (ADR-0049: nothing silently over-shares). Kept here to demonstrate the - * authoring shape; managers actually reach contributor tasks via the - * criteria rules above and their depth grants. + * This replaces the retired owner-based `share_contributor_tasks_with_manager` + * demonstration rule: `type: 'owner'` (`ownedBy`) no longer parses — it + * depended on live position membership, which the static materialiser cannot + * track, so it validated but was silently skipped at seed time (ADR-0078: + * nothing on the authoring surface may be silently inert). The enforced + * equivalent is a criteria rule scoping the rows managers need. */ export const ContributorTaskSharingRule = defineSharingRule({ - type: 'owner', - name: 'share_contributor_tasks_with_manager', - label: 'Contributor Tasks → Manager', - description: "Share each contributor's tasks with managers for oversight.", + type: 'criteria', + name: 'share_open_tasks_with_manager', + label: 'Open Tasks → Manager', + description: 'Share open (not-done) tasks with managers for oversight.', object: 'showcase_task', - ownedBy: { type: 'position', value: 'contributor' }, + condition: 'record.done == false', accessLevel: 'read', sharedWith: { type: 'position', value: 'manager' }, active: true, diff --git a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts index b7b1043450..9d40e4a561 100644 --- a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts +++ b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts @@ -4,19 +4,21 @@ * bootstrapDeclaredSharingRules — seed stack-declared `sharingRules` into * `sys_sharing_rule` (ADR-0057 D6, closes #2077; reconciles #1887). * - * The spec authoring shape (`SharingRuleSchema`: CEL `condition`, `ownedBy`, + * The spec authoring shape (`SharingRuleSchema`: CEL `condition`, * `sharedWith{type,value}`) diverges from the enforced runtime shape * (`criteria_json` JSON filter + `recipient_type`/`recipient_id`). ADR-0057 D6 - * makes the RUNTIME shape canonical and translates only the directly-mappable - * fields. Parts the runtime cannot enforce statically are SKIPPED (logged - * `[experimental]`) rather than seeded as a match-all rule — silently - * over-sharing would be worse than not enforcing (ADR-0049): - * - `owner`-type rules (`ownedBy`): role membership is dynamic, no static - * `criteria_json` equivalent. + * makes the RUNTIME shape canonical and translates the authorable fields. + * Every currently-authorable recipient (`user` / `team` / `position` / + * `unit_and_subordinates` / `business_unit`) maps 1:1 and ENFORCES — the + * retired `group`/`guest` recipients and `owner`-type rules no longer parse + * at the spec (ADR-0078; `group` was renamed → `team`). What the runtime + * still cannot enforce is SKIPPED (logged) rather than seeded as a match-all + * rule — silently over-sharing would be worse than not enforcing (ADR-0049): * - a CEL `condition` the canonical compiler cannot lower (functions, * cross-object traversal) — ADR-0058 D2. Compound predicates (AND/OR, - * comparisons, null, in) DO lower now and are enforced (ADR-0058 D3, #1887). - * - `sharedWith.type` of `group`/`guest`: no runtime recipient mapping. + * comparisons, null, in) DO lower and are enforced (ADR-0058 D3, #1887). + * - defensively, any stale pre-built package that still registers an old + * `owner`-type / unmapped-recipient shape. * * Seeding upserts via `SharingRuleService.defineRule` (idempotent by name) and * MUST run before `listRules()`/`bindRuleHooks` so the lifecycle hooks bind to @@ -35,11 +37,17 @@ type Logger = { info?: (m: string, meta?: any) => void; warn?: (m: string, meta? function mapRecipientType(t: unknown): SharingRuleRecipientType | null { switch (t) { case 'user': return 'user'; + // Flat sys_team membership (ADR-0090 D3 vocabulary; the pre-D3 `group` + // spelling was retired) — expanded by TeamGraphService in expandRecipient. + case 'team': return 'team'; case 'position': return 'position'; // ADR-0057 D5: business-unit subtree recipient. case 'business_unit': return 'business_unit' as SharingRuleRecipientType; case 'unit_and_subordinates': return 'unit_and_subordinates' as SharingRuleRecipientType; - default: return null; // group / guest — no runtime mapping yet + // Defensive only: the authoring enum matches the cases above 1:1, but a + // stale pre-built package could still register a retired shape — skip, + // never seed match-all. + default: return null; } } @@ -96,9 +104,11 @@ export async function bootstrapDeclaredSharingRules( logger?.warn?.('[sharing-rule] skipped (unmappable recipient) [experimental]', { rule: r.name, sharedWith: r.sharedWith?.type }); skipped += 1; continue; } - // owner-type rules have no static criteria_json equivalent. + // Defensive: `owner`-type rules were removed from the authoring spec + // (live-membership-dependent, no static criteria_json equivalent) — this + // guards stale pre-built packages that still register the old shape. if (r.type === 'owner') { - logger?.warn?.('[sharing-rule] skipped owner-based rule (no static criteria) [experimental]', { rule: r.name }); + logger?.warn?.('[sharing-rule] skipped owner-based rule (retired authoring shape — use a criteria rule)', { rule: r.name }); skipped += 1; continue; } // criteria rules: translate CEL → filter. Empty condition = match-all (intentional). diff --git a/packages/qa/dogfood/test/authz-conformance.matrix.ts b/packages/qa/dogfood/test/authz-conformance.matrix.ts index dbe33b9b3c..83c66dd615 100644 --- a/packages/qa/dogfood/test/authz-conformance.matrix.ts +++ b/packages/qa/dogfood/test/authz-conformance.matrix.ts @@ -141,8 +141,8 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ note: 'Proof file carries the forged owner_id/organization_id submit case; the route-level matrix is covered unit-side in rest public-form-routes.test.ts + plugin-security security-plugin.test.ts (publicFormGrant strip suite).' }, { id: 'record-share', summary: 'manual record shares (sys_record_share)', state: 'enforced', enforcement: 'plugin-sharing/sharing-service.ts buildReadFilter/canEdit' }, - { id: 'sharing-rules', summary: 'criteria/owner sharing rules', state: 'enforced', - enforcement: 'plugin-sharing/sharing-rule-service.ts (materialized into sys_record_share)', proof: 'showcase-bu-hierarchy-sharing.dogfood.test.ts' }, + { id: 'sharing-rules', summary: 'criteria sharing rules (recipients: user/team/position/unit_and_subordinates/business_unit)', state: 'enforced', + enforcement: 'plugin-sharing/sharing-rule-service.ts (materialized into sys_record_share); every authorable recipient expands in expandRecipient. The never-enforced owner-type rules + group/guest recipients were removed from the authoring spec (ADR-0078; group renamed → team)', proof: 'showcase-bu-hierarchy-sharing.dogfood.test.ts' }, { id: 'hierarchy-widening', summary: 'hierarchy widening — a unit + its subordinate units gain access', state: 'enforced', enforcement: 'plugin-sharing/business-unit-graph.ts BusinessUnitGraphService subtree (business_unit recipient) — ADR-0057 D5 re-homed off the never-existent sys_position.parent', proof: 'showcase-bu-hierarchy-sharing.dogfood.test.ts' }, { id: 'rls-compiler-fail-closed', summary: 'uncompilable RLS predicate is surfaced/denied, not dropped', state: 'enforced', diff --git a/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts b/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts index 53f62ba2f9..1c0705b867 100644 --- a/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts @@ -42,11 +42,21 @@ describe('showcase: declarative RBAC seeding (ADR-0057 D6 / #2077)', () => { expect(criteria).toEqual({ health: 'red' }); }); - it('owner-based rule is NOT seeded as a match-all (experimental, ADR-0049 honesty)', async () => { + it('retired owner-based rule is gone; its criteria replacement seeds and enforces', async () => { const rules = await ql.find('sys_sharing_rule', { where: {}, context: { isSystem: true } }); + // `type: 'owner'` was removed from the authoring spec (never enforced — + // ADR-0078): the old demonstration rule can no longer exist. const owner = (rules ?? []).find((r: any) => r.name === 'share_contributor_tasks_with_manager'); - // owner-type has no static criteria_json equivalent → skipped, not seeded over-broadly. - expect(owner, 'owner-based rule must not silently over-share').toBeFalsy(); + expect(owner, 'retired owner-based rule must not reappear').toBeFalsy(); + // Its replacement is a real criteria rule — seeded WITH translated + // criteria_json (not skipped, not match-all). + const open = (rules ?? []).find((r: any) => r.name === 'share_open_tasks_with_manager'); + expect(open, 'criteria replacement seeded').toBeTruthy(); + expect(open.object_name).toBe('showcase_task'); + expect(open.recipient_type).toBe('position'); + expect(open.recipient_id).toBe('manager'); + const criteria = JSON.parse(open.criteria_json); + expect(criteria).toEqual({ done: false }); }); it('re-seed is idempotent (no duplicate rows on a second boot)', async () => { diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 8ba9b71df9..737cda6098 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -3944,8 +3944,6 @@ "ObjectAccessScopeSchema (const)", "ObjectPermission (type)", "ObjectPermissionSchema (const)", - "OwnerSharingRule (type)", - "OwnerSharingRuleSchema (const)", "PLATFORM_CAPABILITIES (const)", "PLATFORM_CAPABILITY_NAMES (const)", "PUBLIC_FORM_SERVER_MANAGED_FIELDS (const)", diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index 42995ddac5..b943edf690 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -1194,7 +1194,6 @@ "security/OWDModel", "security/ObjectAccessScope", "security/ObjectPermission", - "security/OwnerSharingRule", "security/PermissionSet", "security/RLSEvaluationResult", "security/RLSOperation", diff --git a/packages/spec/src/security/explain.zod.ts b/packages/spec/src/security/explain.zod.ts index 707e218465..51f839edea 100644 --- a/packages/spec/src/security/explain.zod.ts +++ b/packages/spec/src/security/explain.zod.ts @@ -75,7 +75,7 @@ export const ExplainMatchedRuleSchema = lazySchema(() => z.object({ 'owd_baseline', // the object's OWD default admitted/excluded the row 'ownership', // the principal owns (or does not own) the record 'record_share', // a concrete share row targeting this record - 'sharing_rule', // an owner/criteria sharing rule (SharingRuleType) + 'sharing_rule', // a criteria sharing rule (SharingRuleType) 'team', // team / account-team membership grant 'territory', // territory assignment grant 'rls_policy', // a business RLS policy predicate diff --git a/packages/spec/src/security/sharing.test.ts b/packages/spec/src/security/sharing.test.ts index ac2fe15501..5be0a59593 100644 --- a/packages/spec/src/security/sharing.test.ts +++ b/packages/spec/src/security/sharing.test.ts @@ -2,21 +2,21 @@ import { describe, it, expect } from 'vitest'; import { SharingRuleSchema, SharingRuleType, + ShareRecipientType, SharingLevel, OWDModel, type SharingRule, } from './sharing.zod'; describe('SharingRuleType', () => { - it('should accept valid sharing rule types', () => { - const validTypes = ['owner', 'criteria']; - - validTypes.forEach(type => { - expect(() => SharingRuleType.parse(type)).not.toThrow(); - }); + it('should accept the enforced rule type', () => { + expect(() => SharingRuleType.parse('criteria')).not.toThrow(); }); - it('should reject invalid sharing rule types', () => { + it('should reject retired/invalid sharing rule types', () => { + // `owner` was retired from the authoring surface (live-membership-dependent, + // never enforced by the static materialiser — ADR-0078). + expect(() => SharingRuleType.parse('owner')).toThrow(); expect(() => SharingRuleType.parse('automatic')).toThrow(); expect(() => SharingRuleType.parse('manual')).toThrow(); expect(() => SharingRuleType.parse('guest')).toThrow(); @@ -25,6 +25,22 @@ describe('SharingRuleType', () => { }); }); +describe('ShareRecipientType', () => { + it('accepts exactly the enforced runtime recipients', () => { + const enforced = ['user', 'team', 'position', 'unit_and_subordinates', 'business_unit']; + enforced.forEach((t) => { + expect(() => ShareRecipientType.parse(t)).not.toThrow(); + }); + }); + + it('rejects the retired `group` (renamed → team) and `guest` recipients', () => { + expect(() => ShareRecipientType.parse('group')).toThrow(); + expect(() => ShareRecipientType.parse('guest')).toThrow(); + // Reserved in the runtime contract but not authorable until implemented. + expect(() => ShareRecipientType.parse('queue')).toThrow(); + }); +}); + describe('SharingLevel', () => { it('should accept valid sharing levels', () => { const validLevels = ['read', 'edit', 'full']; @@ -64,8 +80,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "stage = 'Open'", sharedWith: { - type: 'group', - value: 'group_sales_team', + type: 'team', + value: 'team_sales', }, }; @@ -79,8 +95,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).not.toThrow(); @@ -90,8 +106,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).toThrow(); @@ -101,8 +117,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).toThrow(); }); @@ -114,8 +130,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, }); @@ -134,8 +150,8 @@ describe('SharingRuleSchema', () => { condition: "stage = 'Closed Won' AND amount > 100000", accessLevel: 'edit', sharedWith: { - type: 'group', - value: 'group_executive_team', + type: 'team', + value: 'team_executive', }, }); @@ -143,90 +159,92 @@ describe('SharingRuleSchema', () => { expect((rule.condition as any).source).toContain('Closed Won'); }); - it('should accept different sharing rule types', () => { - // Criteria-based rule - const criteriaRule = SharingRuleSchema.parse({ - name: 'test_criteria_rule', - object: 'account', - type: 'criteria', - condition: "status = 'Active'", - sharedWith: { - type: 'group', - value: 'group_id', - }, - }); - expect(criteriaRule.type).toBe('criteria'); - - // Owner-based rule - const ownerRule = SharingRuleSchema.parse({ - name: 'test_owner_rule', - object: 'account', - type: 'owner', - ownedBy: { - type: 'position', - value: 'role_sales_rep', - }, - sharedWith: { - type: 'position', - value: 'role_sales_manager', - }, - }); - expect(ownerRule.type).toBe('owner'); - }); - - it('should accept different access levels', () => { - const levels: Array = ['read', 'edit', 'full']; + it('should accept every enforced recipient type', () => { + const recipients: Array = [ + { type: 'user', value: 'user_jane' }, + { type: 'team', value: 'team_sales' }, + { type: 'position', value: 'sales_manager' }, + { type: 'unit_and_subordinates', value: 'bu_field_ops' }, + { type: 'business_unit', value: 'bu_finance' }, + ]; - levels.forEach(level => { + recipients.forEach((sharedWith) => { const rule = SharingRuleSchema.parse({ - name: 'test_rule', + name: 'recipient_matrix_rule', object: 'account', type: 'criteria', condition: "status = 'Active'", - accessLevel: level, - sharedWith: { - type: 'group', - value: 'group_id', - }, + sharedWith, }); - expect(rule.accessLevel).toBe(level); + expect(rule.sharedWith.type).toBe(sharedWith.type); }); }); - it('should accept owner-based sharing rule', () => { - const rule = SharingRuleSchema.parse({ + it('should reject the retired owner-based rule shape', () => { + // Pre-ADR-0078 shape: validated but was silently skipped at seed time. + // It no longer parses — a criteria rule is the enforced alternative. + expect(() => SharingRuleSchema.parse({ name: 'owner_hierarchy_rule', object: 'account', type: 'owner', ownedBy: { type: 'position', - value: 'role_sales_rep', + value: 'sales_rep', }, accessLevel: 'read', sharedWith: { type: 'position', - value: 'role_sales_manager', + value: 'sales_manager', }, - }); - - expect(rule.type).toBe('owner'); + })).toThrow(); }); - it('should accept criteria-based sharing rule', () => { - const rule = SharingRuleSchema.parse({ - name: 'high_value_accounts', - object: 'account', + it('should reject the retired guest recipient', () => { + // Anonymous access is the public-form grant / share-link surface, not a + // sharing-rule recipient. + expect(() => SharingRuleSchema.parse({ + name: 'public_access', + object: 'knowledge_article', type: 'criteria', - condition: "annual_revenue > 1000000 AND status = 'Active'", + condition: "is_published = true", accessLevel: 'read', + sharedWith: { + type: 'guest', + value: 'guest_users', + }, + })).toThrow(); + }); + + it('should reject the retired group recipient (renamed → team)', () => { + expect(() => SharingRuleSchema.parse({ + name: 'sales_team_access', + object: 'opportunity', + type: 'criteria', + condition: "stage = 'Open'", sharedWith: { type: 'group', - value: 'group_executive_team', + value: 'team_sales', }, - }); + })).toThrow(); + }); - expect(rule.type).toBe('criteria'); - expect(rule.condition).toBeDefined(); + it('should accept different access levels', () => { + const levels: Array = ['read', 'edit', 'full']; + + levels.forEach(level => { + const rule = SharingRuleSchema.parse({ + name: 'test_rule', + object: 'account', + type: 'criteria', + condition: "status = 'Active'", + accessLevel: level, + sharedWith: { + type: 'team', + value: 'team_id', + }, + }); + expect(rule.accessLevel).toBe(level); + }); }); it('should accept criteria sharing rule for manual approval workflows', () => { @@ -245,22 +263,6 @@ describe('SharingRuleSchema', () => { expect(rule.type).toBe('criteria'); }); - it('should accept guest sharing rule', () => { - const rule = SharingRuleSchema.parse({ - name: 'public_access', - object: 'knowledge_article', - type: 'criteria', - condition: "is_published = true", - accessLevel: 'read', - sharedWith: { - type: 'guest', - value: 'guest_users', - }, - }); - - expect(rule.type).toBe('criteria'); - }); - it('should accept inactive sharing rule', () => { const rule = SharingRuleSchema.parse({ name: 'disabled_rule', @@ -269,8 +271,8 @@ describe('SharingRuleSchema', () => { condition: "status = 'Inactive'", active: false, sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, }); @@ -286,8 +288,8 @@ describe('SharingRuleSchema', () => { condition: "billing_state IN ('CA', 'OR', 'WA')", accessLevel: 'edit', sharedWith: { - type: 'group', - value: 'group_west_coast_sales', + type: 'team', + value: 'team_west_coast_sales', }, }); @@ -302,8 +304,8 @@ describe('SharingRuleSchema', () => { condition: "department = 'Finance'", accessLevel: 'edit', sharedWith: { - type: 'group', - value: 'group_finance_team', + type: 'business_unit', + value: 'bu_finance', }, }); @@ -318,8 +320,8 @@ describe('SharingRuleSchema', () => { condition: "status = 'Executed'", accessLevel: 'read', sharedWith: { - type: 'group', - value: 'group_all_users', + type: 'team', + value: 'team_all_users', }, }); @@ -334,8 +336,8 @@ describe('SharingRuleSchema', () => { condition: "stage != 'Closed Won'", accessLevel: 'edit', sharedWith: { - type: 'group', - value: 'group_sales_reps', + type: 'team', + value: 'team_sales_reps', }, }); @@ -348,8 +350,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).toThrow(); // Missing name @@ -358,8 +360,8 @@ describe('SharingRuleSchema', () => { type: 'criteria', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).toThrow(); // Missing object @@ -378,8 +380,8 @@ describe('SharingRuleSchema', () => { type: 'invalid_type', condition: "status = 'Active'", sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).toThrow(); }); @@ -392,8 +394,8 @@ describe('SharingRuleSchema', () => { condition: "status = 'Active'", accessLevel: 'delete', sharedWith: { - type: 'group', - value: 'group_id', + type: 'team', + value: 'team_id', }, })).toThrow(); }); diff --git a/packages/spec/src/security/sharing.zod.ts b/packages/spec/src/security/sharing.zod.ts index 63d1346d70..d4bed38d60 100644 --- a/packages/spec/src/security/sharing.zod.ts +++ b/packages/spec/src/security/sharing.zod.ts @@ -18,9 +18,17 @@ export const OWDModel = z.enum([ /** * Sharing Rule Type * How is the data shared? + * + * `criteria` is the single enforced rule form (field-value predicates, + * compiled to `criteria_json` and materialised as `sys_record_share`). + * `owner`-type rules ("share records owned by members of X") were REMOVED + * from the authoring surface: they depend on live team/position membership, + * which the static materialiser cannot track (a membership change would have + * to re-materialise every dependent rule). They return as an enforced form if + * membership-reactive materialisation is designed; until then a rule that + * validates but silently does nothing is an authoring trap (ADR-0078). */ export const SharingRuleType = z.enum([ - 'owner', // Based on record ownership (Role Hierarchy) 'criteria', // Based on field values (e.g. Status = 'Open') ]); @@ -35,15 +43,32 @@ export const SharingLevel = z.enum([ ]); /** - * Recipient Type + * Recipient Type * Who receives the access? + * + * Every member maps 1:1 onto an enforced runtime recipient expansion + * (`plugin-sharing` `expandRecipient`) — this enum is the authorable subset of + * the runtime `SharingRuleRecipientType` contract: + * - `user` — a single user id. + * - `team` — every member of a `sys_team` (flat collaboration grouping; + * ADR-0090 D3 renamed the pre-D3 `group` vocabulary to `team`). + * - `position` — every holder of a position (flat; ADR-0090 D3). + * - `unit_and_subordinates` — a business unit plus every descendant unit's + * members (ADR-0057 D5 subtree widening). + * - `business_unit` — exactly one business unit's members (no subtree). + * + * Removed (never enforced): `group` (renamed → `team`) and `guest` — anonymous + * access is served by the public-form grant and share links, not sharing rules; + * a guest recipient that silently no-ops is an authoring trap (ADR-0078). The + * runtime contract additionally reserves `queue` (no `sys_queue` yet) — it is + * deliberately NOT authorable until the implementation lands. */ export const ShareRecipientType = z.enum([ 'user', - 'group', + 'team', 'position', 'unit_and_subordinates', - 'guest' // for public sharing + 'business_unit', ]); /** @@ -66,7 +91,7 @@ const BaseSharingRuleSchema = z.object({ // Recipient (Whom to share with) sharedWith: z.object({ type: ShareRecipientType, - value: z.string().describe('ID or Code of the recipient (user / group / position / business unit)'), + value: z.string().describe('ID or code of the recipient (user / team / position / business unit)'), }).describe('The recipient of the shared access'), }); @@ -79,18 +104,6 @@ export const CriteriaSharingRuleSchema = lazySchema(() => BaseSharingRuleSchema. condition: ExpressionInputSchema.describe('Predicate (CEL). e.g. P`record.department == "Sales"`'), })); -/** - * 2. Owner-Based Sharing Rule - * Share records owned by a specific group of users. - */ -export const OwnerSharingRuleSchema = lazySchema(() => BaseSharingRuleSchema.extend({ - type: z.literal('owner'), - ownedBy: z.object({ - type: ShareRecipientType, - value: z.string(), - }).describe('Source group/position whose records are being shared'), -})); - /** * Master Sharing Rule Schema * @@ -98,25 +111,28 @@ export const OwnerSharingRuleSchema = lazySchema(() => BaseSharingRuleSchema.ext * COMPILED to the runtime `criteria_json` FilterCondition by the canonical * `@objectstack/formula` compiler at seed / `defineRule` time, and ENFORCED: * records matching the criteria materialise `sys_record_share` grants for the - * resolved recipients. Supported recipients: `user` / `team` / `business_unit` / - * `position` / `unit_and_subordinates` (ADR-0057 D5; renamed by ADR-0090 D3). + * resolved recipients. Supported recipients: `user` / `team` / `position` / + * `unit_and_subordinates` / `business_unit` (ADR-0057 D5; ADR-0090 D3) — every + * authorable recipient expands at runtime (`plugin-sharing` `expandRecipient`). + * + * The whole authorable surface is enforced — nothing here validates and then + * silently does nothing (ADR-0078). Removed to keep it that way: `owner`-type + * rules (`ownedBy` — live-membership-dependent, needs membership-reactive + * re-materialisation that is not designed yet) and the `group` / `guest` + * recipients (`group` renamed → `team`; anonymous access is the public-form + * grant / share-link surface). A `condition` the compiler cannot lower + * (functions, cross-object traversal) is skipped and logged — never seeded as + * a permissive match-all (ADR-0049). * - * Still `[experimental — not enforced]` (ADR-0049): `owner`-type rules - * (`ownedBy` — depends on live role membership, with no static `criteria_json` - * equivalent) and `group` / `guest` recipients (no runtime recipient mapping). - * A `condition` the compiler cannot lower (functions, cross-object traversal) is - * skipped and logged — never seeded as a permissive match-all. + * Kept as the `SharingRuleType`-discriminated form so a future enforced rule + * type (e.g. membership-reactive owner-based) re-joins as a union member. */ -export const SharingRuleSchema = lazySchema(() => z.discriminatedUnion('type', [ - CriteriaSharingRuleSchema, - OwnerSharingRuleSchema -])); +export const SharingRuleSchema = CriteriaSharingRuleSchema; export type SharingRule = z.infer; /** Authoring input for {@link SharingRule} — defaulted fields are optional. */ export type SharingRuleInput = z.input; export type CriteriaSharingRule = z.infer; -export type OwnerSharingRule = z.infer; /** * Type-safe factory for a record sharing rule. Validates at authoring time via