From b3c092f489ff0cce2758f3364ede3248cb4196c2 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Thu, 5 Mar 2026 15:04:33 -0800 Subject: [PATCH] [Fix] Address code review: delegate team admin check to shared helper, fix sidebar team admin exemption - rbac_utils.py: remove duplicated _check_if_team_admin/_is_user_team_admin_for_any_team; delegate to _user_has_admin_privileges from management_endpoints/common_utils with the shared user_api_key_cache (fixes no-op DualCache and missing org admin coverage) - test_rbac_utils.py: update patch target to match new delegation path - SidebarProvider.tsx: pass allowAgentsForTeamAdmins and allowVectorStoresForTeamAdmins props to Sidebar - leftnav.tsx: add useTeams hook + isTeamAdmin memo; exempt team admins from sidebar filtering when allow_*_for_team_admins is enabled (fixes frontend/backend inconsistency) Co-Authored-By: Claude Sonnet 4.6 --- litellm/proxy/common_utils/rbac_utils.py | 88 +++---------------- .../proxy/common_utils/test_rbac_utils.py | 8 +- .../components/SidebarProvider.tsx | 12 +++ .../src/components/leftnav.tsx | 18 ++-- 4 files changed, 40 insertions(+), 86 deletions(-) diff --git a/litellm/proxy/common_utils/rbac_utils.py b/litellm/proxy/common_utils/rbac_utils.py index 2b187d1806..dd6af76cb3 100644 --- a/litellm/proxy/common_utils/rbac_utils.py +++ b/litellm/proxy/common_utils/rbac_utils.py @@ -5,38 +5,9 @@ These helpers are used by agent and vector store endpoints to enforce proxy-admin-configurable toggles that restrict access for internal users. """ -from typing import TYPE_CHECKING - from fastapi import HTTPException -from litellm._logging import verbose_proxy_logger -from litellm.proxy._types import LiteLLM_TeamTable, LitellmUserRoles, UserAPIKeyAuth - -if TYPE_CHECKING: - pass - - -def _is_user_team_admin_for_any_team( - user_api_key_dict: UserAPIKeyAuth, - teams: list, -) -> bool: - """ - Return True if the user is an admin member in at least one of the given teams. - - Args: - user_api_key_dict: The authenticated user. - teams: List of Prisma team records (from litellm_teamtable.find_many). - """ - for team in teams: - team_obj = LiteLLM_TeamTable(**team.model_dump()) - for member in team_obj.members_with_roles: - if ( - member.user_id is not None - and member.user_id == user_api_key_dict.user_id - and member.role == "admin" - ): - return True - return False +from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth async def check_feature_access_for_user( @@ -60,7 +31,7 @@ async def check_feature_access_for_user( ): return - from litellm.proxy.proxy_server import general_settings + from litellm.proxy.proxy_server import general_settings, prisma_client, user_api_key_cache disable_flag = f"disable_{feature_name}_for_internal_users" allow_team_admins_flag = f"allow_{feature_name}_for_team_admins" @@ -69,10 +40,16 @@ async def check_feature_access_for_user( # Feature is not disabled — allow all authenticated users. return - # Feature is disabled. Check if team admins are exempted. + # Feature is disabled. Check if team/org admins are exempted. if general_settings.get(allow_team_admins_flag, False): - is_team_admin = await _check_if_team_admin(user_api_key_dict) - if is_team_admin: + from litellm.proxy.management_endpoints.common_utils import _user_has_admin_privileges + + is_admin = await _user_has_admin_privileges( + user_api_key_dict=user_api_key_dict, + prisma_client=prisma_client, + user_api_key_cache=user_api_key_cache, + ) + if is_admin: return raise HTTPException( @@ -81,46 +58,3 @@ async def check_feature_access_for_user( "error": f"Access to {feature_name} is disabled for your role. Contact your proxy admin." }, ) - - -async def _check_if_team_admin(user_api_key_dict: UserAPIKeyAuth) -> bool: - """ - Return True if the user is a team admin in any team. - Mirrors the logic in management_endpoints/common_utils._user_has_admin_privileges - but scoped to team-admin check only. - """ - from litellm.proxy.proxy_server import prisma_client - - if prisma_client is None or user_api_key_dict.user_id is None: - return False - - from litellm.caching import DualCache - from litellm.proxy.auth.auth_checks import get_user_object - - try: - user_obj = await get_user_object( - user_id=user_api_key_dict.user_id, - prisma_client=prisma_client, - user_api_key_cache=DualCache(), - user_id_upsert=False, - proxy_logging_obj=None, - ) - - if user_obj is None: - return False - - if user_obj.teams is None or len(user_obj.teams) == 0: - return False - - teams = await prisma_client.db.litellm_teamtable.find_many( - where={"team_id": {"in": user_obj.teams}} - ) - - return _is_user_team_admin_for_any_team(user_api_key_dict, teams) - - except Exception as e: - verbose_proxy_logger.debug( - f"rbac_utils: error checking team admin status for user " - f"{user_api_key_dict.user_id}: {e}" - ) - return False diff --git a/tests/litellm/proxy/common_utils/test_rbac_utils.py b/tests/litellm/proxy/common_utils/test_rbac_utils.py index 997a2e19b7..7dd04043e6 100644 --- a/tests/litellm/proxy/common_utils/test_rbac_utils.py +++ b/tests/litellm/proxy/common_utils/test_rbac_utils.py @@ -101,7 +101,7 @@ async def test_agents_disabled_team_admin_allowed(): clear=True, ): with patch( - "litellm.proxy.common_utils.rbac_utils._check_if_team_admin", + "litellm.proxy.management_endpoints.common_utils._user_has_admin_privileges", new=AsyncMock(return_value=True), ): await check_feature_access_for_user(user, "agents") @@ -116,7 +116,7 @@ async def test_agents_disabled_non_team_admin_blocked(): clear=True, ): with patch( - "litellm.proxy.common_utils.rbac_utils._check_if_team_admin", + "litellm.proxy.management_endpoints.common_utils._user_has_admin_privileges", new=AsyncMock(return_value=False), ): with pytest.raises(HTTPException) as exc_info: @@ -133,7 +133,7 @@ async def test_vector_stores_disabled_team_admin_allowed(): clear=True, ): with patch( - "litellm.proxy.common_utils.rbac_utils._check_if_team_admin", + "litellm.proxy.management_endpoints.common_utils._user_has_admin_privileges", new=AsyncMock(return_value=True), ): await check_feature_access_for_user(user, "vector_stores") @@ -148,7 +148,7 @@ async def test_vector_stores_disabled_non_team_admin_blocked(): clear=True, ): with patch( - "litellm.proxy.common_utils.rbac_utils._check_if_team_admin", + "litellm.proxy.management_endpoints.common_utils._user_has_admin_privileges", new=AsyncMock(return_value=False), ): with pytest.raises(HTTPException) as exc_info: diff --git a/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx b/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx index 7dcc3fa8a1..49e6569f1a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/components/SidebarProvider.tsx @@ -16,7 +16,9 @@ const SidebarProvider = ({ setPage, defaultSelectedKey, sidebarCollapsed }: Side const [enabledPagesInternalUsers, setEnabledPagesInternalUsers] = useState(null); const [enableProjectsUI, setEnableProjectsUI] = useState(false); const [disableAgentsForInternalUsers, setDisableAgentsForInternalUsers] = useState(false); + const [allowAgentsForTeamAdmins, setAllowAgentsForTeamAdmins] = useState(false); const [disableVectorStoresForInternalUsers, setDisableVectorStoresForInternalUsers] = useState(false); + const [allowVectorStoresForTeamAdmins, setAllowVectorStoresForTeamAdmins] = useState(false); useEffect(() => { const fetchUISettings = async () => { @@ -46,9 +48,17 @@ const SidebarProvider = ({ setPage, defaultSelectedKey, sidebarCollapsed }: Side setDisableAgentsForInternalUsers(Boolean(settings.values.disable_agents_for_internal_users)); } + if (settings?.values?.allow_agents_for_team_admins !== undefined) { + setAllowAgentsForTeamAdmins(Boolean(settings.values.allow_agents_for_team_admins)); + } + if (settings?.values?.disable_vector_stores_for_internal_users !== undefined) { setDisableVectorStoresForInternalUsers(Boolean(settings.values.disable_vector_stores_for_internal_users)); } + + if (settings?.values?.allow_vector_stores_for_team_admins !== undefined) { + setAllowVectorStoresForTeamAdmins(Boolean(settings.values.allow_vector_stores_for_team_admins)); + } } catch (error) { console.error("[SidebarProvider] Failed to fetch UI settings:", error); } @@ -65,7 +75,9 @@ const SidebarProvider = ({ setPage, defaultSelectedKey, sidebarCollapsed }: Side enabledPagesInternalUsers={enabledPagesInternalUsers} enableProjectsUI={enableProjectsUI} disableAgentsForInternalUsers={disableAgentsForInternalUsers} + allowAgentsForTeamAdmins={allowAgentsForTeamAdmins} disableVectorStoresForInternalUsers={disableVectorStoresForInternalUsers} + allowVectorStoresForTeamAdmins={allowVectorStoresForTeamAdmins} /> ); }; diff --git a/ui/litellm-dashboard/src/components/leftnav.tsx b/ui/litellm-dashboard/src/components/leftnav.tsx index c7250f7602..12ac7e58e4 100644 --- a/ui/litellm-dashboard/src/components/leftnav.tsx +++ b/ui/litellm-dashboard/src/components/leftnav.tsx @@ -1,4 +1,5 @@ import { useOrganizations } from "@/app/(dashboard)/hooks/organizations/useOrganizations"; +import { useTeams } from "@/app/(dashboard)/hooks/teams/useTeams"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import { ApiOutlined, @@ -29,7 +30,7 @@ import { import type { MenuProps } from "antd"; import { ConfigProvider, Layout, Menu } from "antd"; import { useMemo } from "react"; -import { all_admin_roles, internalUserRoles, isAdminRole, rolesWithWriteAccess } from "../utils/roles"; +import { all_admin_roles, internalUserRoles, isAdminRole, isUserTeamAdminForAnyTeam, rolesWithWriteAccess } from "../utils/roles"; import NewBadge from "./common_components/NewBadge"; import type { Organization } from "./networking"; import UsageIndicator from "./UsageIndicator"; @@ -43,7 +44,9 @@ interface SidebarProps { enabledPagesInternalUsers?: string[] | null; enableProjectsUI?: boolean; disableAgentsForInternalUsers?: boolean; + allowAgentsForTeamAdmins?: boolean; disableVectorStoresForInternalUsers?: boolean; + allowVectorStoresForTeamAdmins?: boolean; } // Menu item configuration @@ -356,9 +359,10 @@ const menuGroups: MenuGroup[] = [ }, ]; -const Sidebar: React.FC = ({ setPage, defaultSelectedKey, collapsed = false, enabledPagesInternalUsers, enableProjectsUI, disableAgentsForInternalUsers, disableVectorStoresForInternalUsers }) => { +const Sidebar: React.FC = ({ setPage, defaultSelectedKey, collapsed = false, enabledPagesInternalUsers, enableProjectsUI, disableAgentsForInternalUsers, allowAgentsForTeamAdmins, disableVectorStoresForInternalUsers, allowVectorStoresForTeamAdmins }) => { const { userId, accessToken, userRole } = useAuthorized(); const { data: organizations } = useOrganizations(); + const { data: teams } = useTeams(); // Check if user is an org_admin const isOrgAdmin = useMemo(() => { @@ -368,6 +372,9 @@ const Sidebar: React.FC = ({ setPage, defaultSelectedKey, collapse ); }, [userId, organizations]); + // Check if user is a team admin for any team + const isTeamAdmin = useMemo(() => isUserTeamAdminForAnyTeam(teams ?? null, userId ?? ""), [teams, userId]); + // Navigate to page helper const navigateToPage = (page: string) => { const newSearchParams = new URLSearchParams(window.location.search); @@ -452,9 +459,10 @@ const Sidebar: React.FC = ({ setPage, defaultSelectedKey, collapse // Hide Projects page if enableProjectsUI is not enabled if (item.key === "projects" && !enableProjectsUI) return false; - // Hide agents and vector-stores pages for non-admin users when disabled - if (!isAdmin && item.key === "agents" && disableAgentsForInternalUsers) return false; - if (!isAdmin && item.key === "vector-stores" && disableVectorStoresForInternalUsers) return false; + // Hide agents and vector-stores pages for non-admin users when disabled, + // unless allow_*_for_team_admins is on and the user is a team admin. + if (!isAdmin && item.key === "agents" && disableAgentsForInternalUsers && !(allowAgentsForTeamAdmins && isTeamAdmin)) return false; + if (!isAdmin && item.key === "vector-stores" && disableVectorStoresForInternalUsers && !(allowVectorStoresForTeamAdmins && isTeamAdmin)) return false; // Existing role check if (item.roles && !item.roles.includes(userRole)) return false;