Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/sim/app/api/organizations/[id]/roster/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ describe('GET /api/organizations/[id]/roster', () => {
workspaceId: 'workspace-1',
workspaceName: 'Workspace One',
permission: 'admin',
roleSource: 'org-admin',
isBilledAccount: false,
},
],
}),
Expand All @@ -193,6 +195,8 @@ describe('GET /api/organizations/[id]/roster', () => {
workspaceId: 'workspace-1',
workspaceName: 'Workspace One',
permission: 'read',
roleSource: 'explicit',
isBilledAccount: false,
},
],
}),
Expand Down
34 changes: 28 additions & 6 deletions apps/sim/app/api/organizations/[id]/roster/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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)
}
Expand All @@ -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) ?? []),
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
import {
badRequestResponse,
conflictResponse,
internalErrorResponse,
notFoundResponse,
singleResponse,
Expand Down Expand Up @@ -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 })
Expand Down
84 changes: 63 additions & 21 deletions apps/sim/app/api/v1/admin/workspaces/[id]/members/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -55,6 +61,7 @@ import {
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
import {
badRequestResponse,
conflictResponse,
internalErrorResponse,
listResponse,
notFoundResponse,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -247,28 +264,53 @@ 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,
})

logger.info(`Admin API: Added user ${userId} to workspace ${workspaceId}`, {
permissions: permissionLevel,
permissionId,
})

/**
* 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(
wasCreated
? `Admin API: Added user ${userId} to workspace ${workspaceId}`
: `Admin API: Updated user ${userId} permissions in workspace ${workspaceId}`,
{ permissions: permissionLevel, permissionId }
)

/**
* The conflict branch amended a membership that already existed, so it is
* a role change rather than an addition — recording it as `MEMBER_ADDED`
* would put a join that never happened in the workspace's audit trail.
*/
recordAudit({
workspaceId,
actorId: 'admin-api',
action: AuditAction.MEMBER_ADDED,
action: wasCreated ? AuditAction.MEMBER_ADDED : AuditAction.MEMBER_ROLE_CHANGED,
resourceType: AuditResourceType.WORKSPACE,
resourceId: workspaceId,
description: `Admin API added member to workspace with ${permissionLevel} permissions`,
description: wasCreated
? `Admin API added member to workspace with ${permissionLevel} permissions`
: `Admin API changed workspace member permissions to ${permissionLevel}`,
metadata: { targetUserId: userId, permissions: permissionLevel },
request,
})
Expand All @@ -288,16 +330,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,
Comment thread
icecrasher321 marked this conversation as resolved.
action: wasCreated ? ('created' as const) : ('updated' as const),
})
} catch (error) {
logger.error('Admin API: Failed to add workspace member', { error, workspaceId })
Expand Down
Loading
Loading