From 9bf3907de546beb34bbfddad048c8295e746e50a Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Tue, 7 Jul 2026 16:13:43 -0700 Subject: [PATCH] fix(mcp): bind oauth_delegate discovery resource to the upstream oauth_delegate forwards the caller's token to the upstream, which validates its audience, so the protected-resource metadata must keep resource pointing at the upstream (returned verbatim, like true_passthrough) rather than rewriting it to the gateway. Rewriting to the gateway asks the client to mint a token bound to the gateway audience, which a strict IdP (Entra) refuses to issue for an unregistered resource and a spec-compliant upstream rejects on receipt. The legacy is_oauth_passthrough opt-in keeps the gateway rewrite unchanged. --- .../mcp_server/discoverable_endpoints.py | 16 ++-- .../mcp_server/test_mcp_oauth_passthrough.py | 94 ++++++------------- 2 files changed, 39 insertions(+), 71 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index f383c99c563..415336b1a9e 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -1292,11 +1292,15 @@ async def _build_oauth_protected_resource_response( """ Build OAuth protected resource response with the appropriate URL pattern. - For pass-through MCP servers (``MCPServer.is_oauth_passthrough``), the - gateway proxies the upstream's own ``oauth-protected-resource`` metadata - so that standards-compliant MCP clients discover the **upstream** IdP - instead of the gateway. The ``resource`` field is rewritten to the - gateway's own URL so clients present the bearer token back to the gateway. + For pass-through MCP servers, the gateway proxies the upstream's own + ``oauth-protected-resource`` metadata so standards-compliant MCP clients + discover the **upstream** IdP instead of the gateway. For ``true_passthrough`` + and ``oauth_delegate`` the metadata is returned verbatim (``resource`` stays + the upstream): the caller's token is forwarded to and validated by the + upstream, so its audience must be the upstream — rewriting it to the gateway + would make a strict IdP (e.g. Entra) refuse to mint it or the upstream reject + it. Only the legacy ``is_oauth_passthrough`` opt-in rewrites ``resource`` to + the gateway's own URL so clients present the bearer token back to the gateway. Args: request: FastAPI Request object @@ -1355,7 +1359,7 @@ async def _build_oauth_protected_resource_response( ) if upstream_metadata is not None: - if mcp_server.is_true_passthrough: + if mcp_server.is_true_passthrough or mcp_server.is_oauth_delegate: return upstream_metadata return {**upstream_metadata, "resource": resource_url} diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py index 002e98034c1..ef2a8318d7a 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_passthrough.py @@ -34,8 +34,7 @@ from litellm.types.mcp_server.mcp_server_manager import MCPServer def _mock_mcp_client_ip(): """Bypass IP-based access control in tests.""" with patch( - "litellm.proxy._experimental.mcp_server.discoverable_endpoints" - ".IPAddressUtils.get_mcp_client_ip", + "litellm.proxy._experimental.mcp_server.discoverable_endpoints.IPAddressUtils.get_mcp_client_ip", return_value=None, ): yield @@ -191,9 +190,7 @@ async def test_oauth_protected_resource_passthrough_proxies_upstream_metadata(): extra_headers=["Authorization"], oauth_passthrough=True, ) - global_mcp_server_manager.registry[passthrough_server.server_id] = ( - passthrough_server - ) + global_mcp_server_manager.registry[passthrough_server.server_id] = passthrough_server upstream_payload = { "resource": "https://upstream.example.com/mcp", @@ -207,18 +204,14 @@ async def test_oauth_protected_resource_passthrough_proxies_upstream_metadata(): mock_client = MagicMock() mock_client.get = AsyncMock(return_value=mock_response) - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): result = await _build_oauth_protected_resource_response( request=_make_request(), mcp_server_name="sample_docs", use_standard_pattern=True, ) - assert result["authorization_servers"] == [ - "https://okta.example.com/oauth2/default" - ] + assert result["authorization_servers"] == ["https://okta.example.com/oauth2/default"] # resource is normalized to the gateway URL so bearers are sent back to us assert result["resource"].endswith("/mcp/sample_docs") assert result["scopes_supported"] == ["openid", "profile"] @@ -242,9 +235,7 @@ async def test_oauth_protected_resource_passthrough_cache_hit(): extra_headers=["Authorization"], oauth_passthrough=True, ) - global_mcp_server_manager.registry[passthrough_server.server_id] = ( - passthrough_server - ) + global_mcp_server_manager.registry[passthrough_server.server_id] = passthrough_server mock_response = MagicMock() mock_response.status_code = 200 @@ -254,9 +245,7 @@ async def test_oauth_protected_resource_passthrough_cache_hit(): mock_client = MagicMock() mock_client.get = AsyncMock(return_value=mock_response) - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): await _build_oauth_protected_resource_response( request=_make_request(), mcp_server_name="sample_docs", @@ -348,12 +337,8 @@ async def test_oauth_metadata_cache_expired_entry_is_refetched(): mock_client = MagicMock() mock_client.get = AsyncMock(return_value=mock_response) - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): - result = await discoverable_endpoints.fetch_upstream_oauth_protected_resource( - passthrough_server - ) + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): + result = await discoverable_endpoints.fetch_upstream_oauth_protected_resource(passthrough_server) assert result == {"authorization_servers": ["https://fresh.example.com"]} assert mock_client.get.await_count == 1 @@ -377,16 +362,12 @@ async def test_oauth_protected_resource_passthrough_network_error_returns_502(): extra_headers=["Authorization"], oauth_passthrough=True, ) - global_mcp_server_manager.registry[passthrough_server.server_id] = ( - passthrough_server - ) + global_mcp_server_manager.registry[passthrough_server.server_id] = passthrough_server mock_client = MagicMock() mock_client.get = AsyncMock(side_effect=httpx.ConnectError("boom")) - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): with pytest.raises(HTTPException) as exc_info: await _build_oauth_protected_resource_response( request=_make_request(), @@ -414,16 +395,10 @@ async def test_fetch_upstream_metadata_returns_none_when_not_all_candidates_netw not_found_response = MagicMock() not_found_response.status_code = 404 mock_client = MagicMock() - mock_client.get = AsyncMock( - side_effect=[not_found_response, httpx.ConnectError("path fallback failed")] - ) + mock_client.get = AsyncMock(side_effect=[not_found_response, httpx.ConnectError("path fallback failed")]) - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): - result = await discoverable_endpoints.fetch_upstream_oauth_protected_resource( - passthrough_server - ) + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): + result = await discoverable_endpoints.fetch_upstream_oauth_protected_resource(passthrough_server) assert result is None assert mock_client.get.await_count == 2 @@ -458,9 +433,7 @@ async def test_oauth_protected_resource_gateway_managed_unchanged(): mock_client = MagicMock() mock_client.get = AsyncMock() - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): result = await _build_oauth_protected_resource_response( request=_make_request(), mcp_server_name="keycloak_whoami", @@ -468,9 +441,7 @@ async def test_oauth_protected_resource_gateway_managed_unchanged(): ) mock_client.get.assert_not_awaited() - assert result["authorization_servers"] == [ - "https://gateway.example.com/keycloak_whoami" - ] + assert result["authorization_servers"] == ["https://gateway.example.com/keycloak_whoami"] assert result["scopes_supported"] == ["read"] @@ -490,12 +461,13 @@ def _make_upstream_metadata_client() -> tuple[dict, MagicMock]: @pytest.mark.asyncio -async def test_oauth_protected_resource_oauth_delegate_proxies_upstream_with_gateway_resource(): - """oauth_delegate discovery must proxy the upstream's authorization_servers - (so the client authorizes against the upstream IdP) while rewriting resource - to the gateway (so bearers are presented back to LiteLLM). A regression that - dropped oauth_delegate from the pass-through predicate would fall through to - the gateway-AS branch and advertise LiteLLM as the authorization server.""" +async def test_oauth_protected_resource_oauth_delegate_returns_upstream_metadata_verbatim(): + """oauth_delegate discovery must return the upstream metadata verbatim, + resource included. The caller's token is forwarded to and validated by the + upstream, so its audience must be the upstream; rewriting resource to the + gateway would make a strict IdP refuse to mint it or the upstream reject it. + A regression that dropped oauth_delegate from the pass-through predicate would + fall through to the gateway-AS branch and advertise LiteLLM as the AS.""" from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( global_mcp_server_manager, ) @@ -512,22 +484,18 @@ async def test_oauth_protected_resource_oauth_delegate_proxies_upstream_with_gat ) global_mcp_server_manager.registry[delegate_server.server_id] = delegate_server - _, mock_client = _make_upstream_metadata_client() + upstream_payload, mock_client = _make_upstream_metadata_client() try: - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): result = await _build_oauth_protected_resource_response( request=_make_request(), mcp_server_name="sample_docs", use_standard_pattern=True, ) - assert result["authorization_servers"] == [ - "https://okta.example.com/oauth2/default" - ] - assert result["resource"].endswith("/mcp/sample_docs") - assert result["resource"] != "https://upstream.example.com/mcp" + assert result == upstream_payload + assert result["authorization_servers"] == ["https://okta.example.com/oauth2/default"] + assert result["resource"] == "https://upstream.example.com/mcp" finally: global_mcp_server_manager.registry.clear() @@ -552,15 +520,11 @@ async def test_oauth_protected_resource_true_passthrough_returns_upstream_metada transport=MCPTransport.http, auth_type=MCPAuth.true_passthrough, ) - global_mcp_server_manager.registry[true_passthrough_server.server_id] = ( - true_passthrough_server - ) + global_mcp_server_manager.registry[true_passthrough_server.server_id] = true_passthrough_server upstream_payload, mock_client = _make_upstream_metadata_client() try: - with patch.object( - discoverable_endpoints, "get_async_httpx_client", return_value=mock_client - ): + with patch.object(discoverable_endpoints, "get_async_httpx_client", return_value=mock_client): result = await _build_oauth_protected_resource_response( request=_make_request(), mcp_server_name="sample_docs",