diff --git a/apps/sim/app/api/workspaces/members/[id]/route.ts b/apps/sim/app/api/workspaces/members/[id]/route.ts index 5472850c5e6..1a028d91813 100644 --- a/apps/sim/app/api/workspaces/members/[id]/route.ts +++ b/apps/sim/app/api/workspaces/members/[id]/route.ts @@ -13,7 +13,10 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access' import { captureServerEvent } from '@/lib/posthog/server' import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access' -import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils' +import { + hasWorkspaceAdminAccess, + isOrganizationAdminOrOwner, +} from '@/lib/workspaces/permissions/utils' import { reassignWorkflowOwnershipForWorkspaceMemberRemovalTx, transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx, @@ -51,6 +54,20 @@ export const DELETE = withRouteHandler( return NextResponse.json({ error: 'Workspace not found' }, { status: 404 }) } + const organizationId = workspaceRow[0].organizationId + + /** + * Authority is settled before anything is answered about the target, so + * the standing-specific replies below only ever describe someone the + * caller can already see in the members list. + */ + const hasAdminAccess = await hasWorkspaceAdminAccess(session.user.id, workspaceId) + const isSelf = userId === session.user.id + + if (!hasAdminAccess && !isSelf) { + return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 }) + } + if (workspaceRow[0].billedAccountUserId === userId) { return NextResponse.json( { error: 'Cannot remove the workspace billing account. Please reassign billing first.' }, @@ -58,6 +75,31 @@ export const DELETE = withRouteHandler( ) } + /** + * Organization admins hold workspace admin across the whole organization + * through `member.role`, not through a `permissions` row, so removal has + * nothing to revoke. Left to fall through, the two shapes failed in two + * different ways: with no row it answered "user not found in workspace" + * about someone listed as an Admin on the very screen the caller clicked + * from, and with a row it deleted a grant the derived one immediately + * replaced — while the seat reconciliation below counts rows only, so + * that no-op could still drop the admin's organization membership. + * + * Mirrored by `workspaceMemberRemovalLockReason` on the client, and by the + * same guard on `PATCH /api/workspaces/[id]/permissions`, which refuses to + * re-role an organization admin for the same reason. + */ + if (organizationId && (await isOrganizationAdminOrOwner(userId, organizationId))) { + return NextResponse.json( + { + error: isSelf + ? 'Organization admins are automatically workspace admins. Change your organization role to leave this workspace.' + : 'Organization admins are automatically workspace admins. Change their organization role to remove them from this workspace.', + }, + { status: 400 } + ) + } + // Check if the user to be removed actually has permissions for this workspace const userPermission = await db .select() @@ -78,14 +120,6 @@ export const DELETE = withRouteHandler( return NextResponse.json({ error: 'User not found in workspace' }, { status: 404 }) } - // Check if current user has admin access to this workspace - const hasAdminAccess = await hasWorkspaceAdminAccess(session.user.id, workspaceId) - const isSelf = userId === session.user.id - - if (!hasAdminAccess && !isSelf) { - return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 }) - } - // Removing the workspace owner is allowed for any admin: ownership transfers // to the billing account in the transaction below. The billing account itself // stays protected by the guard above (and personal workspaces, where owner == @@ -113,8 +147,6 @@ export const DELETE = withRouteHandler( } } - const organizationId = workspaceRow[0].organizationId - const { ownershipTransferred, workflowOwnershipReassignment } = await db.transaction( async (tx) => { const didTransferOwnership = diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.test.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.test.tsx new file mode 100644 index 00000000000..f4751675866 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.test.tsx @@ -0,0 +1,115 @@ +/** + * @vitest-environment jsdom + * + * A row action the server would refuse stays visible and greyed rather than + * disappearing, so the row can say why. That only works if three things hold at + * once: the item is actually disabled (Radix greys it), the reason reaches + * pointer users through the platform tooltip — which needs the wrapping span, + * since a disabled item is `pointer-events-none` and never sees the hover — and + * the reason reaches assistive tech through the accessible name, since Radix + * skips disabled items in a menu's roving focus. + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it } from 'vitest' +import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu' + +const LOCK_REASON = 'Organization admins are automatically workspace admins.' + +let root: Root | null = null +let container: HTMLDivElement | null = null + +function mount(ui: ReactNode) { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => root?.render(ui)) +} + +/** Opens the `...` menu the way a pointer does — Radix opens on `pointerdown`. */ +function openMenu() { + const trigger = container?.querySelector('button') + if (!trigger) throw new Error('Menu trigger did not render') + act(() => { + trigger.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, button: 0 })) + }) +} + +function item(): HTMLElement { + const node = document.querySelector('[role="menuitem"]') + if (!node) throw new Error('No menu item rendered') + return node as HTMLElement +} + +afterEach(() => { + if (root) act(() => root?.unmount()) + container?.remove() + root = null + container = null +}) + +describe('a disabled row action explains itself', () => { + function mountLockedRemove() { + mount( + {}, + }, + ]} + /> + ) + openMenu() + } + + it('greys the item out instead of hiding it', () => { + mountLockedRemove() + + const remove = item() + expect(remove.textContent).toBe('Remove') + expect(remove.getAttribute('data-disabled')).not.toBeNull() + expect(remove.className).toContain('data-[disabled]:opacity-50') + }) + + it('shows the reason in the platform tooltip on hover', () => { + mountLockedRemove() + + expect(document.querySelector('[role="tooltip"]')).toBeNull() + + /* The wrapping span, not the item — a disabled item is `pointer-events-none`. */ + const hoverTarget = item().parentElement + if (!hoverTarget) throw new Error('Tooltip trigger wrapper did not render') + act(() => { + hoverTarget.dispatchEvent( + new MouseEvent('pointerover', { bubbles: true, clientX: 120, clientY: 120 }) + ) + }) + + expect(document.querySelector('[role="tooltip"]')?.textContent).toBe(LOCK_REASON) + }) + + it('folds the reason into the accessible name for assistive tech', () => { + mountLockedRemove() + + expect(item().getAttribute('aria-label')).toBe(`Remove — ${LOCK_REASON}`) + }) + + it('leaves an enabled action unlabelled and untooltipped', () => { + mount( + {} }]} + /> + ) + openMenu() + + expect(item().getAttribute('aria-label')).toBeNull() + expect(item().getAttribute('data-disabled')).toBeNull() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.tsx index 5c3d29bb494..645e0b9a7d4 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.tsx @@ -34,6 +34,11 @@ interface RowActionsMenuProps { * An action with a `tooltip` gets its item wrapped in a plain span tooltip * trigger (the settings-header chip pattern) — a disabled item is * `pointer-events-none`, so the wrapper is what keeps hover working. + * + * A disabled item's tooltip also folds into its accessible name, because Radix + * skips disabled items in a menu's roving focus: without this the explanation + * would reach pointer users only, and assistive tech would announce a dead + * "Remove" with no reason attached. */ export function RowActionsMenu({ label, actions, triggerClassName }: RowActionsMenuProps) { return ( @@ -50,6 +55,11 @@ export function RowActionsMenu({ label, actions, triggerClassName }: RowActionsM key={action.label} onSelect={action.onSelect} disabled={action.disabled} + aria-label={ + action.disabled && action.tooltip + ? `${action.label} — ${action.tooltip}` + : undefined + } className={action.destructive ? 'text-[var(--text-error)]' : undefined} > {action.label} 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 78f00902510..b8ba236cf22 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/teammates/teammates.tsx @@ -9,6 +9,7 @@ import { useParams, useRouter } from 'next/navigation' import { RoleLockTooltip, type WorkspaceRoleSource, + workspaceMemberRemovalLockReason, workspaceRoleLockReason, } from '@/components/permissions' import { canMutateWorkspaceSettingsSection } from '@/components/settings/navigation' @@ -56,6 +57,7 @@ interface Teammate { invitationId?: string token?: string roleSource?: WorkspaceRoleSource + isOrgAdmin?: boolean isBilledAccount?: boolean } @@ -139,6 +141,7 @@ export function Teammates() { isPending: false, userId: member.userId, roleSource: member.roleSource, + isOrgAdmin: member.isOrgAdmin, isBilledAccount: member.isBilledAccount, })) @@ -206,20 +209,33 @@ export function Teammates() { searchTerm.trim() ? `No teammates found matching “${searchTerm}”` : 'No teammates yet' } > - {filteredTeammates.map((teammate) => ( - { - const lockReason = teammate.isPending - ? null - : workspaceRoleLockReason(teammate.roleSource, { - isBilledAccount: teammate.isBilledAccount, - }) - return ( + {filteredTeammates.map((teammate) => { + const lockReason = teammate.isPending + ? null + : workspaceRoleLockReason(teammate.roleSource, { + isBilledAccount: teammate.isBilledAccount, + }) + /** + * Removal is refused for a different set than the role control locks, + * so it carries its own reason — and stays visible-but-disabled rather + * than hidden, so the row explains the derived access instead of + * leaving an Admin who cannot be acted on. + */ + const removalLockReason = teammate.isPending + ? null + : workspaceMemberRemovalLockReason({ + isOrgAdmin: teammate.isOrgAdmin, + isBilledAccount: teammate.isBilledAccount, + }) + + return ( + - ) - })()} - menu={ - copyToClipboard(teammate.email), - }, - ...(canManage && teammate.isPending - ? [ - { - label: 'Resend invite', - onSelect: () => { - if (teammate.invitationId) { - resendInvitation.mutate({ - invitationId: teammate.invitationId, - workspaceId, - }) - } + } + menu={ + copyToClipboard(teammate.email), + }, + ...(canManage && teammate.isPending + ? [ + { + label: 'Resend invite', + onSelect: () => { + if (teammate.invitationId) { + resendInvitation.mutate({ + invitationId: teammate.invitationId, + workspaceId, + }) + } + }, }, - }, - { - label: 'Copy invite link', - onSelect: () => { - if (teammate.invitationId && teammate.token) { - copyToClipboard( - buildInviteLink(teammate.invitationId, teammate.token) - ) - } + { + label: 'Copy invite link', + onSelect: () => { + if (teammate.invitationId && teammate.token) { + copyToClipboard( + buildInviteLink(teammate.invitationId, teammate.token) + ) + } + }, }, - }, - { - label: 'Revoke invite', - destructive: true, - onSelect: () => { - if (teammate.invitationId) { - cancelInvitation.mutate({ - invitationId: teammate.invitationId, - workspaceId, - }) - } + { + label: 'Revoke invite', + destructive: true, + onSelect: () => { + if (teammate.invitationId) { + cancelInvitation.mutate({ + invitationId: teammate.invitationId, + workspaceId, + }) + } + }, }, - }, - ] - : []), - ...(canManage && !teammate.isPending && teammate.userId !== viewer?.userId - ? [ - { - label: 'Remove', - destructive: true, - onSelect: () => { - if (teammate.userId) { - removeMember.mutate( - { userId: teammate.userId, workspaceId }, - { - onError: (error) => { - toast.error("Couldn't remove teammate", { - description: getErrorMessage( - error, - 'Please try again in a moment.' - ), - }) - }, - } - ) - } + ] + : []), + ...(canManage && !teammate.isPending && teammate.userId !== viewer?.userId + ? [ + { + label: 'Remove', + destructive: true, + disabled: removalLockReason !== null, + tooltip: removalLockReason ?? undefined, + onSelect: () => { + if (teammate.userId) { + removeMember.mutate( + { userId: teammate.userId, workspaceId }, + { + onError: (error) => { + toast.error("Couldn't remove teammate", { + description: getErrorMessage( + error, + 'Please try again in a moment.' + ), + }) + }, + } + ) + } + }, }, - }, - ] - : []), - ]} - /> - } - /> - ))} + ] + : []), + ]} + /> + } + /> + ) + })} diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx index bf1b4d20e3e..8cb3982d99c 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/workspace-header.tsx @@ -18,9 +18,11 @@ import { Send, Skeleton, Tooltip, + toast, } from '@sim/emcn' import { MoreHorizontal, PanelLeft, Pin, Search } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { useQueryClient } from '@tanstack/react-query' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { InviteModal } from '@/app/workspace/[workspaceId]/components/invite-modal' @@ -419,6 +421,15 @@ function WorkspaceHeaderImpl({ setLeaveTarget(null) } catch (error) { logger.error('Error leaving workspace:', error) + /** + * The endpoint refuses several standings it can explain — the billing + * account, the last admin, a derived organization admin. Logging alone + * left the confirm modal sitting open with no indication of why, so the + * server's reason is surfaced the way the teammates list surfaces it. + */ + toast.error("Couldn't leave workspace", { + description: getErrorMessage(error, 'Please try again in a moment.'), + }) } } @@ -887,6 +898,15 @@ function WorkspaceHeaderImpl({ const contextCanAdmin = capturedPermissions === 'admin' const capturedWorkspace = workspaces.find((w) => w.id === capturedWorkspaceRef.current?.id) const isOwner = capturedWorkspace && sessionUserId === capturedWorkspace.ownerId + /** + * An organization admin holds this workspace through their org role, not + * a permission row, so there is nothing to give up and the removal + * endpoint refuses it. `permissions === 'admin'` cannot tell them apart + * from an explicit workspace admin, who may leave. This menu has no + * tooltip affordance to explain a greyed row, so the entry is withheld + * rather than shown dead. + */ + const canLeave = !isOwner && !capturedWorkspace?.isOrgAdmin && !!onLeaveWorkspace return ( , + userWorkspaces: Array<{ + workspace: WorkspaceRow + permissionType: PermissionType + viaOrgAdmin: boolean + }>, userId: string ): Promise { const nonOrgBilledUserIds = [ @@ -70,7 +82,7 @@ async function buildWorkspacesWithInviteFlags( }), ]) - return userWorkspaces.map(({ workspace: workspaceDetails, permissionType }) => { + return userWorkspaces.map(({ workspace: workspaceDetails, permissionType, viaOrgAdmin }) => { const billedPlanCategory: PlanCategory = workspaceDetails.workspaceMode === WORKSPACE_MODE.ORGANIZATION ? workspaceDetails.organizationId @@ -88,6 +100,7 @@ async function buildWorkspacesWithInviteFlags( ? ('admin' as const) : ('member' as const), permissions: permissionType, + isOrgAdmin: viaOrgAdmin, ...resolveInviteFlags(invitePolicy, workspaceDetails.billedAccountUserId === userId), } }) diff --git a/apps/sim/lib/workspaces/permissions/utils.test.ts b/apps/sim/lib/workspaces/permissions/utils.test.ts index 9d132ff53bf..534a1e90e5c 100644 --- a/apps/sim/lib/workspaces/permissions/utils.test.ts +++ b/apps/sim/lib/workspaces/permissions/utils.test.ts @@ -196,6 +196,7 @@ describe('Permission Utils', () => { isExternal: false, joinedAt: '2026-04-22T00:00:00.000Z', roleSource: 'explicit', + isOrgAdmin: false, isBilledAccount: false, }, ]) @@ -253,10 +254,41 @@ describe('Permission Utils', () => { expect(orgAdmin).toMatchObject({ permissionType: 'admin', roleSource: 'org-admin', + isOrgAdmin: true, isExternal: false, }) }) + it('reports isOrgAdmin on a workspace owner whose roleSource outranks it', async () => { + mockSelectSequence([ + [{ id: 'ws', ownerId: 'owner-user', organizationId: 'org-1' }], + [ + { + userId: 'owner-user', + email: 'owner@example.com', + name: 'Owner', + image: null, + permissionType: 'admin' as PermissionType, + joinedAt, + userOrganizationId: 'org-1', + }, + ], + [ + { + userId: 'owner-user', + email: 'owner@example.com', + name: 'Owner', + image: null, + joinedAt, + }, + ], + ]) + + const result = await getUsersWithPermissions('ws') + + expect(result[0]).toMatchObject({ roleSource: 'owner', isOrgAdmin: true }) + }) + it('marks users as external when they are not members of the workspace organization', async () => { mockSelectSequence([ [{ id: 'ws', ownerId: 'internal-user', organizationId: 'org-1' }], diff --git a/apps/sim/lib/workspaces/permissions/utils.ts b/apps/sim/lib/workspaces/permissions/utils.ts index 9634157d18d..cf023292aa7 100644 --- a/apps/sim/lib/workspaces/permissions/utils.ts +++ b/apps/sim/lib/workspaces/permissions/utils.ts @@ -327,6 +327,13 @@ export interface WorkspaceMemberWithRole { * derived and cannot be changed through the member UI. */ roleSource: MemberRoleSource + /** + * Admin of the workspace's organization, and so a workspace admin everywhere + * in it. Reported separately from `roleSource` because that field ranks + * `owner` first, which would otherwise hide the org-admin standing on a + * workspace owner — and removal is refused for the org admin, not the owner. + */ + isOrgAdmin: boolean /** * The account the workspace bills to. Its role is pinned to `admin` by the * workspace-permissions route, so the member UI must not offer to change it. @@ -369,6 +376,7 @@ export async function getUsersWithPermissions( isExternal: !isOwner && row.userOrganizationId !== ws.organizationId, joinedAt: row.joinedAt.toISOString(), roleSource: isOwner ? 'owner' : 'explicit', + isOrgAdmin: false, isBilledAccount: row.userId === ws.billedAccountUserId, }) } @@ -397,6 +405,7 @@ export async function getUsersWithPermissions( if (existing) { existing.permissionType = 'admin' existing.isExternal = false + existing.isOrgAdmin = true if (existing.roleSource !== 'owner') { existing.roleSource = isOwner ? 'owner' : 'org-admin' } @@ -410,6 +419,7 @@ export async function getUsersWithPermissions( isExternal: false, joinedAt: row.joinedAt.toISOString(), roleSource: isOwner ? 'owner' : 'org-admin', + isOrgAdmin: true, isBilledAccount: row.userId === ws.billedAccountUserId, }) } diff --git a/apps/sim/lib/workspaces/utils.test.ts b/apps/sim/lib/workspaces/utils.test.ts index 33d9eec1d0d..2f2474c785b 100644 --- a/apps/sim/lib/workspaces/utils.test.ts +++ b/apps/sim/lib/workspaces/utils.test.ts @@ -287,7 +287,7 @@ describe('listAccessibleWorkspaceRowsForUser', () => { const rows = await listAccessibleWorkspaceRowsForUser('user-1', 'active') - expect(rows).toEqual([{ workspace: orgWorkspace, permissionType: 'admin' }]) + expect(rows).toEqual([{ workspace: orgWorkspace, permissionType: 'admin', viaOrgAdmin: true }]) }) it('keeps a lower explicit grant on a workspace owned by a different organization', async () => { @@ -309,8 +309,20 @@ describe('listAccessibleWorkspaceRowsForUser', () => { const rows = await listAccessibleWorkspaceRowsForUser('user-1', 'active') expect(rows).toEqual([ - { workspace: externalWorkspace, permissionType: 'write' }, - { workspace: orgWorkspace, permissionType: 'admin' }, + { workspace: externalWorkspace, permissionType: 'write', viaOrgAdmin: false }, + { workspace: orgWorkspace, permissionType: 'admin', viaOrgAdmin: true }, ]) }) + + it('reports viaOrgAdmin false for every row when the viewer administers no organization', async () => { + const ownWorkspace = { id: 'ws-own', name: 'Own', ownerId: 'user-1', organizationId: null } + + dbChainMockFns.select + .mockReturnValueOnce(createMockChain([{ workspace: ownWorkspace, permissionType: 'admin' }])) + .mockReturnValueOnce(createMockChain([])) + + const rows = await listAccessibleWorkspaceRowsForUser('user-1', 'active') + + expect(rows).toEqual([{ workspace: ownWorkspace, permissionType: 'admin', viaOrgAdmin: false }]) + }) }) diff --git a/apps/sim/lib/workspaces/utils.ts b/apps/sim/lib/workspaces/utils.ts index 545542ba105..760dcb9b1ce 100644 --- a/apps/sim/lib/workspaces/utils.ts +++ b/apps/sim/lib/workspaces/utils.ts @@ -101,7 +101,16 @@ export async function listAccessibleWorkspaceRowsForUser( userId: string, scope: WorkspaceScope = 'active' ): Promise< - Array<{ workspace: typeof workspaceTable.$inferSelect; permissionType: PermissionType }> + Array<{ + workspace: typeof workspaceTable.$inferSelect + permissionType: PermissionType + /** + * The viewer is an admin of this workspace's organization, so their admin + * access is derived and cannot be given up — no permission row to delete. + * True whether or not they also hold an explicit grant. + */ + viaOrgAdmin: boolean + }> > { const explicit = await db .select({ workspace: workspaceTable, permissionType: permissions.permissionType }) @@ -126,18 +135,20 @@ export async function listAccessibleWorkspaceRowsForUser( const orgRows = await getOrgAdminWorkspaceRows(userId, scope) if (orgRows.length === 0) { - return explicit + return explicit.map((row) => ({ ...row, viaOrgAdmin: false })) } const orgWorkspaceIds = new Set(orgRows.map((ws) => ws.id)) const seen = new Set(explicit.map((row) => row.workspace.id)) const elevatedExplicit = explicit.map((row) => - orgWorkspaceIds.has(row.workspace.id) ? { ...row, permissionType: 'admin' as const } : row + orgWorkspaceIds.has(row.workspace.id) + ? { ...row, permissionType: 'admin' as const, viaOrgAdmin: true } + : { ...row, viaOrgAdmin: false } ) const derived = orgRows .filter((ws) => !seen.has(ws.id)) - .map((ws) => ({ workspace: ws, permissionType: 'admin' as const })) + .map((ws) => ({ workspace: ws, permissionType: 'admin' as const, viaOrgAdmin: true })) return [...elevatedExplicit, ...derived] }