fix(mcp): admit a user-subject envelope with the user's own MCP object permission

_reload_admitted_user returned a bare UserAPIKeyAuth(user_id=...), so the shared
get_allowed_mcp_servers found no key/team/object-permission grants and an interactive SSO client could
admit successfully yet see zero tools on a normal (allow_all_keys=False) server. The key path returns
the full key record whose object permission drives that computation; the user path dropped it.

Resolve the user's own MCP object permission and put it on the returned auth, so the same
get_allowed_mcp_servers the key path uses grants the user their litellm-granted servers and access
groups. This reuses get_object_permission (the id-to-grants resolver keys and teams already use) and
does not duplicate any permission logic; get_user_object does not load object_permission, so it is
resolved from the user's object_permission_id the same way the key and team paths do.

Only the user's own object permission is bound. A UserAPIKeyAuth carries a single team_id while a user
may belong to many teams, so team-inherited MCP grants for a user are a follow-up: they need a
many-teams union get_allowed_mcp_servers does not do off one auth object, and faking one here would be
the kind of half-measure that spawns more bugs. Tests cover the user's object permission riding onto the
admitted auth, and the existing admit/SCIM/missing-user/503 cases still hold.
This commit is contained in:
Tin Chi Lo 2026-07-13 11:18:53 -07:00
parent f96899ae2b
commit c46863b0e6
2 changed files with 70 additions and 8 deletions

View file

@ -601,11 +601,14 @@ class MCPRequestHandler:
The DCR client authenticates via SSO at the bridged authorize, which yields a user
subject rather than a virtual key, so the envelope admits under the user's own
identity: the reloaded ``user_id`` rides on the returned ``UserAPIKeyAuth`` and the
caller's centralized policy gate then enforces the user's live budget and org state,
and a SCIM-deactivated owner fails closed here exactly as the key path enforces it. No
team is bound; a user may belong to many teams or none, so the envelope grants the
user's own access rather than silently selecting one team's scope.
identity: the reloaded ``user_id`` and the user's own MCP object permission ride on the
returned ``UserAPIKeyAuth``, and the SAME ``get_allowed_mcp_servers`` the key path uses then
computes which servers the user may reach, so the user's litellm MCP grants and access groups
gate the request exactly as a key's do. Only the user's OWN object permission is bound: a
``UserAPIKeyAuth`` carries a single ``team_id`` while a user may belong to many teams, so
team-inherited MCP grants for a user are a follow-up (they need a many-teams union
``get_allowed_mcp_servers`` does not do off one auth object). The caller's centralized policy
gate enforces the user's live budget and org state, and a SCIM-deactivated owner fails closed.
Error handling mirrors the key path's retryable-503 contract, with one deliberate
difference: ``get_key_object`` raises a ``ProxyException`` for a missing key, but
@ -613,7 +616,7 @@ class MCPRequestHandler:
``ProxyException``/``HTTPException``). So a transient DB outage still surfaces as a retryable
503 via ``_raise_503_if_db_unavailable``, while a missing user, or any other non-outage
resolution failure, fails closed as a 401 rather than propagating as an opaque 500."""
from litellm.proxy.auth.auth_checks import get_user_object
from litellm.proxy.auth.auth_checks import get_object_permission, get_user_object
from litellm.proxy.proxy_server import prisma_client, user_api_key_cache
if prisma_client is None:
@ -634,7 +637,22 @@ class MCPRequestHandler:
raise HTTPException(status_code=401, detail="Invalid or expired credential")
if isinstance(user_object.metadata, dict) and user_object.metadata.get("scim_active") is False:
raise HTTPException(status_code=401, detail="Invalid or expired credential")
return UserAPIKeyAuth(user_id=user_object.user_id)
# Resolve the user's own MCP object permission (get_user_object does not load it) so the shared
# get_allowed_mcp_servers can grant the user their litellm-granted servers. Reuses the same
# get_object_permission resolver the key and team paths use; no permission logic is duplicated.
object_permission = user_object.object_permission
if user_object.object_permission_id and object_permission is None:
object_permission = await get_object_permission(
object_permission_id=user_object.object_permission_id,
prisma_client=prisma_client,
user_api_key_cache=user_api_key_cache,
)
return UserAPIKeyAuth(
user_id=user_object.user_id,
user_role=user_object.user_role,
object_permission=object_permission,
object_permission_id=user_object.object_permission_id,
)
@staticmethod
async def _reload_admitted_key(key_hash: str) -> UserAPIKeyAuth:

View file

@ -5103,7 +5103,13 @@ class TestMCPDcrBridgeDelegateAdmission:
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
patch("litellm.proxy.proxy_server.master_key", self._MASTER_KEY),
self._patch_user_reload(
return_value=MagicMock(user_id="sso-user-7", metadata={"scim_active": True})
return_value=MagicMock(
user_id="sso-user-7",
metadata={"scim_active": True},
user_role=None,
object_permission=None,
object_permission_id=None,
)
) as get_user_object,
):
mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server()
@ -5116,6 +5122,44 @@ class TestMCPDcrBridgeDelegateAdmission:
"bridge_delegate_server": {"Authorization": "Bearer inner-upstream-access-token"}
}
async def test_user_subject_envelope_carries_the_users_mcp_object_permission(self):
"""The admitted user's own MCP object permission rides on the returned auth so the shared
get_allowed_mcp_servers grants the user their litellm-granted servers, rather than admitting a
bare user with no MCP access. Regression for the signed-in SSO client getting zero tools because
the reload dropped the user's object permission."""
object_permission = LiteLLM_ObjectPermissionTable(
object_permission_id="op-user-7", mcp_servers=["bridge_delegate_server"]
)
envelope = self._mint_bridge_envelope(user_id="sso-user-7")
scope = {
"type": "http",
"method": "POST",
"path": "/mcp/bridge_delegate_server",
"headers": [(b"authorization", f"Bearer {envelope}".encode("latin-1"))],
}
with (
patch(
"litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth",
new_callable=AsyncMock,
),
patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager") as mock_mgr,
patch("litellm.proxy.proxy_server.master_key", self._MASTER_KEY),
self._patch_user_reload(
return_value=MagicMock(
user_id="sso-user-7",
metadata={"scim_active": True},
user_role=None,
object_permission=object_permission,
object_permission_id="op-user-7",
)
),
):
mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server()
(auth_result, _h, _s, _headers, _o, _r) = await MCPRequestHandler.process_mcp_request(scope)
assert auth_result.object_permission is not None
assert auth_result.object_permission.mcp_servers == ["bridge_delegate_server"]
async def test_user_subject_envelope_missing_user_fails_closed_401(self):
"""A user_id envelope whose user has since been deleted must fail closed with a 401, not a 500.
get_user_object raises a bare Exception for a missing user (it does not return None on the