Skip to content

Commit 8c52d6b

Browse files
mzxchandramzxchandra
authored
fix(lifecycle): retain shared resources when creators leave (#8762)
* fix(lifecycle): retain shared resources when creators leave * fix(lifecycle): preserve replacement workspace grants * fix(lifecycle): preserve upload history during creator handoff * fix(lifecycle): preserve empty upload attribution through handoff * fix(lifecycle): order workspace locks before file mutations * fix(admin): use renamed member not-found response * fix(lifecycle): acquire organization fences before workspace handoff * fix(lifecycle): lock resource workspaces before account erasure barrier * fix(lifecycle): refuse erasure while creator references are in flight * fix(lifecycle): lock Project workspaces before successor grants * fix(lifecycle): make shared file contexts readonly
1 parent 03450b2 commit 8c52d6b

20 files changed

Lines changed: 3487 additions & 231 deletions

File tree

‎apps/sim/app/api/v1/admin/workspaces/[id]/members/[memberId]/route.ts‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,9 @@ import {
3333
} from '@/lib/api/contracts/v1/admin'
3434
import { parseRequest } from '@/lib/api/server'
3535
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
36-
import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access'
37-
import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access'
36+
import { revokeWorkspaceAccessTx } from '@/lib/workspaces/access/workspace-access'
3837
import { getWorkspaceById } from '@/lib/workspaces/permissions/utils'
39-
import {
40-
reassignWorkflowOwnershipForWorkspaceMemberRemovalTx,
41-
transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx,
42-
WorkspaceBillingAccountRemovalError,
43-
} from '@/lib/workspaces/utils'
38+
import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils'
4439
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
4540
import {
4641
adminBadRequestResponse,
@@ -278,28 +273,19 @@ export const DELETE = withRouteHandler(
278273
)
279274
}
280275

281-
await db.transaction(async (tx) => {
282-
await transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx({
283-
tx,
276+
const removed = await db.transaction(async (tx) => {
277+
const result = await revokeWorkspaceAccessTx(tx, {
284278
workspaceId,
285-
departingUserId: existingMember.userId,
279+
userId: existingMember.userId,
280+
expectedPermissionId: existingMember.id,
286281
})
287-
288-
const workflowOwnershipReassignment =
289-
await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({
290-
tx,
291-
workspaceIds: [workspaceId],
292-
departingUserId: existingMember.userId,
293-
})
294-
if (workflowOwnershipReassignment.unresolved.length > 0) {
282+
if (!result.revoked) {
283+
if (result.reason === 'membership-changed') return false
295284
throw new WorkspaceBillingAccountRemovalError()
296285
}
297-
298-
await tx.delete(permissions).where(eq(permissions.id, memberId))
299-
300-
await revokeWorkspaceCredentialMembershipsTx(tx, workspaceId, existingMember.userId)
301-
await removeWorkspaceSkillMembershipsTx(tx, workspaceId, existingMember.userId)
286+
return true
302287
})
288+
if (!removed) return adminNotFoundResponse('Workspace member')
303289

304290
logger.info(`Admin API: Removed member ${memberId} from workspace ${workspaceId}`, {
305291
userId: existingMember.userId,

‎apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts‎

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,10 @@ import {
4949
} from '@/lib/api/contracts/v1/admin'
5050
import { parseRequest } from '@/lib/api/server'
5151
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
52-
import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access'
5352
import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment'
54-
import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access'
53+
import { revokeWorkspaceAccessTx } from '@/lib/workspaces/access/workspace-access'
5554
import { getWorkspaceById } from '@/lib/workspaces/permissions/utils'
56-
import {
57-
reassignWorkflowOwnershipForWorkspaceMemberRemovalTx,
58-
transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx,
59-
WorkspaceBillingAccountRemovalError,
60-
} from '@/lib/workspaces/utils'
55+
import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils'
6156
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
6257
import {
6358
adminBadRequestResponse,
@@ -395,26 +390,8 @@ export const DELETE = withRouteHandler(
395390
}
396391

397392
await db.transaction(async (tx) => {
398-
await transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx({
399-
tx,
400-
workspaceId,
401-
departingUserId: userId,
402-
})
403-
404-
const workflowOwnershipReassignment =
405-
await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({
406-
tx,
407-
workspaceIds: [workspaceId],
408-
departingUserId: userId,
409-
})
410-
if (workflowOwnershipReassignment.unresolved.length > 0) {
411-
throw new WorkspaceBillingAccountRemovalError()
412-
}
413-
414-
await tx.delete(permissions).where(eq(permissions.id, existingPermission.id))
415-
416-
await revokeWorkspaceCredentialMembershipsTx(tx, workspaceId, userId)
417-
await removeWorkspaceSkillMembershipsTx(tx, workspaceId, userId)
393+
const result = await revokeWorkspaceAccessTx(tx, { workspaceId, userId: userId })
394+
if (!result.revoked) throw new WorkspaceBillingAccountRemovalError()
418395
})
419396

420397
logger.info(`Admin API: Removed user ${userId} from workspace ${workspaceId}`)

‎apps/sim/background/cleanup-soft-deletes.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { and, asc, eq, inArray, isNotNull, isNull, lt, sql } from 'drizzle-orm'
2121
import { type CleanupJobPayload, runCleanupWithLimits } from '@/lib/billing/cleanup-dispatcher'
2222
import {
2323
decrementStorageUsageForBillingContextInTx,
24+
lockWorkspaceStorageForMutationInTx,
2425
resolveStorageBillingContext,
2526
type StorageBillingContext,
2627
} from '@/lib/billing/storage'
@@ -339,6 +340,7 @@ async function deleteExpiredBillableWorkspaceFileRows(
339340
for (const batch of chunkArray(workspaceRows, DEFAULT_DELETE_CHUNK_SIZE)) {
340341
try {
341342
const deletedCount = await db.transaction(async (tx) => {
343+
await lockWorkspaceStorageForMutationInTx(tx, workspaceId)
342344
const fileIds = batch.map(({ id }) => id)
343345
await tx
344346
.delete(workflowTest)

‎apps/sim/lib/billing/organizations/membership-external-removal.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { credential, knowledgeBase, member, workspaceFiles } from '@sim/db/schema'
1+
import { credential, knowledgeBase, member, user, workspaceFiles } from '@sim/db/schema'
22
import { dbChainMockFns, hasMockCondition, queueTableRows, resetDbChainMock } from '@sim/testing'
33
import {
44
organizationMemberLimitsMock,
@@ -39,13 +39,18 @@ describe('external organization access removal', () => {
3939
it('preserves organization indexes and cached documents when a creator leaves with no workspaces', async () => {
4040
queueTableRows(member, [{ id: 'membership', userId: 'departing', role: 'admin' }])
4141
queueTableRows(member, [{ userId: 'surviving-owner' }])
42+
queueTableRows(user, [{ email: 'departing@example.test' }])
43+
queueTableRows(user, [{ email: 'departing@example.test' }])
44+
queueTableRows(user, [{ id: 'surviving-owner' }])
45+
queueTableRows(workspaceFiles, [{ id: 'cached-file', context: 'knowledge-base' }])
4246
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'membership' }])
4347

4448
const result = await removeUserFromOrganization({
4549
userId: 'departing',
4650
organizationId: 'organization-a',
4751
memberId: 'membership',
4852
skipBillingLogic: true,
53+
onError: 'throw',
4954
})
5055

5156
expect(result.success).toBe(true)

‎apps/sim/lib/billing/organizations/membership.ts‎

Lines changed: 14 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
account,
1111
credential,
1212
invitation,
13-
knowledgeBase,
1413
member,
1514
organization,
1615
permissionGroupMember,
@@ -19,7 +18,6 @@ import {
1918
user,
2019
userStats,
2120
workspace,
22-
workspaceFiles,
2321
} from '@sim/db/schema'
2422
import { createLogger } from '@sim/logger'
2523
import { getErrorMessage } from '@sim/utils/errors'
@@ -39,6 +37,7 @@ import {
3937
import { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/billing-identity-lock'
4038
import { setOrgMemberUsageLimit } from '@/lib/billing/organizations/member-limits'
4139
import { MEMBER_BILLING_RECONCILIATION_EVENT_TYPE } from '@/lib/billing/organizations/membership-reconciliation-event'
40+
import { reassignOrganizationSharedResourcesTx } from '@/lib/billing/organizations/resource-handoff'
4241
import { isPaid, sqlIsPro } from '@/lib/billing/plan-helpers'
4342
import { changeOrganizationWorkspaceBilledAccountsInTx } from '@/lib/billing/storage/payer-transfer'
4443
import {
@@ -67,10 +66,8 @@ import {
6766
} from '@/lib/organizations/members/revocation'
6867
import { reassignOrganizationProjects } from '@/lib/projects/membership'
6968
import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access'
70-
import {
71-
reassignWorkflowOwnershipForWorkspaceMemberRemovalTx,
72-
WorkspaceBillingAccountRemovalError,
73-
} from '@/lib/workspaces/utils'
69+
import { reassignSharedResourceOwnershipForWorkspaceMemberRemovalTx } from '@/lib/workspaces/resource-handoff'
70+
import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils'
7471
import { endDirectoryMembershipTx } from '@/ee/scim/lib/identity/end-directory-membership'
7572

7673
export { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/billing-identity-lock'
@@ -571,14 +568,8 @@ async function reassignOwnedOrganizationResourcesTx({
571568
organizationId: string
572569
workspaceIds: string[]
573570
}) {
574-
const [ownerMembership] = await tx
575-
.select({ userId: member.userId })
576-
.from(member)
577-
.where(and(eq(member.organizationId, organizationId), eq(member.role, 'owner')))
578-
.limit(1)
579-
580-
const ownerId = ownerMembership?.userId
581-
if (!ownerId || ownerId === userId) return 0
571+
const ownerId = await reassignOrganizationSharedResourcesTx(tx, organizationId, userId)
572+
if (!ownerId) return 0
582573

583574
await reassignOrganizationProjects(tx, {
584575
organizationId,
@@ -587,28 +578,6 @@ async function reassignOwnedOrganizationResourcesTx({
587578
workspaceIds,
588579
})
589580

590-
/** Creator attribution must survive account deletion without changing document ACLs. */
591-
await tx
592-
.update(knowledgeBase)
593-
.set({ userId: ownerId, updatedAt: new Date() })
594-
.where(
595-
and(
596-
eq(knowledgeBase.organizationId, organizationId),
597-
isNull(knowledgeBase.workspaceId),
598-
eq(knowledgeBase.userId, userId)
599-
)
600-
)
601-
await tx
602-
.update(workspaceFiles)
603-
.set({ userId: ownerId, updatedAt: new Date() })
604-
.where(
605-
and(
606-
eq(workspaceFiles.organizationId, organizationId),
607-
isNull(workspaceFiles.workspaceId),
608-
eq(workspaceFiles.userId, userId)
609-
)
610-
)
611-
612581
if (workspaceIds.length === 0) return 0
613582

614583
const reassignedWorkspaces = await tx
@@ -1239,13 +1208,13 @@ export async function transferUserBetweenOrganizations(
12391208
workspaceIds,
12401209
})
12411210
if (workspaceIds.length > 0) {
1242-
const workflowOwnershipReassignment =
1243-
await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({
1211+
const resourceOwnershipReassignment =
1212+
await reassignSharedResourceOwnershipForWorkspaceMemberRemovalTx({
12441213
tx,
12451214
workspaceIds,
12461215
departingUserId: params.userId,
12471216
})
1248-
if (workflowOwnershipReassignment.unresolved.length > 0) {
1217+
if (resourceOwnershipReassignment.unresolved.length > 0) {
12491218
throw new WorkspaceBillingAccountRemovalError()
12501219
}
12511220
const deletedPermissions = await tx
@@ -1468,13 +1437,13 @@ export async function removeUserFromOrganization(
14681437
}
14691438
}
14701439

1471-
const workflowOwnershipReassignment =
1472-
await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({
1440+
const resourceOwnershipReassignment =
1441+
await reassignSharedResourceOwnershipForWorkspaceMemberRemovalTx({
14731442
tx,
14741443
workspaceIds,
14751444
departingUserId: userId,
14761445
})
1477-
if (workflowOwnershipReassignment.unresolved.length > 0) {
1446+
if (resourceOwnershipReassignment.unresolved.length > 0) {
14781447
throw new WorkspaceBillingAccountRemovalError()
14791448
}
14801449

@@ -1685,13 +1654,13 @@ export async function removeExternalUserFromOrganizationWorkspaces(params: {
16851654
}
16861655
}
16871656

1688-
const workflowOwnershipReassignment =
1689-
await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({
1657+
const resourceOwnershipReassignment =
1658+
await reassignSharedResourceOwnershipForWorkspaceMemberRemovalTx({
16901659
tx,
16911660
workspaceIds,
16921661
departingUserId: userId,
16931662
})
1694-
if (workflowOwnershipReassignment.unresolved.length > 0) {
1663+
if (resourceOwnershipReassignment.unresolved.length > 0) {
16951664
throw new WorkspaceBillingAccountRemovalError()
16961665
}
16971666

‎apps/sim/lib/billing/organizations/resource-handoff.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import {
2+
knowledgeBase,
3+
member,
4+
permissionGroup,
5+
ssoProvider,
6+
user,
7+
workspaceFiles,
8+
} from '@sim/db/schema'
9+
import { and, asc, eq, isNull, ne, sql } from 'drizzle-orm'
10+
import { OrchestrationError } from '@/lib/core/orchestration/types'
11+
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
12+
import { handoffFileCreatorsInTx } from '@/lib/uploads/contexts/workspace/creator-handoff'
13+
14+
/** Organization references survive even when their creator no longer has a membership row. */
15+
export async function listSharedResourceOrganizationIdsForUser(executor: DbOrTx, userId: string) {
16+
const rows = await executor.execute<{ organizationId: string }>(sql`
17+
SELECT DISTINCT "organizationId" FROM (
18+
SELECT ${knowledgeBase.organizationId} AS "organizationId" FROM ${knowledgeBase}
19+
WHERE ${knowledgeBase.userId} = ${userId} AND ${knowledgeBase.workspaceId} IS NULL
20+
UNION ALL SELECT ${workspaceFiles.organizationId} FROM ${workspaceFiles}
21+
WHERE ${workspaceFiles.userId} = ${userId} AND ${workspaceFiles.workspaceId} IS NULL
22+
UNION ALL SELECT ${permissionGroup.organizationId} FROM ${permissionGroup} WHERE ${permissionGroup.createdBy} = ${userId}
23+
UNION ALL SELECT ${ssoProvider.organizationId} FROM ${ssoProvider} WHERE ${ssoProvider.userId} = ${userId}
24+
) owned WHERE "organizationId" IS NOT NULL ORDER BY "organizationId"
25+
`)
26+
return rows.map((row) => row.organizationId)
27+
}
28+
29+
/** Uses the same eligible owner in read-only previews and locked handoffs. */
30+
export async function findOrganizationSharedResourceSuccessor(
31+
tx: DbOrTx,
32+
organizationId: string,
33+
departingUserId: string,
34+
hold = false
35+
): Promise<string | null> {
36+
const successorQuery = tx
37+
.select({ userId: member.userId })
38+
.from(member)
39+
.where(
40+
and(
41+
eq(member.organizationId, organizationId),
42+
eq(member.role, 'owner'),
43+
ne(member.userId, departingUserId)
44+
)
45+
)
46+
.orderBy(asc(member.userId))
47+
.limit(1)
48+
const [successor] = await (hold ? successorQuery.for('share') : successorQuery)
49+
if (!successor) return null
50+
const accountQuery = tx.select({ id: user.id }).from(user).where(eq(user.id, successor.userId))
51+
const [account] = await (hold ? accountQuery.for('key share') : accountQuery)
52+
return account ? successor.userId : null
53+
}
54+
55+
/** Retains organization-owned configuration without rewriting ACLs, credentials, or historical actors. */
56+
export async function reassignOrganizationSharedResourcesTx(
57+
tx: DbTransaction,
58+
organizationId: string,
59+
departingUserId: string
60+
): Promise<string | null> {
61+
const userId = await findOrganizationSharedResourceSuccessor(
62+
tx,
63+
organizationId,
64+
departingUserId,
65+
true
66+
)
67+
if (!userId) {
68+
const owned = await listSharedResourceOrganizationIdsForUser(tx, departingUserId)
69+
if (owned.includes(organizationId)) {
70+
throw new OrchestrationError(
71+
'conflict',
72+
'Shared organization resources need an active organization owner before this account can leave or be deleted.'
73+
)
74+
}
75+
return null
76+
}
77+
const updatedAt = new Date()
78+
await tx
79+
.update(knowledgeBase)
80+
.set({ userId, updatedAt })
81+
.where(
82+
and(
83+
eq(knowledgeBase.organizationId, organizationId),
84+
isNull(knowledgeBase.workspaceId),
85+
eq(knowledgeBase.userId, departingUserId)
86+
)
87+
)
88+
await handoffFileCreatorsInTx(
89+
tx,
90+
and(
91+
eq(workspaceFiles.organizationId, organizationId),
92+
isNull(workspaceFiles.workspaceId),
93+
eq(workspaceFiles.userId, departingUserId)
94+
),
95+
userId
96+
)
97+
await tx
98+
.update(permissionGroup)
99+
.set({ createdBy: userId, updatedAt })
100+
.where(
101+
and(
102+
eq(permissionGroup.organizationId, organizationId),
103+
eq(permissionGroup.createdBy, departingUserId)
104+
)
105+
)
106+
await tx
107+
.update(ssoProvider)
108+
.set({ userId })
109+
.where(
110+
and(eq(ssoProvider.organizationId, organizationId), eq(ssoProvider.userId, departingUserId))
111+
)
112+
return userId
113+
}

‎apps/sim/lib/billing/storage/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export {
1515
incrementAdmittedStorageUsageForBillingContextInTx,
1616
incrementStorageUsageForBillingContextInTx,
1717
type LegacyStorageUsageDelta,
18+
lockWorkspaceStorageForMutationInTx,
1819
maybeNotifyStorageLimitForBillingContext,
1920
type WorkspaceStorageUsageDelta,
2021
} from './tracking'

0 commit comments

Comments
 (0)