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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .changeset/bu-tenant-screen-relanding.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
'@objectstack/plugin-sharing': patch
---

Fix: a sharing rule with a business-unit recipient granted nothing when the unit came from seed data — and tenant-screen the member reads that widening exposes.
Tenant-screen the `sys_business_unit_member` reads, and warn when an active business-unit rule grants nobody.

`BusinessUnitGraphService.orgScope` screened `sys_business_unit` with a strict `organization_id` equality, while the platform's own read-side chokepoint (`SqlDriver.applyTenantScope`) is null-inclusive: `(organization_id = ? OR organization_id IS NULL)`, because a NULL organization marks a platform/seeded row every tenant may see. A sharing rule always carries the caller's organization, but a business unit written by seed data carries none — a seed cannot know the id the runtime mints at boot — so the two never matched. The seed check read the unit as "does not exist", both recipient widths (`business_unit` and `unit_and_subordinates`) expanded to zero users, and the rule stayed active having materialised no `sys_record_share` row and logged nothing. `orgScope` now applies the platform's null-inclusive screen, the same predicate `plugin-approvals` already applies to these very rows and `SharingRuleService.adminOrgScope` applies to the rule table.
⚠️ This entry originally also announced that `BusinessUnitGraphService.orgScope` had been widened to the platform's null-inclusive `(organization_id = ? OR organization_id IS NULL)` screen, fixing the case where a sharing rule naming a **seed-written** business unit granted nobody. That half was **reverted before 17.3 was cut** and does not ship: it re-implemented, a second time and in a second place, the predicate `SqlDriver.applyTenantScope` already owns — the duplication the v18 org-ownership decision (PR #14976) exists to retire — and it had not been released. `orgScope` keeps the strict `organization_id` equality 17.2.0 ships, so a rule naming a seeded unit still expands to nobody. That defect is fixed structurally on the v18 line by the v18 org-ownership decision (PR #14976), C1 (the Default Organization exists before application seed datasets load, and the seed loader stamps `sys_business_unit` seeds). What follows is the part of this change that DOES ship.

The member reads are now tenant-screened, which they were not before. Both `expandUnitMembers` and `expandUsers` queried `sys_business_unit_member` with no organization predicate at all, under a system context that carries no tenant either, so the strict unit screen was the only thing keeping an org-stamped rule away from that unscoped query. Widening the unit screen alone would have turned a silent under-grant into a silent cross-tenant over-grant, since a seeded unit id exists identically in every tenant. The member screen is strict rather than null-inclusive on purpose: seed replay and elevated system writes both leave `sys_business_unit_member.organization_id` NULL, so a NULL there means unknown tenancy rather than platform-global, and an org-scoped rule does not grant to it. The sibling recipient widths already read their membership rows this way.
The member reads are now tenant-screened, which they were not before. Both `expandUnitMembers` and `expandUsers` queried `sys_business_unit_member` with no organization predicate at all, under a system context that carries no tenant either, so the strict unit screen was the only thing keeping an org-stamped rule away from that unscoped query. This matters independently of the unit screen: other organizations' member rows sit on org-stamped — visible — units too, so a strict unit screen narrows which units are reachable but does not close the unscoped member read. The member screen is strict rather than null-inclusive on purpose: seed replay and elevated system writes both leave `sys_business_unit_member.organization_id` NULL, so a NULL there means unknown tenancy rather than platform-global, and an org-scoped rule does not grant to it. The sibling recipient widths already read their membership rows this way.

An active business-unit rule that expands to no recipients now warns once per rule per process, naming the rule, the object, the recipient kind, the unit and the organization. That case — a rule whose unit and membership rows were both seeded — is the one combination that still grants nobody, and it is no longer silent.
An active business-unit rule that expands to no recipients now warns once per rule per process, naming the rule, the object, the recipient kind, the unit and the organization. That is what keeps the remaining seed-data symptom observable at the moment it happens, rather than surfacing only as "the right people cannot see the record".
30 changes: 30 additions & 0 deletions .changeset/revert-null-inclusive-business-unit-screen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
"@objectstack/plugin-sharing": patch
---

revert(plugin-sharing): drop the NULL-inclusive business-unit screen added after 17.2.0

17.3 does not ship the NULL-inclusive business-unit screen added after 17.2.0;
#14547 remains as in 17.2.0 and is fixed structurally in v18 (the v18 org-ownership decision (PR #14976), C1: the
Default Organization exists before application seed datasets load, and the seed
loader stamps `sys_business_unit` seeds).

`BusinessUnitGraphService.orgScope` briefly read
`$or: [{ organization_id: <rule org> }, { organization_id: null }]` so that an
org-stamped sharing rule could name a seeded (org-less) `sys_business_unit`
row. It is restored to the strict `organization_id = <rule org>` equality
17.2.0 ships. That shape re-implemented, a second time and in a second place,
the predicate `SqlDriver.applyTenantScope` already owns — the duplication
the v18 org-ownership decision (PR #14976) exists to retire (#10103 cause 1) — and it had not been released, so
reverting costs nothing while shipping it would have owed v18 a breaking change
and a migration.

The other half of the same change is KEPT and is not touched:
`BusinessUnitGraphService.memberScope` still screens both
`sys_business_unit_member` reads with a strict equality. Those reads previously
carried no organization predicate at all, so an org-stamped rule reaching any
visible unit collected every tenant's membership rows hanging off it; a strict
unit screen narrows which units are reachable but does not close that, because
other organizations' member rows sit on org-stamped units too.
`SharingRuleService.warnOnEmptyUnitExpansion` is also kept: it is what keeps the
remaining #14547 symptom loud instead of silent.
Loading
Loading