Skip to content

Commit 4d78535

Browse files
committed
fix(organizations): transfer ownership for directory-managed owners and explain why they stay
1 parent c2d5e0b commit 4d78535

4 files changed

Lines changed: 53 additions & 36 deletions

File tree

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

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

87-
if (transferResult.refusalCode) {
88-
return NextResponse.json(
89-
{ error: transferResult.error, details: { code: transferResult.refusalCode } },
90-
{ status: 403 }
91-
)
92-
}
9386
if (!transferResult.success) {
9487
return NextResponse.json(
9588
{ error: transferResult.error ?? 'Failed to transfer ownership' },
@@ -162,11 +155,15 @@ export const POST = withRouteHandler(
162155
deliverRunStops(removeResult.stoppedRuns)
163156

164157
if (!removeResult.success) {
165-
logger.error('Transfer succeeded but self-removal failed', {
158+
const outcome = {
166159
organizationId,
167160
userId: session.user.id,
168161
error: removeResult.error,
169-
})
162+
refusalCode: removeResult.refusalCode,
163+
}
164+
if (removeResult.refusalCode)
165+
logger.info('Transfer succeeded; policy kept the previous owner as admin', outcome)
166+
else logger.error('Transfer succeeded but self-removal failed', outcome)
170167
return NextResponse.json(
171168
{
172169
success: true,

‎apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use client'
22

33
import { useState } from 'react'
4-
import { Plus } from '@sim/emcn'
4+
import { Plus, toast } from '@sim/emcn'
55
import { createLogger } from '@sim/logger'
66
import { isOrgAdminRole } from '@sim/platform-authz/predicates'
77
import { getErrorMessage } from '@sim/utils/errors'
@@ -305,6 +305,8 @@ export function TeamManagement({
305305

306306
if (result.left) {
307307
window.location.href = APP_ENTRY_PATH
308+
} else if (result.warning) {
309+
toast.warning(`Ownership transferred. ${result.warning}`)
308310
}
309311
} catch (error) {
310312
logger.error('Failed to transfer ownership', error)

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

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,13 @@ async function loadRuntime() {
1919
import('@/ee/scim/lib/managed-membership'),
2020
import('@/ee/scim/lib/entitlement'),
2121
])
22-
const [{ removeOrganizationMemberRecord }, { transferOrganizationOwnership }] = await Promise.all(
23-
[import('@/lib/organizations/member-manager'), import('@/lib/billing/organizations/membership')]
24-
)
22+
const [
23+
{ removeOrganizationMemberRecord },
24+
{ removeUserFromOrganization, transferOrganizationOwnership },
25+
] = await Promise.all([
26+
import('@/lib/organizations/member-manager'),
27+
import('@/lib/billing/organizations/membership'),
28+
])
2529
return {
2630
db,
2731
schema,
@@ -31,6 +35,7 @@ async function loadRuntime() {
3135
sql,
3236
alias,
3337
removeOrganizationMemberRecord,
38+
removeUserFromOrganization,
3439
transferOrganizationOwnership,
3540
...membership,
3641
...entitlement,
@@ -291,18 +296,36 @@ describe('SCIM managed membership in PostgreSQL', () => {
291296
return row?.role
292297
}
293298

294-
it('refuses a managed owner transferring to leave, leaving ownership unchanged', async () => {
295-
const { transferOrganizationOwnership } = runtime
299+
it('transfers a managed owner’s ownership but keeps them as admin when they try to leave', async () => {
300+
const { db, schema, and, eq, transferOrganizationOwnership, removeUserFromOrganization } =
301+
runtime
296302
await expect(
297303
transferOrganizationOwnership({
298304
organizationId: orgId,
299305
currentOwnerUserId: managedUserId,
300306
newOwnerUserId: adminUserId,
301-
currentOwnerLeaves: true,
302307
})
303-
).resolves.toMatchObject({ success: false, refusalCode: 'SCIM_MANAGED_MEMBERSHIP' })
304-
expect(await roleOf(managedUserId)).toBe('owner')
305-
expect(await roleOf(adminUserId)).toBe('member')
308+
).resolves.toMatchObject({ success: true })
309+
const [previousOwner] = await db
310+
.select({ id: schema.member.id })
311+
.from(schema.member)
312+
.where(
313+
and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, managedUserId))
314+
)
315+
await expect(
316+
removeUserFromOrganization({
317+
userId: managedUserId,
318+
organizationId: orgId,
319+
memberId: previousOwner.id,
320+
actorUserId: managedUserId,
321+
})
322+
).resolves.toMatchObject({
323+
success: false,
324+
refusalCode: 'SCIM_MANAGED_MEMBERSHIP',
325+
error: expect.stringContaining('Ask your identity provider administrator'),
326+
})
327+
expect(await roleOf(managedUserId)).toBe('admin')
328+
expect(await roleOf(adminUserId)).toBe('owner')
306329
})
307330

308331
it('lets a managed owner transfer ownership and stay', async () => {

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,8 @@ export interface RemoveMemberParams {
535535
export interface RemoveMemberResult {
536536
success: boolean
537537
error?: string
538+
/** Set when policy refused the removal, as opposed to it failing. */
539+
refusalCode?: ForbiddenDetailCode
538540
/**
539541
* Whether the member row was actually deleted. `false` (with `success: true`)
540542
* when `requireNoOrgWorkspaceAccess` was set and the user still had workspace
@@ -1621,6 +1623,15 @@ export async function removeUserFromOrganization(
16211623
return { success: false, error: error.message, billingActions, stoppedRuns: [] }
16221624
}
16231625
if (onError === 'throw') throw error
1626+
if (error instanceof ForbiddenOperationError) {
1627+
return {
1628+
success: false,
1629+
error: error.message,
1630+
refusalCode: error.detailCode,
1631+
billingActions,
1632+
stoppedRuns: [],
1633+
}
1634+
}
16241635

16251636
logger.error('Failed to remove user from organization', {
16261637
userId,
@@ -1837,19 +1848,11 @@ export interface TransferOwnershipParams {
18371848
organizationId: string
18381849
currentOwnerUserId: string
18391850
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
18461851
}
18471852

18481853
export interface TransferOwnershipResult {
18491854
success: boolean
18501855
error?: string
1851-
/** Set when the transfer was refused by policy rather than failed. */
1852-
refusalCode?: ForbiddenDetailCode
18531856
workspacesReassigned: number
18541857
billedAccountReassigned: number
18551858
overageMigrated: string
@@ -1859,7 +1862,7 @@ export interface TransferOwnershipResult {
18591862
export async function transferOrganizationOwnership(
18601863
params: TransferOwnershipParams
18611864
): Promise<TransferOwnershipResult> {
1862-
const { organizationId, currentOwnerUserId, newOwnerUserId, currentOwnerLeaves } = params
1865+
const { organizationId, currentOwnerUserId, newOwnerUserId } = params
18631866

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

19021899
const [newOwnerMember] = await tx
19031900
.select({ id: member.id, role: member.role })
@@ -2067,8 +2064,6 @@ export async function transferOrganizationOwnership(
20672064

20682065
return { ...result, success: true }
20692066
} catch (error) {
2070-
if (error instanceof ForbiddenOperationError)
2071-
return { ...result, error: error.message, refusalCode: error.detailCode }
20722067
logger.error('Failed to transfer organization ownership', {
20732068
organizationId,
20742069
currentOwnerUserId,

0 commit comments

Comments
 (0)