Skip to content

Commit 8b5d746

Browse files
improvement(access-controls): ui/ux improvements (#5190)
* improvement(access-controls): ui/ux improvements * remove unused col
1 parent 43fa5ea commit 8b5d746

14 files changed

Lines changed: 16883 additions & 232 deletions

File tree

apps/docs/content/docs/en/platform/enterprise/access-control.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ A workspace-scoped group applies to **all members of its workspaces by default**
145145

146146
A user is governed by one group per workspace, so adding a user is rejected when it would conflict with another of their groups on a shared workspace (skipped rather than added in bulk). The default group ignores members entirely — it always governs everyone not covered by a workspace group.
147147

148-
Manage which workspaces a group governs from the **Workspaces** list in the group's **Details** view (Add and Remove). A non-default group must always target at least one workspace.
148+
Manage which workspaces a group governs from the **Workspaces** list in the group's **Details** view (Add and Remove). A non-default group is created targeting at least one workspace, but you can later remove all of them — a group with no workspaces simply governs nothing until you add one back.
149149

150150
External workspace members (people who have access to a workspace but belong to a different organization) can't be added as named members, but a workspace-scoped group with no members — and the organization default group — still governs them.
151151

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/members/route.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export const DELETE = withRouteHandler(
281281
throw new Error('MEMBER_NOT_FOUND')
282282
}
283283

284-
if (!lockedGroup.isDefault && !lockedGroup.appliesToAllWorkspaces) {
284+
if (!lockedGroup.isDefault) {
285285
const [memberCountRow] = await tx
286286
.select({ value: count() })
287287
.from(permissionGroupMember)

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts

Lines changed: 35 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export const GET = withRouteHandler(
4848
return NextResponse.json({ error: 'Permission group not found' }, { status: 404 })
4949
}
5050

51-
const workspaces = group.appliesToAllWorkspaces ? [] : await getGroupWorkspaces(id)
51+
const workspaces = group.isDefault ? [] : await getGroupWorkspaces(id)
5252

5353
return NextResponse.json({
5454
permissionGroup: {
@@ -117,44 +117,30 @@ export const PUT = withRouteHandler(
117117

118118
// Demoting the org default with no new scope: it becomes a non-default
119119
// group with no workspaces (inert) until an admin re-scopes it. The client
120-
// sends only `isDefault: false`, so this never forwards a workspace list
121-
// (which a non-default group otherwise requires) against the per-group cap.
120+
// sends only `isDefault: false`, so this never forwards a workspace list.
122121
const demotingDefaultToInert =
123-
group.isDefault &&
124-
updates.isDefault === false &&
125-
updates.appliesToAllWorkspaces === undefined &&
126-
updates.workspaceIds === undefined
127-
128-
// Resolve the target workspace scope. Setting the group as default forces
129-
// all-workspaces; otherwise an explicit `appliesToAllWorkspaces` wins, and
130-
// supplying `workspaceIds` alone implies a specific scope.
131-
const scopeProvided =
132-
demotingDefaultToInert ||
133-
updates.appliesToAllWorkspaces !== undefined ||
134-
updates.workspaceIds !== undefined ||
135-
updates.isDefault === true
136-
137-
const resolvedAppliesToAll = demotingDefaultToInert
138-
? false
139-
: updates.isDefault === true
140-
? true
141-
: updates.appliesToAllWorkspaces !== undefined
142-
? updates.appliesToAllWorkspaces
143-
: updates.workspaceIds !== undefined
144-
? false
145-
: group.appliesToAllWorkspaces
122+
group.isDefault && updates.isDefault === false && updates.workspaceIds === undefined
146123

124+
// "Org-wide" is definitionally `isDefault` (the default group), so the
125+
// effective scope follows it: a default group targets no specific
126+
// workspaces; a non-default group targets its `workspaceIds`.
147127
const effectiveIsDefault =
148128
updates.isDefault !== undefined ? updates.isDefault : group.isDefault
149-
if (effectiveIsDefault && !resolvedAppliesToAll) {
150-
return NextResponse.json(
151-
{ error: 'The default group must apply to all workspaces' },
152-
{ status: 400 }
153-
)
154-
}
155-
if (!effectiveIsDefault && resolvedAppliesToAll) {
129+
130+
// Scope is rewritten when the group is promoted to default, demoted to
131+
// inert, or handed an explicit workspace list.
132+
const scopeProvided =
133+
demotingDefaultToInert || updates.workspaceIds !== undefined || updates.isDefault === true
134+
135+
// The default group governs every workspace, so it can't also name specific
136+
// ones. The contract rejects `isDefault: true` + workspaceIds, but a direct
137+
// API caller can still send workspaceIds against a group that is already the
138+
// default — reject rather than silently dropping them.
139+
if (effectiveIsDefault && updates.workspaceIds !== undefined) {
156140
return NextResponse.json(
157-
{ error: 'Non-default groups must target specific workspaces' },
141+
{
142+
error: 'The default group governs all workspaces and cannot target specific workspaces',
143+
},
158144
{ status: 400 }
159145
)
160146
}
@@ -164,14 +150,11 @@ export const PUT = withRouteHandler(
164150
// ("keep current"), they're read under the lock instead (see below) so the
165151
// conflict check and the write share one consistent snapshot.
166152
let providedWorkspaceIds: string[] | null = null
167-
if (!resolvedAppliesToAll && updates.workspaceIds !== undefined) {
153+
if (!effectiveIsDefault && updates.workspaceIds !== undefined) {
154+
// Zero workspaces is allowed on update: the group then governs nothing
155+
// (the resolver inner-joins on the link table, so an empty group never
156+
// matches any workspace). No "at least one" floor here.
168157
providedWorkspaceIds = Array.from(new Set(updates.workspaceIds))
169-
if (providedWorkspaceIds.length === 0) {
170-
return NextResponse.json(
171-
{ error: 'Select at least one workspace when the group targets specific workspaces' },
172-
{ status: 400 }
173-
)
174-
}
175158
const invalid = await findWorkspacesNotInOrganization(providedWorkspaceIds, organizationId)
176159
if (invalid.length > 0) {
177160
return NextResponse.json(
@@ -197,12 +180,12 @@ export const PUT = withRouteHandler(
197180
if (scopeProvided) {
198181
await acquirePermissionGroupOrgLock(tx, organizationId)
199182

200-
if (!resolvedAppliesToAll) {
183+
if (!effectiveIsDefault) {
184+
// May resolve to an empty list — a non-default group is allowed to
185+
// target zero workspaces (governs nothing). The write below deletes
186+
// the old links and inserts none.
201187
resolvedWorkspaceIds =
202188
providedWorkspaceIds ?? (await getGroupWorkspaces(id, tx)).map((ws) => ws.id)
203-
if (resolvedWorkspaceIds.length === 0 && !demotingDefaultToInert) {
204-
throw new Error('NO_WORKSPACES')
205-
}
206189
}
207190

208191
const members = await tx
@@ -225,7 +208,7 @@ export const PUT = withRouteHandler(
225208

226209
// With no explicit members the group governs all members of its
227210
// workspaces; reject when another all-members group already does.
228-
if (!resolvedAppliesToAll && members.length === 0) {
211+
if (!effectiveIsDefault && members.length === 0) {
229212
const conflict = await findAllMembersWorkspaceConflict(
230213
{ organizationId, excludeGroupId: id, workspaceIds: resolvedWorkspaceIds },
231214
tx
@@ -238,12 +221,12 @@ export const PUT = withRouteHandler(
238221
}
239222

240223
if (updates.isDefault === true) {
241-
// Demote the prior default to a non-default group. It must also drop
242-
// the all-workspaces scope (only the default may be org-wide); it ends
243-
// up with no workspaces (inert) until an admin re-scopes it.
224+
// Demote the prior default to a non-default group (only the default may
225+
// be org-wide); it ends up with no workspaces (inert) until an admin
226+
// re-scopes it.
244227
await tx
245228
.update(permissionGroup)
246-
.set({ isDefault: false, appliesToAllWorkspaces: false, updatedAt: now })
229+
.set({ isDefault: false, updatedAt: now })
247230
.where(
248231
and(
249232
eq(permissionGroup.organizationId, organizationId),
@@ -258,7 +241,6 @@ export const PUT = withRouteHandler(
258241
...(updates.name !== undefined && { name: updates.name }),
259242
...(updates.description !== undefined && { description: updates.description }),
260243
...(updates.isDefault !== undefined && { isDefault: updates.isDefault }),
261-
...(scopeProvided && { appliesToAllWorkspaces: resolvedAppliesToAll }),
262244
config: newConfig,
263245
updatedAt: now,
264246
})
@@ -268,7 +250,7 @@ export const PUT = withRouteHandler(
268250
await tx
269251
.delete(permissionGroupWorkspace)
270252
.where(eq(permissionGroupWorkspace.permissionGroupId, id))
271-
if (!resolvedAppliesToAll && resolvedWorkspaceIds.length > 0) {
253+
if (!effectiveIsDefault && resolvedWorkspaceIds.length > 0) {
272254
await tx.insert(permissionGroupWorkspace).values(
273255
resolvedWorkspaceIds.map((workspaceId) => ({
274256
id: generateId(),
@@ -288,7 +270,7 @@ export const PUT = withRouteHandler(
288270
.where(eq(permissionGroup.id, id))
289271
.limit(1)
290272

291-
const finalWorkspaceIds = updated.appliesToAllWorkspaces
273+
const finalWorkspaceIds = updated.isDefault
292274
? []
293275
: (await getGroupWorkspaces(id)).map((ws) => ws.id)
294276

@@ -334,12 +316,6 @@ export const PUT = withRouteHandler(
334316
{ status: 409 }
335317
)
336318
}
337-
if (error instanceof Error && error.message === 'NO_WORKSPACES') {
338-
return NextResponse.json(
339-
{ error: 'Select at least one workspace when the group targets specific workspaces' },
340-
{ status: 400 }
341-
)
342-
}
343319
if (getPostgresErrorCode(error) === '23505') {
344320
const constraint = getPostgresConstraintName(error)
345321
if (constraint === PERMISSION_GROUP_CONSTRAINTS.organizationName) {

apps/sim/app/api/organizations/[id]/permission-groups/route.ts

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ export const GET = withRouteHandler(
5555
createdAt: permissionGroup.createdAt,
5656
updatedAt: permissionGroup.updatedAt,
5757
isDefault: permissionGroup.isDefault,
58-
appliesToAllWorkspaces: permissionGroup.appliesToAllWorkspaces,
5958
creatorName: user.name,
6059
creatorEmail: user.email,
6160
})
@@ -114,21 +113,20 @@ export const POST = withRouteHandler(
114113
const { name, description, config, isDefault } = parsed.data.body
115114

116115
// Only the organization default group is org-wide; every other group
117-
// targets specific workspaces (the contract rejects all-workspaces scope
118-
// for non-default groups).
119-
const appliesToAllWorkspaces = isDefault === true
120-
const workspaceIds = appliesToAllWorkspaces
116+
// targets specific workspaces. "Org-wide" is definitionally `isDefault`.
117+
const isDefaultGroup = isDefault === true
118+
const workspaceIds = isDefaultGroup
121119
? []
122120
: Array.from(new Set(parsed.data.body.workspaceIds ?? []))
123121

124-
if (!appliesToAllWorkspaces && workspaceIds.length === 0) {
122+
if (!isDefaultGroup && workspaceIds.length === 0) {
125123
return NextResponse.json(
126124
{ error: 'Select at least one workspace when the group targets specific workspaces' },
127125
{ status: 400 }
128126
)
129127
}
130128

131-
if (!appliesToAllWorkspaces) {
129+
if (!isDefaultGroup) {
132130
const invalid = await findWorkspacesNotInOrganization(workspaceIds, organizationId)
133131
if (invalid.length > 0) {
134132
return NextResponse.json(
@@ -169,15 +167,14 @@ export const POST = withRouteHandler(
169167
createdAt: now,
170168
updatedAt: now,
171169
isDefault: isDefault || false,
172-
appliesToAllWorkspaces,
173170
}
174171

175172
await db.transaction(async (tx) => {
176173
await acquirePermissionGroupOrgLock(tx, organizationId)
177174

178175
// A new non-default group has no members, so it governs all members of
179176
// its workspaces; reject when another all-members group already does.
180-
if (!appliesToAllWorkspaces) {
177+
if (!isDefaultGroup) {
181178
const conflict = await findAllMembersWorkspaceConflict(
182179
{ organizationId, excludeGroupId: newGroup.id, workspaceIds },
183180
tx
@@ -189,12 +186,12 @@ export const POST = withRouteHandler(
189186
}
190187

191188
if (isDefault) {
192-
// Demote the prior default to a non-default group. It must also drop
193-
// the all-workspaces scope (only the default may be org-wide); it ends
194-
// up with no workspaces (inert) until an admin re-scopes it.
189+
// Demote the prior default to a non-default group (only the default may
190+
// be org-wide); it ends up with no workspaces (inert) until an admin
191+
// re-scopes it.
195192
await tx
196193
.update(permissionGroup)
197-
.set({ isDefault: false, appliesToAllWorkspaces: false, updatedAt: now })
194+
.set({ isDefault: false, updatedAt: now })
198195
.where(
199196
and(
200197
eq(permissionGroup.organizationId, organizationId),
@@ -220,7 +217,6 @@ export const POST = withRouteHandler(
220217
permissionGroupId: newGroup.id,
221218
organizationId,
222219
userId: session.user.id,
223-
appliesToAllWorkspaces,
224220
workspaceCount: workspaceIds.length,
225221
})
226222

@@ -236,7 +232,6 @@ export const POST = withRouteHandler(
236232
metadata: {
237233
organizationId,
238234
isDefault: isDefault || false,
239-
appliesToAllWorkspaces,
240235
workspaceCount: workspaceIds.length,
241236
},
242237
request: req,

apps/sim/app/api/organizations/[id]/permission-groups/utils.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ export async function loadGroupInOrganization(
8888
createdAt: permissionGroup.createdAt,
8989
updatedAt: permissionGroup.updatedAt,
9090
isDefault: permissionGroup.isDefault,
91-
appliesToAllWorkspaces: permissionGroup.appliesToAllWorkspaces,
9291
})
9392
.from(permissionGroup)
9493
.where(and(eq(permissionGroup.id, groupId), eq(permissionGroup.organizationId, organizationId)))

0 commit comments

Comments
 (0)