Skip to content

Commit f2e285c

Browse files
committed
fix(organizations): keep members removed from the instance organization out of the backfill
The instance-mode backfill added everyone in a personal organization to the instance organization, including people an admin had removed from it. Every departure from an organization is recorded in the audit log (org_member.removed naming the user, or member.removed with organizationRemoval when losing the last workspace took the membership), so candidate selection now excludes users with a recorded departure from the instance organization, as two anti-joins in the per-page query. External workspace removals are not departures.
1 parent ffaf52c commit f2e285c

3 files changed

Lines changed: 274 additions & 38 deletions

File tree

‎apps/sim/scripts/backfill-personal-organizations.integration.ts‎

Lines changed: 196 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,38 @@
33
* modes: a team member, or a personal-organization owner with nothing left to attach, selected
44
* (wasted or wrong writes); a personal-organization owner who still owns an organization-less
55
* workspace skipped (that workspace never moves); instance mode selecting a team member, or
6-
* skipping a personal-organization owner (stranded outside the instance organization); cursor
7-
* paging that skips or repeats a user; an out-of-range `--batch-size`.
6+
* skipping a personal-organization owner (stranded outside the instance organization); instance
7+
* mode re-adding someone removed from the instance organization, directly or with their last
8+
* workspace in it, or skipping someone who was only ever removed from another organization or
9+
* from the instance organization's workspaces as an outsider; cursor paging that skips or repeats
10+
* a user; an out-of-range `--batch-size`.
811
*/
912
import { db } from '@sim/db'
10-
import { member, organization, project, user, userStats, workspace } from '@sim/db/schema'
13+
import {
14+
auditLog,
15+
member,
16+
organization,
17+
permissions,
18+
project,
19+
user,
20+
userStats,
21+
workspace,
22+
} from '@sim/db/schema'
23+
import { insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
24+
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
1125
import { generateId } from '@sim/utils/id'
12-
import { inArray } from 'drizzle-orm'
13-
import { afterAll, describe, expect, it } from 'vitest'
26+
import { inArray, or, sql } from 'drizzle-orm'
27+
import { afterAll, describe, expect, it, vi } from 'vitest'
1428
import { createOrganizationWithOwner } from '@/lib/billing/organizations/create-organization'
29+
import { removeOrganizationMember } from '@/lib/organizations/application/members'
1530
import { createPersonalOrganization } from '@/lib/organizations/provision-personal'
31+
import { removeWorkspaceMember } from '@/lib/workspaces/application/remove-member'
1632
import { createWorkspaceInTransaction } from '@/lib/workspaces/create'
17-
import { parseBatchSize, selectCandidateBatch } from '@/scripts/backfill-personal-organizations'
33+
import {
34+
type InstanceOrganizationScope,
35+
parseBatchSize,
36+
selectCandidateBatch,
37+
} from '@/scripts/backfill-personal-organizations'
1838

1939
const prefix = `backfill-${generateId()}-`
2040
const userIds: string[] = []
@@ -54,6 +74,50 @@ async function ownOrganizationlessWorkspace(
5474
)
5575
}
5676

77+
async function createTeam(ownerId: string): Promise<string> {
78+
const { organizationId } = await createOrganizationWithOwner({
79+
actor: { type: 'system', name: 'Test' },
80+
ownerUserId: ownerId,
81+
name: 'Team',
82+
slug: `team-${generateId()}`.toLowerCase(),
83+
})
84+
return organizationId
85+
}
86+
87+
async function joinTeam(userId: string, organizationId: string): Promise<void> {
88+
await db.insert(member).values({ id: generateId(), organizationId, userId, role: 'member' })
89+
}
90+
91+
async function grantWorkspace(userId: string, workspaceId: string): Promise<void> {
92+
await db.insert(permissions).values({
93+
id: generateId(),
94+
userId,
95+
entityType: 'workspace',
96+
entityId: workspaceId,
97+
permissionType: 'write',
98+
})
99+
}
100+
101+
/** Removal records its audit entry after the commit, without waiting for it. */
102+
async function waitForRemovalAudit(userId: string): Promise<void> {
103+
await vi.waitFor(
104+
async () => {
105+
const [row] = await db
106+
.select({ id: auditLog.id })
107+
.from(auditLog)
108+
.where(
109+
or(
110+
sql`${auditLog.metadata}->>'targetUserId' = ${userId}`,
111+
sql`${auditLog.metadata}->>'removedUserId' = ${userId}`
112+
)
113+
)
114+
.limit(1)
115+
expect(row).toBeDefined()
116+
},
117+
{ timeout: 10_000, interval: 50 }
118+
)
119+
}
120+
57121
const seeded = await (async () => {
58122
const noOrganization = await createUser('a-no-org')
59123
await ownOrganizationlessWorkspace(noOrganization)
@@ -65,18 +129,99 @@ const seeded = await (async () => {
65129
const personalDone = await createUser('c-personal-done')
66130
await createPersonalOrganization(personalDone)
67131
const teamMember = await createUser('d-team')
68-
const { organizationId: teamId } = await createOrganizationWithOwner({
69-
actor: { type: 'system', name: 'Test' },
70-
ownerUserId: teamMember,
71-
name: 'Team',
72-
slug: `team-${generateId()}`.toLowerCase(),
73-
})
132+
const teamId = await createTeam(teamMember)
74133
await ownOrganizationlessWorkspace(teamMember, teamId)
75134
const secondNoOrganization = await createUser('e-no-org')
76135
return { noOrganization, personalWithWork, personalDone, teamMember, secondNoOrganization }
77136
})()
78137

138+
/**
139+
* An instance organization (any team serves: selection reads only its id) whose admin removed one
140+
* member directly and another by taking away their last workspace in it, beside accounts that were
141+
* never in it: one removed from another team, one removed as an outsider from an instance
142+
* workspace, and one untouched.
143+
*/
144+
const instanceSeeded = await (async () => {
145+
const owner = await createUser('f-instance-owner')
146+
const organizationId = await createTeam(owner)
147+
const workspaceId = generateId()
148+
await insertWorkspaceFixture(db, {
149+
id: workspaceId,
150+
name: 'Instance workspace',
151+
ownerId: owner,
152+
billedAccountUserId: owner,
153+
organizationId,
154+
})
155+
await db.insert(permissions).values({
156+
id: generateId(),
157+
userId: owner,
158+
entityType: 'workspace',
159+
entityId: workspaceId,
160+
permissionType: 'admin',
161+
})
162+
const ownerPrincipal = createSessionPrincipal({ userId: owner, sessionId: `session-${owner}` })
163+
164+
const removed = await createUser('g-removed')
165+
await joinTeam(removed, organizationId)
166+
await removeOrganizationMember.execute({
167+
principal: ownerPrincipal,
168+
input: { organizationId, userId: removed },
169+
})
170+
171+
const removedWithWorkspace = await createUser('h-removed-with-workspace')
172+
await joinTeam(removedWithWorkspace, organizationId)
173+
await grantWorkspace(removedWithWorkspace, workspaceId)
174+
await removeWorkspaceMember.execute({
175+
principal: ownerPrincipal,
176+
input: { workspaceId, userId: removedWithWorkspace },
177+
})
178+
179+
const removedOutsider = await createUser('i-removed-outsider')
180+
await createPersonalOrganization(removedOutsider)
181+
await grantWorkspace(removedOutsider, workspaceId)
182+
await removeOrganizationMember.execute({
183+
principal: ownerPrincipal,
184+
input: { organizationId, userId: removedOutsider },
185+
})
186+
187+
const otherTeamOwner = await createUser('j-other-team-owner')
188+
const otherTeamId = await createTeam(otherTeamOwner)
189+
const removedElsewhere = await createUser('k-removed-elsewhere')
190+
await joinTeam(removedElsewhere, otherTeamId)
191+
await removeOrganizationMember.execute({
192+
principal: createSessionPrincipal({
193+
userId: otherTeamOwner,
194+
sessionId: `session-${otherTeamOwner}`,
195+
}),
196+
input: { organizationId: otherTeamId, userId: removedElsewhere },
197+
})
198+
199+
const neverMember = await createUser('l-never-member')
200+
await createPersonalOrganization(neverMember)
201+
202+
for (const userId of [removed, removedWithWorkspace, removedOutsider, removedElsewhere]) {
203+
await waitForRemovalAudit(userId)
204+
}
205+
return {
206+
organizationId,
207+
removed,
208+
removedWithWorkspace,
209+
removedOutsider,
210+
removedElsewhere,
211+
neverMember,
212+
}
213+
})()
214+
79215
afterAll(async () => {
216+
await db
217+
.delete(auditLog)
218+
.where(
219+
or(
220+
inArray(sql`${auditLog.metadata}->>'targetUserId'`, userIds),
221+
inArray(sql`${auditLog.metadata}->>'removedUserId'`, userIds),
222+
inArray(auditLog.actorId, userIds)
223+
)
224+
)
80225
const workspaces = await db
81226
.select({ id: workspace.id, projectId: workspace.projectId })
82227
.from(workspace)
@@ -110,21 +255,26 @@ afterAll(async () => {
110255
await db.delete(user).where(inArray(user.id, userIds))
111256
})
112257

113-
/** Every candidate after the seeded prefix in one page, narrowed to the seeded users. */
114-
async function seededCandidates(instanceMode: boolean): Promise<string[]> {
115-
const all = await selectCandidateBatch(prefix, 100_000, instanceMode)
116-
return all.filter((id) => userIds.includes(id))
258+
/** Every candidate after the seeded prefix in one page, narrowed to the given seeded users. */
259+
async function seededCandidates(
260+
instance: InstanceOrganizationScope | null,
261+
among: string[] = userIds
262+
): Promise<string[]> {
263+
const all = await selectCandidateBatch(prefix, 100_000, instance)
264+
return all.filter((id) => among.includes(id))
117265
}
118266

267+
const baseSeededUsers = () => Object.values(seeded)
268+
119269
describe('backfill candidate selection', () => {
120270
it('selects users in no organization and personal owners with workspaces left to attach', async () => {
121-
expect(new Set(await seededCandidates(false))).toEqual(
271+
expect(new Set(await seededCandidates(null, baseSeededUsers()))).toEqual(
122272
new Set([seeded.noOrganization, seeded.personalWithWork, seeded.secondNoOrganization])
123273
)
124274
})
125275

126276
it('selects everyone outside a team on an instance-organization deployment', async () => {
127-
expect(new Set(await seededCandidates(true))).toEqual(
277+
expect(new Set(await seededCandidates({ organizationId: null }, baseSeededUsers()))).toEqual(
128278
new Set([
129279
seeded.noOrganization,
130280
seeded.personalWithWork,
@@ -134,22 +284,47 @@ describe('backfill candidate selection', () => {
134284
)
135285
})
136286

287+
it('keeps everyone removed from the instance organization out of it, and no one else', async () => {
288+
const instanceUsers = [
289+
instanceSeeded.removed,
290+
instanceSeeded.removedWithWorkspace,
291+
instanceSeeded.removedOutsider,
292+
instanceSeeded.removedElsewhere,
293+
instanceSeeded.neverMember,
294+
]
295+
expect(new Set(await seededCandidates({ organizationId: null }, instanceUsers))).toEqual(
296+
new Set(instanceUsers)
297+
)
298+
299+
expect(
300+
new Set(
301+
await seededCandidates({ organizationId: instanceSeeded.organizationId }, instanceUsers)
302+
)
303+
).toEqual(
304+
new Set([
305+
instanceSeeded.removedOutsider,
306+
instanceSeeded.removedElsewhere,
307+
instanceSeeded.neverMember,
308+
])
309+
)
310+
})
311+
137312
it('pages one user at a time through exactly the single-page result, in order', async () => {
138-
const reference = await selectCandidateBatch(prefix, 100_000, false)
313+
const reference = await selectCandidateBatch(prefix, 100_000, null)
139314
const lastSeeded = Math.max(...userIds.map((id) => reference.indexOf(id)))
140315
const expected = reference.slice(0, lastSeeded + 1)
141316

142317
const paged: string[] = []
143318
let cursor: string | null = prefix
144319
while (paged.length < expected.length) {
145-
const batch = await selectCandidateBatch(cursor, 1, false)
320+
const batch = await selectCandidateBatch(cursor, 1, null)
146321
if (batch.length === 0) break
147322
paged.push(...batch)
148323
cursor = batch[batch.length - 1]
149324
}
150325

151326
expect(paged).toEqual(expected)
152-
expect(await selectCandidateBatch(seeded.secondNoOrganization, 100_000, false)).not.toContain(
327+
expect(await selectCandidateBatch(seeded.secondNoOrganization, 100_000, null)).not.toContain(
153328
seeded.secondNoOrganization
154329
)
155330
})

‎apps/sim/scripts/backfill-personal-organizations.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
Every account outside a team gets a personal organization (`organization.kind = 'personal'`)
44
that holds its personal workspaces; on an instance-organization deployment it joins the instance
5-
organization instead. The release that introduces it must roll out in this order.
5+
organization instead, unless the audit log records that it left the instance organization (an
6+
admin, the Admin API or SCIM removed it, or it left). The release that introduces it must roll out in this order.
67

78
## Rollout
89

0 commit comments

Comments
 (0)