Skip to content

fix(credentials): grant admin credential role to workspace admins #4736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
serhiizghama wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(credentials): grant admin credential role to workspace admins #4736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3009cd
fix(credentials): grant admin credential role to workspace admins
serhiizghama May 25, 2026
7819ed3
fix(credentials): apply admin role check in POST credential handler
serhiizghama May 25, 2026
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
9 changes: 6 additions & 3 deletions apps/sim/app/api/credentials/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
normalizeAtlassianDomain,
validateAtlassianServiceAccount,
} from '@/lib/credentials/atlassian-service-account'
import { getWorkspaceMemberUserIds } from '@/lib/credentials/environment'
import { getWorkspaceAdminUserIds, getWorkspaceMemberUserIds } from '@/lib/credentials/environment'
import { syncWorkspaceOAuthCredentialsForUser } from '@/lib/credentials/oauth'
import { getServiceConfigByProviderId } from '@/lib/oauth'
import {
Expand Down Expand Up @@ -535,15 +535,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
})

if ((type === 'env_workspace' || type === 'service_account') && workspaceRow?.ownerId) {
const workspaceUserIds = await getWorkspaceMemberUserIds(workspaceId)
const [workspaceUserIds, adminUserIds] = await Promise.all([
getWorkspaceMemberUserIds(workspaceId),
getWorkspaceAdminUserIds(workspaceId),
])
if (workspaceUserIds.length > 0) {
for (const memberUserId of workspaceUserIds) {
await tx.insert(credentialMember).values({
id: generateId(),
credentialId,
userId: memberUserId,
role:
memberUserId === workspaceRow.ownerId || memberUserId === session.user.id
adminUserIds.has(memberUserId) || memberUserId === session.user.id
? 'admin'
: 'member',
status: 'active',
Expand Down
40 changes: 34 additions & 6 deletions apps/sim/lib/credentials/environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,31 @@ export async function getWorkspaceMemberUserIds(workspaceId: string): Promise<st
return Array.from(memberIds)
}

export async function getWorkspaceAdminUserIds(workspaceId: string): Promise<Set<string>> {
const [workspaceRows, adminPermissionRows] = await Promise.all([
db
.select({ ownerId: workspace.ownerId })
.from(workspace)
.where(eq(workspace.id, workspaceId))
.limit(1),
db
.select({ userId: permissions.userId })
.from(permissions)
.where(
and(
eq(permissions.entityType, 'workspace'),
eq(permissions.entityId, workspaceId),
eq(permissions.permissionType, 'admin')
)
),
])
const adminIds = new Set<string>(adminPermissionRows.map((row) => row.userId))
if (workspaceRows[0]?.ownerId) {
adminIds.add(workspaceRows[0].ownerId)
}
return adminIds
}

export async function getUserWorkspaceIds(userId: string): Promise<string[]> {
const [permissionRows, ownedWorkspaceRows] = await Promise.all([
db
Expand Down Expand Up @@ -64,7 +89,8 @@ export async function getUserWorkspaceIds(userId: string): Promise<string[]> {
async function ensureWorkspaceCredentialMemberships(
credentialId: string,
memberUserIds: string[],
ownerUserId: string
ownerUserId: string,
adminUserIds: Set<string>
) {
Comment on lines 89 to 94
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 ownerUserId parameter now serves only as invitedBy

After this change, ownerUserId is no longer used for role determination inside ensureWorkspaceCredentialMemberships — adminUserIds handles that entirely. The parameter is still used for invitedBy, but its name no longer reflects that narrowed purpose. Callers reading the signature may incorrectly assume it still drives the admin-role check. Consider renaming it to invitedByUserId (or a similar name) to avoid confusion.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

if (!memberUserIds.length) return

Expand All @@ -87,7 +113,7 @@ async function ensureWorkspaceCredentialMemberships(
const now = new Date()

for (const memberUserId of memberUserIds) {
const targetRole = memberUserId === ownerUserId ? 'admin' : 'member'
const targetRole = adminUserIds.has(memberUserId) ? 'admin' : 'member'
const existing = byUserId.get(memberUserId)
if (existing) {
if (existing.status === 'revoked') {
Expand Down Expand Up @@ -126,13 +152,14 @@ export async function syncWorkspaceEnvCredentials(params: {
actingUserId: string
}) {
const { workspaceId, envKeys, actingUserId } = params
const [[workspaceRow], memberUserIds] = await Promise.all([
const [[workspaceRow], memberUserIds, adminUserIds] = await Promise.all([
db
.select({ ownerId: workspace.ownerId })
.from(workspace)
.where(eq(workspace.id, workspaceId))
.limit(1),
getWorkspaceMemberUserIds(workspaceId),
getWorkspaceAdminUserIds(workspaceId),
])
Comment on lines +155 to 163
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant workspace queries from helper functions

syncWorkspaceEnvCredentials (and createWorkspaceEnvCredentials at line 246) now issues three separate workspace-table SELECTs per call: one directly in the Promise.all, one inside getWorkspaceMemberUserIds, and one inside getWorkspaceAdminUserIds. The permissions table is also queried twice (once for all members, once filtered to admin). A single SELECT permissions WHERE entityId = workspaceId joined with the workspace owner fetch would cover both sets, eliminating the extra round-trips.


if (!workspaceRow) return
Expand Down Expand Up @@ -182,7 +209,7 @@ export async function syncWorkspaceEnvCredentials(params: {
}

for (const credentialId of credentialIdsToEnsureMembership) {
await ensureWorkspaceCredentialMemberships(credentialId, memberUserIds, workspaceRow.ownerId)
await ensureWorkspaceCredentialMemberships(credentialId, memberUserIds, workspaceRow.ownerId, adminUserIds)
}

if (normalizedKeys.length > 0) {
Expand Down Expand Up @@ -216,13 +243,14 @@ export async function createWorkspaceEnvCredentials(params: {
const keys = Array.from(new Set(newKeys.filter(Boolean)))
if (keys.length === 0) return

const [[workspaceRow], memberUserIds] = await Promise.all([
const [[workspaceRow], memberUserIds, adminUserIds] = await Promise.all([
db
.select({ ownerId: workspace.ownerId })
.from(workspace)
.where(eq(workspace.id, workspaceId))
.limit(1),
getWorkspaceMemberUserIds(workspaceId),
getWorkspaceAdminUserIds(workspaceId),
])

if (!workspaceRow) return
Expand Down Expand Up @@ -259,7 +287,7 @@ export async function createWorkspaceEnvCredentials(params: {
id: generateId(),
credentialId,
userId: memberUserId,
role: (memberUserId === ownerUserId ? 'admin' : 'member') as 'admin' | 'member',
role: (adminUserIds.has(memberUserId) ? 'admin' : 'member') as 'admin' | 'member',
status: 'active' as const,
joinedAt: now,
invitedBy: ownerUserId,
Expand Down