improvement(permissions): confine workspace role changes to existing members

Author: icecrasher321

apps/sim/app/api/organizations/[id]/roster/route.test.ts

@@ -174,6 +174,8 @@ describe('GET /api/organizations/[id]/roster', () => {
               workspaceId: 'workspace-1',
               workspaceName: 'Workspace One',
               permission: 'admin',
+              roleSource: 'org-admin',
+              isBilledAccount: false,
             },
           ],
         }),
@@ -193,6 +195,8 @@ describe('GET /api/organizations/[id]/roster', () => {
             workspaceId: 'workspace-1',
             workspaceName: 'Workspace One',
             permission: 'read',
+            roleSource: 'explicit',
+            isBilledAccount: false,
           },
         ],
       }),

apps/sim/app/api/organizations/[id]/roster/route.ts

@@ -89,12 +89,17 @@ export const GET = withRouteHandler(
       await expireStalePendingInvitationsForOrganization(organizationId)
 
       const orgWorkspaces = await db
-        .select({ id: workspace.id, name: workspace.name })
+        .select({
+          id: workspace.id,
+          name: workspace.name,
+          ownerId: workspace.ownerId,
+          billedAccountUserId: workspace.billedAccountUserId,
+        })
         .from(workspace)
         .where(and(eq(workspace.organizationId, organizationId), isNull(workspace.archivedAt)))
 
       const orgWorkspaceIds = orgWorkspaces.map((ws) => ws.id)
-      const workspaceNameById = new Map(orgWorkspaces.map((ws) => [ws.id, ws.name]))
+      const workspaceById = new Map(orgWorkspaces.map((ws) => [ws.id, ws]))
       const memberUserIds = memberRows.map((row) => row.userId)
 
       const memberPermissions =
@@ -117,11 +122,14 @@ export const GET = withRouteHandler(
 
       const permissionsByUser = new Map<string, RosterWorkspaceAccess[]>()
       for (const row of memberPermissions) {
+        const ws = workspaceById.get(row.workspaceId)
         const list = permissionsByUser.get(row.userId) ?? []
         list.push({
           workspaceId: row.workspaceId,
-          workspaceName: workspaceNameById.get(row.workspaceId) ?? 'Workspace',
+          workspaceName: ws?.name ?? 'Workspace',
           permission: row.permission,
+          roleSource: ws?.ownerId === row.userId ? 'owner' : 'explicit',
+          isBilledAccount: ws?.billedAccountUserId === row.userId,
         })
         permissionsByUser.set(row.userId, list)
       }
@@ -135,6 +143,14 @@ export const GET = withRouteHandler(
                 workspaceId: ws.id,
                 workspaceName: ws.name,
                 permission: 'admin' as const,
+                /**
+                 * Owner wins over the derived organization grant, matching
+                 * `getUsersWithPermissions` — otherwise the same person reads as
+                 * `owner` in the teammates list and `org-admin` here.
+                 */
+                roleSource:
+                  ws.ownerId === rosterMember.userId ? ('owner' as const) : ('org-admin' as const),
+                isBilledAccount: ws.billedAccountUserId === rosterMember.userId,
               }))
             : (permissionsByUser.get(rosterMember.userId) ?? []),
         }
@@ -183,10 +199,13 @@ export const GET = withRouteHandler(
 
       for (const row of externalPermissionRows) {
         const existing = externalMembersByUser.get(row.userId)
+        const externalWorkspace = workspaceById.get(row.workspaceId)
         const workspaceAccess: RosterWorkspaceAccess = {
           workspaceId: row.workspaceId,
-          workspaceName: workspaceNameById.get(row.workspaceId) ?? 'Workspace',
+          workspaceName: externalWorkspace?.name ?? 'Workspace',
           permission: row.permission,
+          roleSource: externalWorkspace?.ownerId === row.userId ? 'owner' : 'explicit',
+          isBilledAccount: externalWorkspace?.billedAccountUserId === row.userId,
         }
 
         if (existing) {
@@ -247,8 +266,11 @@ export const GET = withRouteHandler(
         const list = grantsByInvitation.get(row.invitationId) ?? []
         list.push({
           workspaceId: row.workspaceId,
-          workspaceName: workspaceNameById.get(row.workspaceId) ?? 'Workspace',
+          workspaceName: workspaceById.get(row.workspaceId)?.name ?? 'Workspace',
           permission: row.permission,
+          /** A pending invitee holds no row yet, so nothing is inherited. */
+          roleSource: 'explicit',
+          isBilledAccount: false,
         })
         grantsByInvitation.set(row.invitationId, list)
       }
@@ -269,7 +291,7 @@ export const GET = withRouteHandler(
       const data = {
         members: rosterMembers,
         pendingInvitations,
-        workspaces: orgWorkspaces,
+        workspaces: orgWorkspaces.map((ws) => ({ id: ws.id, name: ws.name })),
       } satisfies OrganizationRoster
       return NextResponse.json({
         success: true,

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

@@ -44,6 +44,7 @@ import {
 import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
 import {
   badRequestResponse,
+  conflictResponse,
   internalErrorResponse,
   notFoundResponse,
   singleResponse,
@@ -170,10 +171,20 @@ export const PATCH = withRouteHandler(
 
       const now = new Date()
 
-      await db
+      /**
+       * Conditional on the row read above still existing: a concurrent removal
+       * between that read and this write would otherwise match nothing and be
+       * reported to the caller as a successful update.
+       */
+      const updated = await db
         .update(permissions)
         .set({ permissionType: permissionLevel, updatedAt: now })
         .where(eq(permissions.id, memberId))
+        .returning({ id: permissions.id })
+
+      if (updated.length === 0) {
+        return conflictResponse('Workspace member changed during the update. Retry.')
+      }
 
       const [userData] = await db
         .select({ name: user.name, email: user.email, image: user.image })

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

@@ -9,6 +9,12 @@
  *
  * Response: AdminListResponse<AdminWorkspaceMember>
  *
+ * `createdAt` is the member's join time. It previously moved on every role
+ * change, because the in-app role-change endpoint replaced the permission row
+ * rather than amending it; that endpoint now updates in place, so only
+ * `updatedAt` tracks role changes. Consumers that diffed `createdAt` to detect
+ * recently-changed members must read `updatedAt` instead.
+ *
  * POST /api/v1/admin/workspaces/[id]/members
  *
  * Add a user to a workspace with a specific permission level.
@@ -55,6 +61,7 @@ import {
 import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
 import {
   badRequestResponse,
+  conflictResponse,
   internalErrorResponse,
   listResponse,
   notFoundResponse,
@@ -191,10 +198,20 @@ export const POST = withRouteHandler(
       if (existingPermission) {
         if (existingPermission.permissionType !== permissionLevel) {
           const now = new Date()
-          await db
+          /**
+           * Conditional on the row read above still existing: a concurrent
+           * removal between that read and this write would otherwise match
+           * nothing and be reported to the caller as a successful update.
+           */
+          const updated = await db
             .update(permissions)
             .set({ permissionType: permissionLevel, updatedAt: now })
             .where(eq(permissions.id, existingPermission.id))
+            .returning({ id: permissions.id })
+
+          if (updated.length === 0) {
+            return conflictResponse('Workspace member changed during the update. Retry.')
+          }
 
           logger.info(`Admin API: Updated user ${userId} permissions in workspace ${workspaceId}`, {
             previousPermissions: existingPermission.permissionType,
@@ -247,15 +264,31 @@ export const POST = withRouteHandler(
       const now = new Date()
       const permissionId = generateId()
 
-      await db.insert(permissions).values({
-        id: permissionId,
-        userId,
-        entityType: 'workspace',
-        entityId: workspaceId,
-        permissionType: permissionLevel,
-        createdAt: now,
-        updatedAt: now,
-      })
+      /**
+       * The existence read above is unlocked, so two concurrent adds for the
+       * same user both reach here. Conflicting on the uniqueness constraint
+       * settles it as the requested role instead of failing the loser with a
+       * 500 for a request that did what it asked.
+       */
+      const [written] = await db
+        .insert(permissions)
+        .values({
+          id: permissionId,
+          userId,
+          entityType: 'workspace',
+          entityId: workspaceId,
+          permissionType: permissionLevel,
+          createdAt: now,
+          updatedAt: now,
+        })
+        .onConflictDoUpdate({
+          target: [permissions.userId, permissions.entityType, permissions.entityId],
+          set: { permissionType: permissionLevel, updatedAt: now },
+        })
+        .returning({ id: permissions.id, createdAt: permissions.createdAt })
+
+      /** A returned id we did not mint means the conflict branch ran. */
+      const wasCreated = written?.id === permissionId
 
       logger.info(`Admin API: Added user ${userId} to workspace ${workspaceId}`, {
         permissions: permissionLevel,
@@ -288,16 +321,16 @@ export const POST = withRouteHandler(
       }
 
       return singleResponse({
-        id: permissionId,
+        id: written?.id ?? permissionId,
         workspaceId,
         userId,
         permissions: permissionLevel,
-        createdAt: now.toISOString(),
+        createdAt: (written?.createdAt ?? now).toISOString(),
         updatedAt: now.toISOString(),
         userName: userData.name,
         userEmail: userData.email,
         userImage: userData.image,
-        action: 'created' as const,
+        action: wasCreated ? ('created' as const) : ('updated' as const),
       })
     } catch (error) {
       logger.error('Admin API: Failed to add workspace member', { error, workspaceId })