Merge pull request #27007 from stuxf/fix/admin-viewer-write-route-blocklist

fix(auth): block missing write routes for proxy admin viewers
2026-09-08 22:21:35 +00:00 · 2026-05-04 14:45:14 -07:00 · 2026-05-04 14:45:14 -07:00 · 6f5678bcd8
commit 6f5678bcd8
parent e995156462 2a8fe32850
3 changed files with 137 additions and 0 deletions
--- a/litellm/proxy/_types.py
+++ b/litellm/proxy/_types.py
@ -567,6 +567,7 @@ class LiteLLMRoutes(enum.Enum):
            "/team/available",
            "/team/permissions_list",
            "/team/permissions_update",
+            "/team/permissions_bulk_update",
            "/team/daily/activity",
            # model
            "/model/new",
--- a/litellm/proxy/auth/route_checks.py
+++ b/litellm/proxy/auth/route_checks.py
@ -6,6 +6,7 @@ from fastapi import HTTPException, Request, status
 from litellm._logging import verbose_proxy_logger
 from litellm.proxy._types import (
    CommonProxyErrors,
+    KeyManagementRoutes,
    LiteLLM_UserTable,
    LiteLLMRoutes,
    LitellmUserRoles,
@ -14,6 +15,49 @@ from litellm.proxy._types import (

 from .auth_checks_organization import _user_is_org_admin

+# Management write routes denied to PROXY_ADMIN_VIEW_ONLY. Adding a new write
+# endpoint to a management router REQUIRES adding it here too — the surrounding
+# check falls through to "allow" if the route is not matched, which previously
+# let view-only admins call /team/block, /team/unblock, /key/bulk_update, etc.
+_PROXY_ADMIN_VIEW_ONLY_BLOCKED_ROUTES = frozenset(
+    [
+        # user
+        "/user/new",
+        "/user/delete",
+        "/user/bulk_update",
+        # team
+        "/team/new",
+        "/team/update",
+        "/team/delete",
+        "/team/block",
+        "/team/unblock",
+        "/team/permissions_update",
+        "/team/permissions_bulk_update",
+        # model
+        "/model/new",
+        "/model/update",
+        "/model/delete",
+        # JWT key mapping
+        "/jwt/key/mapping/new",
+        "/jwt/key/mapping/update",
+        "/jwt/key/mapping/delete",
+        # key management — keep in sync with KeyManagementRoutes write entries
+        KeyManagementRoutes.KEY_GENERATE.value,
+        KeyManagementRoutes.KEY_UPDATE.value,
+        KeyManagementRoutes.KEY_DELETE.value,
+        KeyManagementRoutes.KEY_REGENERATE.value,
+        KeyManagementRoutes.KEY_GENERATE_SERVICE_ACCOUNT.value,
+        KeyManagementRoutes.KEY_BLOCK.value,
+        KeyManagementRoutes.KEY_UNBLOCK.value,
+        KeyManagementRoutes.KEY_BULK_UPDATE.value,
+    ]
+)
+
+# Suffixes for `/key/{key_id}/...` path-parameterized write routes that the
+# enum templates with `{key_id}`. The blocklist above can't match templated
+# paths directly because the request route carries the resolved key id.
+_PROXY_ADMIN_VIEW_ONLY_BLOCKED_KEY_SUFFIXES = ("/regenerate", "/reset_spend")
+

 class RouteChecks:
    @staticmethod
@ -664,6 +708,31 @@ 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 _PROXY_ADMIN_VIEW_ONLY_BLOCKED_ROUTES or (
+                route.startswith("/key/")
+                and route.endswith(_PROXY_ADMIN_VIEW_ONLY_BLOCKED_KEY_SUFFIXES)
+            ):
+                # 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

--- a/tests/test_litellm/proxy/auth/test_route_checks.py
+++ b/tests/test_litellm/proxy/auth/test_route_checks.py
@ -90,6 +90,73 @@ def test_proxy_admin_viewer_config_update_route_rejected():
    assert "role= proxy_admin_viewer" in str(exc_info.value.detail)


+@pytest.mark.parametrize(
+    "blocked_route",
+    [
+        # team write routes that previously fell through the blocklist
+        "/team/block",
+        "/team/unblock",
+        "/team/permissions_update",
+        "/team/permissions_bulk_update",
+        # JWT key mapping write routes
+        "/jwt/key/mapping/new",
+        "/jwt/key/mapping/update",
+        "/jwt/key/mapping/delete",
+        # key write routes
+        "/key/bulk_update",
+        # path-parameterized key write routes (suffix match)
+        "/key/abc123/regenerate",
+        "/key/abc123/reset_spend",
+        # baseline coverage of routes that were already blocked
+        "/team/new",
+        "/team/delete",
+        "/key/generate",
+        "/key/delete",
+        "/model/new",
+        "/model/delete",
+    ],
+)
+def test_proxy_admin_viewer_blocked_management_writes(blocked_route):
+    """View-only admins must be denied on every management write route — the
+    fall-through path previously allowed /team/block, /team/unblock,
+    /key/bulk_update, /key/{id}/reset_spend, and the JWT key-mapping routes."""
+    with pytest.raises(HTTPException) as exc_info:
+        RouteChecks._check_proxy_admin_viewer_access(
+            route=blocked_route,
+            _user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value,
+            request_data={},
+        )
+    assert exc_info.value.status_code == 403
+    assert blocked_route in str(exc_info.value.detail)
+
+
+@pytest.mark.parametrize(
+    "allowed_read_route",
+    [
+        "/team/info",
+        "/team/list",
+        "/v2/team/list",
+        "/team/permissions_list",
+        "/team/daily/activity",
+        "/user/info",
+        "/user/list",
+        "/key/info",
+        "/key/list",
+        "/model/info",
+        "/jwt/key/mapping/list",
+        "/jwt/key/mapping/info",
+    ],
+)
+def test_proxy_admin_viewer_allowed_management_reads(allowed_read_route):
+    """View-only admins must still be allowed to read management routes."""
+    # Should not raise
+    RouteChecks._check_proxy_admin_viewer_access(
+        route=allowed_read_route,
+        _user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY.value,
+        request_data={},
+    )
+
+
 def test_virtual_key_allowed_routes_with_litellm_routes_member_name_allowed():
    """Test that virtual key is allowed to call routes when allowed_routes contains LiteLLMRoutes member name"""