diff --git a/apps/sim/app/api/organizations/[id]/transfer-ownership/route.ts b/apps/sim/app/api/organizations/[id]/transfer-ownership/route.ts index 53ef7dab812..7a6fc78758b 100644 --- a/apps/sim/app/api/organizations/[id]/transfer-ownership/route.ts +++ b/apps/sim/app/api/organizations/[id]/transfer-ownership/route.ts @@ -149,16 +149,21 @@ export const POST = withRouteHandler( userId: session.user.id, organizationId, memberId: selfMember.id, + actorUserId: session.user.id, spareSessionToken: session.session.token, }) deliverRunStops(removeResult.stoppedRuns) if (!removeResult.success) { - logger.error('Transfer succeeded but self-removal failed', { + const outcome = { organizationId, userId: session.user.id, error: removeResult.error, - }) + refusalCode: removeResult.refusalCode, + } + if (removeResult.refusalCode) + logger.info('Transfer succeeded; policy kept the previous owner as admin', outcome) + else logger.error('Transfer succeeded but self-removal failed', outcome) return NextResponse.json( { success: true, diff --git a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx index c0e4946dd0c..6f44c352d39 100644 --- a/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx +++ b/apps/sim/app/o/[organizationId]/components/organization-sidebar/components/organization-header/organization-header.tsx @@ -66,7 +66,14 @@ export function OrganizationHeader({ const [isInviteModalOpen, setIsInviteModalOpen] = useState(false) const [isMenuOpen, setIsMenuOpen] = useState(false) const [draftName, setDraftName] = useState(null) - const initial = getOrganizationInitial(organization.name) + /** + * The name just saved, shown until the refreshed server prop replaces the one + * it was saved over; without it the old name flashes back in between. + */ + const [savedName, setSavedName] = useState<{ name: string; replaces: string } | null>(null) + if (savedName && savedName.replaces !== organization.name) setSavedName(null) + const organizationName = savedName?.name ?? organization.name + const initial = getOrganizationInitial(organizationName) if (isCollapsed) { return ( @@ -103,21 +110,24 @@ export function OrganizationHeader({ const submitRename = () => { if (draftName === null || renameOrganization.isPending) return const name = draftName.trim() - if (!name || name === organization.name) { + if (!name || name === organizationName) { stopRenaming() return } renameOrganization.mutate( { orgId: organization.id, name }, { - onSuccess: stopRenaming, + onSuccess: () => { + setSavedName({ name, replaces: organization.name }) + stopRenaming() + }, onError: () => requestAnimationFrame(() => nameInputRef.current?.focus()), } ) } const logo = ( - + ) return ( @@ -156,7 +166,7 @@ export function OrganizationHeader({ leftAdornment={} rightAdornment={} > - {organization.name} + {organizationName} { event.preventDefault() - setDraftName(organization.name) + setDraftName(organizationName) }} > - + ) : ( - + )} {renameOrganization.isError && ( diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx index df43655f00c..8e25128c180 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/team-management/team-management.tsx @@ -1,7 +1,7 @@ 'use client' import { useState } from 'react' -import { Plus } from '@sim/emcn' +import { Plus, toast } from '@sim/emcn' import { createLogger } from '@sim/logger' import { isOrgAdminRole } from '@sim/platform-authz/predicates' import { getErrorMessage } from '@sim/utils/errors' @@ -305,6 +305,8 @@ export function TeamManagement({ if (result.left) { window.location.href = APP_ENTRY_PATH + } else if (result.warning) { + toast.warning(`Ownership transferred. ${result.warning}`) } } catch (error) { logger.error('Failed to transfer ownership', error) diff --git a/apps/sim/ee/scim/lib/managed-membership.integration.ts b/apps/sim/ee/scim/lib/managed-membership.integration.ts index 58ad21e7e34..e1550819cee 100644 --- a/apps/sim/ee/scim/lib/managed-membership.integration.ts +++ b/apps/sim/ee/scim/lib/managed-membership.integration.ts @@ -10,7 +10,7 @@ vi.mock('@/lib/core/config/env-flags', () => ({ })) async function loadRuntime() { - const [{ db }, schema, { eq, inArray, sql }, { alias }, membership, entitlement] = + const [{ db }, schema, { and, eq, inArray, sql }, { alias }, membership, entitlement] = await Promise.all([ import('@sim/db'), import('@sim/db/schema'), @@ -19,7 +19,27 @@ async function loadRuntime() { import('@/ee/scim/lib/managed-membership'), import('@/ee/scim/lib/entitlement'), ]) - return { db, schema, eq, inArray, sql, alias, ...membership, ...entitlement } + const [ + { removeOrganizationMemberRecord }, + { removeUserFromOrganization, transferOrganizationOwnership }, + ] = await Promise.all([ + import('@/lib/organizations/member-manager'), + import('@/lib/billing/organizations/membership'), + ]) + return { + db, + schema, + and, + eq, + inArray, + sql, + alias, + removeOrganizationMemberRecord, + removeUserFromOrganization, + transferOrganizationOwnership, + ...membership, + ...entitlement, + } } describe('SCIM managed membership in PostgreSQL', () => { @@ -29,6 +49,7 @@ describe('SCIM managed membership in PostgreSQL', () => { let connectionId: string let managedUserId: string let unmanagedUserId: string + let adminUserId: string let managedEmail: string beforeAll(async () => { @@ -42,6 +63,7 @@ describe('SCIM managed membership in PostgreSQL', () => { connectionId = generateId() managedUserId = generateId() unmanagedUserId = generateId() + adminUserId = generateId() managedEmail = `${managedUserId}@scim-membership.test` const { db, schema } = runtime const now = new Date() @@ -54,7 +76,7 @@ describe('SCIM managed membership in PostgreSQL', () => { })) ) await db.insert(schema.user).values( - [managedUserId, unmanagedUserId].map((id) => ({ + [managedUserId, unmanagedUserId, adminUserId].map((id) => ({ id, name: 'SCIM predicate test', email: `${id}@scim-membership.test`, @@ -99,7 +121,9 @@ describe('SCIM managed membership in PostgreSQL', () => { const { db, schema, eq, inArray } = runtime await db.delete(schema.subscription).where(eq(schema.subscription.referenceId, orgId)) await db.delete(schema.organization).where(inArray(schema.organization.id, [orgId, otherOrgId])) - await db.delete(schema.user).where(inArray(schema.user.id, [managedUserId, unmanagedUserId])) + await db + .delete(schema.user) + .where(inArray(schema.user.id, [managedUserId, unmanagedUserId, adminUserId])) }) async function invitee(organizationId: string, email: string) { @@ -194,4 +218,127 @@ describe('SCIM managed membership in PostgreSQL', () => { }) ).rejects.toMatchObject({ cause: { code: '42P01' } }) }) + + describe('removing a member', () => { + beforeEach(async () => { + const { db, schema } = runtime + await db.insert(schema.member).values([ + { id: generateId(), organizationId: orgId, userId: adminUserId, role: 'owner' }, + { id: generateId(), organizationId: orgId, userId: managedUserId, role: 'member' }, + { id: generateId(), organizationId: orgId, userId: unmanagedUserId, role: 'member' }, + ]) + }) + + async function isMember(userId: string) { + const { db, schema, and, eq } = runtime + const rows = await db + .select({ id: schema.member.id }) + .from(schema.member) + .where(and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, userId))) + return rows.length > 0 + } + + it('refuses a managed member leaving on their own and keeps the membership', async () => { + const { removeOrganizationMemberRecord } = runtime + await expect( + removeOrganizationMemberRecord({ + organizationId: orgId, + userId: managedUserId, + actorUserId: managedUserId, + }) + ).rejects.toMatchObject({ + detailCode: 'SCIM_MANAGED_MEMBERSHIP', + message: expect.stringContaining('Ask your identity provider administrator'), + }) + expect(await isMember(managedUserId)).toBe(true) + }) + + it('lets an administrator remove the same managed member', async () => { + const { removeOrganizationMemberRecord } = runtime + await expect( + removeOrganizationMemberRecord({ + organizationId: orgId, + userId: managedUserId, + actorUserId: adminUserId, + }) + ).resolves.toMatchObject({ membershipType: 'internal' }) + expect(await isMember(managedUserId)).toBe(false) + }) + + it('lets an unmanaged member leave on their own', async () => { + const { removeOrganizationMemberRecord } = runtime + await expect( + removeOrganizationMemberRecord({ + organizationId: orgId, + userId: unmanagedUserId, + actorUserId: unmanagedUserId, + }) + ).resolves.toMatchObject({ membershipType: 'internal' }) + expect(await isMember(unmanagedUserId)).toBe(false) + }) + }) + + describe('transferring ownership', () => { + beforeEach(async () => { + const { db, schema } = runtime + await db.insert(schema.member).values([ + { id: generateId(), organizationId: orgId, userId: managedUserId, role: 'owner' }, + { id: generateId(), organizationId: orgId, userId: adminUserId, role: 'member' }, + ]) + }) + + async function roleOf(userId: string) { + const { db, schema, and, eq } = runtime + const [row] = await db + .select({ role: schema.member.role }) + .from(schema.member) + .where(and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, userId))) + return row?.role + } + + it('transfers a managed owner’s ownership but keeps them as admin when they try to leave', async () => { + const { db, schema, and, eq, transferOrganizationOwnership, removeUserFromOrganization } = + runtime + await expect( + transferOrganizationOwnership({ + organizationId: orgId, + currentOwnerUserId: managedUserId, + newOwnerUserId: adminUserId, + }) + ).resolves.toMatchObject({ success: true }) + const [previousOwner] = await db + .select({ id: schema.member.id }) + .from(schema.member) + .where( + and(eq(schema.member.organizationId, orgId), eq(schema.member.userId, managedUserId)) + ) + await expect( + removeUserFromOrganization({ + userId: managedUserId, + organizationId: orgId, + memberId: previousOwner.id, + actorUserId: managedUserId, + }) + ).resolves.toMatchObject({ + success: false, + refusalCode: 'SCIM_MANAGED_MEMBERSHIP', + error: expect.stringContaining('Ask your identity provider administrator'), + }) + expect(await roleOf(managedUserId)).toBe('admin') + expect(await roleOf(adminUserId)).toBe('owner') + }) + + it('lets a managed owner transfer ownership and stay', async () => { + const { transferOrganizationOwnership } = runtime + await expect( + transferOrganizationOwnership({ + organizationId: orgId, + currentOwnerUserId: managedUserId, + newOwnerUserId: adminUserId, + }) + ).resolves.toMatchObject({ success: true }) + expect(await roleOf(managedUserId)).toBe('admin') + expect(await roleOf(adminUserId)).toBe('owner') + }) + }) }) diff --git a/apps/sim/ee/scim/lib/managed-membership.ts b/apps/sim/ee/scim/lib/managed-membership.ts index 8361c1af55a..2ab304e587f 100644 --- a/apps/sim/ee/scim/lib/managed-membership.ts +++ b/apps/sim/ee/scim/lib/managed-membership.ts @@ -14,7 +14,8 @@ import { isScimDeploymentEnabled, isScimEntitledForOrganization } from '@/ee/sci * remedy. * * Deliberately not applied to deprovisioning: an administrator must always be - * able to remove someone in an emergency, whatever the directory believes. + * able to remove someone in an emergency, whatever the directory believes. A + * member removing themselves is refused, since the directory would re-add them. */ /** @@ -63,6 +64,32 @@ export async function assertMembershipNotScimManaged(params: { userId: string executor: DbOrTx }): Promise { + await refuseIfDirectoryManaged( + params, + '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.' + ) +} + +/** + * Refuses a directory-managed member leaving on their own. Unlike an + * administrator's removal, which must always succeed, a self-leave only lasts + * until the next sync re-adds the member, so it is refused with the remedy. + */ +export async function assertSelfLeaveNotScimManaged(params: { + organizationId: string + userId: string + executor: DbOrTx +}): Promise { + await refuseIfDirectoryManaged( + params, + 'Your membership is managed by your organization’s identity provider. Ask your identity provider administrator to remove you there.' + ) +} + +async function refuseIfDirectoryManaged( + params: { organizationId: string; userId: string; executor: DbOrTx }, + message: string +): Promise { if (!isScimDeploymentEnabled()) return const [row] = await params.executor .select({ managed: scimManagedUserPredicate(params.organizationId, sql`${params.userId}`) }) @@ -74,10 +101,7 @@ export async function assertMembershipNotScimManaged(params: { * once a managed row is found, so the common case costs nothing. */ if (!(await isScimEntitledForOrganization(params.organizationId, params.executor))) return - throw new ForbiddenOperationError( - 'SCIM_MANAGED_MEMBERSHIP', - '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.' - ) + throw new ForbiddenOperationError('SCIM_MANAGED_MEMBERSHIP', message) } /** Refuses an invitation to someone the directory already provisions, while the directory can still sync. */ diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index a76ec3243e2..7b1ae93aa44 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -52,6 +52,7 @@ import { enqueueCancelAtPeriodEndSync, isCancelAtPeriodEndSettled, } from '@/lib/billing/webhooks/subscription-sync' +import { type ForbiddenDetailCode, ForbiddenOperationError } from '@/lib/core/application/forbidden' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { OrchestrationError } from '@/lib/core/orchestration/types' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' @@ -76,6 +77,7 @@ import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access' import { reassignSharedResourceOwnershipForWorkspaceMemberRemovalTx } from '@/lib/workspaces/resource-handoff' import { WorkspaceBillingAccountRemovalError } from '@/lib/workspaces/utils' import { endDirectoryMembershipTx } from '@/ee/scim/lib/identity/end-directory-membership' +import { assertSelfLeaveNotScimManaged } from '@/ee/scim/lib/managed-membership' export { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/identity-lock' export { WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR } from '@/lib/workspaces/utils' @@ -533,6 +535,8 @@ export interface RemoveMemberParams { export interface RemoveMemberResult { success: boolean error?: string + /** Set when policy refused the removal, as opposed to it failing. */ + refusalCode?: ForbiddenDetailCode /** * Whether the member row was actually deleted. `false` (with `success: true`) * when `requireNoOrgWorkspaceAccess` was set and the user still had workspace @@ -1384,8 +1388,11 @@ export async function removeUserFromOrganization( const result = await withInvitationSafeOrganizationAccessMutation( { userId, organizationId, scope: 'all' }, async (tx, { workspaceIds, invitationIds }) => { - if (actorUserId) + if (actorUserId) { await requireMemberManagementAuthority(tx, organizationId, actorUserId, userId) + if (actorUserId === userId) + await assertSelfLeaveNotScimManaged({ organizationId, userId, executor: tx }) + } if (requireNoOrgWorkspaceAccess && workspaceIds.length > 0) { const [remainingAccess] = await tx .select({ id: permissions.id }) @@ -1616,6 +1623,15 @@ export async function removeUserFromOrganization( return { success: false, error: error.message, billingActions, stoppedRuns: [] } } if (onError === 'throw') throw error + if (error instanceof ForbiddenOperationError) { + return { + success: false, + error: error.message, + refusalCode: error.detailCode, + billingActions, + stoppedRuns: [], + } + } logger.error('Failed to remove user from organization', { userId,