diff --git a/.changeset/29398-org-members-union-first-read.md b/.changeset/29398-org-members-union-first-read.md new file mode 100644 index 00000000000..fa6f556ed58 --- /dev/null +++ b/.changeset/29398-org-members-union-first-read.md @@ -0,0 +1,10 @@ +--- +'@objectstack/core': patch +--- + +fix(core): the fellow-org peer list keeps every member the first membership read returned, even when the completion read sees fewer + +Clause-②: no +Metadata-protocol: none + +In an organization of 1,000 members or more, the resolver reads the membership twice: a first read bounded at 1,000 rows, then a completion read in id order. The completion's rows used to replace the first read's, so a completion that saw fewer members than the first read (an older snapshot on another connection) left `org_user_ids` shorter than the first read alone would have, and those members vanished from ordinary members' `sys_user` reads with no signal. The two reads are now unioned. Both are filtered to the active organization, so the union cannot add anyone outside it. No read is added or changed. diff --git a/packages/core/src/security/resolve-authz-context.org-members-past-first-read.test.ts b/packages/core/src/security/resolve-authz-context.org-members-past-first-read.test.ts index f04f6cb78ea..2799595d0f0 100644 --- a/packages/core/src/security/resolve-authz-context.org-members-past-first-read.test.ts +++ b/packages/core/src/security/resolve-authz-context.org-members-past-first-read.test.ts @@ -35,7 +35,10 @@ const PAGE_ROWS = 10000; interface Read { object: string; where: Record; limit?: number; orderBy?: unknown } -function makeQl(tables: Record, opts: { ignoreCursor?: boolean; failAfter?: number } = {}) { +function makeQl( + tables: Record, + opts: { ignoreCursor?: boolean; failAfter?: number; staleForOrderedReads?: ReadonlySet } = {}, +) { const reads: Read[] = []; const holds = (row: any, where: Record) => Object.entries(where).every(([k, v]) => { @@ -53,8 +56,11 @@ function makeQl(tables: Record, opts: { ignoreCursor?: boolean; f if (opts.failAfter !== undefined && object === 'sys_member' && reads.filter((r) => r.object === 'sys_member').length > opts.failAfter) { throw new Error('connection reset'); } - let rows = (tables[object] ?? []).filter((r) => holds(r, q?.where ?? {})); const order = Array.isArray(q?.orderBy) ? q.orderBy[0] : undefined; + // An ordered read (the completion's) answering from an older snapshot: the + // members named here had not joined yet as far as it can see. + const stale = order && object === 'sys_member' ? opts.staleForOrderedReads : undefined; + let rows = (tables[object] ?? []).filter((r) => holds(r, q?.where ?? {}) && !stale?.has(r.user_id)); if (order) { rows = [...rows].sort((a, b) => (a[order.field] < b[order.field] ? -1 : a[order.field] > b[order.field] ? 1 : 0)); if (order.order === 'desc') rows.reverse(); @@ -163,6 +169,23 @@ describe('[#29281] org_user_ids holds the whole active organization', () => { expect(lines[0]).toContain('org_user_ids'); }); + it('a completion that sees fewer members than the first read still yields every member the first read returned', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const tables = tablesOf(FIRST_READ_BOUND + 5); + // Five members the unordered first read returns (it answers in insertion + // order) that the completion's older snapshot does not hold. + const unseen = new Set(['u000001', 'u000002', 'u000003', 'u000004', 'u000005']); + const ql = makeQl(tables, { staleForOrderedReads: unseen }); + const grants = await resolveUserAuthzGrants(ql, ME, { tenantId: ORG }); + + // The completion did run, and did come back short of them. + expect(orgReads(ql.reads)).toHaveLength(2); + for (const u of unseen) expect(grants.org_user_ids).toContain(u); + expect([...grants.org_user_ids].sort()).toEqual(membersOf(tables)); + expect(grants.org_user_ids).not.toContain('u_elsewhere_1'); + expect(grants.org_user_ids).not.toContain('u_elsewhere_2'); + }); + it('a page that fails to read fails the resolution loudly — never a shorter list', async () => { vi.spyOn(console, 'warn').mockImplementation(() => undefined); const tables = tablesOf(FIRST_READ_BOUND + 5); diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index 544406f0ddc..c3eddd26215 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -1922,9 +1922,16 @@ export async function resolveUserAuthzGrants( // membership only when it came back under its bound; at the bound it is a // first page and the membership is read to completion here. Under the // bound nothing more is read. + // + // The completion's rows are UNIONED with Leg 1's, never put in their + // place: a completion that comes back with fewer members than Leg 1 + // already held (a snapshot older than Leg 1's on another connection) would + // otherwise shorten the list below what the first read alone answered. + // The union cannot add a non-member — both reads are filtered to the + // active organization — so the list is never shorter than Leg 1's. if (tenantId) { const orgMembers = orgMembersLeg.length >= ORG_MEMBERS_FIRST_READ_ROWS - ? await readOrganizationMembersToCompletion(ql, tenantId) + ? [...orgMembersLeg, ...await readOrganizationMembersToCompletion(ql, tenantId)] : orgMembersLeg; // (read in Leg 1 above) const ids = new Set( orgMembers