diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index e46e6299277..1ca4c657706 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -1341,7 +1341,7 @@ async def _persist_dcr_client_registration( ``update_mcp_server`` merges credential blobs: a re-registered public client must not inherit the previous client's secret or auth method. """ - if mcp_server.is_true_passthrough or mcp_server.is_oauth_delegate: + if mcp_server.is_client_forwarded_token: return "skipped" try: @@ -2187,7 +2187,7 @@ async def _build_oauth_protected_resource_response( ) if upstream_metadata is not None: - if mcp_server.is_true_passthrough or mcp_server.is_oauth_delegate: + if mcp_server.is_client_forwarded_token: return upstream_metadata return {**upstream_metadata, "resource": resource_url} diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 9835c7f01ed..ecce959143a 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -1740,12 +1740,14 @@ class MCPServerManager: server: The MCP server whose OAuth metadata must be resolved. Returns: - The resolved server, or the registered server when no discovery is - pending. + The resolved server; the registered server when no discovery is + pending, or when discovery failed for a client-forwarded-token + server, whose session consumes no discovered endpoint. Raises: HTTPException: Status 503 when discovery times out or returns - incomplete metadata. + incomplete metadata for a server whose OAuth flow the gateway + runs itself. """ acquisition: Final = self._get_or_start_oauth_discovery_task(server) if acquisition is None: @@ -1764,6 +1766,8 @@ class MCPServerManager: return await self.ensure_oauth_metadata_discovered(server) case _OAuthDiscoveryFailed(timed_out=timed_out): current: Final = self._registered_server(server) + if current.is_client_forwarded_token: + return current server_ref: Final = current.alias or current.server_name or current.name or current.server_id reason: Final = "timed out" if timed_out else "returned incomplete metadata" raise HTTPException( @@ -5256,7 +5260,7 @@ class MCPServerManager: user_api_key_auth=user_api_key_auth, ): extra_headers = _without_authorization(extra_headers) - elif mcp_server.is_true_passthrough or mcp_server.is_oauth_delegate: + elif mcp_server.is_client_forwarded_token: extra_headers = _client_forwarded_authorization_headers( mcp_server=mcp_server, oauth2_headers=oauth2_headers, @@ -5363,7 +5367,7 @@ class MCPServerManager: # Scoped to the two client-forwarded token modes this stack introduced; legacy # oauth2 + delegate_auth_to_upstream (is_oauth_passthrough) is being removed, so it is not # added here even though the list path still relays for it. - relays_upstream_auth: Final = mcp_server.is_true_passthrough or mcp_server.is_oauth_delegate + relays_upstream_auth: Final = mcp_server.is_client_forwarded_token server_label: Final = mcp_server.name or mcp_server.server_name or mcp_server.alias or "" async def _call_tool_via_client(client, params): diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index d4c5b198fbc..7365fc4efbd 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -1704,7 +1704,7 @@ if MCP_AVAILABLE: ) extra_headers: dict[str, str] | None = None - is_client_forwarded_mode: Final = server.is_true_passthrough or server.is_oauth_delegate + is_client_forwarded_mode: Final = server.is_client_forwarded_token # In a multi-server listing scope the request-wide Authorization can only carry one token, # so it is withheld from a client-forwarded server when another server in scope also consumes # it (RFC 9700 cross-resource replay); such scopes must bind per-server via diff --git a/litellm/types/mcp_server/mcp_server_manager.py b/litellm/types/mcp_server/mcp_server_manager.py index aeeeca21d3b..d09503cdc4d 100644 --- a/litellm/types/mcp_server/mcp_server_manager.py +++ b/litellm/types/mcp_server/mcp_server_manager.py @@ -224,6 +224,14 @@ class MCPServer(BaseModel): JWT) but forwards the caller's separate upstream ``Authorization`` unchanged, minting nothing.""" return self.auth_type == MCPAuth.oauth_delegate + @property + def is_client_forwarded_token(self) -> bool: + """True for the two modes whose upstream credential is the caller's own bearer, forwarded + unchanged: the gateway mints nothing for them and holds no OAuth client identity, so a + discovered ``authorization_url`` / ``token_url`` enriches only the gateway's own OAuth front + door and is never a precondition for opening a session.""" + return self.is_true_passthrough or self.is_oauth_delegate + @property def is_dcr_bridge(self) -> bool: """True when this client-forwarded-token server serves the gateway-hosted DCR front door @@ -231,7 +239,7 @@ class MCPServer(BaseModel): authorize, and token relays) instead of relaying the upstream's own OAuth discovery verbatim. ``dcr_bridge`` is rejected on every other auth type at create, update, and config load, so the mode gate here only defends rows edited outside those paths.""" - return bool(self.dcr_bridge) and (self.is_true_passthrough or self.is_oauth_delegate) + return bool(self.dcr_bridge) and self.is_client_forwarded_token @property def requires_per_user_auth(self) -> bool: @@ -248,7 +256,7 @@ class MCPServer(BaseModel): if self.needs_user_oauth_token: return True - if self.is_true_passthrough or self.is_oauth_delegate: + if self.is_client_forwarded_token: return True # PAT passthrough: auth_type is none but extra_headers includes auth headers diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index 14400ef6376..309f3cfb572 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -10525,3 +10525,142 @@ class TestSessionResourceScopeIntersect: ): fallback = await manager.get_allowed_mcp_servers(auth) assert fallback == ["granted-id"] + + +class TestClientForwardedDiscoveryFailureIsNotFatal: + """A failed OAuth metadata discovery may only brick the flows the gateway runs itself. + + ``true_passthrough`` / ``oauth_delegate`` forward the caller's own bearer and mint nothing, so an + upstream that publishes no RFC 9728 metadata (an internal API, or any IdP unreachable from the + pod) must still serve sessions instead of 503-ing before the upstream is ever contacted. + """ + + @staticmethod + def _config(auth_type: MCPAuthType, dcr_bridge: bool | None) -> dict[str, dict[str, object]]: + entry: Final[dict[str, object]] = { + "url": "https://up.example.com/mcp", + "transport": MCPTransport.http, + "auth_type": auth_type, + **({"oauth2_flow": "authorization_code"} if auth_type == MCPAuth.oauth2 else {}), + **({"dcr_bridge": dcr_bridge} if dcr_bridge is not None else {}), + } + return {"upstream": entry} + + async def _registered(self, manager: MCPServerManager, auth_type: MCPAuthType, dcr_bridge: bool | None): + with ( + patch.object(manager, "_discover_oauth_metadata_for_server", new=AsyncMock(return_value=None)), + patch.object(manager, "initialize_tool_name_to_mcp_server_name_mapping"), + ): + await manager.load_servers_from_config(self._config(auth_type, dcr_bridge)) + return next(iter(manager.config_mcp_servers.values())) + + @pytest.mark.parametrize( + "auth_type, dcr_bridge, serves_without_endpoints", + [ + (MCPAuth.true_passthrough, None, True), + (MCPAuth.true_passthrough, True, True), + (MCPAuth.oauth_delegate, None, True), + (MCPAuth.oauth_delegate, True, True), + (MCPAuth.oauth2, None, False), + (MCPAuth.oauth2_token_exchange, None, False), + ], + ) + @pytest.mark.parametrize("failure", ["incomplete", "timed_out"]) + @pytest.mark.asyncio + async def test_discovery_failure_blocks_only_gateway_run_flows( + self, + auth_type: MCPAuthType, + dcr_bridge: bool | None, + serves_without_endpoints: bool, + failure: str, + ): + manager = MCPServerManager() + server = await self._registered(manager, auth_type, dcr_bridge) + manager._set_oauth_discovery_deferred(server.server_id, True) + + async def never_returns(_server): + await asyncio.Future() + + discovery_patch: Final = ( + {"new": AsyncMock(return_value=None)} if failure == "incomplete" else {"side_effect": never_returns} + ) + with ( + patch("litellm.proxy._experimental.mcp_server.mcp_server_manager.MCP_METADATA_TIMEOUT", 0.01), + patch.object(manager, "_discover_oauth_metadata_for_server", **discovery_patch), + ): + if not serves_without_endpoints: + with pytest.raises(HTTPException) as exc: + await manager.ensure_oauth_metadata_discovered(server) + assert exc.value.status_code == 503 + return + + resolved = await manager.ensure_oauth_metadata_discovered(server) + + assert resolved is manager.config_mcp_servers[server.server_id] + assert resolved.authorization_url is None + assert resolved.token_url is None + assert manager._oauth_discovery_slot(server.server_id) is not None + + @pytest.mark.parametrize( + "auth_type, serves_the_listing", + [(MCPAuth.true_passthrough, True), (MCPAuth.oauth_delegate, True), (MCPAuth.oauth2, False)], + ) + @pytest.mark.asyncio + async def test_listing_leg_serves_a_forwarding_server_whose_discovery_failed( + self, auth_type: MCPAuthType, serves_the_listing: bool + ): + """The listing leg is where the 503 became an empty tool list, so pin the fix there too. + + ``_get_tools_from_server`` is the per-server leg the aggregate absorbs: a failure here is what + the fan-out turns into HTTP 200 with ``tools: []``, which is why the outage carried no + diagnostic. A forwarding server must now reach its upstream, and a gateway-run flow must still + surface the fault rather than be silently listed as empty. + """ + manager = MCPServerManager() + server = await self._registered(manager, auth_type, None) + manager._set_oauth_discovery_deferred(server.server_id, True) + manager._fetch_tools_with_timeout = AsyncMock( + return_value=[MCPTool(name="list_reports", description="d", inputSchema={"type": "object"})] + ) + + with patch.object(manager, "_discover_oauth_metadata_for_server", new=AsyncMock(return_value=None)): + if not serves_the_listing: + with pytest.raises(MCPServerListError): + await manager._get_tools_from_server(server=server) + return + tools = await manager._get_tools_from_server(server=server) + + assert [tool.name for tool in tools] == ["upstream-list_reports"] + manager._fetch_tools_with_timeout.assert_awaited_once() + + @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 + ): + """Exempting these modes from the FAILURE must not exempt them from discovery itself. + + ``/authorize``, ``/token`` and ``/register`` read the discovered endpoints for these servers + (``_resolve_ephemeral_dcr_client`` mints for ``true_passthrough`` whatever ``dcr_bridge`` + says), so an exemption written into the unresolved-endpoints predicate would disarm the slot + and silently drop a working front door. + """ + manager = MCPServerManager() + metadata: Final = MCPOAuthMetadata( + authorization_url="https://idp.example.com/authorize", + token_url="https://idp.example.com/token", + registration_url="https://idp.example.com/register", + ) + with ( + patch.object(manager, "_discover_oauth_metadata_for_server", new=AsyncMock(return_value=metadata)), + patch.object(manager, "initialize_tool_name_to_mcp_server_name_mapping"), + ): + await manager.load_servers_from_config(self._config(auth_type, None)) + server = next(iter(manager.config_mcp_servers.values())) + resolved = await manager.ensure_oauth_metadata_discovered(server) + + assert resolved.authorization_url == "https://idp.example.com/authorize" + assert resolved.token_url == "https://idp.example.com/token" + assert resolved.registration_url == "https://idp.example.com/register" + assert manager.config_mcp_servers[server.server_id].authorization_url == "https://idp.example.com/authorize" + assert manager._oauth_discovery_slot(server.server_id) is None