Skip to content

Commit ef115bf

Browse files
committed
fix(organizations): refuse self-leave for directory-managed members and keep the renamed name visible
1 parent 0bf55bc commit ef115bf

4 files changed

Lines changed: 131 additions & 18 deletions

File tree

‎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/ee/scim/lib/managed-membership.integration.ts‎

Lines changed: 79 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,19 @@ 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 { removeOrganizationMemberRecord } = await import('@/lib/organizations/member-manager')
23+
return {
24+
db,
25+
schema,
26+
and,
27+
eq,
28+
inArray,
29+
sql,
30+
alias,
31+
removeOrganizationMemberRecord,
32+
...membership,
33+
...entitlement,
34+
}
2335
}
2436

2537
describe('SCIM managed membership in PostgreSQL', () => {
@@ -29,6 +41,7 @@ describe('SCIM managed membership in PostgreSQL', () => {
2941
let connectionId: string
3042
let managedUserId: string
3143
let unmanagedUserId: string
44+
let adminUserId: string
3245
let managedEmail: string
3346

3447
beforeAll(async () => {
@@ -42,6 +55,7 @@ describe('SCIM managed membership in PostgreSQL', () => {
4255
connectionId = generateId()
4356
managedUserId = generateId()
4457
unmanagedUserId = generateId()
58+
adminUserId = generateId()
4559
managedEmail = `${managedUserId}@scim-membership.test`
4660
const { db, schema } = runtime
4761
const now = new Date()
@@ -54,7 +68,7 @@ describe('SCIM managed membership in PostgreSQL', () => {
5468
}))
5569
)
5670
await db.insert(schema.user).values(
57-
[managedUserId, unmanagedUserId].map((id) => ({
71+
[managedUserId, unmanagedUserId, adminUserId].map((id) => ({
5872
id,
5973
name: 'SCIM predicate test',
6074
email: `${id}@scim-membership.test`,
@@ -99,7 +113,9 @@ describe('SCIM managed membership in PostgreSQL', () => {
99113
const { db, schema, eq, inArray } = runtime
100114
await db.delete(schema.subscription).where(eq(schema.subscription.referenceId, orgId))
101115
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]))
116+
await db
117+
.delete(schema.user)
118+
.where(inArray(schema.user.id, [managedUserId, unmanagedUserId, adminUserId]))
103119
})
104120

105121
async function invitee(organizationId: string, email: string) {
@@ -194,4 +210,63 @@ describe('SCIM managed membership in PostgreSQL', () => {
194210
})
195211
).rejects.toMatchObject({ cause: { code: '42P01' } })
196212
})
213+
214+
describe('removing a member', () => {
215+
beforeEach(async () => {
216+
const { db, schema } = runtime
217+
await db.insert(schema.member).values([
218+
{ id: generateId(), organizationId: orgId, userId: adminUserId, role: 'owner' },
219+
{ id: generateId(), organizationId: orgId, userId: managedUserId, role: 'member' },
220+
{ id: generateId(), organizationId: orgId, userId: unmanagedUserId, role: 'member' },
221+
])
222+
})
223+
224+
async function isMember(userId: string) {
225+
const { db, schema, and, eq } = runtime
226+
const rows = await db
227+
.select({ id: schema.member.id })
228+
.from(schema.member)
229+
.where(and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, userId)))
230+
return rows.length > 0
231+
}
232+
233+
it('refuses a managed member leaving on their own and keeps the membership', async () => {
234+
const { removeOrganizationMemberRecord } = runtime
235+
await expect(
236+
removeOrganizationMemberRecord({
237+
organizationId: orgId,
238+
userId: managedUserId,
239+
actorUserId: managedUserId,
240+
})
241+
).rejects.toMatchObject({
242+
detailCode: 'SCIM_MANAGED_MEMBERSHIP',
243+
message: expect.stringContaining('Ask your identity provider administrator'),
244+
})
245+
expect(await isMember(managedUserId)).toBe(true)
246+
})
247+
248+
it('lets an administrator remove the same managed member', async () => {
249+
const { removeOrganizationMemberRecord } = runtime
250+
await expect(
251+
removeOrganizationMemberRecord({
252+
organizationId: orgId,
253+
userId: managedUserId,
254+
actorUserId: adminUserId,
255+
})
256+
).resolves.toMatchObject({ membershipType: 'internal' })
257+
expect(await isMember(managedUserId)).toBe(false)
258+
})
259+
260+
it('lets an unmanaged member leave on their own', async () => {
261+
const { removeOrganizationMemberRecord } = runtime
262+
await expect(
263+
removeOrganizationMemberRecord({
264+
organizationId: orgId,
265+
userId: unmanagedUserId,
266+
actorUserId: unmanagedUserId,
267+
})
268+
).resolves.toMatchObject({ membershipType: 'internal' })
269+
expect(await isMember(unmanagedUserId)).toBe(false)
270+
})
271+
})
197272
})

‎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. */

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access'
7676
import { reassignSharedResourceOwnershipForWorkspaceMemberRemovalTx } from '@/lib/workspaces/resource-handoff'
7777
import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils'
7878
import { endDirectoryMembershipTx } from '@/ee/scim/lib/identity/end-directory-membership'
79+
import { assertSelfLeaveNotScimManaged } from '@/ee/scim/lib/managed-membership'
7980

8081
export { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/identity-lock'
8182
export { WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR } from '@/lib/workspaces/utils'
@@ -1384,8 +1385,11 @@ export async function removeUserFromOrganization(
13841385
const result = await withInvitationSafeOrganizationAccessMutation(
13851386
{ userId, organizationId, scope: 'all' },
13861387
async (tx, { workspaceIds, invitationIds }) => {
1387-
if (actorUserId)
1388+
if (actorUserId) {
13881389
await requireMemberManagementAuthority(tx, organizationId, actorUserId, userId)
1390+
if (actorUserId === userId)
1391+
await assertSelfLeaveNotScimManaged({ organizationId, userId, executor: tx })
1392+
}
13891393
if (requireNoOrgWorkspaceAccess && workspaceIds.length > 0) {
13901394
const [remainingAccess] = await tx
13911395
.select({ id: permissions.id })

0 commit comments

Comments
 (0)