mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-05 08:07:05 +00:00
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.
This commit is contained in:
parent
446a6e8cdd
commit
9bf3907de5
2 changed files with 39 additions and 71 deletions
|
|
@ -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}
|
||||
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue