scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-14 23:21:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 102

fix(mcp): gate the team-grant union on an admission marker, not api_key absence

Browse source 
A JWT-authenticated caller is also keyless with a user_id and, absent a team claim, no team_id, so gating the multi-team union on api_key-is-None silently broadened JWT MCP access to the union of every team the user belongs to. _reload_admitted_user now stamps MCP_ADMITTED_USER_SUBJECT_METADATA and the union fires only for that positive marker, so the gateway session and bridge user paths union while JWT and other keyless auth keep their prior behavior. Regression-tested.
This commit is contained in:
Tin Chi Lo 2026-07-14 10:01:13 -07:00
parent b82fb75292
commit 5df5fa50fa
2 changed files with 65 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

53
litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py
View file

@ -128,6 +128,21 @@ def _has_client_supplied_mcp_auth(
return bool(mcp_auth_header) or bool(mcp_server_auth_headers)
MCP_ADMITTED_USER_SUBJECT_METADATA = "mcp_admitted_user_subject"
"""Marker key stamped into ``UserAPIKeyAuth.metadata`` by ``_reload_admitted_user`` for a
subject admitted keyless through the gateway session / bridge user path. It is what lets
``_team_ids_for_mcp_grant`` union the user's teams for exactly those admissions without also
broadening JWT auth, which produces a structurally identical keyless auth."""
def _is_mcp_admitted_user_subject(user_api_key_auth: UserAPIKeyAuth) -> bool:
"""True when this auth is a keyless subject admitted by the gateway session / bridge user
path (stamped at admission), as opposed to a JWT or other keyless auth that merely lacks a
``team_id``."""
metadata = user_api_key_auth.metadata
return isinstance(metadata, dict) and metadata.get(MCP_ADMITTED_USER_SUBJECT_METADATA) is True
def _is_aggregate_mcp_scope(route: str, mcp_servers: list[str] | None) -> bool:
"""True when a request targets the aggregate ``/mcp`` endpoint rather than any named
server. Named targets arrive either through ``x-mcp-servers`` (``mcp_servers``) or a
@ -811,6 +826,7 @@ class MCPRequestHandler:
org_id=user_object.organization_id,
object_permission=object_permission,
object_permission_id=user_object.object_permission_id,
metadata={MCP_ADMITTED_USER_SUBJECT_METADATA: True},
)
@staticmethod
@ -1599,16 +1615,17 @@ class MCPRequestHandler:
"""
Get allowed MCP servers a caller inherits from team membership.
For a key-based caller the ``team_id`` on the auth is the one team, and the result
is that team's grants (byte-identical to before this method learned about multiple
teams). For a user-subject caller admitted WITHOUT a key (the gateway DCR session
bearer and the bridge user-envelope, which carry a ``user_id`` and no ``api_key`` or
``team_id``), a ``UserAPIKeyAuth`` can only pin one team while the user may belong to
many, so the inherited grant is the UNION across every team the user belongs to.
Without this a signed-in user would see only servers granted to them directly and
none granted through their teams, which is how servers are meant to be shared
(assign teams, not individuals). Key-based auth never enters the union branch, so its
access is unchanged.
For a caller with a ``team_id`` (every key-based caller) the result is that one team's
grants, byte-identical to before this method learned about multiple teams. For a
subject admitted keyless through the gateway DCR session or bridge user path, a
``UserAPIKeyAuth`` can only pin one team while the user may belong to many, so the
inherited grant is the UNION across every team the user belongs to. Without this a
signed-in user would see only servers granted to them directly and none granted
through their teams, which is how servers are meant to be shared (assign teams, not
individuals). The union is gated on the admission marker
``_team_ids_for_mcp_grant`` checks, NOT on ``api_key is None``, so JWT auth (also
keyless, also possibly team-less) keeps its prior behavior and is not silently
broadened.
"""
team_ids = await MCPRequestHandler._team_ids_for_mcp_grant(user_api_key_auth)
if not team_ids:
@ -1625,16 +1642,20 @@ class MCPRequestHandler:
async def _team_ids_for_mcp_grant(user_api_key_auth: UserAPIKeyAuth | None) -> list[str]:
"""The team ids whose MCP grants a caller inherits.
A key-based caller (``api_key`` set) or any caller with an explicit ``team_id`` uses
that single team, so key auth is unchanged. Only a keyless user-subject caller (no
``api_key``, no ``team_id``, a ``user_id``) fans out to the user's full team list,
resolved once from the live user record. The ``UI_TEAM_ID`` sentinel resolves to no
teams exactly as before."""
A caller with an explicit ``team_id`` (every key-based caller, and any auth that pins
a team) uses that single team, so key auth is byte-identical. The fan-out to the
user's full team list happens ONLY for a subject admitted keyless through the gateway
session or bridge user path, which ``_reload_admitted_user`` stamps with
``MCP_ADMITTED_USER_SUBJECT_METADATA``. Gating on that positive marker rather than on
``api_key is None`` is deliberate: JWT auth also produces a keyless ``user_id`` auth
with no ``team_id``, and it must keep its prior behavior (no team-inherited grants)
rather than silently gaining the union across every team the user belongs to. The
``UI_TEAM_ID`` sentinel resolves to no teams exactly as before."""
if user_api_key_auth is None:
return []
if user_api_key_auth.team_id:
return [] if user_api_key_auth.team_id == UI_TEAM_ID else [user_api_key_auth.team_id]
if user_api_key_auth.api_key is not None or not user_api_key_auth.user_id:
if not user_api_key_auth.user_id or not _is_mcp_admitted_user_subject(user_api_key_auth):
return []
return await MCPRequestHandler._resolve_user_team_ids(user_api_key_auth.user_id, user_api_key_auth)

34
tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py
View file

@ -6272,9 +6272,17 @@ class TestUserSubjectTeamUnion:
):
yield
@staticmethod
def _admitted_subject(user_id):
from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import (
MCP_ADMITTED_USER_SUBJECT_METADATA,
)
return UserAPIKeyAuth(user_id=user_id, api_key=None, metadata={MCP_ADMITTED_USER_SUBJECT_METADATA: True})
async def test_keyless_user_unions_servers_across_all_their_teams(self):
teams = {"team-a": self._team("team-a", ["srv1", "srv2"]), "team-b": self._team("team-b", ["srv2", "srv3"])}
auth = UserAPIKeyAuth(user_id="sso-user", api_key=None)
auth = self._admitted_subject("sso-user")
with self._patch(teams_by_id=teams, user_teams=["team-a", "team-b"]):
result = await MCPRequestHandler._get_allowed_mcp_servers_for_team(auth)
assert set(result) == {"srv1", "srv2", "srv3"}
@ -6298,7 +6306,7 @@ class TestUserSubjectTeamUnion:
assert set(result) == {"srv1"}
async def test_keyless_user_with_no_teams_gets_nothing_from_teams(self):
auth = UserAPIKeyAuth(user_id="lonely-user", api_key=None)
auth = self._admitted_subject("lonely-user")
with self._patch(teams_by_id={}, user_teams=[]):
result = await MCPRequestHandler._get_allowed_mcp_servers_for_team(auth)
assert result == []
@ -6318,14 +6326,28 @@ class TestUserSubjectTeamUnion:
assert await MCPRequestHandler._team_ids_for_mcp_grant(
UserAPIKeyAuth(api_key="sk", team_id="t1", user_id="u")
) == ["t1"]
# keyless user-subject, no team -> resolved from user record
# keyless subject admitted by the gateway/bridge path (marked), no team -> resolved from record
with self._patch(teams_by_id={}, user_teams=["t2", "t3"]):
assert await MCPRequestHandler._team_ids_for_mcp_grant(
UserAPIKeyAuth(api_key=None, user_id="u")
) == ["t2", "t3"]
assert await MCPRequestHandler._team_ids_for_mcp_grant(self._admitted_subject("u")) == ["t2", "t3"]
# keyless, no user_id -> nothing
assert await MCPRequestHandler._team_ids_for_mcp_grant(UserAPIKeyAuth(api_key=None)) == []
# keyless with a user_id but NOT admission-marked (JWT auth) -> nothing (unchanged behavior)
with self._patch(teams_by_id={}, user_teams=["t2", "t3"]):
assert await MCPRequestHandler._team_ids_for_mcp_grant(
UserAPIKeyAuth(api_key=None, user_id="jwt-user")
) == []
# UI sentinel -> nothing
assert await MCPRequestHandler._team_ids_for_mcp_grant(
UserAPIKeyAuth(api_key="sk", team_id=UI_TEAM_ID, user_id="u")
) == []
async def test_jwt_keyless_user_without_team_claim_does_not_union(self):
"""Regression for the review finding: a JWT-authenticated caller is also keyless with a
user_id and (with no team claim) no team_id, but it is NOT admission-marked, so it must
keep its prior behavior of inheriting no team grants rather than silently gaining the
union across every team the user belongs to."""
teams = {"team-a": self._team("team-a", ["srv1"]), "team-b": self._team("team-b", ["srv2"])}
jwt_auth = UserAPIKeyAuth(user_id="jwt-user", api_key=None) # no admission marker
with self._patch(teams_by_id=teams, user_teams=["team-a", "team-b"]):
result = await MCPRequestHandler._get_allowed_mcp_servers_for_team(jwt_auth)
assert result == []