scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-09 03:18:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 109

fix(mcp): resolve admin OAuth sessions to the same server set the connect page shows (#37900)

Browse source 
* fix(mcp): resolve admin OAuth sessions to the same server set the connect page shows

* fix(mcp): bind admitted admin rows through the entitlement ceiling, not the credential scope clause
This commit is contained in:
tin-berri 2026-08-21 18:16:49 -07:00 • committed by GitHub
parent 843e90c2f5
commit 7cb100af63
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 208 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -39,6 +39,7 @@ from litellm.proxy._types import (
SpecialMCPServerName,
SpecialMCPServerNames,
UserAPIKeyAuth,
user_api_key_has_admin_view,
)
from litellm.proxy.auth.ip_address_utils import IPAddressUtils
from litellm.proxy.auth.user_api_key_auth import (
@ -1785,11 +1786,14 @@ class MCPRequestHandler:
global_mcp_server_manager,
)
# An OPEN channel (allow_all_keys, the user's own BYOM) makes the server REACHABLE through the
# user, though no grant source names it — without this the union returns [], listable but
# uninvokable. Reachability is ALL it confers, NOT a ceiling waiver: the user's own
# mcp_tool_permissions and org tool ceiling still bind, exactly as a key's do on an allow_all server.
reachable_via_open_channel: Final = server_id in await global_mcp_server_manager.operator_open_server_ids(auth)
# An OPEN channel (allow_all_keys, the user's own BYOM, an unscoped admin-view role) makes the
# server REACHABLE through the user, though no grant source names it — without this the union
# returns [], listable but uninvokable. Reachability is ALL it confers, NOT a ceiling waiver:
# the user's own mcp_tool_permissions and org tool ceiling still bind, exactly as a key's do
# on an allow_all server or an admin key's do on any server.
reachable_via_open_channel: Final = server_id in await global_mcp_server_manager.operator_open_server_ids(
auth
) or await MCPRequestHandler.admin_view_unscoped(auth)
allowed: Final[set[str]] = set()
for source, granted in await MCPRequestHandler.admitted_source_grants(auth):
@ -2723,6 +2727,32 @@ class MCPRequestHandler:
entitled_servers: Final = await MCPRequestHandler._get_allowed_mcp_servers_for_user(user_api_key_auth)
return entitled_servers is None or len(entitled_servers) > 0
@staticmethod
async def admin_view_unscoped(user_api_key_auth: UserAPIKeyAuth | None = None) -> bool:
"""Whether this principal's admin-view role grants the unscoped MCP resolution, whatever
credential carries it (admin key, dashboard session, or OAuth-admitted session subject).
Two bounds disqualify, one per ownership of the row. A CREDENTIAL's explicit
``object_permission.mcp_servers`` scope wins even for admins, including the empty list. An
admitted subject's object_permission is the user's own row, whose ``mcp_servers`` column is
[] by DB default, so for that shape the row binds through the entitlement ceiling instead
(any non-empty entitlement, or an unresolved one, disqualifies), exactly as
``operator_open_server_ids`` reads the same row. The one owner of this predicate: the
server-axis registry resolution in ``get_allowed_mcp_servers`` and the tools-axis open
channel in ``_resolve_admitted_subject_tools`` both consult it, so the two axes cannot
disagree."""
if user_api_key_auth is None or not user_api_key_has_admin_view(user_api_key_auth):
return False
object_permission: Final = user_api_key_auth.object_permission
credential_scoped: Final = (
not _is_mcp_admitted_user_subject(user_api_key_auth)
and object_permission is not None
and object_permission.mcp_servers is not None
)
if credential_scoped:
return False
return not await MCPRequestHandler._user_places_mcp_ceiling(user_api_key_auth)
@staticmethod
async def _apply_user_tool_ceiling(
allowed_tools: Sequence[str] | None,

41
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -2943,17 +2943,14 @@ class MCPServerManager:
2. If admin and no object_permission, return all servers
3. Otherwise, use standard permission checks
"""
from litellm.proxy.management_endpoints.common_utils import _user_has_admin_view
allow_all_server_ids: Final = self.get_allow_all_keys_server_ids()
# A keyless admitted subject is resolved per grant source, and channel decisions that are
# absolute for a scoped KEY credential are not absolute for it: its own opt-out silences its
# own source (handled per source in the resolver), never its teams' grants, and its admin
# role does not swallow the grant model — a session bearer is a third-party client
# credential, not the dashboard, so an admin signing in through the connect flow gets their
# grants like anyone else rather than handing the client the full registry ahead of every
# per-team org ceiling.
# own source (handled per source in the resolver), never its teams' grants. Its admin role
# rides the HUMAN, not the credential: an admin's session resolves the same registry their
# dashboard shows (connect-page parity), bounded like an admin key by explicit
# object_permission scope, the entitlement ceiling, and the session resource scope below.
is_admitted_subject: Final = _is_mcp_admitted_user_subject(user_api_key_auth)
# The key explicitly opted out of every MCP server. Return zero before
@ -2982,26 +2979,16 @@ class MCPServerManager:
)
try:
# If admin but NO explicit object permission, get all servers (never for an admitted
# subject — see is_admitted_subject above)
if (
user_api_key_auth
and not is_admitted_subject
and _user_has_admin_view(user_api_key_auth)
and not has_explicit_object_permission
# An entitlement attached to the HUMAN binds them whatever their role: it is the
# person's scope, not the credential's, so an admin role is not a waiver of it. An
# UNRESOLVED entitlement also skips the shortcut, so the resolver denies rather than
# handing over the whole registry on a transient fault.
and not await MCPRequestHandler._user_places_mcp_ceiling(user_api_key_auth)
):
verbose_logger.debug("Admin user without explicit object_permission - returning all servers")
return list(self.get_registry().keys())
# Get allowed servers from object permissions (respects object_permission even for admins)
allowed_mcp_servers: Final = await MCPRequestHandler.get_allowed_mcp_servers(user_api_key_auth)
verbose_logger.debug("Allowed MCP Servers for user api key auth: %s", allowed_mcp_servers)
combined_servers: Final = set(allowed_mcp_servers)
# Admin view with no explicit object permission and no entitlement ceiling resolves the
# whole registry, for keys AND admitted session subjects alike (one predicate owns the
# question). Seeded into the union rather than returned early so the session resource
# scope below still bounds a per-server envelope held by an admin.
combined_servers: Final = (
set(self.get_registry().keys())
if await MCPRequestHandler.admin_view_unscoped(user_api_key_auth)
else set(await MCPRequestHandler.get_allowed_mcp_servers(user_api_key_auth))
)
verbose_logger.debug("Allowed MCP Servers for user api key auth: %s", combined_servers)
combined_servers.update(
await self.operator_open_server_ids(
user_api_key_auth,

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

@ -6430,9 +6430,7 @@ class TestAggregateGatewayDcrChallenge:
assert _gateway_dcr_challenge_target("/mcp/srv", None, None) == expected, resolved
assert _gateway_dcr_challenge_target("/mcp/a,b", None, None) is None
assert _gateway_dcr_challenge_target("/mcp", None, None) is None
with patch(
"litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager"
) as mock_mgr:
with patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr:
mock_mgr.get_mcp_server_by_name.return_value = _server(MCPAuth.oauth2)
assert _gateway_dcr_challenge_target("/mcp/srv", ["other"], None) is None
@ -7073,25 +7071,109 @@ class TestUserSubjectTeamUnion:
assert await manager.operator_open_server_ids(admitted) == {"srv-byom"}
assert await manager.operator_open_server_ids(scoped_key) == set(), "explicit key scope still suppresses BYOM"
async def test_admitted_admin_is_scoped_to_grants_not_full_registry(self):
"""The wrapper's admin short-circuit hands the FULL registry to any admin-role auth before
the grant union or the per-team org ceilings run. A session bearer is a third-party client
credential, not the dashboard: an admin signing in through the connect flow gets their
grants like anyone else. A real admin key keeps the dashboard behavior unchanged."""
@pytest.mark.parametrize(
"role", ["PROXY_ADMIN", "PROXY_ADMIN_VIEW_ONLY"], ids=["proxy_admin", "proxy_admin_view_only"]
)
async def test_admitted_admin_gets_registry_like_an_admin_key(self, role):
"""Connect-page parity: admin view rides the HUMAN, not the credential. An admitted session
subject with an admin-view role resolves the same full registry an admin KEY does, so the
servers the dashboard shows an admin are the servers their OAuth session serves. Regression
pin for the customer report where an admin's Claude Code session showed zero tools."""
from litellm.proxy._types import LitellmUserRoles
manager = self._manager_with(["srv-granted", "srv-secret"])
admitted = _make_admitted_subject("admin-user")
admitted.user_role = LitellmUserRoles[role]
key_admin = UserAPIKeyAuth(user_id="admin-user", api_key="sk-hash", user_role=LitellmUserRoles[role])
with patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=["srv-granted"])):
admitted_view = set(await manager.get_allowed_mcp_servers(admitted))
key_admin_view = set(await manager.get_allowed_mcp_servers(key_admin))
assert admitted_view == {"srv-granted", "srv-secret"}, "an admitted admin resolves the registry"
assert key_admin_view == admitted_view, "session and key admin views must be identical"
async def test_admitted_admin_explicit_scope_still_wins(self):
"""An admin whose own user row names servers is entitlement-bound whatever their role: the
row binds through the ceiling for an admitted subject (a user row's mcp_servers is the
human's grant list, not a credential scope), so the registry seed must not fire. A KEY
carrying an explicit scope disqualifies directly, empty list included."""
from litellm.proxy._types import LiteLLM_ObjectPermissionTable, LitellmUserRoles
manager = self._manager_with(["srv-granted", "srv-secret"])
admitted = _make_admitted_subject("admin-user", own_servers=["srv-granted"])
admitted.user_role = LitellmUserRoles.PROXY_ADMIN
with (
patch.object(
MCPRequestHandler, "_get_allowed_mcp_servers_for_user", AsyncMock(return_value=["srv-granted"])
),
patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=["srv-granted"])),
):
assert set(await manager.get_allowed_mcp_servers(admitted)) == {"srv-granted"}
scoped_key = UserAPIKeyAuth(
user_id="admin-user",
api_key="sk-hash",
user_role=LitellmUserRoles.PROXY_ADMIN,
object_permission=LiteLLM_ObjectPermissionTable(object_permission_id="op-k", mcp_servers=[]),
)
with patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=[])):
assert await manager.get_allowed_mcp_servers(scoped_key) == []
async def test_admitted_admin_db_default_empty_scope_still_gets_registry(self):
"""The admitted subject's object_permission is the user's own row, whose mcp_servers column
is [] by DB default whenever the row exists for any other field: default noise, never an
explicit scope. The registry seed must fire through it, or every admin with a shared
permission row keeps resolving zero servers while their dashboard shows all of them."""
from litellm.proxy._types import LiteLLM_ObjectPermissionTable, LitellmUserRoles
manager = self._manager_with(["srv-granted", "srv-secret"])
admitted = _make_admitted_subject("admin-user")
admitted.user_role = LitellmUserRoles.PROXY_ADMIN
admitted.object_permission = LiteLLM_ObjectPermissionTable(object_permission_id="op-u", mcp_servers=[])
with patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=[])):
assert set(await manager.get_allowed_mcp_servers(admitted)) == {"srv-granted", "srv-secret"}
async def test_non_admin_admitted_subject_never_gets_registry(self):
"""The negative control for the registry seed: a plain admitted subject with no admin-view
role resolves only their grant union, however many servers the registry holds."""
manager = self._manager_with(["srv-granted", "srv-secret"])
plain = _make_admitted_subject("plain-user")
with patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=["srv-granted"])):
assert set(await manager.get_allowed_mcp_servers(plain)) == {"srv-granted"}
async def test_admitted_admin_entitlement_ceiling_disables_registry(self):
"""An entitlement ceiling, including an UNRESOLVED one, binds the human whatever their role:
the registry seed must not fire on a transient fault, and the grant union answers instead."""
from litellm.proxy._types import LitellmUserRoles
manager = self._manager_with(["srv-granted", "srv-secret"])
admitted = _make_admitted_subject("admin-user")
admitted.user_role = LitellmUserRoles.PROXY_ADMIN
with patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=["srv-granted"])):
admitted_view = set(await manager.get_allowed_mcp_servers(admitted))
key_admin_view = set(
await manager.get_allowed_mcp_servers(
UserAPIKeyAuth(user_id="admin-user", api_key="sk-hash", user_role=LitellmUserRoles.PROXY_ADMIN)
)
)
assert admitted_view == {"srv-granted"}, "an admitted admin gets their grants, not the registry"
assert key_admin_view == {"srv-granted", "srv-secret"}, "admin KEY behavior must be unchanged"
with (
patch.object(MCPRequestHandler, "_get_allowed_mcp_servers_for_user", AsyncMock(return_value=None)),
patch.object(MCPRequestHandler, "get_allowed_mcp_servers", AsyncMock(return_value=["srv-granted"])),
):
assert set(await manager.get_allowed_mcp_servers(admitted)) == {"srv-granted"}
async def test_admitted_admin_tools_ride_own_source_on_ungranted_server(self):
"""Admin view is an open channel on the tools axis too: the user's OWN source resolves the
tools for a server no grant names, so an admin session's registry-wide servers are invokable
rather than listable-but-uninvokable. A non-admin subject on the same server stays denied.
An admin whose row carries any entitlement never reaches this channel: the ceiling clause
disqualifies the predicate first, so their own tool permissions keep binding on the grants path."""
from litellm.proxy._types import LitellmUserRoles
admin = _make_admitted_subject("admin-user")
admin.user_role = LitellmUserRoles.PROXY_ADMIN
plain = _make_admitted_subject("plain-user")
with self._patch(teams_by_id={}, user_teams=[]):
with patch(
"litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager.operator_open_server_ids",
AsyncMock(return_value=set()),
):
admin_tools = await MCPRequestHandler.get_allowed_tools_for_server("srv-any", admin)
plain_tools = await MCPRequestHandler.get_allowed_tools_for_server("srv-any", plain)
assert admin_tools is None, "admin channel resolves allow-all through the user's own source"
assert plain_tools == [], "a non-admin subject with no granting source stays denied"
async def test_admitted_opt_out_via_wrapper_keeps_team_servers(self):
"""The wrapper's no_mcp_servers early-return is a KEY rule (a scoped credential's opt-out is

69
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py
View file

@ -4846,7 +4846,9 @@ class TestMCPServerManager:
@staticmethod
def _manager_with_deepwiki_and_huggingface() -> MCPServerManager:
manager = MCPServerManager()
deepwiki = MCPServer(server_id="deepwiki-id", name="deepwiki", server_name="deepwiki", transport=MCPTransport.http)
deepwiki = MCPServer(
server_id="deepwiki-id", name="deepwiki", server_name="deepwiki", transport=MCPTransport.http
)
huggingface = MCPServer(
server_id="huggingface-id", name="huggingface", server_name="huggingface", transport=MCPTransport.http
)
@ -4867,8 +4869,14 @@ class TestMCPServerManager:
with pytest.raises(ValueError, match="Tool hub_repo_search not found"):
manager._resolve_mcp_server_for_tool_call("deepwiki", "hub_repo_search")
assert manager._resolve_mcp_server_for_tool_call("deepwiki", "read_wiki_structure") is manager.registry["deepwiki-id"]
assert manager._resolve_mcp_server_for_tool_call("huggingface", "hub_repo_search") is manager.registry["huggingface-id"]
assert (
manager._resolve_mcp_server_for_tool_call("deepwiki", "read_wiki_structure")
is manager.registry["deepwiki-id"]
)
assert (
manager._resolve_mcp_server_for_tool_call("huggingface", "hub_repo_search")
is manager.registry["huggingface-id"]
)
def test_get_mcp_server_from_tool_name_rejects_other_servers_prefix(self):
manager = self._manager_with_deepwiki_and_huggingface()
@ -4876,7 +4884,9 @@ class TestMCPServerManager:
assert manager._get_mcp_server_from_tool_name("huggingface-read_wiki_structure") is None
assert manager._get_mcp_server_from_tool_name("deepwiki-hub_repo_search") is None
assert manager._get_mcp_server_from_tool_name("deepwiki-read_wiki_structure") is manager.registry["deepwiki-id"]
assert manager._get_mcp_server_from_tool_name("huggingface-hub_repo_search") is manager.registry["huggingface-id"]
assert (
manager._get_mcp_server_from_tool_name("huggingface-hub_repo_search") is manager.registry["huggingface-id"]
)
def test_resolve_mcp_server_for_tool_call_shared_bare_name_resolves_via_own_prefixed_spelling(self):
manager = MCPServerManager()
@ -10500,6 +10510,39 @@ class TestSessionResourceScopeIntersect:
assert MCPServerManager._admitted_session_resource_scope(self._admitted_auth("b")) == "b"
@pytest.mark.asyncio
async def test_admin_registry_seed_still_bounded_by_session_resource_scope(self):
"""The admin-view registry seed flows through the same scoped exit as every union: a
session envelope sealed to one server never widens past it, even held by an admin whose
role resolves the whole registry. Pin for the connect-page-parity change; without the
single-exit shape, the old early return would hand a per-server bearer the registry."""
from unittest.mock import AsyncMock, patch
from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager
from litellm.proxy._types import LitellmUserRoles
from litellm.types.mcp import MCPTransport
from litellm.types.mcp_server.mcp_server_manager import MCPServer
manager = MCPServerManager()
for sid in ("granted-id", "other-id"):
manager.registry[sid] = MCPServer(
server_id=sid, name=sid, server_name=sid, url="https://example.com/mcp", transport=MCPTransport.http
)
auth = self._admitted_auth("granted-id")
auth.user_role = LitellmUserRoles.PROXY_ADMIN
with (
patch.object(MCPServerManager, "get_allow_all_keys_server_ids", return_value=[]),
patch.object(
MCPServerManager,
"_get_active_submitted_mcp_server_ids_for_user",
new_callable=AsyncMock,
return_value=[],
),
):
assert await manager.get_allowed_mcp_servers(auth) == ["granted-id"]
auth.mcp_session_resource_server_id = None
assert set(await manager.get_allowed_mcp_servers(auth)) == {"granted-id", "other-id"}
@pytest.mark.asyncio
async def test_get_allowed_mcp_servers_scopes_past_operator_open_union(self):
"""The intersect applies AFTER the operator-open (allow_all_keys) union, so a scoped
@ -10519,7 +10562,12 @@ class TestSessionResourceScopeIntersect:
new_callable=AsyncMock,
return_value=["granted-id", "other-id"],
),
patch.object(MCPServerManager, "_get_active_submitted_mcp_server_ids_for_user", new_callable=AsyncMock, return_value=[]),
patch.object(
MCPServerManager,
"_get_active_submitted_mcp_server_ids_for_user",
new_callable=AsyncMock,
return_value=[],
),
):
allowed = await manager.get_allowed_mcp_servers(auth)
assert allowed == ["granted-id"]
@ -10531,7 +10579,12 @@ class TestSessionResourceScopeIntersect:
new_callable=AsyncMock,
side_effect=RuntimeError("resolver down"),
),
patch.object(MCPServerManager, "_get_active_submitted_mcp_server_ids_for_user", new_callable=AsyncMock, return_value=[]),
patch.object(
MCPServerManager,
"_get_active_submitted_mcp_server_ids_for_user",
new_callable=AsyncMock,
return_value=[],
),
):
fallback = await manager.get_allowed_mcp_servers(auth)
assert fallback == ["granted-id"]
@ -10645,9 +10698,7 @@ class TestClientForwardedDiscoveryFailureIsNotFatal:
@pytest.mark.parametrize("auth_type", [MCPAuth.true_passthrough, MCPAuth.oauth_delegate])
@pytest.mark.asyncio
async def test_client_forwarded_servers_keep_discovering_their_front_door_endpoints(
self, auth_type: MCPAuthType
):
async def test_client_forwarded_servers_keep_discovering_their_front_door_endpoints(self, auth_type: MCPAuthType):
"""Exempting these modes from the FAILURE must not exempt them from discovery itself.
``/authorize``, ``/token`` and ``/register`` read the discovered endpoints for these servers