From 41d89ab84eeea0c75b96223b3317ebbd8c61dedb Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Sat, 7 Mar 2026 18:28:01 -0800 Subject: [PATCH] fix: address review feedback for org admin user management - Backend authorization: enforce server-side org scoping on /user/list. Non-proxy-admin callers must be org_admin; their permitted org IDs are fetched from the DB and intersected with any requested org filter. - Race condition: gate userListQuery on org data being loaded for non-proxy-admin users (undefined = loading, null = no filter needed). - Silent failure: surface organizationMemberAddCall errors via NotificationsManager instead of only console.error. - Multi-org: org admins managing multiple orgs can choose which org to add the new user to via a dropdown (defaults to first org). - Change organization_id param to List[str] (repeated query params) instead of comma-separated string. Co-Authored-By: Claude Opus 4.6 --- .../internal_user_endpoints.py | 79 +++++++++++++++++-- .../src/app/(dashboard)/users/page.tsx | 16 ++-- .../src/components/CreateUserButton.tsx | 29 +++++-- .../src/components/networking.tsx | 4 +- .../src/components/view_users.tsx | 4 +- 5 files changed, 110 insertions(+), 22 deletions(-) diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index 1ddb7b47c1d..2e7cf5e7b1e 100644 --- a/litellm/proxy/management_endpoints/internal_user_endpoints.py +++ b/litellm/proxy/management_endpoints/internal_user_endpoints.py @@ -1502,10 +1502,11 @@ async def get_users( sort_order: str = fastapi.Query( default="asc", description="Sort order ('asc' or 'desc')" ), - organization_id: Optional[str] = fastapi.Query( + organization_id: Optional[List[str]] = fastapi.Query( default=None, - description="Filter users by organization membership. Comma-separated for multiple orgs.", + description="Filter users by organization membership. Pass multiple values for multiple orgs.", ), + user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), ): """ Get a paginated list of users with filtering and sorting options. @@ -1534,7 +1535,11 @@ async def get_users( sort_order: Optional[str] Sort order ('asc' or 'desc') """ - from litellm.proxy.proxy_server import prisma_client + from litellm.proxy.proxy_server import ( + prisma_client, + proxy_logging_obj, + user_api_key_cache, + ) if prisma_client is None: raise HTTPException( @@ -1542,6 +1547,67 @@ async def get_users( detail={"error": f"No db connected. prisma client={prisma_client}"}, ) + # Server-side authorization: proxy admins see all, org admins see only their org(s) + is_proxy_admin = _user_has_admin_view(user_api_key_dict) + allowed_org_ids: Optional[List[str]] = None + if not is_proxy_admin: + if user_api_key_dict.user_id is None: + raise HTTPException( + status_code=403, + detail={ + "error": "Only proxy admins and organization admins can list users." + }, + ) + try: + caller_user = await get_user_object( + user_id=user_api_key_dict.user_id, + prisma_client=prisma_client, + user_api_key_cache=user_api_key_cache, + user_id_upsert=False, + proxy_logging_obj=proxy_logging_obj, + ) + except ValueError: + raise HTTPException( + status_code=403, + detail={ + "error": "Only proxy admins and organization admins can list users." + }, + ) + if caller_user is None: + raise HTTPException( + status_code=403, + detail={ + "error": "Only proxy admins and organization admins can list users." + }, + ) + allowed_org_ids = [ + m.organization_id + for m in (caller_user.organization_memberships or []) + if m.user_role == LitellmUserRoles.ORG_ADMIN.value + ] + if not allowed_org_ids: + raise HTTPException( + status_code=403, + detail={ + "error": "Only proxy admins and organization admins can list users." + }, + ) + # If client also sent organization_id, intersect with allowed orgs + if organization_id: + requested = set(organization_id) + allowed = set(allowed_org_ids) + intersection = list(requested & allowed) + if not intersection: + raise HTTPException( + status_code=403, + detail={ + "error": "You do not have org_admin access to the requested organization(s)." + }, + ) + allowed_org_ids = intersection + # For org admins, always enforce org scoping + organization_id = allowed_org_ids + # Calculate skip and take for pagination skip = (page - 1) * page_size @@ -1580,12 +1646,9 @@ async def get_users( "in": sso_id_list, } - if organization_id is not None and isinstance(organization_id, str): - org_id_list = [ - oid.strip() for oid in organization_id.split(",") if oid.strip() - ] + if organization_id: where_conditions["organization_memberships"] = { - "some": {"organization_id": {"in": org_id_list}} + "some": {"organization_id": {"in": organization_id}} } ## Filter any none fastapi.Query params - e.g. where_conditions: {'user_email': {'contains': Query(None), 'mode': 'insensitive'}, 'teams': {'has': Query(None)}} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/users/page.tsx b/ui/litellm-dashboard/src/app/(dashboard)/users/page.tsx index ae7a0b9767a..75a50a0d388 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/users/page.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/users/page.tsx @@ -13,14 +13,20 @@ const UsersPage = () => { const [keys, setKeys] = useState([]); const { teams } = useTeams(); - const { data: organizations } = useOrganizations(); + const { data: organizations, isLoading: isOrgsLoading } = useOrganizations(); - // Compute org IDs where the user is an org_admin, but only if they're NOT a proxy admin - const orgAdminOrgIds = useMemo(() => { - if (!userId || !organizations || !userRole) return null; + // Three states: + // - undefined: org data still loading (non-proxy-admin) — query should wait + // - null: proxy admin or no org filtering needed — query runs unfiltered + // - string[]: org admin org IDs — query runs filtered + const orgAdminOrgIds = useMemo((): string[] | null | undefined => { + if (!userId || !userRole) return null; // Proxy admins see all users — no org filtering if (isProxyAdminRole(userRole)) return null; + // Still loading org data — signal "not ready yet" + if (isOrgsLoading || !organizations) return undefined; + const adminOrgIds = organizations .filter((org: Organization) => org.members?.some((member) => member.user_id === userId && member.user_role === "org_admin") @@ -28,7 +34,7 @@ const UsersPage = () => { .map((org: Organization) => org.organization_id); return adminOrgIds.length > 0 ? adminOrgIds : null; - }, [userId, organizations, userRole]); + }, [userId, organizations, userRole, isOrgsLoading]); return ( >; onUserCreated?: (userId: string) => void; isEmbedded?: boolean; - organizationId?: string | null; + organizationIds?: string[] | null; } // Define an interface for the UI settings @@ -57,7 +57,7 @@ interface UISettings { } export const CreateUserButton: React.FC = ({ - userID, accessToken, teams, possibleUIRoles, onUserCreated, isEmbedded = false, organizationId }) => { + userID, accessToken, teams, possibleUIRoles, onUserCreated, isEmbedded = false, organizationIds }) => { const queryClient = useQueryClient(); const [uiSettings, setUISettings] = useState(null); const [form] = Form.useForm(); @@ -100,7 +100,7 @@ export const CreateUserButton: React.FC = ({ form.resetFields(); }; - const handleCreate = async (formValues: { user_id: string; models?: string[]; user_role: string }) => { + const handleCreate = async (formValues: { user_id: string; models?: string[]; user_role: string; organization_id?: string }) => { try { NotificationsManager.info("Making API Call"); if (!isEmbedded) { @@ -114,15 +114,17 @@ export const CreateUserButton: React.FC = ({ setApiuser(true); const user_id = response.data?.user_id || response.user_id; - // Auto-add user to the org admin's organization - if (organizationId && user_id) { + // Auto-add user to the selected organization (org admin flow) + const targetOrgId = formValues.organization_id || (organizationIds?.length === 1 ? organizationIds[0] : null); + if (targetOrgId && user_id) { try { - await organizationMemberAddCall(accessToken, organizationId, { + await organizationMemberAddCall(accessToken, targetOrgId, { role: "internal_user", user_id: user_id, }); } catch (orgError) { console.error("Failed to add user to organization:", orgError); + NotificationsManager.fromBackend("User created but failed to add to organization. Please add them manually."); } } @@ -293,6 +295,21 @@ export const CreateUserButton: React.FC = ({ + {organizationIds && organizationIds.length > 1 && ( + + + + )} + diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 1dd9afc698e..8c5e1157218 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -1153,7 +1153,9 @@ export const userListCall = async ( } if (organizationIds && organizationIds.length > 0) { - queryParams.append("organization_id", organizationIds.join(",")); + for (const orgId of organizationIds) { + queryParams.append("organization_id", orgId); + } } const queryString = queryParams.toString(); diff --git a/ui/litellm-dashboard/src/components/view_users.tsx b/ui/litellm-dashboard/src/components/view_users.tsx index 49a55f393ff..2d3dde2f0fd 100644 --- a/ui/litellm-dashboard/src/components/view_users.tsx +++ b/ui/litellm-dashboard/src/components/view_users.tsx @@ -265,7 +265,7 @@ const ViewUserDashboard: React.FC = ({ accessToken, toke orgAdminOrgIds ?? null, ); }, - enabled: Boolean(accessToken && token && userRole && userID), + enabled: Boolean(accessToken && token && userRole && userID && orgAdminOrgIds !== undefined), placeholderData: (previousData) => previousData, }); const userListResponse = userListQuery.data; @@ -304,7 +304,7 @@ const ViewUserDashboard: React.FC = ({ accessToken, toke ) : userID && accessToken ? ( <> - + {isProxyAdmin && (