diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index bb62e479b42..24aff5548e3 100644 --- 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", diff --git a/litellm/proxy/auth/route_checks.py b/litellm/proxy/auth/route_checks.py index dba29f84133..8bcfbb67539 100644 --- 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 diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index 39f832256d0..cf6feabf85f 100644 --- 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"""