[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 <noreply@anthropic.com>
This commit is contained in:
parent
96b75be03d
commit
b3c092f489
@ -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
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -16,7 +16,9 @@ const SidebarProvider = ({ setPage, defaultSelectedKey, sidebarCollapsed }: Side
|
||||
const [enabledPagesInternalUsers, setEnabledPagesInternalUsers] = useState<string[] | null>(null);
|
||||
const [enableProjectsUI, setEnableProjectsUI] = useState<boolean>(false);
|
||||
const [disableAgentsForInternalUsers, setDisableAgentsForInternalUsers] = useState<boolean>(false);
|
||||
const [allowAgentsForTeamAdmins, setAllowAgentsForTeamAdmins] = useState<boolean>(false);
|
||||
const [disableVectorStoresForInternalUsers, setDisableVectorStoresForInternalUsers] = useState<boolean>(false);
|
||||
const [allowVectorStoresForTeamAdmins, setAllowVectorStoresForTeamAdmins] = useState<boolean>(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}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
@ -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<SidebarProps> = ({ setPage, defaultSelectedKey, collapsed = false, enabledPagesInternalUsers, enableProjectsUI, disableAgentsForInternalUsers, disableVectorStoresForInternalUsers }) => {
|
||||
const Sidebar: React.FC<SidebarProps> = ({ 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<SidebarProps> = ({ 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<SidebarProps> = ({ 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;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user