Skip to content

Commit 1fdb43f

Browse files
improvement(perms): followup to org scoping of permission groups
1 parent 522ba8e commit 1fdb43f

4 files changed

Lines changed: 59 additions & 18 deletions

File tree

apps/sim/app/api/permission-groups/user/route.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import { getSession } from '@/lib/auth'
77
import { isOrganizationOnEnterprisePlan } from '@/lib/billing'
88
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import { parsePermissionGroupConfig } from '@/lib/permission-groups/types'
10-
import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils'
10+
import {
11+
checkWorkspaceAccess,
12+
isOrganizationAdminOrOwner,
13+
} from '@/lib/workspaces/permissions/utils'
1114

1215
export const GET = withRouteHandler(async (req: Request) => {
1316
const session = await getSession()
@@ -32,12 +35,33 @@ export const GET = withRouteHandler(async (req: Request) => {
3235
}
3336

3437
const organizationId = access.workspace?.organizationId ?? null
35-
if (!organizationId || !(await isOrganizationOnEnterprisePlan(organizationId))) {
38+
39+
// Workspaces without an organization have no permission groups, and the caller
40+
// can never be an org admin in that case.
41+
if (!organizationId) {
42+
return NextResponse.json({
43+
permissionGroupId: null,
44+
groupName: null,
45+
config: null,
46+
entitled: false,
47+
organizationId: null,
48+
isOrgAdmin: false,
49+
})
50+
}
51+
52+
// Resolve role + entitlement against the WORKSPACE's owning organization (not
53+
// the caller's active org) so management gating is scoped to the org that
54+
// actually governs this workspace. External members are not org admins here.
55+
const isOrgAdmin = await isOrganizationAdminOrOwner(session.user.id, organizationId)
56+
57+
if (!(await isOrganizationOnEnterprisePlan(organizationId))) {
3658
return NextResponse.json({
3759
permissionGroupId: null,
3860
groupName: null,
3961
config: null,
4062
entitled: false,
63+
organizationId,
64+
isOrgAdmin,
4165
})
4266
}
4367

@@ -80,6 +104,8 @@ export const GET = withRouteHandler(async (req: Request) => {
80104
groupName: null,
81105
config: null,
82106
entitled: true,
107+
organizationId,
108+
isOrgAdmin,
83109
})
84110
}
85111

@@ -88,5 +114,7 @@ export const GET = withRouteHandler(async (req: Request) => {
88114
groupName: resolved.groupName,
89115
config: parsePermissionGroupConfig(resolved.config),
90116
entitled: true,
117+
organizationId,
118+
isOrgAdmin,
91119
})
92120
})

apps/sim/app/api/v1/admin/access-control/route.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
3535
import { withAdminAuth } from '@/app/api/v1/admin/middleware'
3636
import {
3737
adminValidationErrorResponse,
38+
badRequestResponse,
3839
internalErrorResponse,
3940
singleResponse,
4041
} from '@/app/api/v1/admin/responses'
@@ -133,6 +134,12 @@ export const DELETE = withRouteHandler(
133134
if (!parsed.success) return parsed.response
134135

135136
const { organizationId, reason: rawReason } = parsed.data.query
137+
// The contract's refine guarantees this at the boundary; this explicit guard
138+
// narrows the type (avoiding a non-null assertion) and stays correct even if
139+
// the contract changes.
140+
if (!organizationId) {
141+
return badRequestResponse('organizationId is required')
142+
}
136143
const reason = rawReason || 'Enterprise plan churn cleanup'
137144

138145
try {
@@ -142,7 +149,7 @@ export const DELETE = withRouteHandler(
142149
name: permissionGroup.name,
143150
})
144151
.from(permissionGroup)
145-
.where(eq(permissionGroup.organizationId, organizationId!))
152+
.where(eq(permissionGroup.organizationId, organizationId))
146153

147154
if (existingGroups.length === 0) {
148155
logger.info('Admin API: No permission groups to delete', { organizationId })

apps/sim/ee/access-control/components/access-control.tsx

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,10 @@ import {
2525
Switch,
2626
} from '@/components/emcn'
2727
import { ArrowLeft } from '@/components/emcn/icons'
28-
import { useSession } from '@/lib/auth/auth-client'
2928
import { getEnv, isTruthy } from '@/lib/core/config/env'
3029
import { cn } from '@/lib/core/utils/cn'
3130
import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
3231
import { getUserColor } from '@/lib/workspaces/colors'
33-
import { getUserRole } from '@/lib/workspaces/organization'
3432
import { getAllBlocks } from '@/blocks'
3533
import {
3634
type PermissionGroup,
@@ -44,7 +42,7 @@ import {
4442
useUserPermissionConfig,
4543
} from '@/ee/access-control/hooks/permission-groups'
4644
import { useBlacklistedProviders } from '@/hooks/queries/allowed-providers'
47-
import { useOrganizationRoster, useOrganizations } from '@/hooks/queries/organization'
45+
import { useOrganizationRoster } from '@/hooks/queries/organization'
4846
import { useProviderModels } from '@/hooks/queries/providers'
4947
import {
5048
DYNAMIC_MODEL_PROVIDERS,
@@ -407,27 +405,31 @@ export function AccessControl() {
407405
const params = useParams()
408406
const workspaceId = typeof params?.workspaceId === 'string' ? params.workspaceId : undefined
409407

410-
const { data: session } = useSession()
411-
const { data: organizationsData, isPending: orgLoading } = useOrganizations()
412-
const activeOrganization = organizationsData?.activeOrganization
413-
const organizationId = activeOrganization?.id
408+
// Access control is governed by the workspace's OWNING organization, which may
409+
// differ from the caller's active org (e.g. external members). Resolve the org
410+
// id and the caller's admin status server-side from the workspace so gating is
411+
// never keyed off the session's active org.
412+
const { data: userPermissionConfig, isPending: entitlementLoading } =
413+
useUserPermissionConfig(workspaceId)
414+
const organizationId = userPermissionConfig?.organizationId ?? undefined
415+
const currentUserIsOrgAdmin = userPermissionConfig?.isOrgAdmin ?? false
414416

417+
// Group + roster reads require org admin/owner on the host org; only fetch them
418+
// for admins to avoid surfacing expected 403s for non-admins/external members.
415419
const { data: permissionGroups = [], isPending: groupsLoading } = usePermissionGroups(
416420
organizationId,
417-
!!organizationId
421+
!!organizationId && currentUserIsOrgAdmin
418422
)
419-
const { data: roster } = useOrganizationRoster(organizationId)
420-
const { data: userPermissionConfig, isPending: entitlementLoading } =
421-
useUserPermissionConfig(workspaceId)
422-
423-
const userRole = getUserRole(activeOrganization, session?.user?.email ?? undefined)
424-
const currentUserIsOrgAdmin = userRole === 'owner' || userRole === 'admin'
423+
const { data: roster } = useOrganizationRoster(currentUserIsOrgAdmin ? organizationId : undefined)
425424

426425
const accessControlEnabledLocally = isTruthy(getEnv('NEXT_PUBLIC_ACCESS_CONTROL_ENABLED'))
427426
const isEntitled = accessControlEnabledLocally || !!userPermissionConfig?.entitled
428427
const canManage = isEntitled && currentUserIsOrgAdmin && !!organizationId
429428

430-
const isLoading = !workspaceId || orgLoading || groupsLoading || entitlementLoading
429+
const isLoading =
430+
!workspaceId ||
431+
entitlementLoading ||
432+
(!!organizationId && currentUserIsOrgAdmin && groupsLoading)
431433

432434
const createPermissionGroup = useCreatePermissionGroup()
433435
const updatePermissionGroup = useUpdatePermissionGroup()

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ export const userPermissionConfigSchema = z.object({
9090
groupName: z.string().nullable(),
9191
config: permissionGroupFullConfigSchema.nullable(),
9292
entitled: z.boolean(),
93+
/** The workspace's owning organization id (null when the workspace has no org). */
94+
organizationId: z.string().nullable(),
95+
/** Whether the caller is an owner/admin of the workspace's owning organization. */
96+
isOrgAdmin: z.boolean(),
9397
})
9498
export type UserPermissionConfig = z.output<typeof userPermissionConfigSchema>
9599

0 commit comments

Comments
 (0)