From 3c4234c594022291bdebdaf56e72f5862779a6f2 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 13 Jun 2026 17:59:18 -0700 Subject: [PATCH] followup --- .../app/api/permission-groups/user/route.ts | 32 +++++++++++++++++-- .../app/api/v1/admin/access-control/route.ts | 9 +++++- .../components/access-control.tsx | 32 ++++++++++--------- .../lib/api/contracts/permission-groups.ts | 4 +++ 4 files changed, 59 insertions(+), 18 deletions(-) diff --git a/apps/sim/app/api/permission-groups/user/route.ts b/apps/sim/app/api/permission-groups/user/route.ts index 9d50d56284..50ed121d5a 100644 --- a/apps/sim/app/api/permission-groups/user/route.ts +++ b/apps/sim/app/api/permission-groups/user/route.ts @@ -7,7 +7,10 @@ import { getSession } from '@/lib/auth' import { isOrganizationOnEnterprisePlan } from '@/lib/billing' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { parsePermissionGroupConfig } from '@/lib/permission-groups/types' -import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils' +import { + checkWorkspaceAccess, + isOrganizationAdminOrOwner, +} from '@/lib/workspaces/permissions/utils' export const GET = withRouteHandler(async (req: Request) => { const session = await getSession() @@ -32,12 +35,33 @@ export const GET = withRouteHandler(async (req: Request) => { } const organizationId = access.workspace?.organizationId ?? null - if (!organizationId || !(await isOrganizationOnEnterprisePlan(organizationId))) { + + // Workspaces without an organization have no permission groups, and the caller + // can never be an org admin in that case. + if (!organizationId) { + return NextResponse.json({ + permissionGroupId: null, + groupName: null, + config: null, + entitled: false, + organizationId: null, + isOrgAdmin: false, + }) + } + + // Resolve role + entitlement against the WORKSPACE's owning organization (not + // the caller's active org) so management gating is scoped to the org that + // actually governs this workspace. External members are not org admins here. + const isOrgAdmin = await isOrganizationAdminOrOwner(session.user.id, organizationId) + + if (!(await isOrganizationOnEnterprisePlan(organizationId))) { return NextResponse.json({ permissionGroupId: null, groupName: null, config: null, entitled: false, + organizationId, + isOrgAdmin, }) } @@ -80,6 +104,8 @@ export const GET = withRouteHandler(async (req: Request) => { groupName: null, config: null, entitled: true, + organizationId, + isOrgAdmin, }) } @@ -88,5 +114,7 @@ export const GET = withRouteHandler(async (req: Request) => { groupName: resolved.groupName, config: parsePermissionGroupConfig(resolved.config), entitled: true, + organizationId, + isOrgAdmin, }) }) diff --git a/apps/sim/app/api/v1/admin/access-control/route.ts b/apps/sim/app/api/v1/admin/access-control/route.ts index 9f0b72832d..6df0f9f288 100644 --- a/apps/sim/app/api/v1/admin/access-control/route.ts +++ b/apps/sim/app/api/v1/admin/access-control/route.ts @@ -35,6 +35,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { withAdminAuth } from '@/app/api/v1/admin/middleware' import { adminValidationErrorResponse, + badRequestResponse, internalErrorResponse, singleResponse, } from '@/app/api/v1/admin/responses' @@ -133,6 +134,12 @@ export const DELETE = withRouteHandler( if (!parsed.success) return parsed.response const { organizationId, reason: rawReason } = parsed.data.query + // The contract's refine guarantees this at the boundary; this explicit guard + // narrows the type (avoiding a non-null assertion) and stays correct even if + // the contract changes. + if (!organizationId) { + return badRequestResponse('organizationId is required') + } const reason = rawReason || 'Enterprise plan churn cleanup' try { @@ -142,7 +149,7 @@ export const DELETE = withRouteHandler( name: permissionGroup.name, }) .from(permissionGroup) - .where(eq(permissionGroup.organizationId, organizationId!)) + .where(eq(permissionGroup.organizationId, organizationId)) if (existingGroups.length === 0) { logger.info('Admin API: No permission groups to delete', { organizationId }) diff --git a/apps/sim/ee/access-control/components/access-control.tsx b/apps/sim/ee/access-control/components/access-control.tsx index 0a6897603d..37c2ebf6c0 100644 --- a/apps/sim/ee/access-control/components/access-control.tsx +++ b/apps/sim/ee/access-control/components/access-control.tsx @@ -25,12 +25,10 @@ import { Switch, } from '@/components/emcn' import { ArrowLeft } from '@/components/emcn/icons' -import { useSession } from '@/lib/auth/auth-client' import { getEnv, isTruthy } from '@/lib/core/config/env' import { cn } from '@/lib/core/utils/cn' import type { PermissionGroupConfig } from '@/lib/permission-groups/types' import { getUserColor } from '@/lib/workspaces/colors' -import { getUserRole } from '@/lib/workspaces/organization' import { getAllBlocks } from '@/blocks' import { type PermissionGroup, @@ -44,7 +42,7 @@ import { useUserPermissionConfig, } from '@/ee/access-control/hooks/permission-groups' import { useBlacklistedProviders } from '@/hooks/queries/allowed-providers' -import { useOrganizationRoster, useOrganizations } from '@/hooks/queries/organization' +import { useOrganizationRoster } from '@/hooks/queries/organization' import { useProviderModels } from '@/hooks/queries/providers' import { DYNAMIC_MODEL_PROVIDERS, @@ -407,27 +405,31 @@ export function AccessControl() { const params = useParams() const workspaceId = typeof params?.workspaceId === 'string' ? params.workspaceId : undefined - const { data: session } = useSession() - const { data: organizationsData, isPending: orgLoading } = useOrganizations() - const activeOrganization = organizationsData?.activeOrganization - const organizationId = activeOrganization?.id + // Access control is governed by the workspace's OWNING organization, which may + // differ from the caller's active org (e.g. external members). Resolve the org + // id and the caller's admin status server-side from the workspace so gating is + // never keyed off the session's active org. + const { data: userPermissionConfig, isPending: entitlementLoading } = + useUserPermissionConfig(workspaceId) + const organizationId = userPermissionConfig?.organizationId ?? undefined + const currentUserIsOrgAdmin = userPermissionConfig?.isOrgAdmin ?? false + // Group + roster reads require org admin/owner on the host org; only fetch them + // for admins to avoid surfacing expected 403s for non-admins/external members. const { data: permissionGroups = [], isPending: groupsLoading } = usePermissionGroups( organizationId, - !!organizationId + !!organizationId && currentUserIsOrgAdmin ) - const { data: roster } = useOrganizationRoster(organizationId) - const { data: userPermissionConfig, isPending: entitlementLoading } = - useUserPermissionConfig(workspaceId) - - const userRole = getUserRole(activeOrganization, session?.user?.email ?? undefined) - const currentUserIsOrgAdmin = userRole === 'owner' || userRole === 'admin' + const { data: roster } = useOrganizationRoster(currentUserIsOrgAdmin ? organizationId : undefined) const accessControlEnabledLocally = isTruthy(getEnv('NEXT_PUBLIC_ACCESS_CONTROL_ENABLED')) const isEntitled = accessControlEnabledLocally || !!userPermissionConfig?.entitled const canManage = isEntitled && currentUserIsOrgAdmin && !!organizationId - const isLoading = !workspaceId || orgLoading || groupsLoading || entitlementLoading + const isLoading = + !workspaceId || + entitlementLoading || + (!!organizationId && currentUserIsOrgAdmin && groupsLoading) const createPermissionGroup = useCreatePermissionGroup() const updatePermissionGroup = useUpdatePermissionGroup() diff --git a/apps/sim/lib/api/contracts/permission-groups.ts b/apps/sim/lib/api/contracts/permission-groups.ts index 0c82682dce..8e47195249 100644 --- a/apps/sim/lib/api/contracts/permission-groups.ts +++ b/apps/sim/lib/api/contracts/permission-groups.ts @@ -90,6 +90,10 @@ export const userPermissionConfigSchema = z.object({ groupName: z.string().nullable(), config: permissionGroupFullConfigSchema.nullable(), entitled: z.boolean(), + /** The workspace's owning organization id (null when the workspace has no org). */ + organizationId: z.string().nullable(), + /** Whether the caller is an owner/admin of the workspace's owning organization. */ + isOrgAdmin: z.boolean(), }) export type UserPermissionConfig = z.output