Skip to content

Commit c2d5e0b

Browse files
committed
fix(organizations): refuse transfer-and-leave for directory-managed owners
1 parent ef115bf commit c2d5e0b

3 files changed

Lines changed: 76 additions & 2 deletions

File tree

‎apps/sim/app/api/organizations/[id]/transfer-ownership/route.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,15 @@ export const POST = withRouteHandler(
8181
organizationId,
8282
currentOwnerUserId: session.user.id,
8383
newOwnerUserId,
84+
currentOwnerLeaves: alsoLeave,
8485
})
8586

87+
if (transferResult.refusalCode) {
88+
return NextResponse.json(
89+
{ error: transferResult.error, details: { code: transferResult.refusalCode } },
90+
{ status: 403 }
91+
)
92+
}
8693
if (!transferResult.success) {
8794
return NextResponse.json(
8895
{ error: transferResult.error ?? 'Failed to transfer ownership' },
@@ -149,6 +156,7 @@ export const POST = withRouteHandler(
149156
userId: session.user.id,
150157
organizationId,
151158
memberId: selfMember.id,
159+
actorUserId: session.user.id,
152160
spareSessionToken: session.session.token,
153161
})
154162
deliverRunStops(removeResult.stoppedRuns)

‎apps/sim/ee/scim/lib/managed-membership.integration.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ async function loadRuntime() {
1919
import('@/ee/scim/lib/managed-membership'),
2020
import('@/ee/scim/lib/entitlement'),
2121
])
22-
const { removeOrganizationMemberRecord } = await import('@/lib/organizations/member-manager')
22+
const [{ removeOrganizationMemberRecord }, { transferOrganizationOwnership }] = await Promise.all(
23+
[import('@/lib/organizations/member-manager'), import('@/lib/billing/organizations/membership')]
24+
)
2325
return {
2426
db,
2527
schema,
@@ -29,6 +31,7 @@ async function loadRuntime() {
2931
sql,
3032
alias,
3133
removeOrganizationMemberRecord,
34+
transferOrganizationOwnership,
3235
...membership,
3336
...entitlement,
3437
}
@@ -269,4 +272,50 @@ describe('SCIM managed membership in PostgreSQL', () => {
269272
expect(await isMember(unmanagedUserId)).toBe(false)
270273
})
271274
})
275+
276+
describe('transferring ownership', () => {
277+
beforeEach(async () => {
278+
const { db, schema } = runtime
279+
await db.insert(schema.member).values([
280+
{ id: generateId(), organizationId: orgId, userId: managedUserId, role: 'owner' },
281+
{ id: generateId(), organizationId: orgId, userId: adminUserId, role: 'member' },
282+
])
283+
})
284+
285+
async function roleOf(userId: string) {
286+
const { db, schema, and, eq } = runtime
287+
const [row] = await db
288+
.select({ role: schema.member.role })
289+
.from(schema.member)
290+
.where(and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, userId)))
291+
return row?.role
292+
}
293+
294+
it('refuses a managed owner transferring to leave, leaving ownership unchanged', async () => {
295+
const { transferOrganizationOwnership } = runtime
296+
await expect(
297+
transferOrganizationOwnership({
298+
organizationId: orgId,
299+
currentOwnerUserId: managedUserId,
300+
newOwnerUserId: adminUserId,
301+
currentOwnerLeaves: true,
302+
})
303+
).resolves.toMatchObject({ success: false, refusalCode: 'SCIM_MANAGED_MEMBERSHIP' })
304+
expect(await roleOf(managedUserId)).toBe('owner')
305+
expect(await roleOf(adminUserId)).toBe('member')
306+
})
307+
308+
it('lets a managed owner transfer ownership and stay', async () => {
309+
const { transferOrganizationOwnership } = runtime
310+
await expect(
311+
transferOrganizationOwnership({
312+
organizationId: orgId,
313+
currentOwnerUserId: managedUserId,
314+
newOwnerUserId: adminUserId,
315+
})
316+
).resolves.toMatchObject({ success: true })
317+
expect(await roleOf(managedUserId)).toBe('admin')
318+
expect(await roleOf(adminUserId)).toBe('owner')
319+
})
320+
})
272321
})

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import {
5252
enqueueCancelAtPeriodEndSync,
5353
isCancelAtPeriodEndSettled,
5454
} from '@/lib/billing/webhooks/subscription-sync'
55+
import { type ForbiddenDetailCode, ForbiddenOperationError } from '@/lib/core/application/forbidden'
5556
import { isBillingEnabled } from '@/lib/core/config/env-flags'
5657
import { OrchestrationError } from '@/lib/core/orchestration/types'
5758
import { enqueueOutboxEvent } from '@/lib/core/outbox/service'
@@ -1836,11 +1837,19 @@ export interface TransferOwnershipParams {
18361837
organizationId: string
18371838
currentOwnerUserId: string
18381839
newOwnerUserId: string
1840+
/**
1841+
* The current owner leaves once the transfer commits. Their self-leave is
1842+
* vetted inside the transfer, so a leave the directory would undo refuses the
1843+
* transfer too instead of half-applying it.
1844+
*/
1845+
currentOwnerLeaves?: boolean
18391846
}
18401847

18411848
export interface TransferOwnershipResult {
18421849
success: boolean
18431850
error?: string
1851+
/** Set when the transfer was refused by policy rather than failed. */
1852+
refusalCode?: ForbiddenDetailCode
18441853
workspacesReassigned: number
18451854
billedAccountReassigned: number
18461855
overageMigrated: string
@@ -1850,7 +1859,7 @@ export interface TransferOwnershipResult {
18501859
export async function transferOrganizationOwnership(
18511860
params: TransferOwnershipParams
18521861
): Promise<TransferOwnershipResult> {
1853-
const { organizationId, currentOwnerUserId, newOwnerUserId } = params
1862+
const { organizationId, currentOwnerUserId, newOwnerUserId, currentOwnerLeaves } = params
18541863

18551864
const result: TransferOwnershipResult = {
18561865
success: false,
@@ -1883,6 +1892,12 @@ export async function transferOrganizationOwnership(
18831892
if (!currentOwnerMember) {
18841893
throw new Error('Current user is not the owner of this organization')
18851894
}
1895+
if (currentOwnerLeaves)
1896+
await assertSelfLeaveNotScimManaged({
1897+
organizationId,
1898+
userId: currentOwnerUserId,
1899+
executor: tx,
1900+
})
18861901

18871902
const [newOwnerMember] = await tx
18881903
.select({ id: member.id, role: member.role })
@@ -2052,6 +2067,8 @@ export async function transferOrganizationOwnership(
20522067

20532068
return { ...result, success: true }
20542069
} catch (error) {
2070+
if (error instanceof ForbiddenOperationError)
2071+
return { ...result, error: error.message, refusalCode: error.detailCode }
20552072
logger.error('Failed to transfer organization ownership', {
20562073
organizationId,
20572074
currentOwnerUserId,

0 commit comments

Comments
 (0)