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
10 changes: 10 additions & 0 deletions .changeset/29398-org-members-union-first-read.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ const PAGE_ROWS = 10000;

interface Read { object: string; where: Record<string, unknown>; limit?: number; orderBy?: unknown }

function makeQl(tables: Record<string, any[]>, opts: { ignoreCursor?: boolean; failAfter?: number } = {}) {
function makeQl(
tables: Record<string, any[]>,
opts: { ignoreCursor?: boolean; failAfter?: number; staleForOrderedReads?: ReadonlySet<string> } = {},
) {
const reads: Read[] = [];
const holds = (row: any, where: Record<string, unknown>) =>
Object.entries(where).every(([k, v]) => {
Expand All @@ -53,8 +56,11 @@ function makeQl(tables: Record<string, any[]>, 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();
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/security/resolve-authz-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>(
orgMembers
Expand Down
Loading