Skip to content

Commit f332ca6

Browse files
committed
fix(permission-groups): preserve bulk membership behavior and locked entitlement checks
1 parent eba6605 commit f332ca6

18 files changed

Lines changed: 265 additions & 115 deletions

File tree

‎apps/docs/content/docs/cli/permission-groups.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ Bulk Add Permission Group Members (OAuth login or personal API key required)
4646
| `--organization <value>` | Yes | Organization that owns the permission group. |
4747
| `--group <value>` | Yes | Permission group identifier. |
4848
| `--user-ids <json\|@file>` | No | Organization member identifiers. Existing group members are skipped; users outside the organization are ignored. (JSON, or @path / @- to read a file or stdin). |
49-
| `--add-all-organization-members` | No | Add every current organization member, up to 1000. For larger organizations, use userIds in batches. Cannot be combined with userIds. |
49+
| `--add-all-organization-members` | No | Add every current organization member in bounded batches within one transaction. Cannot be combined with userIds. |
5050
| `--no-add-all-organization-members` | No | Send --add-all-organization-members as false. |
5151

5252
</CommandTable>
@@ -254,7 +254,7 @@ Update Permission Group (OAuth login or personal API key required)
254254
| --- | --- | --- |
255255
| `--organization <value>` | Yes | Organization that owns the permission group. |
256256
| `--name <value>` | No | Group name, unique within the organization. |
257-
| `--description <value>` | No | Group description. Null clears it; omission leaves it unchanged. (--description null sends the word, not JSON null). |
257+
| `--description <value>` | No | Group description. Null or an empty string clears it; omission leaves it unchanged. (--description null sends the word, not JSON null). |
258258
| `--config <json\|@file>` | No | Patch of permission restrictions. Omitted keys remain unchanged; each supplied array replaces that entire list. (JSON, or @path / @- to read a file or stdin). |
259259
| `--is-default` | No | Whether the group is the organization default. Only one group can be the default. |
260260
| `--no-is-default` | No | Send --is-default as false. |

‎apps/docs/content/docs/cli/reference.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3037,7 +3037,7 @@ sim permission-groups members batch-add [options]
30373037
| `--organization <value>` | Yes | Organization that owns the permission group. |
30383038
| `--group <value>` | Yes | Permission group identifier. |
30393039
| `--user-ids <json\|@file>` | No | Organization member identifiers. Existing group members are skipped; users outside the organization are ignored. (JSON, or @path / @- to read a file or stdin). |
3040-
| `--add-all-organization-members` | No | Add every current organization member, up to 1000. For larger organizations, use userIds in batches. Cannot be combined with userIds. |
3040+
| `--add-all-organization-members` | No | Add every current organization member in bounded batches within one transaction. Cannot be combined with userIds. |
30413041
| `--no-add-all-organization-members` | No | Send --add-all-organization-members as false. |
30423042

30433043
</CommandTable>
@@ -3245,7 +3245,7 @@ sim permission-groups update <groupId> [options]
32453245
| --- | --- | --- |
32463246
| `--organization <value>` | Yes | Organization that owns the permission group. |
32473247
| `--name <value>` | No | Group name, unique within the organization. |
3248-
| `--description <value>` | No | Group description. Null clears it; omission leaves it unchanged. (--description null sends the word, not JSON null). |
3248+
| `--description <value>` | No | Group description. Null or an empty string clears it; omission leaves it unchanged. (--description null sends the word, not JSON null). |
32493249
| `--config <json\|@file>` | No | Patch of permission restrictions. Omitted keys remain unchanged; each supplied array replaces that entire list. (JSON, or @path / @- to read a file or stdin). |
32503250
| `--is-default` | No | Whether the group is the organization default. Only one group can be the default. |
32513251
| `--no-is-default` | No | Send --is-default as false. |

‎apps/docs/lib/openapi-download.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('OpenAPI download', () => {
3333
const tags = document.tags as Array<{ name: string }>
3434

3535
expect(document.openapi).toBe('3.1.0')
36-
expect(Object.keys(paths)).toHaveLength(157)
36+
expect(Object.keys(paths)).toHaveLength(163)
3737
expect(tags.map((tag) => tag.name)).toEqual([
3838
'Workspace Sync',
3939
'Workflows',
@@ -44,6 +44,7 @@ describe('OpenAPI download', () => {
4444
'Tables',
4545
'Knowledge Bases',
4646
'Billing',
47+
'Permission Groups',
4748
'Meta',
4849
'Workspaces',
4950
'MCP Servers',

‎apps/docs/openapi-v2-resources.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5524,7 +5524,7 @@
55245524
"post": {
55255525
"operationId": "bulkAddPermissionGroupMembers",
55265526
"summary": "Bulk Add Permission Group Members",
5527-
"description": "Assign up to 1000 organization members atomically. Existing assignments are skipped and users outside the organization are ignored. Any overlapping membership conflict rejects the entire batch. Requires organization admin or owner access and active Access Control. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
5527+
"description": "Assign up to 1000 selected organization members, or the entire organization roster, atomically. Existing assignments are skipped and users outside the organization are ignored. Any overlapping membership conflict rejects the entire batch. Requires organization admin or owner access and active Access Control. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.",
55285528
"x-sim-operation": "permission_groups.members.bulk_add",
55295529
"x-oauth-scope": "api:write",
55305530
"tags": ["Permission Groups"],
@@ -12628,7 +12628,7 @@
1262812628
},
1262912629
"isDefault": {
1263012630
"type": "boolean",
12631-
"description": "Whether this is the organization default, which applies across all its workspaces."
12631+
"description": "Whether this is the organization default, which applies to everyone across all its workspaces regardless of member assignments."
1263212632
},
1263312633
"membershipMode": {
1263412634
"type": "string",
@@ -13266,7 +13266,7 @@
1326613266
"description": "Group name, unique within the organization."
1326713267
},
1326813268
"description": {
13269-
"description": "Group description. Null clears it; omission leaves it unchanged.",
13269+
"description": "Group description. Null or an empty string clears it; omission leaves it unchanged.",
1327013270
"anyOf": [
1327113271
{
1327213272
"type": "string",
@@ -13847,7 +13847,7 @@
1384713847
}
1384813848
},
1384913849
"addAllOrganizationMembers": {
13850-
"description": "Add every current organization member, up to 1000. For larger organizations, use userIds in batches. Cannot be combined with userIds.",
13850+
"description": "Add every current organization member in bounded batches within one transaction. Cannot be combined with userIds.",
1385113851
"type": "boolean"
1385213852
}
1385313853
},

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ async function updateUnderLock(body: UpdatePermissionGroupBody) {
7777
})
7878
try {
7979
expect(await Promise.race([lockEntered.promise, pendingResponse.then(() => false)])).toBe(true)
80-
expect(mocks.acquireLock).toHaveBeenCalledExactlyOnceWith(db, ORGANIZATION_ID)
80+
expect(mocks.acquireLock).toHaveBeenCalledExactlyOnceWith(db, ORGANIZATION_ID, {
81+
lockTimeoutAlreadyBounded: true,
82+
})
8183
expect(dbChainMockFns.update).not.toHaveBeenCalled()
8284
} finally {
8385
lockReleased.resolve()
@@ -161,7 +163,9 @@ describe('permission group PUT policy serialization', () => {
161163

162164
expect(response.status).toBe(404)
163165
await expect(response.json()).resolves.toMatchObject({ error: 'Permission group not found' })
164-
expect(mocks.acquireLock).toHaveBeenCalledExactlyOnceWith(db, ORGANIZATION_ID)
166+
expect(mocks.acquireLock).toHaveBeenCalledExactlyOnceWith(db, ORGANIZATION_ID, {
167+
lockTimeoutAlreadyBounded: true,
168+
})
165169
expect(mocks.loadGroup).toHaveBeenLastCalledWith(GROUP_ID, ORGANIZATION_ID, db)
166170
expect(dbChainMockFns.update).not.toHaveBeenCalled()
167171
})

‎apps/sim/lib/api/contracts/permission-groups.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,10 @@ const workspaceIdsSchema = z.array(z.string().min(1)).max(MAX_PERMISSION_GROUP_W
114114
* with no `workspaceIds` is already the all-workspaces case and needs no
115115
* assertion here.
116116
*
117-
* Everything else is left to the routes: a non-default group targets the
117+
* Other scope rules are enforced by the shared manager: a non-default group targets the
118118
* workspaces in `workspaceIds` (empty is allowed on update — the group then
119119
* governs nothing, since the resolver inner-joins the workspace link table), and
120-
* the create route requires at least one workspace up front.
120+
* creation requires at least one workspace up front.
121121
*/
122122
function refineWorkspaceScope(
123123
body: { workspaceIds?: string[]; isDefault?: boolean },
@@ -177,7 +177,9 @@ export const updatePermissionGroupBodySchema = z
177177
.max(500, 'description cannot exceed 500 characters')
178178
.nullable()
179179
.optional()
180-
.describe('Group description. Null clears it; omission leaves it unchanged.'),
180+
.describe(
181+
'Group description. Null or an empty string clears it; omission leaves it unchanged.'
182+
),
181183
config: permissionGroupConfigSchema.optional(),
182184
isDefault: z
183185
.boolean()

‎apps/sim/lib/api/contracts/v2/openapi/permission-groups.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ export const permissionGroupOpenApiRoutes = [
6161
params: documentedSchema(
6262
v2ListPermissionGroupsContract.params,
6363
'ListPermissionGroupsParams',
64-
'Permission group parameters',
65-
'Organization and permission-group identifiers.'
64+
'Organization parameters',
65+
'Organization identifier.'
6666
),
6767
query: documentedSchema(
6868
v2ListPermissionGroupsContract.query,
@@ -94,8 +94,8 @@ export const permissionGroupOpenApiRoutes = [
9494
params: documentedSchema(
9595
v2CreatePermissionGroupContract.params,
9696
'CreatePermissionGroupParams',
97-
'Permission group parameters',
98-
'Organization and permission-group identifiers.'
97+
'Organization parameters',
98+
'Organization identifier.'
9999
),
100100
query: v2CreatePermissionGroupContract.query,
101101
body: documentedSchema(
@@ -324,7 +324,7 @@ export const permissionGroupOpenApiRoutes = [
324324
applicationOperation: permissionGroupOperations.bulkAddMembers,
325325
operationId: 'bulkAddPermissionGroupMembers',
326326
summary: 'Bulk Add Permission Group Members',
327-
description: `Assign up to 1000 organization members atomically. Existing assignments are skipped and users outside the organization are ignored. Any overlapping membership conflict rejects the entire batch. ${AUTHORITY}`,
327+
description: `Assign up to 1000 selected organization members, or the entire organization roster, atomically. Existing assignments are skipped and users outside the organization are ignored. Any overlapping membership conflict rejects the entire batch. ${AUTHORITY}`,
328328
tags: ['Permission Groups'],
329329
errors: [...RESOURCE_CONFLICT_ERRORS, 'PayloadTooLarge'],
330330
success: {
@@ -374,8 +374,8 @@ export const permissionGroupOpenApiRoutes = [
374374
params: documentedSchema(
375375
v2ListPermissionGroupWorkspacesContract.params,
376376
'ListPermissionGroupWorkspacesParams',
377-
'Permission group parameters',
378-
'Organization and permission-group identifiers.'
377+
'Organization parameters',
378+
'Organization identifier.'
379379
),
380380
query: documentedSchema(
381381
v2ListPermissionGroupWorkspacesContract.query,

‎apps/sim/lib/api/contracts/v2/permission-groups.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export const v2PermissionGroupSchema = z
5050
isDefault: z
5151
.boolean()
5252
.describe(
53-
'Whether this is the organization default, which applies across all its workspaces.'
53+
'Whether this is the organization default, which applies to everyone across all its workspaces regardless of member assignments.'
5454
),
5555
membershipMode: z
5656
.string()
@@ -165,7 +165,7 @@ export const v2BulkAddPermissionGroupMembersBodySchema = z
165165
.boolean()
166166
.optional()
167167
.describe(
168-
'Add every current organization member, up to 1000. For larger organizations, use userIds in batches. Cannot be combined with userIds.'
168+
'Add every current organization member in bounded batches within one transaction. Cannot be combined with userIds.'
169169
),
170170
})
171171
.strict()

‎apps/sim/lib/api/mcp/generated/v2-operations.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ export const V2_MCP_OPERATIONS = {
387387
contract: v2BulkAddPermissionGroupMembersContract,
388388
summary: 'Bulk Add Permission Group Members',
389389
description:
390-
'Assign up to 1000 organization members atomically. Existing assignments are skipped and users outside the organization are ignored. Any overlapping membership conflict rejects the entire batch. Requires organization admin or owner access and active Access Control. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
390+
'Assign up to 1000 selected organization members, or the entire organization roster, atomically. Existing assignments are skipped and users outside the organization are ignored. Any overlapping membership conflict rejects the entire batch. Requires organization admin or owner access and active Access Control. Workspace API keys return `403`; use a personal API key or scoped OAuth token.\n\nOAuth scope: `api:write`.',
391391
workspaceKeyUnsupported: true,
392392
handler: () =>
393393
import(

‎apps/sim/lib/permission-groups/application/use-cases.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,9 @@ export const bulkAddPermissionGroupMembers = defineAuthorizedPermissionGroupUseC
181181
description: `Bulk added ${result.added} member(s) to permission group "${result.group.name}"`,
182182
metadata: {
183183
permissionGroupId: result.group.id,
184+
added: result.added,
184185
addedUserIds: result.addedUserIds,
186+
...(result.added > result.addedUserIds.length && { addedUserIdsTruncated: true }),
185187
skipped: result.skipped,
186188
},
187189
},

0 commit comments

Comments
 (0)