scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-20 05:04:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 155

fix(proxy): let proxy_admin_viewer store its own MCP credentials (#46994)

Browse source 
Co-authored-by: michelligabriele <gabriele.michelli@icloud.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Yassin Kortam 2026-10-17 11:31:37 -07:00 • committed by GitHub
parent 8226b2be9a
commit 03e3e71efa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 63 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
litellm/proxy/auth/route_checks.py
View file

@ -72,14 +72,22 @@ _PROXY_ADMIN_VIEW_ONLY_BLOCKED_KEY_SUFFIXES: Final = ("/regenerate", "/reset_spe
_LLM_API_ROUTE_GROUPS: Final = frozenset(("llm_api_routes", "llm_api_routes_without_mcp"))
_AUTH_ENFORCED_PASS_THROUGH_ROUTE_GROUPS: Final = frozenset(("openai_routes", *_LLM_API_ROUTE_GROUPS))
# Method-scoped because the generic credential CRUD handlers share these paths:
# the route check cannot admit PATCH/DELETE on a name like "user_connections"
# or "*/user_connection*" without also opening the admin CRUD handlers.
_INTERNAL_USER_CREDENTIAL_CONNECTION_ROUTES: Final = (
# Writes that only touch the caller's own credential row (BYOK connections and
# per-user MCP credentials). Method-scoped because the generic credential CRUD
# handlers share these paths: the route check cannot admit PATCH/DELETE on a
# name like "user_connections" without also opening the admin CRUD handlers.
_SELF_SCOPED_CREDENTIAL_ROUTES: Final = (
("GET", "/credentials/user_connections"),
("POST", "/credentials/{credential_name:path}/user_connection/start"),
("POST", "/credentials/{credential_name:path}/user_connection/poll"),
("DELETE", "/credentials/{credential_name:path}/user_connection"),
("POST", "/v1/mcp/server/oauth/{server_id}/register"),
("POST", "/v1/mcp/server/{server_id}/oauth-user-credential"),
("DELETE", "/v1/mcp/server/{server_id}/oauth-user-credential"),
("POST", "/v1/mcp/server/{server_id}/user-credential"),
("DELETE", "/v1/mcp/server/{server_id}/user-credential"),
("POST", "/v1/mcp/server/{server_id}/user-env-vars"),
("DELETE", "/v1/mcp/server/{server_id}/user-env-vars"),
)
@ -349,7 +357,7 @@ class RouteChecks:
_user_role == LitellmUserRoles.INTERNAL_USER.value
and (
RouteChecks.check_route_access(route=route, allowed_routes=LiteLLMRoutes.internal_user_routes.value)
or RouteChecks.is_internal_user_credential_connection_route(route=route, request=request)
or RouteChecks.is_self_scoped_credential_route(route=route, request=request)
)
or user_is_org_admin(request_data=request_data, user_object=user_obj)
and RouteChecks.check_route_access(route=route, allowed_routes=LiteLLMRoutes.org_admin_allowed_routes.value)
@ -702,13 +710,13 @@ class RouteChecks:
_get_request_method = get_request_method
@staticmethod
def is_internal_user_credential_connection_route(route: str, request: Request | None) -> bool:
def is_self_scoped_credential_route(route: str, request: Request | None) -> bool:
method: Final = RouteChecks.get_request_method(request)
if method is None:
return False
return any(
method == expected_method and RouteChecks.check_route_access(route=route, allowed_routes=(pattern,))
for expected_method, pattern in _INTERNAL_USER_CREDENTIAL_CONNECTION_ROUTES
for expected_method, pattern in _SELF_SCOPED_CREDENTIAL_ROUTES
)
@staticmethod
@ -1033,6 +1041,9 @@ class RouteChecks:
if route == "/session/logout":
return
if RouteChecks.is_self_scoped_credential_route(route=route, request=request):
return
# 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).

45
tests/unit/proxy/auth/test_route_checks.py
View file

@ -4782,6 +4782,51 @@ def test_view_only_user_denied_credential_connection_routes(method: str, route:
_internal_user_check(route, method, role=LitellmUserRoles.INTERNAL_USER_VIEW_ONLY)
_SELF_SCOPED_CREDENTIAL_WRITES = (
("POST", "/credentials/x/user_connection/start"),
("POST", "/credentials/x/user_connection/poll"),
("DELETE", "/credentials/x/user_connection"),
("POST", "/v1/mcp/server/oauth/srv-1/register"),
("POST", "/v1/mcp/server/srv-1/oauth-user-credential"),
("DELETE", "/v1/mcp/server/srv-1/oauth-user-credential"),
("POST", "/v1/mcp/server/srv-1/user-credential"),
("DELETE", "/v1/mcp/server/srv-1/user-credential"),
("POST", "/v1/mcp/server/srv-1/user-env-vars"),
("DELETE", "/v1/mcp/server/srv-1/user-env-vars"),
)
@pytest.mark.parametrize(("method", "route"), _SELF_SCOPED_CREDENTIAL_WRITES)
def test_proxy_admin_viewer_allowed_self_scoped_credential_writes(method: str, route: str) -> None:
"""Regression for LIT-9320: these handlers only write the caller's own credential
row, so a read-only admin can use them like /user/password/change."""
assert _internal_user_check(route, method, role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY) is None
@pytest.mark.parametrize(
("method", "route"),
[
("POST", "/v1/mcp/server"),
("PUT", "/v1/mcp/server"),
("DELETE", "/v1/mcp/server/srv-1"),
("POST", "/v1/mcp/server/import"),
("POST", "/v1/mcp/server/srv-1/pin"),
("PATCH", "/v1/mcp/server/srv-1/oauth-user-credential"),
("PUT", "/v1/mcp/server/srv-1/user-credential"),
("PATCH", "/v1/mcp/server/srv-1/user-env-vars"),
("POST", "/v1/mcp/server/srv-1/user-credential/rotate"),
("POST", "/v1/mcp/server/oauth/srv-1/token"),
("DELETE", "/v1/mcp/server/oauth/srv-1/register"),
("PATCH", "/credentials/x/user_connection"),
("DELETE", "/credentials/user_connections"),
("POST", "/key/generate"),
],
)
def test_proxy_admin_viewer_denied_mcp_admin_and_sibling_writes(method: str, route: str) -> None:
with pytest.raises(HTTPException, match="user not allowed to access this route"):
_internal_user_check(route, method, role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY)
def test_user_connection_route_registration_order_wins_over_generic_credential_routes() -> None:
"""The :path placeholders in the generic CRUD routes could swallow the
user_connection paths; the concrete routes must register (and match) first."""