Skip to content

Commit 6989ce8

Browse files
authored
fix(sim-links): hand off a departing member's links to the successor (#9597)
* fix(sim-links): hand off a departing member's links to the successor * fix(sim-links): repair leftover links on account deletion * fix(sim-links): lock leftover-link organizations before deletion barriers * fix(sim-links): hold the creator's membership while creating a link
1 parent 0bf55bc commit 6989ce8

6 files changed

Lines changed: 309 additions & 6 deletions

File tree

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ import {
22
knowledgeBase,
33
member,
44
permissionGroup,
5+
simLink,
56
ssoProvider,
67
user,
78
workspaceFiles,
89
} from '@sim/db/schema'
9-
import { and, asc, eq, isNull, ne, sql } from 'drizzle-orm'
10+
import { and, asc, eq, isNull, ne, notInArray, sql } from 'drizzle-orm'
1011
import { OrchestrationError } from '@/lib/core/orchestration/types'
1112
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
1213
import { handoffFileCreatorsInTx } from '@/lib/uploads/contexts/workspace/creator-handoff'
@@ -103,6 +104,7 @@ export async function reassignOrganizationSharedResourcesTx(
103104
eq(permissionGroup.createdBy, departingUserId)
104105
)
105106
)
107+
await reassignSimLinksTx(tx, organizationId, departingUserId, userId)
106108
await tx
107109
.update(ssoProvider)
108110
.set({ userId })
@@ -111,3 +113,60 @@ export async function reassignOrganizationSharedResourcesTx(
111113
)
112114
return userId
113115
}
116+
117+
async function reassignSimLinksTx(
118+
tx: DbTransaction,
119+
organizationId: string,
120+
fromUserId: string,
121+
toUserId: string
122+
) {
123+
await tx
124+
.update(simLink)
125+
.set({ createdBy: toUserId, updatedAt: new Date() })
126+
.where(and(eq(simLink.organizationId, organizationId), eq(simLink.createdBy, fromUserId)))
127+
}
128+
129+
/**
130+
* Organizations where a former member still created Sim links outside `excludedOrganizationIds`.
131+
* Account deletion locks these with the shared organizations, but they never block erasure.
132+
*/
133+
export async function listLeftoverSimLinkOrganizationIdsForUser(
134+
executor: DbOrTx,
135+
userId: string,
136+
excludedOrganizationIds: string[]
137+
) {
138+
const rows = await executor
139+
.selectDistinct({ organizationId: simLink.organizationId })
140+
.from(simLink)
141+
.where(
142+
and(
143+
eq(simLink.createdBy, userId),
144+
excludedOrganizationIds.length > 0
145+
? notInArray(simLink.organizationId, excludedOrganizationIds)
146+
: undefined
147+
)
148+
)
149+
.orderBy(asc(simLink.organizationId))
150+
return rows.map((row) => row.organizationId)
151+
}
152+
153+
/**
154+
* Repairs Sim links a former member kept after leaving before departures handed them off. The
155+
* caller holds each organization's mutation lock. Never blocks erasure: an organization without
156+
* an owner successor keeps the FK's `SET NULL`.
157+
*/
158+
export async function handoffLeftoverSimLinksTx(
159+
tx: DbTransaction,
160+
departingUserId: string,
161+
organizationIds: string[]
162+
) {
163+
for (const organizationId of organizationIds) {
164+
const successorId = await findOrganizationSharedResourceSuccessor(
165+
tx,
166+
organizationId,
167+
departingUserId,
168+
true
169+
)
170+
if (successorId) await reassignSimLinksTx(tx, organizationId, departingUserId, successorId)
171+
}
172+
}

‎apps/sim/lib/core/application/organization-authorization.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ export interface OrganizationMembershipContext extends OrganizationAuthorization
4747
export interface OrganizationAuthorizationOptions {
4848
executor?: DbOrTx
4949
forUpdate?: boolean
50+
/** Holds the membership row until commit, so a concurrent removal waits for this write. */
51+
forShare?: boolean
5052
}
5153

5254
/** Rechecks routed organization membership; owning a workspace is irrelevant to this grant. */
@@ -83,7 +85,11 @@ export async function requireOrganizationSubjectMembership(
8385
.select({ role: member.role })
8486
.from(member)
8587
.where(and(eq(member.organizationId, organizationId), eq(member.userId, userId)))
86-
const [membership] = options.forUpdate ? await query.for('update').limit(1) : await query.limit(1)
88+
const [membership] = options.forUpdate
89+
? await query.for('update').limit(1)
90+
: options.forShare
91+
? await query.for('share').limit(1)
92+
: await query.limit(1)
8793
const parsedRole = organizationRoleSchema.safeParse(membership?.role)
8894
if (!parsedRole.success) throw new OrganizationMembershipNotFoundError()
8995
if (minimumRole === 'admin' && !isOrgAdminRole(parsedRole.data)) {
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import { db } from '@sim/db'
2+
import { member, organization, simLink, user } from '@sim/db/schema'
3+
import { readTestDatabaseUrl } from '@sim/db/testing/test-infrastructure'
4+
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
5+
import { createDeferred } from '@sim/testing/helpers/deferred'
6+
import { featureFlagsMock, featureFlagsMockFns } from '@sim/testing/mocks/feature-flags.mock'
7+
import { generateId } from '@sim/utils/id'
8+
import { eq, inArray } from 'drizzle-orm'
9+
import { afterAll, expect, it, vi } from 'vitest'
10+
import { removeUserFromOrganization } from '@/lib/billing/organizations/membership'
11+
import { createSimLink } from '@/lib/sim-links/application/use-cases'
12+
13+
vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)
14+
15+
readTestDatabaseUrl()
16+
17+
const ownerId = generateId()
18+
const creatorId = generateId()
19+
const organizationId = generateId()
20+
const creatorMemberId = generateId()
21+
22+
afterAll(async () => {
23+
await db.delete(organization).where(eq(organization.id, organizationId))
24+
await db.delete(user).where(inArray(user.id, [ownerId, creatorId]))
25+
})
26+
27+
it('refuses a link whose creator is removed after authorization but before the insert', async () => {
28+
await db.insert(user).values(
29+
[ownerId, creatorId].map((id) => ({
30+
id,
31+
name: 'Sim link fixture',
32+
email: `${id}@example.test`,
33+
emailVerified: true,
34+
createdAt: new Date(),
35+
updatedAt: new Date(),
36+
}))
37+
)
38+
await db.insert(organization).values({
39+
id: organizationId,
40+
name: 'Sim link organization',
41+
slug: organizationId,
42+
createdAt: new Date(),
43+
})
44+
await db.insert(member).values([
45+
{ id: generateId(), organizationId, userId: ownerId, role: 'owner', createdAt: new Date() },
46+
{
47+
id: creatorMemberId,
48+
organizationId,
49+
userId: creatorId,
50+
role: 'member',
51+
createdAt: new Date(),
52+
},
53+
])
54+
55+
const authorized = createDeferred<void>()
56+
const proceed = createDeferred<void>()
57+
featureFlagsMockFns.mockIsFeatureEnabled.mockImplementation(async (flag) => {
58+
if (flag !== 'sim-links') return false
59+
authorized.resolve()
60+
await proceed.promise
61+
return true
62+
})
63+
64+
const creation = createSimLink.execute({
65+
principal: createSessionPrincipal({ userId: creatorId }),
66+
input: {
67+
organizationId,
68+
name: 'handbook',
69+
url: 'https://example.test/handbook',
70+
defaultUrl: null,
71+
description: null,
72+
unlisted: true,
73+
restrictEdit: true,
74+
},
75+
})
76+
const settled = creation.then(
77+
() => null,
78+
(error: unknown) => error
79+
)
80+
await authorized.promise
81+
expect(
82+
(
83+
await removeUserFromOrganization({
84+
organizationId,
85+
userId: creatorId,
86+
memberId: creatorMemberId,
87+
skipBillingLogic: true,
88+
onError: 'throw',
89+
})
90+
).success
91+
).toBe(true)
92+
proceed.resolve()
93+
94+
expect(await settled).toMatchObject({ message: 'Organization not found' })
95+
expect(
96+
await db
97+
.select({ id: simLink.id })
98+
.from(simLink)
99+
.where(eq(simLink.organizationId, organizationId))
100+
).toEqual([])
101+
})

‎apps/sim/lib/sim-links/application/use-cases.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import {
99
type OrganizationUseCaseContext,
1010
} from '@/lib/core/application/authorized-organization-use-case'
1111
import type { WorkspaceUseCaseAuditEntry } from '@/lib/core/application/authorized-workspace-use-case'
12-
import type { OrganizationMembershipContext } from '@/lib/core/application/organization-authorization'
12+
import {
13+
type OrganizationMembershipContext,
14+
requireOrganizationSubjectMembership,
15+
} from '@/lib/core/application/organization-authorization'
1316
import { OrchestrationError } from '@/lib/core/orchestration/types'
1417
import { defineAuthorizedCredentialUserUseCase } from '@/lib/credentials/application/authorized-user-use-case'
1518
import type { DbTransaction } from '@/lib/db/types'
@@ -193,6 +196,18 @@ export const createSimLink = defineAuthorizedOrganizationUseCase({
193196
const { organizationId, ...fields } = input
194197
return withUniqueName(organizationId, fields.name, () =>
195198
db.transaction(async (tx) => {
199+
/** A removal deletes this row before handing off the member's links, so it waits for this insert. */
200+
await requireOrganizationSubjectMembership(
201+
context.userId,
202+
organizationId,
203+
'member',
204+
'none',
205+
undefined,
206+
{
207+
executor: tx,
208+
forShare: true,
209+
}
210+
)
196211
const linkId = await repository.insertSimLink(tx, organizationId, context.userId, fields)
197212
return readView(tx, organizationId, linkId, viewerOf(context))
198213
})

‎apps/sim/lib/users/account-deletion.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
} from '@sim/db/schema'
2020
import { createLogger } from '@sim/logger'
2121
import { getErrorMessage, getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
22-
import { formatQuotedNameList } from '@sim/utils/string'
22+
import { compareStrings, formatQuotedNameList } from '@sim/utils/string'
2323
import { and, eq, gt, inArray, isNotNull, isNull, lte, ne, notExists, or, sql } from 'drizzle-orm'
2424
import type {
2525
AccountDeletionBlocker,
@@ -34,6 +34,8 @@ import {
3434
} from '@/lib/billing/organizations/membership'
3535
import {
3636
findOrganizationSharedResourceSuccessor,
37+
handoffLeftoverSimLinksTx,
38+
listLeftoverSimLinkOrganizationIdsForUser,
3739
listSharedResourceOrganizationIdsForUser,
3840
reassignOrganizationSharedResourcesTx,
3941
} from '@/lib/billing/organizations/resource-handoff'
@@ -812,7 +814,16 @@ export async function deleteUserAccount(
812814
await db.transaction(async (tx) => {
813815
/** Organization and Project gates precede workspace rows, which precede user FK barriers. */
814816
const sharedOrganizationIds = await listSharedResourceOrganizationIdsForUser(tx, userId)
815-
for (const organizationId of sharedOrganizationIds) {
817+
const leftoverSimLinkOrganizationIds = await listLeftoverSimLinkOrganizationIdsForUser(
818+
tx,
819+
userId,
820+
sharedOrganizationIds
821+
)
822+
const lockedOrganizationIds = [
823+
...sharedOrganizationIds,
824+
...leftoverSimLinkOrganizationIds,
825+
].sort(compareStrings)
826+
for (const organizationId of lockedOrganizationIds) {
816827
await acquireOrganizationMutationLock(tx, organizationId)
817828
}
818829
const workspaceIds = new Set([
@@ -852,9 +863,15 @@ export async function deleteUserAccount(
852863
...(await listSharedResourceWorkspaceIdsForUser(tx, userId)),
853864
]
854865
const currentOrganizationIds = await listSharedResourceOrganizationIdsForUser(tx, userId)
866+
const currentSimLinkOrganizationIds = await listLeftoverSimLinkOrganizationIdsForUser(
867+
tx,
868+
userId,
869+
currentOrganizationIds
870+
)
855871
if (
856872
currentWorkspaceIds.some((id) => !workspaceIds.has(id)) ||
857-
currentOrganizationIds.some((id) => !sharedOrganizationIds.includes(id))
873+
currentOrganizationIds.some((id) => !sharedOrganizationIds.includes(id)) ||
874+
currentSimLinkOrganizationIds.some((id) => !lockedOrganizationIds.includes(id))
858875
) {
859876
throw new AccountDeletionBlockedError([
860877
{
@@ -965,6 +982,7 @@ export async function deleteUserAccount(
965982
for (const organizationId of sharedOrganizationIds) {
966983
await reassignOrganizationSharedResourcesTx(tx, organizationId, userId)
967984
}
985+
await handoffLeftoverSimLinksTx(tx, userId, leftoverSimLinkOrganizationIds)
968986

969987
const sharedWorkspaceIds = await listSharedResourceWorkspaceIdsForUser(tx, userId)
970988
const { unresolved: resourceUnresolved } =

0 commit comments

Comments
 (0)