Skip to content

Commit d909889

Browse files
fix(workspaces): explain why org admins can't be removed from a workspace (#6838)
* fix(workspaces): explain why org admins can't be removed from a workspace Organization admins hold workspace admin through their org role, not a permissions row, so removal had nothing to revoke. It failed with "User not found in workspace" for someone listed as an Admin on the same screen, and when they also held an explicit row it deleted a grant the derived one immediately replaced — which could drop their org membership and seat, since the seat reconciliation counts rows only. * fix(workspaces): stop offering leave to org admins and surface refusals Sidebar Leave was still offered to non-owner organization admins, whose access is derived and cannot be given up, and the confirm modal swallowed the refusal — so it sat open with no reason shown. The workspaces list now reports whether the viewer's admin access came from their org role, which `permissions: 'admin'` alone could not distinguish from an explicit grant. Also folds a disabled row action's tooltip into its accessible name, since Radix skips disabled items in a menu's roving focus.
1 parent f82085a commit d909889

13 files changed

Lines changed: 415 additions & 108 deletions

File tree

apps/sim/app/api/workspaces/members/[id]/route.ts

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1313
import { revokeWorkspaceCredentialMembershipsTx } from '@/lib/credentials/access'
1414
import { captureServerEvent } from '@/lib/posthog/server'
1515
import { removeWorkspaceSkillMembershipsTx } from '@/lib/skills/access'
16-
import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils'
16+
import {
17+
hasWorkspaceAdminAccess,
18+
isOrganizationAdminOrOwner,
19+
} from '@/lib/workspaces/permissions/utils'
1720
import {
1821
reassignWorkflowOwnershipForWorkspaceMemberRemovalTx,
1922
transferWorkspaceOwnershipToBilledAccountForMemberRemovalTx,
@@ -51,13 +54,52 @@ export const DELETE = withRouteHandler(
5154
return NextResponse.json({ error: 'Workspace not found' }, { status: 404 })
5255
}
5356

57+
const organizationId = workspaceRow[0].organizationId
58+
59+
/**
60+
* Authority is settled before anything is answered about the target, so
61+
* the standing-specific replies below only ever describe someone the
62+
* caller can already see in the members list.
63+
*/
64+
const hasAdminAccess = await hasWorkspaceAdminAccess(session.user.id, workspaceId)
65+
const isSelf = userId === session.user.id
66+
67+
if (!hasAdminAccess && !isSelf) {
68+
return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
69+
}
70+
5471
if (workspaceRow[0].billedAccountUserId === userId) {
5572
return NextResponse.json(
5673
{ error: 'Cannot remove the workspace billing account. Please reassign billing first.' },
5774
{ status: 400 }
5875
)
5976
}
6077

78+
/**
79+
* Organization admins hold workspace admin across the whole organization
80+
* through `member.role`, not through a `permissions` row, so removal has
81+
* nothing to revoke. Left to fall through, the two shapes failed in two
82+
* different ways: with no row it answered "user not found in workspace"
83+
* about someone listed as an Admin on the very screen the caller clicked
84+
* from, and with a row it deleted a grant the derived one immediately
85+
* replaced — while the seat reconciliation below counts rows only, so
86+
* that no-op could still drop the admin's organization membership.
87+
*
88+
* Mirrored by `workspaceMemberRemovalLockReason` on the client, and by the
89+
* same guard on `PATCH /api/workspaces/[id]/permissions`, which refuses to
90+
* re-role an organization admin for the same reason.
91+
*/
92+
if (organizationId && (await isOrganizationAdminOrOwner(userId, organizationId))) {
93+
return NextResponse.json(
94+
{
95+
error: isSelf
96+
? 'Organization admins are automatically workspace admins. Change your organization role to leave this workspace.'
97+
: 'Organization admins are automatically workspace admins. Change their organization role to remove them from this workspace.',
98+
},
99+
{ status: 400 }
100+
)
101+
}
102+
61103
// Check if the user to be removed actually has permissions for this workspace
62104
const userPermission = await db
63105
.select()
@@ -78,14 +120,6 @@ export const DELETE = withRouteHandler(
78120
return NextResponse.json({ error: 'User not found in workspace' }, { status: 404 })
79121
}
80122

81-
// Check if current user has admin access to this workspace
82-
const hasAdminAccess = await hasWorkspaceAdminAccess(session.user.id, workspaceId)
83-
const isSelf = userId === session.user.id
84-
85-
if (!hasAdminAccess && !isSelf) {
86-
return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
87-
}
88-
89123
// Removing the workspace owner is allowed for any admin: ownership transfers
90124
// to the billing account in the transaction below. The billing account itself
91125
// stays protected by the guard above (and personal workspaces, where owner ==
@@ -113,8 +147,6 @@ export const DELETE = withRouteHandler(
113147
}
114148
}
115149

116-
const organizationId = workspaceRow[0].organizationId
117-
118150
const { ownershipTransferred, workflowOwnershipReassignment } = await db.transaction(
119151
async (tx) => {
120152
const didTransferOwnership =
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* A row action the server would refuse stays visible and greyed rather than
5+
* disappearing, so the row can say why. That only works if three things hold at
6+
* once: the item is actually disabled (Radix greys it), the reason reaches
7+
* pointer users through the platform tooltip — which needs the wrapping span,
8+
* since a disabled item is `pointer-events-none` and never sees the hover — and
9+
* the reason reaches assistive tech through the accessible name, since Radix
10+
* skips disabled items in a menu's roving focus.
11+
*/
12+
import { act, type ReactNode } from 'react'
13+
import { createRoot, type Root } from 'react-dom/client'
14+
import { afterEach, describe, expect, it } from 'vitest'
15+
import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu'
16+
17+
const LOCK_REASON = 'Organization admins are automatically workspace admins.'
18+
19+
let root: Root | null = null
20+
let container: HTMLDivElement | null = null
21+
22+
function mount(ui: ReactNode) {
23+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
24+
container = document.createElement('div')
25+
document.body.appendChild(container)
26+
root = createRoot(container)
27+
act(() => root?.render(ui))
28+
}
29+
30+
/** Opens the `...` menu the way a pointer does — Radix opens on `pointerdown`. */
31+
function openMenu() {
32+
const trigger = container?.querySelector('button')
33+
if (!trigger) throw new Error('Menu trigger did not render')
34+
act(() => {
35+
trigger.dispatchEvent(new MouseEvent('pointerdown', { bubbles: true, button: 0 }))
36+
})
37+
}
38+
39+
function item(): HTMLElement {
40+
const node = document.querySelector('[role="menuitem"]')
41+
if (!node) throw new Error('No menu item rendered')
42+
return node as HTMLElement
43+
}
44+
45+
afterEach(() => {
46+
if (root) act(() => root?.unmount())
47+
container?.remove()
48+
root = null
49+
container = null
50+
})
51+
52+
describe('a disabled row action explains itself', () => {
53+
function mountLockedRemove() {
54+
mount(
55+
<RowActionsMenu
56+
label='Teammate actions'
57+
actions={[
58+
{
59+
label: 'Remove',
60+
destructive: true,
61+
disabled: true,
62+
tooltip: LOCK_REASON,
63+
onSelect: () => {},
64+
},
65+
]}
66+
/>
67+
)
68+
openMenu()
69+
}
70+
71+
it('greys the item out instead of hiding it', () => {
72+
mountLockedRemove()
73+
74+
const remove = item()
75+
expect(remove.textContent).toBe('Remove')
76+
expect(remove.getAttribute('data-disabled')).not.toBeNull()
77+
expect(remove.className).toContain('data-[disabled]:opacity-50')
78+
})
79+
80+
it('shows the reason in the platform tooltip on hover', () => {
81+
mountLockedRemove()
82+
83+
expect(document.querySelector('[role="tooltip"]')).toBeNull()
84+
85+
/* The wrapping span, not the item — a disabled item is `pointer-events-none`. */
86+
const hoverTarget = item().parentElement
87+
if (!hoverTarget) throw new Error('Tooltip trigger wrapper did not render')
88+
act(() => {
89+
hoverTarget.dispatchEvent(
90+
new MouseEvent('pointerover', { bubbles: true, clientX: 120, clientY: 120 })
91+
)
92+
})
93+
94+
expect(document.querySelector('[role="tooltip"]')?.textContent).toBe(LOCK_REASON)
95+
})
96+
97+
it('folds the reason into the accessible name for assistive tech', () => {
98+
mountLockedRemove()
99+
100+
expect(item().getAttribute('aria-label')).toBe(`Remove — ${LOCK_REASON}`)
101+
})
102+
103+
it('leaves an enabled action unlabelled and untooltipped', () => {
104+
mount(
105+
<RowActionsMenu
106+
label='Teammate actions'
107+
actions={[{ label: 'Copy email', onSelect: () => {} }]}
108+
/>
109+
)
110+
openMenu()
111+
112+
expect(item().getAttribute('aria-label')).toBeNull()
113+
expect(item().getAttribute('data-disabled')).toBeNull()
114+
})
115+
})

apps/sim/app/workspace/[workspaceId]/settings/components/row-actions-menu/row-actions-menu.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ interface RowActionsMenuProps {
3434
* An action with a `tooltip` gets its item wrapped in a plain span tooltip
3535
* trigger (the settings-header chip pattern) — a disabled item is
3636
* `pointer-events-none`, so the wrapper is what keeps hover working.
37+
*
38+
* A disabled item's tooltip also folds into its accessible name, because Radix
39+
* skips disabled items in a menu's roving focus: without this the explanation
40+
* would reach pointer users only, and assistive tech would announce a dead
41+
* "Remove" with no reason attached.
3742
*/
3843
export function RowActionsMenu({ label, actions, triggerClassName }: RowActionsMenuProps) {
3944
return (
@@ -50,6 +55,11 @@ export function RowActionsMenu({ label, actions, triggerClassName }: RowActionsM
5055
key={action.label}
5156
onSelect={action.onSelect}
5257
disabled={action.disabled}
58+
aria-label={
59+
action.disabled && action.tooltip
60+
? `${action.label}${action.tooltip}`
61+
: undefined
62+
}
5363
className={action.destructive ? 'text-[var(--text-error)]' : undefined}
5464
>
5565
{action.label}

0 commit comments

Comments
 (0)