diff --git a/.changeset/30621-sharing-rule-object-not-enforced.md b/.changeset/30621-sharing-rule-object-not-enforced.md new file mode 100644 index 00000000000..14eb43dff88 --- /dev/null +++ b/.changeset/30621-sharing-rule-object-not-enforced.md @@ -0,0 +1,19 @@ +--- +'@objectstack/plugin-sharing': patch +--- + +fix(plugin-sharing)!: a sharing rule on an object without record sharing is refused at save, not saved to do nothing + +Clause-②: no (narrowing) +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the sharing-rule save doors (`@objectstack/plugin-sharing`): a rule whose object is not under record-sharing enforcement is refused with `422 SHARING_NOT_ENABLED` instead of saved. + +- **Before.** An administrator could save a sharing rule on an object no sharing gate consults, such as `sys_session`, `sys_account` or `sys_api_key` (a system object with no `owner_id`), a public object without an owner, a `controlled_by_parent` detail or a bypass-listed object. The save answered `201`, the rule showed as active in Setup, and its reconcile then refused every grant (`SHARING_NOT_ENABLED`) and wrote no `sys_record_share`, so the rule's recipients saw nothing more than before. Nothing told the administrator. +- **Now.** Both save doors refuse that rule and save nothing: the `/data` door (Setup's New and edit forms, scripts, imports) through a `sys_sharing_rule` hook guard, and `POST /api/v1/sharing/rules` (`SharingRuleService.defineRule`). The answer is `422 SHARING_NOT_ENABLED`, the pair the per-record share routes and `POST /api/v1/sharing/rules/:idOrName/evaluate` already answer for the same verdict. Its sentence names the object and says how to make it one a rule applies to, for example "give the object a private or read-only sharing model (`sharingModel: 'private'` or `'read'`) and an `owner_id` field", or "author the rule on its master object instead". An update is refused when it moves a rule onto such an object. + +The verdict is the reconcile's own: the save doors read the same ADR-0111 D7 inert-grant check (`SharingService.sharingRuleObjectRefusal`, over `inertGrantReason`) that the grant pre-flight throws on, so the two cannot disagree. A public object that has an owner column is still admitted, because the grant service there still grants an external member of the record's organization (#25549). System writes pass, as with the declared-name guard. + +To fix a rejected save, author the rule on an object that uses record sharing, or turn record sharing on for the object as the sentence says. A rule already stored on such an object is not migrated or deleted: it keeps sharing nothing, and an edit that leaves it on its object (for example switching it off) still saves. diff --git a/content/docs/kernel/runtime-services/sharing-service.mdx b/content/docs/kernel/runtime-services/sharing-service.mdx index 81379f3d0d6..fafa861cd86 100644 --- a/content/docs/kernel/runtime-services/sharing-service.mdx +++ b/content/docs/kernel/runtime-services/sharing-service.mdx @@ -60,7 +60,7 @@ mask AND-ed with object CRUD, not a fourth `access_level`. - `PERMISSION_DENIED` (403) — the caller does not hold `canManageShares` on the record (ADR-0111 D1). On the sharing-**rule** surface (`ISharingRuleService`, declared in the same canonical source — `listRules` / `getRule` / `defineRule` / `deleteRule` / `evaluateRule`) the same code carries a second condition: the caller holds `manage_sharing` but their session resolves **no active organization**, and an org-scoped capability with no organization has no tenant whose rules it authorizes. System contexts and platform operators (`manage_platform_settings`, or the `platform_admin` position) are unaffected — see [Rule administration](/docs/permissions/sharing-rules). - `NOT_FOUND` (404) — the record is missing **or not visible to the caller** (indistinguishable by design), or a `revoke` share id does not exist / does not belong to the `scope` record. - `CONFLICT` (409) — `revoke` on a rule-materialised share (`source != 'manual'`); the next rule reconciliation would silently re-grant it. Deactivate or edit the sharing rule instead. -- `SHARING_NOT_ENABLED` (422) — `grant` on an object the sharing gates never consult for the recipient (a public sharing model, unless the recipient is an `external` member of the record's organization, whose owner-only floor does consult it; no `owner_id` field; a bypass object, `controlled_by_parent`, or a **federated** object whose `owner_id` is the platform's injected anchor rather than a real remote column — the platform provisions no storage for a federated object, so the gates read that column off a table that has not got it and can never admit). +- `SHARING_NOT_ENABLED` (422) — `grant` on an object the sharing gates never consult for the recipient (a public sharing model, unless the recipient is an `external` member of the record's organization, whose owner-only floor does consult it; no `owner_id` field; a bypass object, `controlled_by_parent`, or a **federated** object whose `owner_id` is the platform's injected anchor rather than a real remote column — the platform provisions no storage for a federated object, so the gates read that column off a table that has not got it and can never admit). On the sharing-**rule** surface, `defineRule` (and a `sys_sharing_rule` write through the `/data` door) answers the same code when the rule's object is one no gate consults for any recipient: the rule is refused at save instead of saved to share nothing. ## Enforcement is automatic — do not re-check it in a hook diff --git a/content/docs/permissions/sharing-rules.mdx b/content/docs/permissions/sharing-rules.mdx index 6e36df3125a..084f8fe11e5 100644 --- a/content/docs/permissions/sharing-rules.mdx +++ b/content/docs/permissions/sharing-rules.mdx @@ -137,12 +137,22 @@ the rows it owns and the rows granted to it: the object (for example `supplier in current_user.`). Such a policy lifts the owner-only floor for that object, and the policy bounds the rows. -On a `public_read_write` object, or a system object with no `sharingModel`, a -share or sharing rule takes effect for external recipients only: the grant is -accepted for a recipient who is an `external` member of the record's +On a `public_read_write` object, or a system object with no `sharingModel`, +whose `owner_id` is real, a share or sharing rule takes effect for external +recipients only: the grant is accepted for a recipient who is an `external` member of the record's organization, every other recipient is refused with `SHARING_NOT_ENABLED` (422), and a rule's evaluation writes rows for its external recipients alone. +A sharing rule on an object that no sharing gate consults for any recipient is +refused when it is saved: an object with no real `owner_id` (a system object +without one, such as `sys_session`, or a federated object whose `owner_id` is +the platform's injected anchor), a `controlled_by_parent` detail, or a bypass +object. It is refused with `SHARING_NOT_ENABLED` (422) and a sentence that +names the object and how to turn record sharing on for it. This holds for Setup's form and the +`/data` door as well as `POST /api/v1/sharing/rules`, and for an edit that moves +a rule onto such an object. A rule saved before this check keeps its row and +can still be switched off. + `externalSharingModel`, the external dial ADR-0090 D11 declared, was removed in `@objectstack/spec` 18 (ADR-0049 enforce-or-remove): while it existed no request resolved an external principal, so the value changed no answer. The schema diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 15eb4f3aac9..1d1fac67cc7 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -170,7 +170,7 @@ One of the largest consumers of the flag; | 37 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `buildSharingMiddleware` | | 38 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `createLink`, `revokeLink`, `listLinks` | | 39 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `bindRuleProvenanceStamp` | -| 40 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface and the declared-name guard on `sys_sharing_rule` writes (a new row or a rename taking a declared rule's name, 409) — `defineRule` makes that check itself before its own system write, and the other system writers are the platform's machinery (the ownership ceremony, migrations) | `assertCanManageRules`, `bindDeclaredRuleNameGuard` | +| 40 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface and the declared-name guard on `sys_sharing_rule` writes (a new row or a rename taking a declared rule's name, 409) — `defineRule` makes that check itself before its own system write, and the other system writers are the platform's machinery (the ownership ceremony, migrations); and the object-enforcement guard on the same writes (a rule whose object is not under record-sharing enforcement, 422 `SHARING_NOT_ENABLED`), which `defineRule` likewise makes for its non-system callers | `assertCanManageRules`, `bindDeclaredRuleNameGuard`, `bindRuleObjectEnforcementGuard`, `defineRule` | ### 4. Approvals, attachments, comments, knowledge @@ -601,6 +601,7 @@ and CI refuses one that differs from what the tree produces. | `packages/plugins/plugin-security/src/suggested-audience-bindings.ts#assertDeploymentAuthority` | 1 | 58 | | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite` | 1 | 25 | | `packages/plugins/plugin-sharing/src/rule-hooks.ts#bindRuleHooks` | 1 | 31 | +| `packages/plugins/plugin-sharing/src/rule-hooks.ts#bindRuleObjectEnforcementGuard` | 1 | 40 | | `packages/plugins/plugin-sharing/src/share-link-service.ts#createLink` | 3 | 38 | | `packages/plugins/plugin-sharing/src/share-link-service.ts#listLinks` | 1 | 38 | | `packages/plugins/plugin-sharing/src/share-link-service.ts#revokeLink` | 1 | 38 | @@ -608,6 +609,7 @@ and CI refuses one that differs from what the tree produces. | `packages/plugins/plugin-sharing/src/sharing-rule-provenance.ts#bindRuleProvenanceStamp` | 1 | 39 | | `packages/plugins/plugin-sharing/src/sharing-rule-service.ts#assertCanManageRules` | 1 | 40 | | `packages/plugins/plugin-sharing/src/sharing-rule-service.ts#bindDeclaredRuleNameGuard` | 1 | 40 | +| `packages/plugins/plugin-sharing/src/sharing-rule-service.ts#defineRule` | 1 | 40 | | `packages/plugins/plugin-sharing/src/sharing-service.ts#assertCanManageShares` | 1 | 33 | | `packages/plugins/plugin-sharing/src/sharing-service.ts#bypassVerdict` | 1 | 32 | | `packages/plugins/plugin-sharing/src/sharing-service.ts#canManageShares` | 1 | 33 | diff --git a/packages/plugins/plugin-sharing/src/rule-criteria.ts b/packages/plugins/plugin-sharing/src/rule-criteria.ts index f83ab1b755f..f5eec7ab531 100644 --- a/packages/plugins/plugin-sharing/src/rule-criteria.ts +++ b/packages/plugins/plugin-sharing/src/rule-criteria.ts @@ -115,3 +115,25 @@ export class SharingCriteriaValidationError extends Error { this.fields = [{ field, code: 'required', message }]; } } + +/** + * [#30621] A sharing rule refused at save because its object is not under + * record-sharing enforcement: the rule could never share anything. + * + * `code` + `status` are the declared envelope the `/data` door and the + * sharing-rule routes answer at its own status: `SHARING_NOT_ENABLED` (422), + * the pair the per-record share routes and `POST .../evaluate` already answer + * for the same verdict. The message is `SharingService.sharingRuleObjectRefusal`'s + * sentence, which names the object and how to turn record sharing on for it. + */ +export class SharingRuleObjectNotEnforcedError extends Error { + readonly code = 'SHARING_NOT_ENABLED'; + readonly status = 422; + /** The object the rule names. */ + readonly object: string; + constructor(object: string, message: string) { + super(message); + this.name = 'SharingRuleObjectNotEnforcedError'; + this.object = object; + } +} diff --git a/packages/plugins/plugin-sharing/src/rule-hooks.ts b/packages/plugins/plugin-sharing/src/rule-hooks.ts index 23baa61dae0..9c3dd4553e5 100644 --- a/packages/plugins/plugin-sharing/src/rule-hooks.ts +++ b/packages/plugins/plugin-sharing/src/rule-hooks.ts @@ -2,7 +2,8 @@ import type { SharingRuleService } from './sharing-rule-service.js'; import type { SharingRuleRow } from '@objectstack/spec/contracts'; -import { isMatchAllCriteria, SharingCriteriaValidationError } from './rule-criteria.js'; +import { isMatchAllCriteria, SharingCriteriaValidationError, SharingRuleObjectNotEnforcedError } from './rule-criteria.js'; +import type { SharingService } from './sharing-service.js'; import { RULE_RECOMPUTE_ROW_CAP, RuleRegrantQueue, @@ -29,6 +30,9 @@ export const RULE_REBIND_TRIGGER_PACKAGE = 'plugin-sharing:rule-rebind'; */ export const RULE_CRITERIA_GUARD_PACKAGE = 'plugin-sharing:rule-criteria-guard'; +/** Package id of {@link bindRuleObjectEnforcementGuard}'s hooks. */ +export const RULE_OBJECT_ENFORCEMENT_GUARD_PACKAGE = 'plugin-sharing:rule-object-enforcement-guard'; + /* * RETIRED — the `isSystem` materialisation skip, and with it the INFO line that * announced it (`SYSTEM_WRITE_SKIP_NOTICE`, #6783 / #4707 demand 3). @@ -355,3 +359,45 @@ export function bindRuleCriteriaGuard(engine: MinimalEngine, logger?: MinimalLog engine.registerHook('beforeUpdate', guard(false), opts); logger?.info?.('[sharing-rule] criteria guard bound on sys_sharing_rule (ADR-0049)'); } + +/** + * [#30621] Refuse a `sys_sharing_rule` write whose object is not under + * record-sharing enforcement. + * + * Before, such a rule saved (`201`) and its reconcile then met the ADR-0111 D7 + * inert-grant guard and wrote no `sys_record_share`: the rule sat in Setup + * looking active and never applied. The verdict here is the reconcile's own + * ({@link SharingService.sharingRuleObjectRefusal}), so the two cannot disagree. + * + * An insert is checked. An update is checked only when it MOVES the rule onto + * another object (`object_name` supplied and different from the pre-image), + * so an administrator can still switch off a rule saved before this guard. + * System writes pass, as with the declared-name guard: `defineRule` makes the + * same check itself for its non-system callers, and the remaining system + * writers are the platform's own machinery. + */ +export function bindRuleObjectEnforcementGuard( + engine: MinimalEngine, + sharing: Pick, +): void { + if (typeof engine.registerHook !== 'function') return; + if (typeof engine.unregisterHooksByPackage === 'function') { + engine.unregisterHooksByPackage(RULE_OBJECT_ENFORCEMENT_GUARD_PACKAGE); + } + const guard = (insert: boolean) => (ctx: any) => { + if ((ctx?.session as any)?.isSystem) return; + const data = ctx?.input?.data; + if (!data || typeof data !== 'object' || Array.isArray(data)) return; + const object = data.object_name; + if (typeof object !== 'string' || object.length === 0) return; + if (!insert) { + const previous = ctx?.previous as Record | undefined; + if (previous && typeof previous === 'object' && previous.object_name === object) return; + } + const refusal = sharing.sharingRuleObjectRefusal(object); + if (refusal) throw new SharingRuleObjectNotEnforcedError(object, refusal); + }; + const opts = { object: 'sys_sharing_rule', packageId: RULE_OBJECT_ENFORCEMENT_GUARD_PACKAGE, priority: 100 }; + engine.registerHook('beforeInsert', guard(true), opts); + engine.registerHook('beforeUpdate', guard(false), opts); +} diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index f7f7ffc24d5..1ee65c2b3b4 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -73,7 +73,13 @@ import { import { SharingRuleService, bindDeclaredRuleNameGuard, bindSharingRuleActivationReconcile } from './sharing-rule-service.js'; import { ShareLinkService } from './share-link-service.js'; import { registerShareLinkRoutes } from './share-link-routes.js'; -import { bindRuleHooks, unbindAllRuleHooks, bindRuleCriteriaGuard, RULE_REBIND_TRIGGER_PACKAGE } from './rule-hooks.js'; +import { + bindRuleHooks, + unbindAllRuleHooks, + bindRuleCriteriaGuard, + bindRuleObjectEnforcementGuard, + RULE_REBIND_TRIGGER_PACKAGE, +} from './rule-hooks.js'; import { bindRuleProvenanceStamp, unbindRuleProvenanceStamp } from './sharing-rule-provenance.js'; import { bindPrimaryBuHooks, backfillPrimaryBu } from './primary-bu-projection.js'; import { bindBusinessUnitTreeRecompute } from './bu-tree-recompute.js'; @@ -981,6 +987,10 @@ export class SharingServicePlugin implements Plugin { // match-all criteria gate has to sit on the table itself too. bindRuleCriteriaGuard(engine, ctx.logger as any); + // [#30621] …and so must the refusal of a rule whose object is not + // under record-sharing enforcement: it would save and share nothing. + if (this.service) bindRuleObjectEnforcementGuard(engine, this.service); + // [ADR-0131 D6 regime C] …and so must the refusal of a row, new or // renamed, that takes a declared rule's name: `defineRule` makes it // for its own callers. A declared rule is customised by a clone diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-object-enforcement.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule-object-enforcement.test.ts new file mode 100644 index 00000000000..b6acca223bb --- /dev/null +++ b/packages/plugins/plugin-sharing/src/sharing-rule-object-enforcement.test.ts @@ -0,0 +1,226 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30621] A sharing rule is refused at save when its object is not under + * record-sharing enforcement. + * + * Before: an administrator could save a `sys_sharing_rule` on an object no + * sharing gate consults (measured: `sys_session`, `sys_account`, + * `sys_api_key`). The save answered `201`; the reconcile then met the ADR-0111 + * D7 inert-grant guard (`SHARING_NOT_ENABLED`) and wrote no `sys_record_share`, + * so the rule sat in Setup looking active and never applied. + * + * Now: both save doors refuse it with `SHARING_NOT_ENABLED` (422), the code + * the per-record share routes and `POST .../evaluate` already answer for the + * same verdict, and nothing is saved: + * + * - the `/data` door (Setup's New and edit forms) through the + * `sys_sharing_rule` hook guard; + * - `SharingRuleService.defineRule` (`POST /api/v1/sharing/rules`). + * + * The verdict is the reconcile's own (`SharingService.sharingRuleObjectRefusal` + * reads the same `inertGrantReason` the grant pre-flight throws on), so the + * save door and the reconcile cannot disagree about which objects are + * enforced. A public object WITH an owner column stays admitted: there the + * grant service still admits an external member of the record's organization + * (#25549), so a rule on it is not inert as a whole. + * + * Real `ObjectQL` + `SqlDriver` (better-sqlite3 `:memory:`), so the hook runs + * on the engine's own write path and `ctx.previous` is the engine's pre-image. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; + +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; +import { bindRuleObjectEnforcementGuard } from './rule-hooks.js'; + +const ORG = 'org_30621'; +const ENFORCED = 'os30621_deal'; +const OWNERLESS = 'os30621_session'; +const PUBLIC_OWNED = 'os30621_notice'; +const PUBLIC_FEDERATED = 'os30621_ext_customer'; + +const SYSTEM: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; +const ADMIN = { + userId: 'usr_admin', + organizationId: ORG, + tenantId: ORG, + positions: [], + permissions: [], + systemPermissions: ['manage_sharing'], +} as unknown as ExecutionContext; + +const text = (name: string) => ({ type: 'text', name, label: name }); + +const RULE_FIELDS = Object.fromEntries( + ['organization_id', 'name', 'label', 'description', 'object_name', 'criteria_json', 'recipient_type', + 'recipient_id', 'access_level', 'managed_by', 'created_at', 'updated_at'].map((f) => [f, text(f)]), +); + +const open: SqlDriver[] = []; +afterEach(async () => { + while (open.length) await open.pop()!.disconnect().catch(() => undefined); +}); + +async function boot() { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + open.push(driver); + const ql = new ObjectQL(); + ql.registerDriver(driver as never, true); + await ql.init(); + ql.setTenancyPostureProvider(() => 'single'); + const recordFields = { + id: { ...text('id'), primary: true }, + stage: text('stage'), + organization_id: text('organization_id'), + }; + ql.registerObject({ + name: ENFORCED, label: 'Deal', sharingModel: 'private', + fields: { ...recordFields, owner_id: text('owner_id') }, + } as never); + // The `sys_session` shape as the registry holds it: no `owner_id`, no + // declared model (`ownership: 'none'`, so the registry injects no owner). + ql.registerObject({ name: OWNERLESS, label: 'Session', ownership: 'none', fields: recordFields } as never); + ql.registerObject({ + name: PUBLIC_OWNED, label: 'Notice', sharingModel: 'public_read_write', + fields: { ...recordFields, owner_id: text('owner_id') }, + } as never); + // A PUBLIC federated object (ADR-0015): the registry injects `owner_id` as an + // anchor the remote table does not have. Registered only, never written to. + ql.registerObject({ + name: PUBLIC_FEDERATED, label: 'External customer', sharingModel: 'public_read_write', + external: { remoteName: 'customers' }, fields: { id: { ...text('id'), primary: true }, name: text('name') }, + } as never); + ql.registerObject({ + name: 'sys_sharing_rule', label: 'Sharing Rule', isSystem: true, + fields: { + id: { ...text('id'), primary: true }, + ...RULE_FIELDS, + active: { type: 'boolean', name: 'active', label: 'Active' }, + customized: { type: 'boolean', name: 'customized', label: 'Customized' }, + }, + } as never); + await driver.initObjects([ENFORCED, OWNERLESS, PUBLIC_OWNED, 'sys_sharing_rule'] + .map((name) => ({ name, fields: (ql as any).getSchema(name).fields }) as never)); + const sharing = new SharingService({ engine: ql as never }); + const rules = new SharingRuleService({ engine: ql as never, sharing }); + bindRuleObjectEnforcementGuard(ql as never, sharing); + return { ql, rules, sharing }; +} + +const rowFor = (object: string, name: string) => ({ + id: `srule_${name}`, + organization_id: ORG, + name, + label: name, + object_name: object, + criteria_json: '{"stage":"won"}', + recipient_type: 'position', + recipient_id: 'sales_rep', + access_level: 'read', + active: true, + managed_by: 'admin', +}); + +async function storedRules(ql: ObjectQL) { + return ((await ql.find('sys_sharing_rule', { context: SYSTEM } as never)) ?? []) as Array>; +} + +describe('[#30621] the /data door refuses a sharing rule on an object without record sharing', () => { + it('an insert on an owner-less object (the sys_session shape) is refused and nothing is saved', async () => { + const { ql } = await boot(); + const err = await ql.insert('sys_sharing_rule', rowFor(OWNERLESS, 'session_rule'), { context: ADMIN } as never) + .catch((x: unknown) => x as any); + expect(err?.code).toBe('SHARING_NOT_ENABLED'); + expect(err?.status).toBe(422); + // The sentence names the object and how to turn record sharing on for it. + expect(String(err?.message)).toContain(`'${OWNERLESS}'`); + expect(String(err?.message)).toMatch(/sharingModel/); + expect(String(err?.message)).toMatch(/owner_id/); + expect(await storedRules(ql)).toHaveLength(0); + }); + + it('CONTROL: an insert on an enforced object saves as before', async () => { + const { ql } = await boot(); + await ql.insert('sys_sharing_rule', rowFor(ENFORCED, 'deal_rule'), { context: ADMIN } as never); + expect((await storedRules(ql)).map((r) => r.name)).toEqual(['deal_rule']); + }); + + it('CONTROL: a public object WITH an owner column is admitted (the reconcile still grants its external members)', async () => { + const { ql } = await boot(); + await ql.insert('sys_sharing_rule', rowFor(PUBLIC_OWNED, 'notice_rule'), { context: ADMIN } as never); + expect((await storedRules(ql)).map((r) => r.name)).toEqual(['notice_rule']); + }); + + it('an update that moves a rule onto a non-enforced object is refused and the row keeps its object', async () => { + const { ql } = await boot(); + await ql.insert('sys_sharing_rule', rowFor(ENFORCED, 'deal_rule'), { context: ADMIN } as never); + const err = await ql.update('sys_sharing_rule', { id: 'srule_deal_rule', object_name: OWNERLESS }, { context: ADMIN } as never) + .catch((x: unknown) => x as any); + expect(err?.code).toBe('SHARING_NOT_ENABLED'); + expect(err?.status).toBe(422); + expect((await storedRules(ql))[0]?.object_name).toBe(ENFORCED); + }); + + it('an update that leaves a legacy rule on its object passes, so it can still be switched off', async () => { + const { ql } = await boot(); + // A row saved before this guard existed: written under the system context. + await ql.insert('sys_sharing_rule', rowFor(OWNERLESS, 'legacy_rule'), { context: SYSTEM } as never); + await ql.update('sys_sharing_rule', { id: 'srule_legacy_rule', object_name: OWNERLESS, active: false }, { context: ADMIN } as never); + const [row] = await storedRules(ql); + expect(row?.active === false || row?.active === 0).toBe(true); + }); +}); + +describe('[#30621] the refusal sentence names the remedy for the arm that refused', () => { + it('a PUBLIC federated object with the injected owner anchor is told to declare a real remote owner_id', async () => { + const { ql, sharing } = await boot(); + expect((ql as any).getSchema(PUBLIC_FEDERATED)?.fields?.owner_id, 'the registry injected the anchor').toBeTruthy(); + const refusal = sharing.sharingRuleObjectRefusal(PUBLIC_FEDERATED); + expect(refusal).toContain(`'${PUBLIC_FEDERATED}'`); + expect(refusal).toMatch(/declare a real remote 'owner_id' column/); + // Not the generic remedy: the object already has an owner_id, and a + // private model would still meet the phantom-anchor refusal. + expect(refusal).not.toMatch(/sharingModel: 'private'/); + const err = await ql.insert('sys_sharing_rule', rowFor(PUBLIC_FEDERATED, 'ext_rule'), { context: ADMIN } as never) + .catch((x: unknown) => x as any); + expect(err?.code).toBe('SHARING_NOT_ENABLED'); + expect(err?.status).toBe(422); + expect(await storedRules(ql)).toHaveLength(0); + }); +}); + +describe('[#30621] defineRule (POST /api/v1/sharing/rules) refuses the same rule', () => { + const input = (object: string, name: string) => ({ + name, label: name, object, recipientType: 'position', recipientId: 'sales_rep', criteria: { stage: 'won' }, + }); + + it('a rule on an owner-less object is refused with SHARING_NOT_ENABLED 422 and nothing is saved', async () => { + const { ql, rules } = await boot(); + const err = await rules.defineRule(input(OWNERLESS, 'session_rule') as never, ADMIN).catch((x: unknown) => x as any); + expect(err?.code).toBe('SHARING_NOT_ENABLED'); + expect(err?.status).toBe(422); + expect(String(err?.message)).toContain(`'${OWNERLESS}'`); + expect(await storedRules(ql)).toHaveLength(0); + }); + + it('CONTROL: a rule on an enforced object is saved', async () => { + const { ql, rules } = await boot(); + const row = await rules.defineRule(input(ENFORCED, 'deal_rule') as never, ADMIN); + expect(row.object_name).toBe(ENFORCED); + expect((await storedRules(ql)).map((r) => r.name)).toEqual(['deal_rule']); + }); + + it('an upsert that moves an existing rule onto a non-enforced object is refused', async () => { + const { ql, rules } = await boot(); + await rules.defineRule(input(ENFORCED, 'deal_rule') as never, ADMIN); + const err = await rules.defineRule(input(OWNERLESS, 'deal_rule') as never, ADMIN).catch((x: unknown) => x as any); + expect(err?.code).toBe('SHARING_NOT_ENABLED'); + expect(err?.status).toBe(422); + expect((await storedRules(ql))[0]?.object_name).toBe(ENFORCED); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index 3e9f1cb56ae..e2df976916d 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -42,7 +42,12 @@ import type { SharingEngine } from './sharing-service.js'; import type { SharingService } from './sharing-service.js'; import { isInternalRecipientRefusal } from './sharing-service.js'; import { normalizeAccessLevel, normalizeStoredAccessLevel } from './access-level.js'; -import { parseCriteria, isMatchAllCriteria, MATCH_ALL_CRITERIA_MESSAGE } from './rule-criteria.js'; +import { + parseCriteria, + isMatchAllCriteria, + MATCH_ALL_CRITERIA_MESSAGE, + SharingRuleObjectNotEnforcedError, +} from './rule-criteria.js'; import { TeamGraphService, teamHasMember } from './team-graph.js'; import { PositionGraphService } from './position-graph.js'; import { BusinessUnitGraphService } from './business-unit-graph.js'; @@ -833,6 +838,20 @@ export class SharingRuleService implements ISharingRuleService { // `active: false` on an over-sharing rule must survive redeploys. const seedMode = input.managedBy === 'package' || input.managedBy === 'platform'; + // [#30621] A rule on an object no sharing gate consults would save and then + // share nothing: refuse it here, with the reconcile's own verdict. The + // `sys_sharing_rule` hook guard makes the same check on the `/data` door; + // this write runs under the system context, which that guard lets pass. + // An upsert that keeps a stored rule on its object is not a move, so a + // rule saved before this check can still be switched off. + if (!context?.isSystem) { + const stored: any = Array.isArray(existing) ? existing[0] : undefined; + if (!stored || stored.object_name !== input.object) { + const refusal = this.sharing.sharingRuleObjectRefusal(input.object); + if (refusal) throw new SharingRuleObjectNotEnforcedError(input.object, refusal); + } + } + if (Array.isArray(existing) && existing[0]) { const row: any = existing[0]; if (seedMode) { diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 558afc3b321..28e392bc439 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -1766,11 +1766,55 @@ export class SharingService implements ISharingService { * input to this question). */ private assertNotInertGrant(object: string): void { + const reason = this.objectInertReason(object); + if (reason) throw new Error(`SHARING_NOT_ENABLED: ${reason}`); + } + + /** + * The caller-independent D7 verdict on `object`: why no share row on it + * could ever be consulted, whoever the recipient is, or `null`. + */ + private objectInertReason(object: string): string | null { // [#25549] The arm the recipient's population decides is the recipient // half's ({@link assertRecipientNotInert}), asked once the organization is known. - if (this.recipientDecidesInertness(object)) return; - const reason = this.inertGrantReason(object); - if (reason) throw new Error(`SHARING_NOT_ENABLED: ${reason}`); + if (this.recipientDecidesInertness(object)) return null; + return this.inertGrantReason(object); + } + + /** + * [#30621] The save door's refusal for a sharing rule on `object`, or `null` + * when a rule there can grant. + * + * A rule on an object no sharing gate consults saved and then did nothing: + * its reconcile met {@link assertNotInertGrant} and wrote no row. This reads + * the SAME verdict ({@link objectInertReason}), so the save door and the + * reconcile cannot disagree about which objects are under enforcement. The + * sentence adds what the administrator can do about it. + */ + sharingRuleObjectRefusal(object: string): string | null { + const reason = this.objectInertReason(object); + if (!reason) return null; + return `A sharing rule on '${object}' would never share anything: ${reason}. ${this.sharingRuleObjectRemedy(object)}`; + } + + /** [#30621] How to make `object` one a sharing rule applies to, per D7 arm. */ + private sharingRuleObjectRemedy(object: string): string { + if (this.bypassObjects.has(object)) { + return 'Record sharing cannot be turned on for this platform object; author the rule on an object that uses record sharing.'; + } + const schema = typeof this.engine.getSchema === 'function' ? this.engine.getSchema(object) : undefined; + const declared = schema?.sharingModel ?? schema?.security?.sharingModel; + if (declared === 'controlled_by_parent') { + return 'Author the rule on its master object instead.'; + } + // Asked before the model: a federated object whose `owner_id` is the + // injected anchor already HAS that field, public or not, and a private + // model would still meet the phantom-anchor refusal. + if (schema && hasOwnerField(schema) && hasPhantomOwnerAnchor(schema)) { + return `To use sharing rules on it, declare a real remote '${OWNER_FIELD}' column on the federated object.`; + } + return `To use sharing rules on it, give the object a private or read-only sharing model ` + + `(sharingModel: 'private' or 'read') and an '${OWNER_FIELD}' field.`; } /**