mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
cb7da3044d
commit
41d89ab84e
5 changed files with 110 additions and 22 deletions
|
|
@ -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)}}
|
||||
|
|
|
|||
|
|
@ -13,14 +13,20 @@ const UsersPage = () => {
|
|||
const [keys, setKeys] = useState<null | any[]>([]);
|
||||
|
||||
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 (
|
||||
<ViewUserDashboard
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ interface CreateuserProps {
|
|||
possibleUIRoles: null | Record<string, Record<string, string>>;
|
||||
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<CreateuserProps> = ({
|
||||
userID, accessToken, teams, possibleUIRoles, onUserCreated, isEmbedded = false, organizationId }) => {
|
||||
userID, accessToken, teams, possibleUIRoles, onUserCreated, isEmbedded = false, organizationIds }) => {
|
||||
const queryClient = useQueryClient();
|
||||
const [uiSettings, setUISettings] = useState<UISettings | null>(null);
|
||||
const [form] = Form.useForm();
|
||||
|
|
@ -100,7 +100,7 @@ export const CreateUserButton: React.FC<CreateuserProps> = ({
|
|||
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<CreateuserProps> = ({
|
|||
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<CreateuserProps> = ({
|
|||
<TeamDropdown teams={teams} />
|
||||
</Form.Item>
|
||||
|
||||
{organizationIds && organizationIds.length > 1 && (
|
||||
<Form.Item
|
||||
label="Organization"
|
||||
name="organization_id"
|
||||
initialValue={organizationIds[0]}
|
||||
help="The user will be added to this organization."
|
||||
>
|
||||
<Select placeholder="Select Organization" style={{ width: "100%" }}>
|
||||
{organizationIds.map((orgId) => (
|
||||
<Option key={orgId} value={orgId}>{orgId}</Option>
|
||||
))}
|
||||
</Select>
|
||||
</Form.Item>
|
||||
)}
|
||||
|
||||
<Form.Item label="Metadata" name="metadata">
|
||||
<Input.TextArea rows={4} placeholder="Enter metadata as JSON" />
|
||||
</Form.Item>
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -265,7 +265,7 @@ const ViewUserDashboard: React.FC<ViewUserDashboardProps> = ({ 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<ViewUserDashboardProps> = ({ accessToken, toke
|
|||
</>
|
||||
) : userID && accessToken ? (
|
||||
<>
|
||||
<CreateUserButton userID={userID} accessToken={accessToken} teams={teams} possibleUIRoles={possibleUIRoles} organizationId={orgAdminOrgIds?.[0] ?? null} />
|
||||
<CreateUserButton userID={userID} accessToken={accessToken} teams={teams} possibleUIRoles={possibleUIRoles} organizationIds={orgAdminOrgIds ?? null} />
|
||||
|
||||
{isProxyAdmin && (
|
||||
<Button
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue