diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index a428c42d689..fe1ac8c2d52 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -716,13 +716,18 @@ class LiteLLMRoutes(enum.Enum): "/organization/member_delete", ] - # Routes accessible by Admin Viewer (read-only admin access) + # Routes accessible by Admin Viewer (read-only admin access). # # Admin Viewer follows a read-parity-with-Proxy-Admin rule: anything Proxy # Admin can read/list/get, Admin Viewer can too (no writes, no cost-incurring - # actions). When extending this list, the only valid exclusions are write - # endpoints and cost-incurring endpoints (e.g. /chat/completions, the - # Playground). Pure GET/list/info endpoints belong here. + # actions). + # + # NOTE: This list is no longer the primary mechanism for granting access — + # `_check_proxy_admin_viewer_access()` in route_checks.py default-allows + # any safe HTTP method (GET/HEAD/OPTIONS) on non-inference routes. This + # list now matters only for non-GET routes that are semantically reads + # (e.g. POST /spend/calculate). Adding a new GET endpoint does not require + # updating this list — the default-allow behavior covers it automatically. admin_viewer_routes = ( [ "/user/list", diff --git a/litellm/proxy/auth/route_checks.py b/litellm/proxy/auth/route_checks.py index 6417307f691..c30c5459beb 100644 --- a/litellm/proxy/auth/route_checks.py +++ b/litellm/proxy/auth/route_checks.py @@ -202,6 +202,7 @@ class RouteChecks: route=route, _user_role=_user_role, request_data=request_data, + request=request, ) elif ( _user_role == LitellmUserRoles.INTERNAL_USER.value @@ -596,14 +597,66 @@ class RouteChecks: return True return False + # HTTP methods that are intrinsically read-only and therefore safe to + # default-allow for PROXY_ADMIN_VIEW_ONLY. Anything else (POST/PUT/PATCH/ + # DELETE) is treated as a write attempt and goes through the explicit + # write-allowlist below. + _SAFE_HTTP_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) + + # Explicit write routes that PROXY_ADMIN_VIEW_ONLY must NEVER call. The + # role-principle is "no writes, ever" — the management_routes list is the + # authoritative source for which non-llm routes are writes; we just need + # to filter out the read endpoints (info / list) that share the prefix. + # A cleaner approach is to denylist by HTTP verb (POST/PUT/PATCH/DELETE); + # this block stays as a backstop in case a write is implemented as GET. + _ADMIN_VIEWER_BLOCKED_WRITE_ROUTES = frozenset( + [ + "/user/new", + "/user/delete", + "/user/bulk_update", + "/team/new", + "/team/update", + "/team/delete", + "/model/new", + "/model/update", + "/model/delete", + "/key/generate", + "/key/delete", + "/key/update", + "/key/regenerate", + "/key/service-account/generate", + "/key/block", + "/key/unblock", + ] + ) + @staticmethod def _check_proxy_admin_viewer_access( route: str, _user_role: str, request_data: dict, + request: Optional[Request] = None, ) -> None: """ - Check access for PROXY_ADMIN_VIEW_ONLY role + Check access for PROXY_ADMIN_VIEW_ONLY role. + + Admin Viewer follows a read-parity-with-Proxy-Admin rule: anything Proxy + Admin can read/list/get, Admin Viewer can read/list/get. The only + exclusions are cost-incurring inference routes (Playground, /chat/ + completions, etc.) and any state-mutating request. + + Implementation: + 1. LLM/inference routes → 403 (cost-incurring). + 2. Safe HTTP method (GET/HEAD/OPTIONS) → allow by default. This is + the read-parity guarantee — every new GET endpoint added anywhere + in the codebase is automatically readable by Admin Viewer + without needing to remember to add it to an allowlist. + 3. Unsafe HTTP method (POST/PUT/PATCH/DELETE): + - Allow `/user/update` only when restricted to user_email/password. + - Block all explicit writes in `_ADMIN_VIEWER_BLOCKED_WRITE_ROUTES`. + - Otherwise allow only if the route is in admin_viewer_routes / + global_spend_tracking_routes (legacy explicit-allow set). + - Else 403. """ if RouteChecks.is_llm_api_route(route=route): raise HTTPException( @@ -611,65 +664,58 @@ class RouteChecks: detail=f"user not allowed to access this OpenAI routes, role= {_user_role}", ) - # Check if this is a write operation on management routes - if RouteChecks.check_route_access( - route=route, allowed_routes=LiteLLMRoutes.management_routes.value - ): - # For management routes, only allow read operations or specific allowed updates - if route == "/user/update": - # Check the Request params are valid for PROXY_ADMIN_VIEW_ONLY - if request_data is not None and isinstance(request_data, dict): - _params_updated = request_data.keys() - for param in _params_updated: - if param not in ["user_email", "password"]: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route} and updating invalid param: {param}. only user_email and password can be updated", - ) - elif ( - route - in [ - "/user/new", - "/user/delete", - "/user/bulk_update", - "/team/new", - "/team/update", - "/team/delete", - "/model/new", - "/model/update", - "/model/delete", - "/key/generate", - "/key/delete", - "/key/update", - "/key/regenerate", - "/key/service-account/generate", - "/key/block", - "/key/unblock", - ] - or route.startswith("/key/") - and route.endswith("/regenerate") - ): - # Block write operations for PROXY_ADMIN_VIEW_ONLY - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route}", - ) - # Allow read operations on management routes (like /user/info, /team/info, /model/info) + method = request.method.upper() if request is not None else "GET" + is_safe_method = method in RouteChecks._SAFE_HTTP_METHODS + + # ── Safe HTTP method: default-allow ────────────────────────────── + if is_safe_method: return - elif RouteChecks.check_route_access( - route=route, allowed_routes=LiteLLMRoutes.admin_viewer_routes.value - ): - # Allow access to admin viewer routes (read-only admin endpoints) + + # ── Unsafe HTTP method: explicit checks ────────────────────────── + # Allow `/user/update` for self-service email / password change. + if route == "/user/update": + if request_data is not None and isinstance(request_data, dict): + for param in request_data.keys(): + if param not in ["user_email", "password"]: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=( + f"user not allowed to access this route, role= {_user_role}. " + f"Trying to access: {route} and updating invalid param: {param}. " + "only user_email and password can be updated" + ), + ) return - elif RouteChecks.check_route_access( - route=route, allowed_routes=LiteLLMRoutes.global_spend_tracking_routes.value + + # Hard-block known write routes regardless of HTTP method (defensive + # — these are POSTs in practice, but pinning them here protects + # against future GET-shaped writes). + if route in RouteChecks._ADMIN_VIEWER_BLOCKED_WRITE_ROUTES or ( + route.startswith("/key/") and route.endswith("/regenerate") ): - # Allow access to global spend tracking routes (read-only spend endpoints) - # proxy_admin_viewer role description: "view all keys, view all spend" - return - else: - # For other routes, block access raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route}", ) + + # Legacy explicit-allow sets (kept for routes that are POST but + # semantically read-only, e.g. /spend/calculate). + if RouteChecks.check_route_access( + route=route, allowed_routes=LiteLLMRoutes.admin_viewer_routes.value + ): + return + if RouteChecks.check_route_access( + route=route, allowed_routes=LiteLLMRoutes.global_spend_tracking_routes.value + ): + return + if RouteChecks.check_route_access( + route=route, allowed_routes=LiteLLMRoutes.management_routes.value + ): + # On management routes, allow non-blocked writes (e.g. read-only + # info/list endpoints implemented as POST). + return + + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route}", + ) diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index a1613aad94f..ad71e765fa6 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -1373,6 +1373,117 @@ def test_proxy_admin_viewer_can_access_settings_read_endpoints(route): ) +# ── Admin Viewer parity: default-allow GET semantics ───────────────────────── +# +# The route-check layer is structured to default-allow safe HTTP methods +# (GET / HEAD / OPTIONS) for PROXY_ADMIN_VIEW_ONLY. This eliminates the +# whack-a-mole where every newly-added GET endpoint silently 403'd until +# someone remembered to add it to admin_viewer_routes. +# +# These tests pin the new contract: +# - Any GET endpoint not on the LLM/inference path is readable. +# - Any unsafe method (POST/PUT/PATCH/DELETE) outside the explicit allow +# sets is still 403. + +# Routes the user reported as broken in production — they're in disparate +# corners of the codebase and represent the long tail of GETs we'd otherwise +# need to enumerate manually. Default-allow makes them all work. +ADMIN_VIEWER_REPORTED_GET_ROUTES = [ + "/in_product_nudges", + "/health/latest", + "/credentials", + "/v1/mcp/network/client-ip", + "/claude-code/plugins", + "/policy/templates", + # Routes we already had to enumerate manually (regression coverage). + "/spend/logs/ui", + "/customer/list", + "/guardrails/list", + "/policies/attachments/list", + # Hypothetical future GETs — must not require an allowlist entry. + "/some/future/read/endpoint", + "/another/admin-tool/status", +] + + +@pytest.mark.parametrize("route", ADMIN_VIEWER_REPORTED_GET_ROUTES) +def test_proxy_admin_viewer_default_allows_any_get(route): + """ + PROXY_ADMIN_VIEW_ONLY must be able to GET any non-inference endpoint. + + This is a structural guarantee: the route-check defaults to allow for + safe HTTP methods so we don't have to maintain an explicit allowlist. + """ + user_obj = LiteLLM_UserTable( + user_id="viewer_user", + user_email="viewer@example.com", + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + ) + valid_token = UserAPIKeyAuth( + user_id="viewer_user", + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + ) + request = MagicMock(spec=Request) + request.method = "GET" + request.query_params = {} + request.url = MagicMock() + request.url.path = route + + try: + RouteChecks.non_proxy_admin_allowed_routes_check( + user_obj=user_obj, + _user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + route=route, + request=request, + valid_token=valid_token, + request_data={}, + ) + except Exception as e: + pytest.fail(f"proxy_admin_viewer GET should default-allow {route!r}. Got: {e}") + + +@pytest.mark.parametrize( + "route", + [ + # Random path that isn't in any allowlist — POST must still 403. + "/some/future/write/endpoint", + # Hard-blocked write routes. + "/user/new", + "/team/new", + "/key/generate", + "/model/new", + ], +) +def test_proxy_admin_viewer_post_blocked_outside_allowlists(route): + """ + Default-allow only applies to safe HTTP methods. POST/PUT/PATCH/DELETE + on a route not in any allow set must still 403. + """ + user_obj = LiteLLM_UserTable( + user_id="viewer_user", + user_email="viewer@example.com", + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + ) + valid_token = UserAPIKeyAuth( + user_id="viewer_user", + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + ) + request = MagicMock(spec=Request) + request.method = "POST" + request.query_params = {} + + with pytest.raises(HTTPException) as exc_info: + RouteChecks.non_proxy_admin_allowed_routes_check( + user_obj=user_obj, + _user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value, + route=route, + request=request, + valid_token=valid_token, + request_data={}, + ) + assert exc_info.value.status_code == 403 + + class TestModelsRouteExemptFromDisableLLMEndpoints: """ Test that /models and /v1/models are exempt from DISABLE_LLM_API_ENDPOINTS. diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/ModelsAndEndpointsView.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/ModelsAndEndpointsView.tsx index 8f248dacc92..5ba93f9b724 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/ModelsAndEndpointsView.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/ModelsAndEndpointsView.tsx @@ -367,102 +367,149 @@ const ModelsAndEndpointsView: React.FC = ({ premiumUser, te modelAccessGroups={availableModelAccessGroups} /> ) : ( - - -
- {all_admin_roles.includes(userRole) ? All Models : Your Models} - {!shouldHideAddModelTab && Add Model} - {all_admin_roles.includes(userRole) && LLM Credentials} - {all_admin_roles.includes(userRole) && Pass-Through Endpoints} - {all_admin_roles.includes(userRole) && Health Status} - {all_admin_roles.includes(userRole) && Model Retry Settings} - {all_admin_roles.includes(userRole) && Model Group Alias} - {all_admin_roles.includes(userRole) && Price Data Reload} -
- -
- {lastRefreshed && Last Refreshed: {lastRefreshed}} - -
- - - - {!shouldHideAddModelTab && ( - - { + // Build a single source-of-truth list of {tab, panel} pairs. + // Conditionally-hidden tabs (e.g. "Add Model" for non-admin) get + // filtered out as a unit so tab indices and panel indices can + // never drift apart — Tremor's TabList and TabPanels filter + // falsy children inconsistently, which previously caused + // "click LLM Credentials, see nothing" for Admin Viewer. + const isAdmin = all_admin_roles.includes(userRole); + const visibleTabs: Array<{ tab: React.ReactElement; panel: React.ReactElement }> = [ + { + tab: {isAdmin ? "All Models" : "Your Models"}, + panel: ( + - - )} - - - - - - - - - - - - - - - - + ), + }, + ]; + if (!shouldHideAddModelTab) { + visibleTabs.push({ + tab: Add Model, + panel: ( + + + + ), + }); + } + if (isAdmin) { + visibleTabs.push( + { + tab: LLM Credentials, + panel: ( + + + + ), + }, + { + tab: Pass-Through Endpoints, + panel: ( + + + + ), + }, + { + tab: Health Status, + panel: ( + + + + ), + }, + { + tab: Model Retry Settings, + panel: ( + + ), + }, + { + tab: Model Group Alias, + panel: ( + + + + ), + }, + { + tab: Price Data Reload, + panel: , + }, + ); + } + return ( + + +
{visibleTabs.map((t) => t.tab)}
+ +
+ {lastRefreshed && Last Refreshed: {lastRefreshed}} + +
+ + {visibleTabs.map((t) => t.panel)} + + ); + })() )}