Skip to content

Commit 8dc26b3

Browse files
committed
fix(plugin-sharing): re-evaluate position and team sharing rules for the user whose membership changed
A write to sys_user_position, to the sys_member.role transition source, or to sys_team_member now re-evaluates every sharing rule whose recipient the row names, for the one user the row names, in the same write path. A new holder gains the rule's grants and a former holder loses them before the write returns. WIP: tests and changeset follow. Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r Co-authored-by: Claude <noreply@anthropic.com>
1 parent 580fd4e commit 8dc26b3

6 files changed

Lines changed: 616 additions & 11 deletions

File tree

‎packages/plugins/plugin-sharing/src/bu-tree-recompute.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@
5151
* Everything else is deliberately NOT recomputed. That exclusion is a
5252
* requirement, not an optimisation: a fix that recomputed every rule on every
5353
* BU write would close the security hole and replace it with a load problem —
54-
* see {@link BU_TREE_RECIPIENT_TYPES}.
54+
* see {@link BU_TREE_RECIPIENT_TYPES}. The `position` and `team` kinds are
55+
* recomputed by writes to THEIR membership tables instead, per affected user —
56+
* `recipient-membership-recompute.ts`.
5557
*
5658
* ## The design: synchronous revoke, asynchronous re-grant
5759
*

‎packages/plugins/plugin-sharing/src/position-graph.ts‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,26 @@
33
import { isGrantActive } from '@objectstack/core';
44

55
import type { SharingEngine } from './sharing-service.js';
6-
import { TeamGraphService } from './team-graph.js';
6+
import { TeamGraphService, holdsRole } from './team-graph.js';
77

88
const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
99

10+
/**
11+
* The `sys_user_position` predicate "assignments of `positionName`", scoped to
12+
* `scopeOrg` when one is given — ONE spelling for the rule-wide expansion
13+
* ({@link PositionGraphService.expandPositionUsers}) and the user-scoped check
14+
* ({@link PositionGraphService.holdsPosition}), so the two can never disagree
15+
* about which assignment rows count.
16+
*/
17+
function positionAssignmentPredicate(positionName: string, scopeOrg: string | null | undefined): Record<string, unknown> {
18+
const filter: Record<string, unknown> = { position: positionName };
19+
if (scopeOrg) filter.organization_id = scopeOrg;
20+
return filter;
21+
}
22+
23+
/** The columns both reads project: the holder and the ADR-0091 D2 validity window. */
24+
const ASSIGNMENT_FIELDS = ['user_id', 'valid_from', 'valid_until'];
25+
1026
type PositionCache = {
1127
expand?: Map<string, string[]>;
1228
};
@@ -73,16 +89,14 @@ export class PositionGraphService {
7389
const users = new Set<string>();
7490

7591
// 1) Platform assignment table (source of truth).
76-
const filter: Record<string, unknown> = { position: positionName };
7792
const scopeOrg = organizationId ?? this.organizationId;
78-
if (scopeOrg) filter.organization_id = scopeOrg;
7993
try {
8094
// valid_from / valid_until ride the projection so the ADR-0091 D2
8195
// validity filter below sees them — expired holders stop receiving
8296
// position-recipient shares at resolution time, fail-closed.
8397
const rows = await this.engine.find('sys_user_position', {
84-
filter,
85-
fields: ['user_id', 'valid_from', 'valid_until'],
98+
filter: positionAssignmentPredicate(positionName, scopeOrg),
99+
fields: ASSIGNMENT_FIELDS,
86100
limit: 10000,
87101
context: SYSTEM_CTX,
88102
});
@@ -105,4 +119,42 @@ export class PositionGraphService {
105119
this.cache.expand!.set(key, result);
106120
return result;
107121
}
122+
123+
/**
124+
* Does `userId` hold `positionName`? The user-scoped twin of
125+
* {@link expandPositionUsers}: the same two sources, the same organization
126+
* scope and the same ADR-0091 D2 validity filter, each narrowed to one user,
127+
* so the answer is `expandPositionUsers(positionName, organizationId)
128+
* .includes(userId)` — pinned in `position-graph.test.ts` — read in two
129+
* indexed lookups instead of the position's whole holder list.
130+
*
131+
* Exists for the recipient-membership recompute
132+
* (`recipient-membership-recompute.ts`), which re-evaluates a position rule
133+
* for the ONE user whose assignment was written. That runs on the write path,
134+
* once per written row, so a full expansion there would make a bulk
135+
* assignment import quadratic in the position's holder count.
136+
*
137+
* Uncached on purpose: the answer is wanted right after a write that may
138+
* have changed it. A failed assignment read falls through to the transition
139+
* source, as the expansion's does.
140+
*/
141+
async holdsPosition(userId: string, positionName: string, organizationId?: string): Promise<boolean> {
142+
if (!userId || !positionName) return false;
143+
const scopeOrg = organizationId ?? this.organizationId;
144+
try {
145+
const rows = await this.engine.find('sys_user_position', {
146+
filter: { ...positionAssignmentPredicate(positionName, scopeOrg), user_id: userId },
147+
fields: ASSIGNMENT_FIELDS,
148+
limit: 1000,
149+
context: SYSTEM_CTX,
150+
});
151+
const nowMs = Date.now();
152+
for (const r of (rows ?? []) as any[]) {
153+
if (String(r.user_id ?? '') === userId && isGrantActive(r, nowMs)) return true;
154+
}
155+
} catch {
156+
/* table may not exist on minimal stacks — the transition source below still applies */
157+
}
158+
return holdsRole(this.engine, positionName, userId, scopeOrg);
159+
}
108160
}

0 commit comments

Comments
 (0)