Skip to content

Commit 1dc9b35

Browse files
committed
add ws id, user constraint
1 parent df7d486 commit 1dc9b35

9 files changed

Lines changed: 124 additions & 12 deletions

File tree

apps/sim/app/api/workspaces/[workspaceId]/permission-groups/[id]/members/bulk/route.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { db } from '@sim/db'
22
import { permissionGroup, permissionGroupMember, permissions } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4+
import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
45
import { generateId } from '@sim/utils/id'
56
import { and, eq, inArray } from 'drizzle-orm'
67
import { type NextRequest, NextResponse } from 'next/server'
@@ -9,6 +10,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
910
import { getSession } from '@/lib/auth'
1011
import { isWorkspaceOnEnterprisePlan } from '@/lib/billing'
1112
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
13+
import { PERMISSION_GROUP_MEMBER_CONSTRAINTS } from '@/lib/permission-groups/types'
1214
import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils'
1315

1416
const logger = createLogger('WorkspacePermissionGroupBulkMembers')
@@ -141,6 +143,7 @@ export const POST = withRouteHandler(
141143
const newMembers = usersToAdd.map((userId) => ({
142144
id: generateId(),
143145
permissionGroupId: id,
146+
workspaceId,
144147
userId,
145148
assignedBy: session.user.id,
146149
assignedAt: new Date(),
@@ -186,6 +189,21 @@ export const POST = withRouteHandler(
186189
if (error instanceof z.ZodError) {
187190
return NextResponse.json({ error: error.errors[0].message }, { status: 400 })
188191
}
192+
if (getPostgresErrorCode(error) === '23505') {
193+
const constraint = getPostgresConstraintName(error)
194+
if (
195+
constraint === PERMISSION_GROUP_MEMBER_CONSTRAINTS.workspaceUser ||
196+
constraint === PERMISSION_GROUP_MEMBER_CONSTRAINTS.groupUser
197+
) {
198+
return NextResponse.json(
199+
{
200+
error:
201+
'One or more users were concurrently added to a group in this workspace. Please refresh and try again.',
202+
},
203+
{ status: 409 }
204+
)
205+
}
206+
}
189207
logger.error('Error bulk adding members to permission group', error)
190208
return NextResponse.json({ error: 'Failed to add members' }, { status: 500 })
191209
}

apps/sim/app/api/workspaces/[workspaceId]/permission-groups/[id]/members/route.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { db } from '@sim/db'
22
import { permissionGroup, permissionGroupMember, permissions, user } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4+
import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors'
45
import { generateId } from '@sim/utils/id'
56
import { and, eq, inArray } from 'drizzle-orm'
67
import { type NextRequest, NextResponse } from 'next/server'
@@ -9,6 +10,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
910
import { getSession } from '@/lib/auth'
1011
import { isWorkspaceOnEnterprisePlan } from '@/lib/billing'
1112
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
13+
import { PERMISSION_GROUP_MEMBER_CONSTRAINTS } from '@/lib/permission-groups/types'
1214
import { checkWorkspaceAccess, hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils'
1315

1416
const logger = createLogger('WorkspacePermissionGroupMembers')
@@ -169,6 +171,7 @@ export const POST = withRouteHandler(
169171
const memberData = {
170172
id: generateId(),
171173
permissionGroupId: id,
174+
workspaceId,
172175
userId,
173176
assignedBy: session.user.id,
174177
assignedAt: new Date(),
@@ -214,6 +217,24 @@ export const POST = withRouteHandler(
214217
{ status: 409 }
215218
)
216219
}
220+
if (getPostgresErrorCode(error) === '23505') {
221+
const constraint = getPostgresConstraintName(error)
222+
if (constraint === PERMISSION_GROUP_MEMBER_CONSTRAINTS.workspaceUser) {
223+
return NextResponse.json(
224+
{
225+
error:
226+
'User was concurrently added to another group in this workspace. Please refresh and try again.',
227+
},
228+
{ status: 409 }
229+
)
230+
}
231+
if (constraint === PERMISSION_GROUP_MEMBER_CONSTRAINTS.groupUser) {
232+
return NextResponse.json(
233+
{ error: 'User is already in this permission group' },
234+
{ status: 409 }
235+
)
236+
}
237+
}
217238
logger.error('Error adding member to permission group', error)
218239
return NextResponse.json({ error: 'Failed to add member' }, { status: 500 })
219240
}

apps/sim/lib/permission-groups/auto-add.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export async function applyWorkspaceAutoAddGroup(
5454
await client.insert(permissionGroupMember).values({
5555
id: generateId(),
5656
permissionGroupId: autoAddGroup.id,
57+
workspaceId,
5758
userId,
5859
assignedBy: null,
5960
assignedAt: new Date(),

apps/sim/lib/permission-groups/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { z } from 'zod'
22

3+
export const PERMISSION_GROUP_MEMBER_CONSTRAINTS = {
4+
groupUser: 'permission_group_member_group_user_unique',
5+
workspaceUser: 'permission_group_member_workspace_user_unique',
6+
} as const
7+
38
export const permissionGroupConfigSchema = z.object({
49
allowedIntegrations: z.array(z.string()).nullable().optional(),
510
allowedModelProviders: z.array(z.string()).nullable().optional(),

packages/db/migrations/0194_organic_talisman.sql renamed to packages/db/migrations/0194_careless_pete_wisdom.sql

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
-- Rebase permission groups from organization-scoped to workspace-scoped.
22
-- Each existing org-scoped permission_group is cloned onto every workspace
33
-- owned by that org; members are copied to each clone only if the user has
4-
-- workspace-level permissions on the target workspace.
4+
-- workspace-level permissions on the target workspace. The
5+
-- permission_group_member table also gains a denormalized workspace_id column
6+
-- so the database can enforce the "one group per user per workspace"
7+
-- invariant with a composite unique index.
58

69
-- 0. Backfill workspace -> organization links for grandfathered workspaces whose
710
-- billed account user is the sole owner of exactly one organization. This is a
@@ -25,9 +28,11 @@ WHERE w."organization_id" IS NULL
2528
AND w."workspace_mode" = 'grandfathered_shared'
2629
AND w."billed_account_user_id" = owner_orgs."user_id";--> statement-breakpoint
2730

28-
-- 1. Add workspace_id as nullable so existing rows can coexist during the data migration.
31+
-- 1. Add workspace_id columns as nullable so existing rows can coexist during the data migration.
2932
ALTER TABLE "permission_group" ADD COLUMN "workspace_id" text;--> statement-breakpoint
3033
ALTER TABLE "permission_group" ADD CONSTRAINT "permission_group_workspace_id_workspace_id_fk" FOREIGN KEY ("workspace_id") REFERENCES "public"."workspace"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
34+
ALTER TABLE "permission_group_member" ADD COLUMN "workspace_id" text;--> statement-breakpoint
35+
ALTER TABLE "permission_group_member" ADD CONSTRAINT "permission_group_member_workspace_id_workspace_id_fk" FOREIGN KEY ("workspace_id") REFERENCES "public"."workspace"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
3136

3237
-- 2. Materialize a plan of (source permission group, target workspace, new clone id)
3338
-- so we can insert the clone rows AND the member rows with stable references.
@@ -70,12 +75,15 @@ SELECT
7075
FROM "__permission_group_clone_plan" plan
7176
JOIN "permission_group" pg ON pg."id" = plan."source_id";--> statement-breakpoint
7277

73-
-- 4. Copy member rows to each workspace clone, but only if the user has
74-
-- workspace-level permissions on that target workspace.
75-
INSERT INTO "permission_group_member" ("id", "permission_group_id", "user_id", "assigned_by", "assigned_at")
78+
-- 4. Copy member rows to each workspace clone, populating the denormalized
79+
-- workspace_id. Only include users who have workspace-level permissions on
80+
-- that target workspace, OR who are the workspace owner (legacy workspaces
81+
-- may have an owner without an explicit permissions row).
82+
INSERT INTO "permission_group_member" ("id", "permission_group_id", "workspace_id", "user_id", "assigned_by", "assigned_at")
7683
SELECT
7784
gen_random_uuid()::text,
7885
plan."cloned_id",
86+
plan."workspace_id",
7987
m."user_id",
8088
m."assigned_by",
8189
m."assigned_at"
@@ -100,8 +108,9 @@ WHERE "permission_group_id" IN (
100108

101109
DELETE FROM "permission_group" WHERE "organization_id" IS NOT NULL;--> statement-breakpoint
102110

103-
-- 6. Enforce NOT NULL on workspace_id now that every surviving row has one.
111+
-- 6. Enforce NOT NULL on both workspace_id columns now that every surviving row has one.
104112
ALTER TABLE "permission_group" ALTER COLUMN "workspace_id" SET NOT NULL;--> statement-breakpoint
113+
ALTER TABLE "permission_group_member" ALTER COLUMN "workspace_id" SET NOT NULL;--> statement-breakpoint
105114

106115
-- 7. Drop legacy structures and swap indexes.
107116
ALTER TABLE "permission_group" DROP CONSTRAINT "permission_group_organization_id_organization_id_fk";--> statement-breakpoint
@@ -112,6 +121,8 @@ ALTER TABLE "permission_group" DROP COLUMN "organization_id";--> statement-break
112121
CREATE UNIQUE INDEX "permission_group_workspace_name_unique" ON "permission_group" USING btree ("workspace_id","name");--> statement-breakpoint
113122
CREATE UNIQUE INDEX "permission_group_workspace_auto_add_unique" ON "permission_group" USING btree ("workspace_id") WHERE auto_add_new_members = true;--> statement-breakpoint
114123
CREATE UNIQUE INDEX "permission_group_member_group_user_unique" ON "permission_group_member" USING btree ("permission_group_id","user_id");--> statement-breakpoint
124+
CREATE UNIQUE INDEX "permission_group_member_workspace_user_unique" ON "permission_group_member" USING btree ("workspace_id","user_id");--> statement-breakpoint
115125

116126
-- 8. Sweep any residual dead config keys from pre-existing workspace-scoped rows (if any).
117127
UPDATE "permission_group" SET "config" = ("config" - 'hideEnvironmentTab' - 'hideTemplates') WHERE "config" ? 'hideEnvironmentTab' OR "config" ? 'hideTemplates';
128+

packages/db/migrations/meta/0194_snapshot.json

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"id": "2b83a8e3-99b2-4ac1-ac93-fa25a416a2ef",
2+
"id": "dce19071-748a-432a-86f3-6dd4021df35f",
33
"prevId": "8caf2eaf-a548-4c7f-83ae-546b416c5d88",
44
"version": "7",
55
"dialect": "postgresql",
@@ -8584,6 +8584,12 @@
85848584
"primaryKey": false,
85858585
"notNull": true
85868586
},
8587+
"workspace_id": {
8588+
"name": "workspace_id",
8589+
"type": "text",
8590+
"primaryKey": false,
8591+
"notNull": true
8592+
},
85878593
"user_id": {
85888594
"name": "user_id",
85898595
"type": "text",
@@ -8640,6 +8646,27 @@
86408646
"concurrently": false,
86418647
"method": "btree",
86428648
"with": {}
8649+
},
8650+
"permission_group_member_workspace_user_unique": {
8651+
"name": "permission_group_member_workspace_user_unique",
8652+
"columns": [
8653+
{
8654+
"expression": "workspace_id",
8655+
"isExpression": false,
8656+
"asc": true,
8657+
"nulls": "last"
8658+
},
8659+
{
8660+
"expression": "user_id",
8661+
"isExpression": false,
8662+
"asc": true,
8663+
"nulls": "last"
8664+
}
8665+
],
8666+
"isUnique": true,
8667+
"concurrently": false,
8668+
"method": "btree",
8669+
"with": {}
86438670
}
86448671
},
86458672
"foreignKeys": {
@@ -8652,6 +8679,15 @@
86528679
"onDelete": "cascade",
86538680
"onUpdate": "no action"
86548681
},
8682+
"permission_group_member_workspace_id_workspace_id_fk": {
8683+
"name": "permission_group_member_workspace_id_workspace_id_fk",
8684+
"tableFrom": "permission_group_member",
8685+
"tableTo": "workspace",
8686+
"columnsFrom": ["workspace_id"],
8687+
"columnsTo": ["id"],
8688+
"onDelete": "cascade",
8689+
"onUpdate": "no action"
8690+
},
86558691
"permission_group_member_user_id_user_id_fk": {
86568692
"name": "permission_group_member_user_id_user_id_fk",
86578693
"tableFrom": "permission_group_member",

packages/db/migrations/meta/_journal.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,8 +1356,8 @@
13561356
{
13571357
"idx": 194,
13581358
"version": "7",
1359-
"when": 1776798198298,
1360-
"tag": "0194_organic_talisman",
1359+
"when": 1776800382630,
1360+
"tag": "0194_careless_pete_wisdom",
13611361
"breakpoints": true
13621362
}
13631363
]

packages/db/schema.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2678,6 +2678,9 @@ export const permissionGroupMember = pgTable(
26782678
permissionGroupId: text('permission_group_id')
26792679
.notNull()
26802680
.references(() => permissionGroup.id, { onDelete: 'cascade' }),
2681+
workspaceId: text('workspace_id')
2682+
.notNull()
2683+
.references(() => workspace.id, { onDelete: 'cascade' }),
26812684
userId: text('user_id')
26822685
.notNull()
26832686
.references(() => user.id, { onDelete: 'cascade' }),
@@ -2690,6 +2693,10 @@ export const permissionGroupMember = pgTable(
26902693
table.permissionGroupId,
26912694
table.userId
26922695
),
2696+
workspaceUserUnique: uniqueIndex('permission_group_member_workspace_user_unique').on(
2697+
table.workspaceId,
2698+
table.userId
2699+
),
26932700
})
26942701
)
26952702

packages/utils/src/errors.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,19 @@ export function toError(value: unknown): Error {
1313
* Normalizes common Drizzle / `postgres` driver shapes and walks `cause` chains.
1414
*/
1515
export function getPostgresErrorCode(error: unknown): string | undefined {
16+
return readPgErrorField(error, 'code')
17+
}
18+
19+
/**
20+
* Returns the name of the PostgreSQL constraint that triggered the error (e.g. the unique index
21+
* name on a `23505`), when present on a thrown value. Mirrors the field populated by the
22+
* `postgres` / `pg` drivers, walking `cause` chains the same way as `getPostgresErrorCode`.
23+
*/
24+
export function getPostgresConstraintName(error: unknown): string | undefined {
25+
return readPgErrorField(error, 'constraint_name') ?? readPgErrorField(error, 'constraint')
26+
}
27+
28+
function readPgErrorField(error: unknown, field: string): string | undefined {
1629
const seen = new Set<unknown>()
1730
let current: unknown = error
1831

@@ -23,9 +36,9 @@ export function getPostgresErrorCode(error: unknown): string | undefined {
2336
seen.add(current)
2437

2538
if (typeof current === 'object') {
26-
const code = (current as { code?: unknown }).code
27-
if (typeof code === 'string') {
28-
return code
39+
const value = (current as Record<string, unknown>)[field]
40+
if (typeof value === 'string') {
41+
return value
2942
}
3043
}
3144

0 commit comments

Comments
 (0)