diff --git a/.changeset/23325-record-share-remedy-text.md b/.changeset/23325-record-share-remedy-text.md new file mode 100644 index 00000000000..15a823a5b9a --- /dev/null +++ b/.changeset/23325-record-share-remedy-text.md @@ -0,0 +1,19 @@ +--- +'@objectstack/plugin-sharing': patch +--- + +fix(plugin-sharing): a record-share grant refused for a non-user recipient names the sharing-rule recipient that delivers it, so a `group` caller is sent to `team` + +Clause-②: no + +**What was wrong.** `ISharingService.grant` writes a record share for a `user` only (ADR-0078), and refuses `group`, `position` and `unit_and_subordinates` with one sentence. FROM: "VALIDATION_FAILED: recipientType must be 'user' (received "group") — group/position recipients are delivered via sharing rules (ADR-0111 D7)". The sharing-rule vocabulary has no `group` recipient: a rule spells it `team`, since the ADR-0090 D3 rename. A caller or an AI that followed the remedy wrote a rule naming `group`, which neither the authoring `sharedWith.type` enum nor the `sys_sharing_rule.recipient_type` select offers. A `unit_and_subordinates` caller read a sentence that did not name its value. + +**What changed.** Each declared value's remedy names its own rule recipient. TO: + +- `group`: "… — a group recipient is delivered via a sharing rule with recipient type 'team', its name since the ADR-0090 D3 rename (ADR-0111 D7)" +- `position`: "… — a position recipient is delivered via a sharing rule with recipient type 'position' (ADR-0111 D7)" +- `unit_and_subordinates`: "… — a unit_and_subordinates recipient is delivered via a sharing rule with recipient type 'unit_and_subordinates' (ADR-0111 D7)" + +A value no record share declares (an untyped REST body sending `team`, say) is told so, and is named no route: "… — "team" is not a record-share recipient type (ADR-0111 D7)". + +**Unchanged.** Which values `grant()` accepts (`user` only), the `VALIDATION_FAILED` code and its 400, the first clause of every refusal, the retired `guest` value's own prescription, and the record-share vocabulary. Nothing is written for a refused grant. diff --git a/content/docs/kernel/runtime-services/sharing-service.mdx b/content/docs/kernel/runtime-services/sharing-service.mdx index ced5b91088c..67069ebf986 100644 --- a/content/docs/kernel/runtime-services/sharing-service.mdx +++ b/content/docs/kernel/runtime-services/sharing-service.mdx @@ -56,7 +56,7 @@ mask AND-ed with object CRUD, not a fourth `access_level`. ## Typical Errors - `FORBIDDEN` (403) — a write denied by the `canEdit` gate. Thrown by the sharing engine middleware; `canEdit` itself returns `false` rather than throwing. -- `VALIDATION_FAILED` (400) — `grant`/`revoke` called without a required field (`object`, `recordId`, `recipientId`, or `shareId`), or `grant` with a non-`user` `recipientType` (only `user` rows are enforced by the gates; group/position recipients are delivered via sharing rules). +- `VALIDATION_FAILED` (400) — `grant`/`revoke` called without a required field (`object`, `recordId`, `recipientId`, or `shareId`), or `grant` with a non-`user` `recipientType` (only `user` rows are enforced by the gates). The refusal names the sharing-rule recipient that delivers each declared value: `group` → `team` (a rule spells it `team` since the ADR-0090 D3 rename), `position` → `position`, `unit_and_subordinates` → `unit_and_subordinates`. - `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. diff --git a/packages/plugins/plugin-sharing/src/sharing-service.test.ts b/packages/plugins/plugin-sharing/src/sharing-service.test.ts index 1d148be4ad7..9b44a430272 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.test.ts @@ -5,6 +5,8 @@ import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { SharingService, type SharingServiceOptions } from './sharing-service.js'; import { buildSharingMiddleware } from './sharing-plugin.js'; import { bootRequestContext } from './exec-context-seam.testkit.js'; +import { SysRecordShare } from './objects/sys-record-share.object.js'; +import { SysSharingRule } from './objects/sys-sharing-rule.object.js'; // ───────────────────────────────────────────────────────────────────── // In-memory fake engine @@ -1111,8 +1113,8 @@ describe('[ADR-0111 D7] no inert grants', () => { // body) can still send it. The grant's one error channel is the message // prefix (`rest-server.ts`'s `respondSharingError` maps it to a 400); the // pin asserts that prefix, the prescription, and that nothing was written. - // A sharing rule cannot deliver a guest either, so the generic sentence - // ("delivered via sharing rules") would misdirect the caller. + // A sharing rule cannot deliver a guest either, so a remedy naming a rule + // route ("delivered via a sharing rule …") would misdirect the caller. const refusal = await svc.grant( { object: 'account', recordId: 'a1', recipientId: 'anyone', recipientType: 'guest' as never }, { userId: 'alice' }, @@ -1122,10 +1124,82 @@ describe('[ADR-0111 D7] no inert grants', () => { expect(message.startsWith("VALIDATION_FAILED: recipientType 'guest' was removed (ADR-0138 D8)")).toBe(true); expect(message).toMatch(/guest access flows through the guest position and share links \(ADR-0090 D11\)/); expect(message).toMatch(/Expose the record through a public form or a share link instead\.$/); - expect(message).not.toMatch(/sharing rules/); + expect(message).not.toMatch(/sharing rule/); expect(engine._tables.sys_record_share ?? []).toHaveLength(0); }); + describe('[ADR-0111 D7] the refusal names the sharing-rule recipient that delivers each non-user value', () => { + /** A select field's option values, in declared order. */ + const selectValues = (field: unknown): unknown[] => + ((field as { options?: unknown[] }).options ?? []) + .map((o) => (typeof o === 'object' && o !== null ? (o as { value: unknown }).value : o)); + + // The pin the guest case above sets for its own value, for every value the + // generic refusal answers. A rule spells `group` as `team` (the ADR-0090 D3 + // rename, recorded by the ADR-0087 entry `sharing-rule-recipient-reconcile`); + // the other two keep their own name. + const RULE_ROUTES = { + group: 'team', + position: 'position', + unit_and_subordinates: 'unit_and_subordinates', + } as const; + + const refusalFor = async (recipientType: string): Promise => { + const refusal = await svc.grant( + { object: 'account', recordId: 'a1', recipientId: 'g1', recipientType: recipientType as never }, + { userId: 'alice' }, + ).then(() => null, (e: unknown) => e); + expect(refusal, `${recipientType} was granted`).toBeInstanceOf(Error); + return (refusal as Error).message; + }; + + it('covers exactly the non-user values a record-share row declares, and routes each to a rule recipient', () => { + // Read from both selects, so neither vocabulary moves without this table. + expect(Object.keys(RULE_ROUTES)).toEqual( + selectValues(SysRecordShare.fields.recipient_type).filter((v) => v !== 'user'), + ); + const ruleRecipients = selectValues(SysSharingRule.fields.recipient_type); + for (const rule of Object.values(RULE_ROUTES)) expect(ruleRecipients).toContain(rule); + // The record-share spelling `group` is no rule recipient: naming it would + // send the caller to a route that does not exist. + expect(ruleRecipients).not.toContain('group'); + }); + + for (const [recipientType, rule] of Object.entries(RULE_ROUTES)) { + it(`${recipientType} → a sharing rule with recipient type '${rule}'`, async () => { + const message = await refusalFor(recipientType); + expect(message.startsWith( + `VALIDATION_FAILED: recipientType must be 'user' (received "${recipientType}") — ` + + `a ${recipientType} recipient is delivered via a sharing rule with recipient type '${rule}'`, + )).toBe(true); + expect(message).toMatch(/\(ADR-0111 D7\)$/); + expect(engine._tables.sys_record_share ?? []).toHaveLength(0); + }); + } + + it("group's remedy cites the rename and never names a `group` rule recipient", async () => { + const message = await refusalFor('group'); + expect(message).toMatch(/recipient type 'team', its name since the ADR-0090 D3 rename \(ADR-0111 D7\)$/); + expect(message).not.toMatch(/recipient type 'group'/); + // The two values whose rule spelling is their own carry no rename clause. + for (const unchanged of ['position', 'unit_and_subordinates']) { + expect(await refusalFor(unchanged)).not.toMatch(/rename/); + } + }); + + it('a value no record share declares is told so, and is named no route', async () => { + // `team` is a RULE recipient an untyped caller (a REST body) may send here. + // It is no record-share value, so no route is invented for it. + const message = await refusalFor('team'); + expect(message).toBe( + `VALIDATION_FAILED: recipientType must be 'user' (received "team") — ` + + '"team" is not a record-share recipient type (ADR-0111 D7)', + ); + expect(message).not.toMatch(/delivered via/); + expect(engine._tables.sys_record_share ?? []).toHaveLength(0); + }); + }); + it('refuses a grant on a public object (no gate would ever consult it)', async () => { await expect( svc.grant({ object: 'whiteboard', recordId: 'w1', recipientId: 'bob' }, { userId: 'alice' }), diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 6e08c38d10a..52704b19f94 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -7,6 +7,7 @@ import type { ISharingService, IHierarchyScopeResolver, RecordShare, + RecordShareRecipientType, GrantShareInput, ShareAccessLevel, SharingWriteVerdict, @@ -15,6 +16,7 @@ import { normalizeTenancyPosture, postureEnforcesWall, type ExplainDecision, + type ShareRecipientType, type TenancyPosture, } from '@objectstack/spec/security'; // [#7136] Every enforcement method below takes the FULL `resolveAuthzContext` @@ -101,6 +103,50 @@ function retiredRecipientTypeRefusal(value: unknown): string | undefined { : undefined; } +/** + * [ADR-0111 D7] The sharing-rule recipient that delivers each declared + * non-`user` record-share value: the route {@link SharingService.grant} names + * when it refuses one. Only a `user` row is ever written (ADR-0078), so a + * caller who meant a set of people is sent to the sharing rule that expands + * that set into per-user rows, spelled the way a rule spells it. + * + * Typed against both vocabularies, so neither can move away from it unnoticed. + * The keys are every non-`user` `RecordShareRecipientType` member: a member + * added there fails to compile here until it names its route. Each value is an + * authorable `ShareRecipientType` (`spec/security`), the list the + * `sys_sharing_rule.recipient_type` select is held equal to, so the refusal can + * never name a recipient a rule cannot be written with (the reserved `queue` + * included). A rule spells `group` as `team`: the ADR-0090 D3 rename, recorded + * by the ADR-0087 entry `sharing-rule-recipient-reconcile`. + */ +const RULE_ROUTE_BY_RECORD_SHARE_RECIPIENT: Readonly, + { readonly recipientType: ShareRecipientType; readonly renamedBy?: string } +>> = { + group: { recipientType: 'team', renamedBy: 'ADR-0090 D3' }, + position: { recipientType: 'position' }, + unit_and_subordinates: { recipientType: 'unit_and_subordinates' }, +}; + +/** + * The remedy `grant()` appends to its refusal of a non-`user` `recipientType` + * that is not retired: the sharing-rule recipient that delivers a declared + * value, or, for a value no record share declares, that it is not one. It + * never names a route for a value it does not know. + */ +function nonUserRecipientTypeRemedy(value: unknown): string { + const route = typeof value === 'string' + && Object.prototype.hasOwnProperty.call(RULE_ROUTE_BY_RECORD_SHARE_RECIPIENT, value) + ? RULE_ROUTE_BY_RECORD_SHARE_RECIPIENT[value as keyof typeof RULE_ROUTE_BY_RECORD_SHARE_RECIPIENT] + : undefined; + if (route === undefined) { + return `${JSON.stringify(value)} is not a record-share recipient type (ADR-0111 D7)`; + } + return `a ${value as string} recipient is delivered via a sharing rule with recipient type '${route.recipientType}'` + + (route.renamedBy !== undefined ? `, its name since the ${route.renamedBy} rename` : '') + + ' (ADR-0111 D7)'; +} + /** * Owner field convention. Hard-coded to `owner_id` for MVP — the * sharing model in Salesforce / ServiceNow / Dynamics all assume a @@ -1383,8 +1429,11 @@ export class SharingService implements ISharingService { const recipientType = input.recipientType ?? 'user'; // [ADR-0111 D7] Only `user` recipients are consulted by the read/write // gates; persisting any other type would be a silently inert grant - // (ADR-0078). Group / business-unit principals are delivered via sharing - // rules, whose evaluator expands them into per-user rows. + // (ADR-0078). Group / position / business-unit principals are delivered + // via sharing rules, whose evaluator expands them into per-user rows; the + // refusal names the rule recipient for each declared value + // (`RULE_ROUTE_BY_RECORD_SHARE_RECIPIENT`), because the rule vocabulary + // does not spell every one of them the same way (`group` is `team`). if (recipientType !== 'user') { // [ADR-0138 D8] A retired value no longer typechecks, yet still reaches // here untyped. It is told where guest access lives, never that a sharing @@ -1394,7 +1443,7 @@ export class SharingService implements ISharingService { retired !== undefined ? `VALIDATION_FAILED: ${retired}` : `VALIDATION_FAILED: recipientType must be 'user' (received ${JSON.stringify(recipientType)}) — ` - + `group/position recipients are delivered via sharing rules (ADR-0111 D7)`, + + nonUserRecipientTypeRemedy(recipientType), ); } // Validate BEFORE any write. Previously anything at all was persisted