From 24640fb8bcae79a8322c19b9aae181b128d571a0 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 8 Jun 2026 11:28:27 -0700 Subject: [PATCH 1/3] improvement(perms): member removal reassignment policies --- .../[id]/members/[memberId]/route.ts | 8 +- .../[id]/members/[memberId]/route.ts | 8 +- .../[id]/members/[memberId]/route.ts | 69 ++++++- .../v1/admin/workspaces/[id]/members/route.ts | 71 ++++++- .../app/api/workspaces/members/[id]/route.ts | 99 ++++----- .../lib/billing/organizations/membership.ts | 72 +++++-- apps/sim/lib/workspaces/utils.test.ts | 108 ++++++++++ apps/sim/lib/workspaces/utils.ts | 191 +++++++++++++++++- 8 files changed, 538 insertions(+), 88 deletions(-) create mode 100644 apps/sim/lib/workspaces/utils.test.ts diff --git a/apps/sim/app/api/organizations/[id]/members/[memberId]/route.ts b/apps/sim/app/api/organizations/[id]/members/[memberId]/route.ts index 270d62374fc..0cb2dd979b4 100644 --- a/apps/sim/app/api/organizations/[id]/members/[memberId]/route.ts +++ b/apps/sim/app/api/organizations/[id]/members/[memberId]/route.ts @@ -13,6 +13,7 @@ import { getUserUsageData } from '@/lib/billing/core/usage' import { removeExternalUserFromOrganizationWorkspaces, removeUserFromOrganization, + WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR, } from '@/lib/billing/organizations/membership' import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -340,7 +341,9 @@ export const DELETE = withRouteHandler( ? 404 : error === 'User is an organization member' ? 409 - : 500 + : error === WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR + ? 400 + : 500 return NextResponse.json({ error }, { status }) } @@ -406,6 +409,9 @@ export const DELETE = withRouteHandler( if (result.error === 'Member not found') { return NextResponse.json({ error: result.error }, { status: 404 }) } + if (result.error === WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR) { + return NextResponse.json({ error: result.error }, { status: 400 }) + } return NextResponse.json({ error: result.error }, { status: 500 }) } diff --git a/apps/sim/app/api/v1/admin/organizations/[id]/members/[memberId]/route.ts b/apps/sim/app/api/v1/admin/organizations/[id]/members/[memberId]/route.ts index 7a33e113133..6bb367feb26 100644 --- a/apps/sim/app/api/v1/admin/organizations/[id]/members/[memberId]/route.ts +++ b/apps/sim/app/api/v1/admin/organizations/[id]/members/[memberId]/route.ts @@ -36,7 +36,10 @@ import { } from '@/lib/api/contracts/v1/admin' import { parseRequest } from '@/lib/api/server' import { getOrgMemberLedgerByUser } from '@/lib/billing/core/organization' -import { removeUserFromOrganization } from '@/lib/billing/organizations/membership' +import { + removeUserFromOrganization, + WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR, +} from '@/lib/billing/organizations/membership' import { isBillingEnabled } from '@/lib/core/config/feature-flags' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { withAdminAuthParams } from '@/app/api/v1/admin/middleware' @@ -261,6 +264,9 @@ export const DELETE = withRouteHandler( if (result.error === 'Member not found') { return notFoundResponse('Member') } + if (result.error === WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR) { + return badRequestResponse(result.error) + } return internalErrorResponse(result.error || 'Failed to remove member') } diff --git a/apps/sim/app/api/v1/admin/workspaces/[id]/members/[memberId]/route.ts b/apps/sim/app/api/v1/admin/workspaces/[id]/members/[memberId]/route.ts index 72ed9bf961d..e8552fdf091 100644 --- a/apps/sim/app/api/v1/admin/workspaces/[id]/members/[memberId]/route.ts +++ b/apps/sim/app/api/v1/admin/workspaces/[id]/members/[memberId]/route.ts @@ -22,7 +22,7 @@ */ import { db } from '@sim/db' -import { permissions, user } from '@sim/db/schema' +import { permissionGroupMember, permissions, user, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { and, eq } from 'drizzle-orm' import { @@ -32,10 +32,16 @@ import { } from '@/lib/api/contracts/v1/admin' import { parseRequest } from '@/lib/api/server' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' -import { revokeWorkspaceCredentialMemberships } from '@/lib/credentials/access' +import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access' import { getWorkspaceById } from '@/lib/workspaces/permissions/utils' +import { + reassignWorkflowOwnershipForWorkspaceMemberRemovalTx, + transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx, + WorkspaceBillingAccountRemovalError, +} from '@/lib/workspaces/utils' import { withAdminAuthParams } from '@/app/api/v1/admin/middleware' import { + badRequestResponse, internalErrorResponse, notFoundResponse, singleResponse, @@ -147,6 +153,19 @@ export const PATCH = withRouteHandler( return notFoundResponse('Workspace member') } + const [workspaceBilling] = await db + .select({ billedAccountUserId: workspace.billedAccountUserId }) + .from(workspace) + .where(eq(workspace.id, workspaceId)) + .limit(1) + + if ( + workspaceBilling?.billedAccountUserId === existingMember.userId && + permissionLevel !== 'admin' + ) { + return badRequestResponse('Workspace billing account must retain admin permissions') + } + const now = new Date() await db @@ -218,9 +237,48 @@ export const DELETE = withRouteHandler( return notFoundResponse('Workspace member') } - await db.delete(permissions).where(eq(permissions.id, memberId)) + const [workspaceBilling] = await db + .select({ billedAccountUserId: workspace.billedAccountUserId }) + .from(workspace) + .where(eq(workspace.id, workspaceId)) + .limit(1) + + if (workspaceBilling?.billedAccountUserId === existingMember.userId) { + return badRequestResponse( + 'Cannot remove the workspace billing account. Please reassign billing first.' + ) + } + + await db.transaction(async (tx) => { + await transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx({ + tx, + workspaceId, + departingUserId: existingMember.userId, + }) - await revokeWorkspaceCredentialMemberships(workspaceId, existingMember.userId) + const workflowOwnershipReassignment = + await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx, + workspaceIds: [workspaceId], + departingUserId: existingMember.userId, + }) + if (workflowOwnershipReassignment.unresolved.length > 0) { + throw new WorkspaceBillingAccountRemovalError() + } + + await tx.delete(permissions).where(eq(permissions.id, memberId)) + + await revokeWorkspaceCredentialMembershipsTx(tx, workspaceId, existingMember.userId) + + await tx + .delete(permissionGroupMember) + .where( + and( + eq(permissionGroupMember.userId, existingMember.userId), + eq(permissionGroupMember.workspaceId, workspaceId) + ) + ) + }) logger.info(`Admin API: Removed member ${memberId} from workspace ${workspaceId}`, { userId: existingMember.userId, @@ -233,6 +291,9 @@ export const DELETE = withRouteHandler( workspaceId, }) } catch (error) { + if (error instanceof WorkspaceBillingAccountRemovalError) { + return badRequestResponse(error.message) + } logger.error('Admin API: Failed to remove workspace member', { error, workspaceId, memberId }) return internalErrorResponse('Failed to remove workspace member') } diff --git a/apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts b/apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts index f7ab35c47d2..9cf5c2e3866 100644 --- a/apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts +++ b/apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts @@ -31,7 +31,13 @@ */ import { db } from '@sim/db' -import { permissions, user, workspaceEnvironment } from '@sim/db/schema' +import { + permissionGroupMember, + permissions, + user, + workspace, + workspaceEnvironment, +} from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { and, count, eq } from 'drizzle-orm' @@ -42,11 +48,18 @@ import { } from '@/lib/api/contracts/v1/admin' import { parseRequest } from '@/lib/api/server' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access' import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment' import { applyWorkspaceAutoAddGroup } from '@/lib/permission-groups/auto-add' import { getWorkspaceById } from '@/lib/workspaces/permissions/utils' +import { + reassignWorkflowOwnershipForWorkspaceMemberRemovalTx, + transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx, + WorkspaceBillingAccountRemovalError, +} from '@/lib/workspaces/utils' import { withAdminAuthParams } from '@/app/api/v1/admin/middleware' import { + badRequestResponse, internalErrorResponse, listResponse, notFoundResponse, @@ -143,6 +156,16 @@ export const POST = withRouteHandler( return notFoundResponse('Workspace') } + const [workspaceBilling] = await db + .select({ billedAccountUserId: workspace.billedAccountUserId }) + .from(workspace) + .where(eq(workspace.id, workspaceId)) + .limit(1) + + if (workspaceBilling?.billedAccountUserId === userId && permissionLevel !== 'admin') { + return badRequestResponse('Workspace billing account must retain admin permissions') + } + const [userData] = await db .select({ id: user.id, name: user.name, email: user.email, image: user.image }) .from(user) @@ -282,6 +305,18 @@ export const DELETE = withRouteHandler( return notFoundResponse('Workspace') } + const [workspaceBilling] = await db + .select({ billedAccountUserId: workspace.billedAccountUserId }) + .from(workspace) + .where(eq(workspace.id, workspaceId)) + .limit(1) + + if (workspaceBilling?.billedAccountUserId === userId) { + return badRequestResponse( + 'Cannot remove the workspace billing account. Please reassign billing first.' + ) + } + const [existingPermission] = await db .select({ id: permissions.id }) .from(permissions) @@ -298,12 +333,44 @@ export const DELETE = withRouteHandler( return notFoundResponse('Workspace member') } - await db.delete(permissions).where(eq(permissions.id, existingPermission.id)) + await db.transaction(async (tx) => { + await transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx({ + tx, + workspaceId, + departingUserId: userId, + }) + + const workflowOwnershipReassignment = + await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx, + workspaceIds: [workspaceId], + departingUserId: userId, + }) + if (workflowOwnershipReassignment.unresolved.length > 0) { + throw new WorkspaceBillingAccountRemovalError() + } + + await tx.delete(permissions).where(eq(permissions.id, existingPermission.id)) + + await revokeWorkspaceCredentialMembershipsTx(tx, workspaceId, userId) + + await tx + .delete(permissionGroupMember) + .where( + and( + eq(permissionGroupMember.userId, userId), + eq(permissionGroupMember.workspaceId, workspaceId) + ) + ) + }) logger.info(`Admin API: Removed user ${userId} from workspace ${workspaceId}`) return singleResponse({ removed: true, userId, workspaceId }) } catch (error) { + if (error instanceof WorkspaceBillingAccountRemovalError) { + return badRequestResponse(error.message) + } logger.error('Admin API: Failed to remove workspace member', { error, workspaceId, diff --git a/apps/sim/app/api/workspaces/members/[id]/route.ts b/apps/sim/app/api/workspaces/members/[id]/route.ts index 74b3c33ed64..66bedfca965 100644 --- a/apps/sim/app/api/workspaces/members/[id]/route.ts +++ b/apps/sim/app/api/workspaces/members/[id]/route.ts @@ -2,7 +2,6 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { db } from '@sim/db' import { member, permissionGroupMember, permissions, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { generateId } from '@sim/utils/id' import { and, eq } from 'drizzle-orm' import { type NextRequest, NextResponse } from 'next/server' import { removeWorkspaceMemberContract } from '@/lib/api/contracts/invitations' @@ -14,6 +13,11 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access' import { captureServerEvent } from '@/lib/posthog/server' import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils' +import { + reassignWorkflowOwnershipForWorkspaceMemberRemovalTx, + transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx, + WorkspaceBillingAccountRemovalError, +} from '@/lib/workspaces/utils' const logger = createLogger('WorkspaceMemberAPI') @@ -116,78 +120,49 @@ export const DELETE = withRouteHandler( const organizationId = workspaceRow[0].organizationId - const ownershipTransferred = await db.transaction(async (tx) => { - let didTransferOwnership = false + const { ownershipTransferred, workflowOwnershipReassignment } = await db.transaction( + async (tx) => { + const didTransferOwnership = + await transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx({ + tx, + workspaceId, + departingUserId: userId, + }) - if (isRemovingWorkspaceOwner) { - /** - * Invariant: the billed account is the org owner for org workspaces, - * the owner for personal workspaces, and a workspace admin for - * grandfathered shared workspaces. - */ - const newOwnerId = workspaceRow[0].billedAccountUserId + const workflowOwnershipReassignment = + await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx, + workspaceIds: [workspaceId], + departingUserId: userId, + }) + if (workflowOwnershipReassignment.unresolved.length > 0) { + throw new WorkspaceBillingAccountRemovalError() + } await tx - .update(workspace) - .set({ ownerId: newOwnerId, updatedAt: new Date() }) - .where(eq(workspace.id, workspaceId)) - - const [existingNewOwnerPermission] = await tx - .select({ id: permissions.id }) - .from(permissions) + .delete(permissions) .where( and( - eq(permissions.userId, newOwnerId), + eq(permissions.userId, userId), eq(permissions.entityType, 'workspace'), eq(permissions.entityId, workspaceId) ) ) - .limit(1) - if (existingNewOwnerPermission) { - await tx - .update(permissions) - .set({ permissionType: 'admin', updatedAt: new Date() }) - .where(eq(permissions.id, existingNewOwnerPermission.id)) - } else { - const now = new Date() - await tx.insert(permissions).values({ - id: generateId(), - userId: newOwnerId, - entityType: 'workspace', - entityId: workspaceId, - permissionType: 'admin', - createdAt: now, - updatedAt: now, - }) - } - - didTransferOwnership = true - } + await revokeWorkspaceCredentialMembershipsTx(tx, workspaceId, userId) - await tx - .delete(permissions) - .where( - and( - eq(permissions.userId, userId), - eq(permissions.entityType, 'workspace'), - eq(permissions.entityId, workspaceId) - ) - ) - - await revokeWorkspaceCredentialMembershipsTx(tx, workspaceId, userId) - - await tx - .delete(permissionGroupMember) - .where( - and( - eq(permissionGroupMember.userId, userId), - eq(permissionGroupMember.workspaceId, workspaceId) + await tx + .delete(permissionGroupMember) + .where( + and( + eq(permissionGroupMember.userId, userId), + eq(permissionGroupMember.workspaceId, workspaceId) + ) ) - ) - return didTransferOwnership - }) + return { ownershipTransferred: didTransferOwnership, workflowOwnershipReassignment } + } + ) /** * Seats are tied to organization membership (one per member), so a @@ -275,6 +250,7 @@ export const DELETE = withRouteHandler( removedUserRole: userPermission?.permissionType ?? 'owner', selfRemoval: isSelf, ownershipTransferred, + workflowOwnershipReassignment, organizationRemoval, seatReduction, }, @@ -283,6 +259,9 @@ export const DELETE = withRouteHandler( return NextResponse.json({ success: true }) } catch (error) { + if (error instanceof WorkspaceBillingAccountRemovalError) { + return NextResponse.json({ error: error.message }, { status: 400 }) + } logger.error('Error removing workspace member:', error) return NextResponse.json({ error: 'Failed to remove workspace member' }, { status: 500 }) } diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index 1e4960f4f08..cd57cb28d07 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -31,6 +31,12 @@ import { validateSeatAvailability } from '@/lib/billing/validation/seat-manageme import { OUTBOX_EVENT_TYPES } from '@/lib/billing/webhooks/outbox-handlers' import { enqueueOutboxEvent } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' +import { + reassignWorkflowOwnershipForWorkspaceMemberRemovalTx, + WorkspaceBillingAccountRemovalError, +} from '@/lib/workspaces/utils' + +export { WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR } from '@/lib/workspaces/utils' const logger = createLogger('OrganizationMembership') @@ -343,6 +349,8 @@ async function reassignOwnedOrganizationWorkspacesTx({ organizationId: string workspaceIds: string[] }) { + if (workspaceIds.length === 0) return 0 + const [ownerMembership] = await tx .select({ userId: member.userId }) .from(member) @@ -350,7 +358,7 @@ async function reassignOwnedOrganizationWorkspacesTx({ .limit(1) const ownerId = ownerMembership?.userId - if (!ownerId || ownerId === userId || workspaceIds.length === 0) return 0 + if (!ownerId || ownerId === userId) return 0 const reassignedWorkspaces = await tx .update(workspace) @@ -362,25 +370,15 @@ async function reassignOwnedOrganizationWorkspacesTx({ inArray(workspace.id, workspaceIds) ) ) - .returning({ id: workspace.id }) + .returning({ + id: workspace.id, + }) - if (reassignedWorkspaces.length === 0) return 0 + if (reassignedWorkspaces.length === 0) { + return 0 + } const now = new Date() - await tx - .update(permissions) - .set({ permissionType: 'admin', updatedAt: now }) - .where( - and( - eq(permissions.userId, ownerId), - eq(permissions.entityType, 'workspace'), - inArray( - permissions.entityId, - reassignedWorkspaces.map((row) => row.id) - ) - ) - ) - await tx .insert(permissions) .values( @@ -394,7 +392,10 @@ async function reassignOwnedOrganizationWorkspacesTx({ updatedAt: now, })) ) - .onConflictDoNothing() + .onConflictDoUpdate({ + target: [permissions.userId, permissions.entityType, permissions.entityId], + set: { permissionType: 'admin', updatedAt: now }, + }) return reassignedWorkspaces.length } @@ -1075,6 +1076,16 @@ export async function removeUserFromOrganization( workspaceIds, }) + const workflowOwnershipReassignment = + await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx, + workspaceIds, + departingUserId: userId, + }) + if (workflowOwnershipReassignment.unresolved.length > 0) { + throw new WorkspaceBillingAccountRemovalError() + } + const deletedPerms = await tx .delete(permissions) .where( @@ -1182,6 +1193,10 @@ export async function removeUserFromOrganization( return { success: true, removed: true, billingActions } } catch (error) { + if (error instanceof WorkspaceBillingAccountRemovalError) { + return { success: false, error: error.message, billingActions } + } + logger.error('Failed to remove user from organization', { userId, organizationId, @@ -1254,6 +1269,16 @@ export async function removeExternalUserFromOrganizationWorkspaces(params: { workspaceIds, }) + const workflowOwnershipReassignment = + await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx, + workspaceIds, + departingUserId: userId, + }) + if (workflowOwnershipReassignment.unresolved.length > 0) { + throw new WorkspaceBillingAccountRemovalError() + } + const deletedPermissions = await tx .delete(permissions) .where( @@ -1337,6 +1362,17 @@ export async function removeExternalUserFromOrganizationWorkspaces(params: { pendingInvitationsCancelled, } } catch (error) { + if (error instanceof WorkspaceBillingAccountRemovalError) { + return { + success: false, + error: error.message, + workspaceAccessRevoked: 0, + permissionGroupsRevoked: 0, + credentialMembershipsRevoked: 0, + pendingInvitationsCancelled: 0, + } + } + logger.error('Failed to remove external workspace member from organization workspaces', { organizationId, userId, diff --git a/apps/sim/lib/workspaces/utils.test.ts b/apps/sim/lib/workspaces/utils.test.ts new file mode 100644 index 00000000000..677475d1c5d --- /dev/null +++ b/apps/sim/lib/workspaces/utils.test.ts @@ -0,0 +1,108 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@sim/db', () => ({ + db: {}, +})) + +import { reassignWorkflowOwnershipForWorkspaceMemberRemovalTx } from '@/lib/workspaces/utils' + +function createSelectChain(result: unknown) { + const limit = vi.fn().mockResolvedValue(result) + const where = vi.fn().mockReturnValue({ limit }) + const from = vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue(result), + limit, + }) + + return { from, where, limit } +} + +function createGroupedSelectChain(result: unknown) { + const groupBy = vi.fn().mockResolvedValue(result) + const where = vi.fn().mockReturnValue({ groupBy }) + const from = vi.fn().mockReturnValue({ where }) + + return { from, where, groupBy } +} + +function createUpdateChain(result: unknown) { + const returning = vi.fn().mockResolvedValue(result) + const where = vi.fn().mockReturnValue({ returning }) + const set = vi.fn().mockReturnValue({ where }) + + return { set, where, returning } +} + +describe('reassignWorkflowOwnershipForWorkspaceMemberRemovalTx', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('reassigns departing member workflows to the workspace billed account', async () => { + const workspaceSelect = createSelectChain([ + { id: 'workspace-1', billedAccountUserId: 'billed-1' }, + ]) + const workflowCountSelect = createGroupedSelectChain([ + { workspaceId: 'workspace-1', workflowCount: 2 }, + ]) + const workflowUpdate = createUpdateChain([]) + const tx = { + select: vi.fn().mockReturnValueOnce(workspaceSelect).mockReturnValueOnce(workflowCountSelect), + update: vi.fn().mockReturnValue(workflowUpdate), + } + + const result = await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx: tx as any, + workspaceIds: ['workspace-1'], + departingUserId: 'departing-1', + }) + + expect(tx.update).toHaveBeenCalledTimes(1) + expect(workflowUpdate.set).toHaveBeenCalledWith( + expect.objectContaining({ updatedAt: expect.any(Date) }) + ) + expect(result).toEqual({ + reassigned: [{ workspaceId: 'workspace-1', newWorkflowUserId: 'billed-1', workflowCount: 2 }], + unresolved: [], + }) + }) + + it('marks a workspace unresolved when the departing member is the billed account', async () => { + const workspaceSelect = createSelectChain([ + { id: 'workspace-1', billedAccountUserId: 'departing-1' }, + ]) + const tx = { + select: vi.fn().mockReturnValue(workspaceSelect), + update: vi.fn(), + } + + const result = await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx: tx as any, + workspaceIds: ['workspace-1'], + departingUserId: 'departing-1', + }) + + expect(tx.update).not.toHaveBeenCalled() + expect(result).toEqual({ reassigned: [], unresolved: ['workspace-1'] }) + }) + + it('marks a workspace unresolved when no billed account is configured', async () => { + const workspaceSelect = createSelectChain([{ id: 'workspace-1', billedAccountUserId: null }]) + const tx = { + select: vi.fn().mockReturnValue(workspaceSelect), + update: vi.fn(), + } + + const result = await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx: tx as any, + workspaceIds: ['workspace-1'], + departingUserId: 'departing-1', + }) + + expect(tx.update).not.toHaveBeenCalled() + expect(result).toEqual({ reassigned: [], unresolved: ['workspace-1'] }) + }) +}) diff --git a/apps/sim/lib/workspaces/utils.ts b/apps/sim/lib/workspaces/utils.ts index f1ddf97c444..3984427ddbe 100644 --- a/apps/sim/lib/workspaces/utils.ts +++ b/apps/sim/lib/workspaces/utils.ts @@ -1,7 +1,9 @@ import { db } from '@sim/db' -import { permissions, workspace as workspaceTable } from '@sim/db/schema' +import { permissions, workflow, workspace as workspaceTable } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, desc, eq, isNull, ne, sql } from 'drizzle-orm' +import { generateId } from '@sim/utils/id' +import { and, count, desc, eq, inArray, isNull, ne, sql } from 'drizzle-orm' +import type { DbOrTx } from '@/lib/db/types' const logger = createLogger('WorkspaceUtils') @@ -82,6 +84,191 @@ export interface ReassignBilledAccountResult { unresolved: string[] } +export interface ReassignWorkflowOwnershipResult { + reassigned: Array<{ workspaceId: string; newWorkflowUserId: string; workflowCount: number }> + unresolved: string[] +} + +export const WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR = + 'Cannot remove the workspace billing account. Please reassign billing first.' + +export class WorkspaceBillingAccountRemovalError extends Error { + constructor() { + super(WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR) + this.name = 'WorkspaceBillingAccountRemovalError' + } +} + +export async function transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx({ + tx, + workspaceId, + departingUserId, +}: { + tx: DbOrTx + workspaceId: string + departingUserId: string +}): Promise { + const [workspaceRow] = await tx + .select({ + ownerId: workspaceTable.ownerId, + billedAccountUserId: workspaceTable.billedAccountUserId, + }) + .from(workspaceTable) + .where(eq(workspaceTable.id, workspaceId)) + .limit(1) + + if (!workspaceRow || workspaceRow.ownerId !== departingUserId) { + return false + } + + const newOwnerId = workspaceRow.billedAccountUserId + if (newOwnerId === departingUserId) { + throw new WorkspaceBillingAccountRemovalError() + } + + await tx + .update(workspaceTable) + .set({ ownerId: newOwnerId, updatedAt: new Date() }) + .where(eq(workspaceTable.id, workspaceId)) + + const [existingNewOwnerPermission] = await tx + .select({ id: permissions.id }) + .from(permissions) + .where( + and( + eq(permissions.userId, newOwnerId), + eq(permissions.entityType, 'workspace'), + eq(permissions.entityId, workspaceId) + ) + ) + .limit(1) + + if (existingNewOwnerPermission) { + await tx + .update(permissions) + .set({ permissionType: 'admin', updatedAt: new Date() }) + .where(eq(permissions.id, existingNewOwnerPermission.id)) + return true + } + + const now = new Date() + await tx.insert(permissions).values({ + id: generateId(), + userId: newOwnerId, + entityType: 'workspace', + entityId: workspaceId, + permissionType: 'admin', + createdAt: now, + updatedAt: now, + }) + + return true +} + +/** + * Reassigns workflows owned by a user who is about to lose access to one or more workspaces. + * + * Workflow execution, webhook provider config, and environment variables intentionally resolve + * through `workflow.userId`, so that identity must remain an active workspace identity. The + * replacement is the workspace billed account: the same stable identity used for server-side + * billing/permission actor resolution, and one the member-removal routes protect from removal. + */ +export async function reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx, + workspaceIds, + departingUserId, +}: { + tx: DbOrTx + workspaceIds: string[] + departingUserId: string +}): Promise { + const uniqueWorkspaceIds = Array.from(new Set(workspaceIds.filter(Boolean))) + if (uniqueWorkspaceIds.length === 0) { + return { reassigned: [], unresolved: [] } + } + + const workspaceRows = await tx + .select({ + id: workspaceTable.id, + billedAccountUserId: workspaceTable.billedAccountUserId, + }) + .from(workspaceTable) + .where(inArray(workspaceTable.id, uniqueWorkspaceIds)) + + const reassigned: ReassignWorkflowOwnershipResult['reassigned'] = [] + const unresolved: string[] = [] + const reassignmentWorkspaceIds: string[] = [] + + for (const ws of workspaceRows) { + const replacementUserId = + ws.billedAccountUserId !== departingUserId ? ws.billedAccountUserId : null + + if (!replacementUserId) { + unresolved.push(ws.id) + continue + } + + reassignmentWorkspaceIds.push(ws.id) + } + + if (reassignmentWorkspaceIds.length > 0) { + const workflowCounts = await tx + .select({ + workspaceId: workflow.workspaceId, + workflowCount: count(workflow.id), + }) + .from(workflow) + .where( + and( + eq(workflow.userId, departingUserId), + inArray(workflow.workspaceId, reassignmentWorkspaceIds) + ) + ) + .groupBy(workflow.workspaceId) + + await tx + .update(workflow) + .set({ + userId: sql`( + select ${workspaceTable.billedAccountUserId} + from ${workspaceTable} + where ${workspaceTable.id} = ${workflow.workspaceId} + )`, + updatedAt: new Date(), + }) + .where( + and( + eq(workflow.userId, departingUserId), + inArray(workflow.workspaceId, reassignmentWorkspaceIds) + ) + ) + + const billedAccountByWorkspaceId = new Map( + workspaceRows.map((ws) => [ws.id, ws.billedAccountUserId]) + ) + for (const { workspaceId, workflowCount } of workflowCounts) { + if (!workspaceId || workflowCount === 0) continue + const newWorkflowUserId = billedAccountByWorkspaceId.get(workspaceId) + if (!newWorkflowUserId) continue + reassigned.push({ + workspaceId, + newWorkflowUserId, + workflowCount, + }) + } + } + + if (reassigned.length > 0 || unresolved.length > 0) { + logger.info('Reassigned workflow ownership for removed workspace member', { + departingUserId, + reassigned, + unresolved, + }) + } + + return { reassigned, unresolved } +} + /** * Reassigns `billedAccountUserId` on every workspace that points to `departingUserId` to * another eligible user, so the user can be deleted without violating the `workspace.billed_account_user_id` From 5493e386e4cef9ea5ca2603dcbddfd8748ddc8db Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 8 Jun 2026 11:42:04 -0700 Subject: [PATCH 2/3] improve UI notif --- .../settings/components/teammates/teammates.tsx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx index c6c37bf4c1c..2cf87bb1f04 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx @@ -1,6 +1,7 @@ 'use client' import { useCallback, useMemo, useState } from 'react' +import { getErrorMessage } from '@sim/utils/errors' import { useQueryClient } from '@tanstack/react-query' import { useParams, useRouter } from 'next/navigation' import { @@ -15,6 +16,7 @@ import { MoreHorizontal, Plus, Search, + toast, } from '@/components/emcn' import type { WorkspacePermission } from '@/lib/api/contracts/workspaces' import { @@ -284,7 +286,19 @@ export function Teammates() { className='text-[var(--text-error)]' onSelect={() => { if (teammate.userId) { - removeMember.mutate({ userId: teammate.userId, workspaceId }) + removeMember.mutate( + { userId: teammate.userId, workspaceId }, + { + onError: (error) => { + toast.error("Couldn't remove teammate", { + description: getErrorMessage( + error, + 'Please try again in a moment.' + ), + }) + }, + } + ) } }} > From 3b84506ffce695bc512ed18e49437cc4027efbfe Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 8 Jun 2026 11:49:00 -0700 Subject: [PATCH 3/3] address comments --- apps/sim/lib/workspaces/utils.test.ts | 30 ++++++++++++++++++-- apps/sim/lib/workspaces/utils.ts | 41 ++++++++++++++++----------- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/apps/sim/lib/workspaces/utils.test.ts b/apps/sim/lib/workspaces/utils.test.ts index 677475d1c5d..00227a9d795 100644 --- a/apps/sim/lib/workspaces/utils.test.ts +++ b/apps/sim/lib/workspaces/utils.test.ts @@ -74,8 +74,11 @@ describe('reassignWorkflowOwnershipForWorkspaceMemberRemovalTx', () => { const workspaceSelect = createSelectChain([ { id: 'workspace-1', billedAccountUserId: 'departing-1' }, ]) + const workflowCountSelect = createGroupedSelectChain([ + { workspaceId: 'workspace-1', workflowCount: 1 }, + ]) const tx = { - select: vi.fn().mockReturnValue(workspaceSelect), + select: vi.fn().mockReturnValueOnce(workspaceSelect).mockReturnValueOnce(workflowCountSelect), update: vi.fn(), } @@ -89,10 +92,33 @@ describe('reassignWorkflowOwnershipForWorkspaceMemberRemovalTx', () => { expect(result).toEqual({ reassigned: [], unresolved: ['workspace-1'] }) }) + it('does not mark a workspace unresolved when the departing billing account owns no workflows', async () => { + const workspaceSelect = createSelectChain([ + { id: 'workspace-1', billedAccountUserId: 'departing-1' }, + ]) + const workflowCountSelect = createGroupedSelectChain([]) + const tx = { + select: vi.fn().mockReturnValueOnce(workspaceSelect).mockReturnValueOnce(workflowCountSelect), + update: vi.fn(), + } + + const result = await reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ + tx: tx as any, + workspaceIds: ['workspace-1'], + departingUserId: 'departing-1', + }) + + expect(tx.update).not.toHaveBeenCalled() + expect(result).toEqual({ reassigned: [], unresolved: [] }) + }) + it('marks a workspace unresolved when no billed account is configured', async () => { const workspaceSelect = createSelectChain([{ id: 'workspace-1', billedAccountUserId: null }]) + const workflowCountSelect = createGroupedSelectChain([ + { workspaceId: 'workspace-1', workflowCount: 1 }, + ]) const tx = { - select: vi.fn().mockReturnValue(workspaceSelect), + select: vi.fn().mockReturnValueOnce(workspaceSelect).mockReturnValueOnce(workflowCountSelect), update: vi.fn(), } diff --git a/apps/sim/lib/workspaces/utils.ts b/apps/sim/lib/workspaces/utils.ts index 3984427ddbe..88ea402ecdf 100644 --- a/apps/sim/lib/workspaces/utils.ts +++ b/apps/sim/lib/workspaces/utils.ts @@ -122,7 +122,7 @@ export async function transferWorkspaceOwnershipToBilledAccountForMemberRemovalT } const newOwnerId = workspaceRow.billedAccountUserId - if (newOwnerId === departingUserId) { + if (!newOwnerId || newOwnerId === departingUserId) { throw new WorkspaceBillingAccountRemovalError() } @@ -198,8 +198,29 @@ export async function reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ const reassigned: ReassignWorkflowOwnershipResult['reassigned'] = [] const unresolved: string[] = [] const reassignmentWorkspaceIds: string[] = [] + const workflowCounts = await tx + .select({ + workspaceId: workflow.workspaceId, + workflowCount: count(workflow.id), + }) + .from(workflow) + .where( + and(eq(workflow.userId, departingUserId), inArray(workflow.workspaceId, uniqueWorkspaceIds)) + ) + .groupBy(workflow.workspaceId) + + const workflowCountsByWorkspaceId = new Map() + for (const { workspaceId, workflowCount } of workflowCounts) { + if (!workspaceId || workflowCount === 0) continue + workflowCountsByWorkspaceId.set(workspaceId, workflowCount) + } for (const ws of workspaceRows) { + const workflowCount = workflowCountsByWorkspaceId.get(ws.id) ?? 0 + if (workflowCount === 0) { + continue + } + const replacementUserId = ws.billedAccountUserId !== departingUserId ? ws.billedAccountUserId : null @@ -212,20 +233,6 @@ export async function reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ } if (reassignmentWorkspaceIds.length > 0) { - const workflowCounts = await tx - .select({ - workspaceId: workflow.workspaceId, - workflowCount: count(workflow.id), - }) - .from(workflow) - .where( - and( - eq(workflow.userId, departingUserId), - inArray(workflow.workspaceId, reassignmentWorkspaceIds) - ) - ) - .groupBy(workflow.workspaceId) - await tx .update(workflow) .set({ @@ -246,8 +253,8 @@ export async function reassignWorkflowOwnershipForWorkspaceMemberRemovalTx({ const billedAccountByWorkspaceId = new Map( workspaceRows.map((ws) => [ws.id, ws.billedAccountUserId]) ) - for (const { workspaceId, workflowCount } of workflowCounts) { - if (!workspaceId || workflowCount === 0) continue + for (const workspaceId of reassignmentWorkspaceIds) { + const workflowCount = workflowCountsByWorkspaceId.get(workspaceId) ?? 0 const newWorkflowUserId = billedAccountByWorkspaceId.get(workspaceId) if (!newWorkflowUserId) continue reassigned.push({