Skip to content

Commit 0a2970f

Browse files
authored
fix(organizations): refuse self-leave for directory-managed members and keep the renamed name visible (#9609)
* fix(organizations): refuse self-leave for directory-managed members and keep the renamed name visible * fix(organizations): refuse transfer-and-leave for directory-managed owners * fix(organizations): transfer ownership for directory-managed owners and explain why they stay
1 parent eede06c commit 0a2970f

6 files changed

Lines changed: 225 additions & 21 deletions

File tree

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,21 @@ export const POST = withRouteHandler(
149149
userId: session.user.id,
150150
organizationId,
151151
memberId: selfMember.id,
152+
actorUserId: session.user.id,
152153
spareSessionToken: session.session.token,
153154
})
154155
deliverRunStops(removeResult.stoppedRuns)
155156

156157
if (!removeResult.success) {
157-
logger.error('Transfer succeeded but self-removal failed', {
158+
const outcome = {
158159
organizationId,
159160
userId: session.user.id,
160161
error: removeResult.error,
161-
})
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)
162167
return NextResponse.json(
163168
{
164169
success: true,

‎apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,14 @@ export function OrganizationHeader({
6666
const [isInviteModalOpen, setIsInviteModalOpen] = useState(false)
6767
const [isMenuOpen, setIsMenuOpen] = useState(false)
6868
const [draftName, setDraftName] = useState<string | null>(null)
69-
const initial = getOrganizationInitial(organization.name)
69+
/**
70+
* The name just saved, shown until the refreshed server prop replaces the one
71+
* it was saved over; without it the old name flashes back in between.
72+
*/
73+
const [savedName, setSavedName] = useState<{ name: string; replaces: string } | null>(null)
74+
if (savedName && savedName.replaces !== organization.name) setSavedName(null)
75+
const organizationName = savedName?.name ?? organization.name
76+
const initial = getOrganizationInitial(organizationName)
7077

7178
if (isCollapsed) {
7279
return (
@@ -103,21 +110,24 @@ export function OrganizationHeader({
103110
const submitRename = () => {
104111
if (draftName === null || renameOrganization.isPending) return
105112
const name = draftName.trim()
106-
if (!name || name === organization.name) {
113+
if (!name || name === organizationName) {
107114
stopRenaming()
108115
return
109116
}
110117
renameOrganization.mutate(
111118
{ orgId: organization.id, name },
112119
{
113-
onSuccess: stopRenaming,
120+
onSuccess: () => {
121+
setSavedName({ name, replaces: organization.name })
122+
stopRenaming()
123+
},
114124
onError: () => requestAnimationFrame(() => nameInputRef.current?.focus()),
115125
}
116126
)
117127
}
118128

119129
const logo = (
120-
<IdentityTile size='lg' initial={initial} logoUrl={organization.logo} alt={organization.name} />
130+
<IdentityTile size='lg' initial={initial} logoUrl={organization.logo} alt={organizationName} />
121131
)
122132

123133
return (
@@ -156,7 +166,7 @@ export function OrganizationHeader({
156166
leftAdornment={<IdentityTile initial={initial} logoUrl={organization.logo} />}
157167
rightAdornment={<ChipChevronDown />}
158168
>
159-
{organization.name}
169+
{organizationName}
160170
</Chip>
161171
</DropdownMenuTrigger>
162172
<DropdownMenuContent
@@ -222,13 +232,13 @@ export function OrganizationHeader({
222232
className='h-auto min-w-0 p-0 hover-hover:opacity-70 focus-visible:opacity-70'
223233
onSelect={(event) => {
224234
event.preventDefault()
225-
setDraftName(organization.name)
235+
setDraftName(organizationName)
226236
}}
227237
>
228-
<OverflowText label={organization.name} />
238+
<OverflowText label={organizationName} />
229239
</DropdownMenuItem>
230240
) : (
231-
<OverflowText label={organization.name} />
241+
<OverflowText label={organizationName} />
232242
)}
233243
{renameOrganization.isError && (
234244
<span role='alert' className='text-[var(--text-error)] text-caption'>

‎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: 151 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ vi.mock('@/lib/core/config/env-flags', () => ({
1010
}))
1111

1212
async function loadRuntime() {
13-
const [{ db }, schema, { eq, inArray, sql }, { alias }, membership, entitlement] =
13+
const [{ db }, schema, { and, eq, inArray, sql }, { alias }, membership, entitlement] =
1414
await Promise.all([
1515
import('@sim/db'),
1616
import('@sim/db/schema'),
@@ -19,7 +19,27 @@ async function loadRuntime() {
1919
import('@/ee/scim/lib/managed-membership'),
2020
import('@/ee/scim/lib/entitlement'),
2121
])
22-
return { db, schema, eq, inArray, sql, alias, ...membership, ...entitlement }
22+
const [
23+
{ removeOrganizationMemberRecord },
24+
{ removeUserFromOrganization, transferOrganizationOwnership },
25+
] = await Promise.all([
26+
import('@/lib/organizations/member-manager'),
27+
import('@/lib/billing/organizations/membership'),
28+
])
29+
return {
30+
db,
31+
schema,
32+
and,
33+
eq,
34+
inArray,
35+
sql,
36+
alias,
37+
removeOrganizationMemberRecord,
38+
removeUserFromOrganization,
39+
transferOrganizationOwnership,
40+
...membership,
41+
...entitlement,
42+
}
2343
}
2444

2545
describe('SCIM managed membership in PostgreSQL', () => {
@@ -29,6 +49,7 @@ describe('SCIM managed membership in PostgreSQL', () => {
2949
let connectionId: string
3050
let managedUserId: string
3151
let unmanagedUserId: string
52+
let adminUserId: string
3253
let managedEmail: string
3354

3455
beforeAll(async () => {
@@ -42,6 +63,7 @@ describe('SCIM managed membership in PostgreSQL', () => {
4263
connectionId = generateId()
4364
managedUserId = generateId()
4465
unmanagedUserId = generateId()
66+
adminUserId = generateId()
4567
managedEmail = `${managedUserId}@scim-membership.test`
4668
const { db, schema } = runtime
4769
const now = new Date()
@@ -54,7 +76,7 @@ describe('SCIM managed membership in PostgreSQL', () => {
5476
}))
5577
)
5678
await db.insert(schema.user).values(
57-
[managedUserId, unmanagedUserId].map((id) => ({
79+
[managedUserId, unmanagedUserId, adminUserId].map((id) => ({
5880
id,
5981
name: 'SCIM predicate test',
6082
email: `${id}@scim-membership.test`,
@@ -99,7 +121,9 @@ describe('SCIM managed membership in PostgreSQL', () => {
99121
const { db, schema, eq, inArray } = runtime
100122
await db.delete(schema.subscription).where(eq(schema.subscription.referenceId, orgId))
101123
await db.delete(schema.organization).where(inArray(schema.organization.id, [orgId, otherOrgId]))
102-
await db.delete(schema.user).where(inArray(schema.user.id, [managedUserId, unmanagedUserId]))
124+
await db
125+
.delete(schema.user)
126+
.where(inArray(schema.user.id, [managedUserId, unmanagedUserId, adminUserId]))
103127
})
104128

105129
async function invitee(organizationId: string, email: string) {
@@ -194,4 +218,127 @@ describe('SCIM managed membership in PostgreSQL', () => {
194218
})
195219
).rejects.toMatchObject({ cause: { code: '42P01' } })
196220
})
221+
222+
describe('removing a member', () => {
223+
beforeEach(async () => {
224+
const { db, schema } = runtime
225+
await db.insert(schema.member).values([
226+
{ id: generateId(), organizationId: orgId, userId: adminUserId, role: 'owner' },
227+
{ id: generateId(), organizationId: orgId, userId: managedUserId, role: 'member' },
228+
{ id: generateId(), organizationId: orgId, userId: unmanagedUserId, role: 'member' },
229+
])
230+
})
231+
232+
async function isMember(userId: string) {
233+
const { db, schema, and, eq } = runtime
234+
const rows = await db
235+
.select({ id: schema.member.id })
236+
.from(schema.member)
237+
.where(and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, userId)))
238+
return rows.length > 0
239+
}
240+
241+
it('refuses a managed member leaving on their own and keeps the membership', async () => {
242+
const { removeOrganizationMemberRecord } = runtime
243+
await expect(
244+
removeOrganizationMemberRecord({
245+
organizationId: orgId,
246+
userId: managedUserId,
247+
actorUserId: managedUserId,
248+
})
249+
).rejects.toMatchObject({
250+
detailCode: 'SCIM_MANAGED_MEMBERSHIP',
251+
message: expect.stringContaining('Ask your identity provider administrator'),
252+
})
253+
expect(await isMember(managedUserId)).toBe(true)
254+
})
255+
256+
it('lets an administrator remove the same managed member', async () => {
257+
const { removeOrganizationMemberRecord } = runtime
258+
await expect(
259+
removeOrganizationMemberRecord({
260+
organizationId: orgId,
261+
userId: managedUserId,
262+
actorUserId: adminUserId,
263+
})
264+
).resolves.toMatchObject({ membershipType: 'internal' })
265+
expect(await isMember(managedUserId)).toBe(false)
266+
})
267+
268+
it('lets an unmanaged member leave on their own', async () => {
269+
const { removeOrganizationMemberRecord } = runtime
270+
await expect(
271+
removeOrganizationMemberRecord({
272+
organizationId: orgId,
273+
userId: unmanagedUserId,
274+
actorUserId: unmanagedUserId,
275+
})
276+
).resolves.toMatchObject({ membershipType: 'internal' })
277+
expect(await isMember(unmanagedUserId)).toBe(false)
278+
})
279+
})
280+
281+
describe('transferring ownership', () => {
282+
beforeEach(async () => {
283+
const { db, schema } = runtime
284+
await db.insert(schema.member).values([
285+
{ id: generateId(), organizationId: orgId, userId: managedUserId, role: 'owner' },
286+
{ id: generateId(), organizationId: orgId, userId: adminUserId, role: 'member' },
287+
])
288+
})
289+
290+
async function roleOf(userId: string) {
291+
const { db, schema, and, eq } = runtime
292+
const [row] = await db
293+
.select({ role: schema.member.role })
294+
.from(schema.member)
295+
.where(and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, userId)))
296+
return row?.role
297+
}
298+
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
302+
await expect(
303+
transferOrganizationOwnership({
304+
organizationId: orgId,
305+
currentOwnerUserId: managedUserId,
306+
newOwnerUserId: adminUserId,
307+
})
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')
329+
})
330+
331+
it('lets a managed owner transfer ownership and stay', async () => {
332+
const { transferOrganizationOwnership } = runtime
333+
await expect(
334+
transferOrganizationOwnership({
335+
organizationId: orgId,
336+
currentOwnerUserId: managedUserId,
337+
newOwnerUserId: adminUserId,
338+
})
339+
).resolves.toMatchObject({ success: true })
340+
expect(await roleOf(managedUserId)).toBe('admin')
341+
expect(await roleOf(adminUserId)).toBe('owner')
342+
})
343+
})
197344
})

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

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import { isScimDeploymentEnabled, isScimEntitledForOrganization } from '@/ee/sci
1414
* remedy.
1515
*
1616
* Deliberately not applied to deprovisioning: an administrator must always be
17-
* able to remove someone in an emergency, whatever the directory believes.
17+
* able to remove someone in an emergency, whatever the directory believes. A
18+
* member removing themselves is refused, since the directory would re-add them.
1819
*/
1920

2021
/**
@@ -63,6 +64,32 @@ export async function assertMembershipNotScimManaged(params: {
6364
userId: string
6465
executor: DbOrTx
6566
}): Promise<void> {
67+
await refuseIfDirectoryManaged(
68+
params,
69+
'This member is managed by the organization’s identity provider. Make the change there, or turn off managed-membership locking in the organization’s directory settings.'
70+
)
71+
}
72+
73+
/**
74+
* Refuses a directory-managed member leaving on their own. Unlike an
75+
* administrator's removal, which must always succeed, a self-leave only lasts
76+
* until the next sync re-adds the member, so it is refused with the remedy.
77+
*/
78+
export async function assertSelfLeaveNotScimManaged(params: {
79+
organizationId: string
80+
userId: string
81+
executor: DbOrTx
82+
}): Promise<void> {
83+
await refuseIfDirectoryManaged(
84+
params,
85+
'Your membership is managed by your organization’s identity provider. Ask your identity provider administrator to remove you there.'
86+
)
87+
}
88+
89+
async function refuseIfDirectoryManaged(
90+
params: { organizationId: string; userId: string; executor: DbOrTx },
91+
message: string
92+
): Promise<void> {
6693
if (!isScimDeploymentEnabled()) return
6794
const [row] = await params.executor
6895
.select({ managed: scimManagedUserPredicate(params.organizationId, sql`${params.userId}`) })
@@ -74,10 +101,7 @@ export async function assertMembershipNotScimManaged(params: {
74101
* once a managed row is found, so the common case costs nothing.
75102
*/
76103
if (!(await isScimEntitledForOrganization(params.organizationId, params.executor))) return
77-
throw new ForbiddenOperationError(
78-
'SCIM_MANAGED_MEMBERSHIP',
79-
'This member is managed by the organization’s identity provider. Make the change there, or turn off managed-membership locking in the organization’s directory settings.'
80-
)
104+
throw new ForbiddenOperationError('SCIM_MANAGED_MEMBERSHIP', message)
81105
}
82106

83107
/** Refuses an invitation to someone the directory already provisions, while the directory can still sync. */

0 commit comments

Comments
 (0)