scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-11 22:51:28 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 123

fix(mcp): let an admin-pinned issuer drive OAuth discovery for url-less servers

Browse source
This commit is contained in:
Tin Chi Lo 2026-07-20 19:37:20 -07:00
parent 1d36290551
commit 930676a9bf
4 changed files with 187 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py
View file

@ -597,7 +597,14 @@ async def authorize_with_server(
):
_raise_if_not_oauth2(mcp_server)
if mcp_server.authorization_url is None:
raise HTTPException(status_code=400, detail="MCP server authorization url is not set")
raise HTTPException(
status_code=400,
detail=(
"MCP server authorization url is not configured. Servers with no url (OpenAPI "
"spec or stdio) run no resource discovery, so set Authorization URL and Token URL "
"manually, or set Issuer to discover them from the identity provider (RFC 8414)."
),
)
if mcp_server.is_dcr_bridge:
# Enforce S256 PKCE on both bridge arms. The relay arm forwards the validated,
@ -702,7 +709,14 @@ async def exchange_token_with_server(
raise HTTPException(status_code=400, detail="Unsupported grant_type")
if mcp_server.token_url is None:
raise HTTPException(status_code=400, detail="MCP server token url is not set")
raise HTTPException(
status_code=400,
detail=(
"MCP server token url is not configured. Servers with no url (OpenAPI spec or "
"stdio) run no resource discovery, so set Token URL manually, or set Issuer to "
"discover it from the identity provider (RFC 8414)."
),
)
# The id and secret must come from the same source. When the server-side client_id wins,
# falling back to the caller's secret pairs the persisted client with a foreign secret; the
@ -1262,7 +1276,14 @@ async def register_client_with_server(
return dummy_return
if mcp_server.authorization_url is None:
raise HTTPException(status_code=400, detail="MCP server authorization url is not set")
raise HTTPException(
status_code=400,
detail=(
"MCP server authorization url is not configured. Servers with no url (OpenAPI "
"spec or stdio) run no resource discovery, so set Authorization URL and Token URL "
"manually, or set Issuer to discover them from the identity provider (RFC 8414)."
),
)
if mcp_server.registration_url is None:
return dummy_return

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

@ -224,6 +224,20 @@ def _uses_issuer_anchor(manual_issuer: str | None, is_discovery_auth_type: bool)
return _blank_to_none(manual_issuer) is not None and is_discovery_auth_type
def _has_oauth_discovery_source(server_url: str | None, use_issuer_anchor: bool) -> bool:
"""Whether the server has any source OAuth discovery can fetch metadata from.
Resource-rooted discovery (RFC 9728) is fetched from the server ``url``, so spec-only
(OpenAPI) and stdio servers, which have none, could never discover: their OAuth endpoints
stayed unset unless entered manually and ``/authorize`` served its 400 with no hint of why.
An admin-pinned issuer is a trust anchor in its own right (RFC 8414 section 3.3) whose
metadata fetch does not touch the resource at all, so an anchored server can discover with
no ``url``. Called by both build paths (config and DB) so the two cannot disagree on when
discovery is reachable.
"""
return bool(server_url) or use_issuer_anchor
def _endpoints_yield_to_issuer(
issuer: str | None,
is_discovery_auth_type: bool,
@ -1254,7 +1268,12 @@ class MCPServerManager:
manual_token_url = _blank_to_none(server_config.get("token_url"))
manual_registration_url = _blank_to_none(server_config.get("registration_url"))
is_discovery_auth_type = auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES
use_issuer_anchor = _uses_issuer_anchor(manual_issuer, is_discovery_auth_type)
obo_needs_discovery = self._obo_needs_endpoint_discovery(
auth_type,
server_config.get("token_exchange_endpoint"),
manual_token_url,
)
use_issuer_anchor = _uses_issuer_anchor(manual_issuer, is_discovery_auth_type or obo_needs_discovery)
manual_authorization_url, manual_token_url, manual_registration_url = _endpoints_yield_to_issuer(
manual_issuer,
is_discovery_auth_type,
@ -1262,17 +1281,12 @@ class MCPServerManager:
manual_token_url,
manual_registration_url,
)
should_discover = bool(server_url) and (
is_discovery_auth_type
or self._obo_needs_endpoint_discovery(
auth_type,
server_config.get("token_exchange_endpoint"),
manual_token_url,
)
should_discover = _has_oauth_discovery_source(server_url, use_issuer_anchor) and (
is_discovery_auth_type or obo_needs_discovery
)
if not should_discover:
mcp_oauth_metadata = None
elif manual_issuer is not None and is_discovery_auth_type:
elif use_issuer_anchor and manual_issuer is not None:
mcp_oauth_metadata = await self._fetch_issuer_anchored_oauth_metadata(manual_issuer, server_url)
else:
mcp_oauth_metadata = await self._descovery_metadata(
@ -1668,7 +1682,7 @@ class MCPServerManager:
token_exchange_endpoint: Optional[str],
) -> Optional[MCPOAuthMetadata]:
has_all_upstream_oauth_fields = bool(manual_authorization_url and manual_token_url and scopes)
needs_discovery = bool(server_url) and (
needs_discovery = _has_oauth_discovery_source(server_url, use_issuer_anchor) and (
(is_discovery_auth_type and not has_all_upstream_oauth_fields)
or self._obo_needs_endpoint_discovery(auth_type, token_exchange_endpoint, manual_token_url)
)
@ -1787,13 +1801,17 @@ class MCPServerManager:
manual_token_url = _blank_to_none(mcp_server.token_url)
manual_registration_url = _blank_to_none(mcp_server.registration_url)
is_discovery_auth_type = auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES
use_issuer_anchor = _uses_issuer_anchor(manual_issuer, is_discovery_auth_type)
manual_authorization_url, manual_token_url, manual_registration_url = _endpoints_yield_to_issuer(
manual_issuer, is_discovery_auth_type, manual_authorization_url, manual_token_url, manual_registration_url
)
token_exchange_endpoint = mcp_server.token_exchange_endpoint or (
credentials_dict.get("token_exchange_endpoint") if credentials_dict else None
)
use_issuer_anchor = _uses_issuer_anchor(
manual_issuer,
is_discovery_auth_type
or self._obo_needs_endpoint_discovery(auth_type, token_exchange_endpoint, manual_token_url),
)
manual_authorization_url, manual_token_url, manual_registration_url = _endpoints_yield_to_issuer(
manual_issuer, is_discovery_auth_type, manual_authorization_url, manual_token_url, manual_registration_url
)
gated_oauth_metadata = await self._resolve_table_oauth_metadata(
mcp_server=mcp_server,
auth_type=auth_type,
@ -1971,7 +1989,7 @@ class MCPServerManager:
family: discovered ``authorization_url``/``token_url``/``scopes`` otherwise live only on
the in-memory registry entry, which is rebuilt on every client connect (the DCR reuse path
calls ``update_server``) and on every post-write DB reload, so one failed re-discovery
serves 400 "authorization url is not set" from /authorize until a later rebuild succeeds.
serves the 400 "authorization url is not configured" from /authorize until a later rebuild succeeds.
Only fills row fields that are currently empty, never persists origin-fallback guesses
(RFC 9728/8414-advertised metadata only), and deliberately skips ``registration_url``
because ``_dcr_bridge_relays_client_registration`` keys off that column. Best-effort: a

37
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py
View file

@ -8031,3 +8031,40 @@ async def test_bare_origin_discovery_resolves_single_server_not_aggregate():
assert resource_response["authorization_servers"] == ["https://llm.example.com/test_oauth"]
finally:
global_mcp_server_manager.registry.clear()
@pytest.mark.asyncio
async def test_authorize_wall_names_the_fix_for_urlless_servers():
"""LIT-4629: the authorize wall previously said only "authorization url is not set" with no
hint that spec-only servers never discover; the detail must now name both remedies (manual
Authorization URL + Token URL, or an Issuer for RFC 8414 discovery)."""
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
authorize_with_server,
)
from litellm.types.mcp import MCPAuth, MCPTransport
from litellm.types.mcp_server.mcp_server_manager import MCPServer
server = MCPServer(
server_id="urlless-wall",
name="sheets_wall",
server_name="sheets_wall",
url=None,
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
spec_path="https://example.com/openapi.yaml",
)
mock_request = MagicMock()
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
with pytest.raises(HTTPException) as exc_info:
await authorize_with_server(
request=mock_request,
mcp_server=server,
client_id="client",
redirect_uri="http://localhost/callback",
)
assert exc_info.value.status_code == 400
detail_text = str(exc_info.value.detail)
assert "set Authorization URL and Token URL" in detail_text
assert "Issuer" in detail_text

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

@ -5597,7 +5597,7 @@ class TestMCPServerTimestamps:
async def test_build_mcp_server_from_table_persists_discovered_oauth_endpoints(self):
"""A DB-backed oauth2 server with no configured endpoints discovers them and must write
authorization_url, token_url, and scopes back to the row; otherwise the resolved values
live only in memory and one failed re-discovery serves 400 "authorization url is not set"
live only in memory and one failed re-discovery serves the 400 "authorization url is not configured"
from /authorize. registration_url must never be persisted because
_dcr_bridge_relays_client_registration keys off that column."""
manager = MCPServerManager()
@ -8936,3 +8936,95 @@ class TestMaterializeAuthHeaders:
assert await _materialize_auth_headers(None) is None
assert await _materialize_auth_headers(NoOpAuth()) is None
class TestUrllessIssuerDiscovery:
"""LIT-4629: servers with no url (OpenAPI spec_path, stdio) run no resource discovery, so
their OAuth endpoints could only ever come from manual entry; an admin-pinned issuer is a
url-independent trust anchor (RFC 8414 section 3.3) and must unlock discovery for them."""
def _urlless_row(self, **overrides):
fields = dict(
server_id="urlless-1",
alias="sheets_urlless",
description="spec-only server",
url=None,
spec_path="https://example.com/sheets-openapi.yaml",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
created_at=datetime.now(),
updated_at=datetime.now(),
)
fields.update(overrides)
return LiteLLM_MCPServerTable(**fields)
@pytest.mark.asyncio
async def test_urlless_server_with_issuer_discovers_endpoints(self):
"""The gate previously required bool(server_url), so a url-less server with an issuer
configured never ran the issuer-anchored fetch and /authorize 400d. Kills the mutant that
restores the bare bool(server_url) term."""
manager = MCPServerManager()
row = self._urlless_row(issuer="https://accounts.google.com")
resolved = MCPOAuthMetadata(
authorization_url="https://accounts.google.com/o/oauth2/v2/auth",
token_url="https://oauth2.googleapis.com/token",
)
resource_rooted = AsyncMock(return_value=None)
with (
patch.object(manager, "_fetch_issuer_anchored_oauth_metadata", new=AsyncMock(return_value=resolved)) as anchored,
patch.object(manager, "_descovery_metadata", new=resource_rooted),
):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
anchored.assert_awaited_once_with("https://accounts.google.com", None)
resource_rooted.assert_not_awaited()
assert built.issuer_is_anchored is True
assert built.authorization_url == "https://accounts.google.com/o/oauth2/v2/auth"
assert built.token_url == "https://oauth2.googleapis.com/token"
@pytest.mark.asyncio
async def test_urlless_server_without_issuer_stays_undiscovered(self):
"""With neither a url nor an issuer there is no discovery source; the build must not
attempt any fetch and the endpoints stay unset (manual entry remains the only path)."""
manager = MCPServerManager()
row = self._urlless_row()
anchored = AsyncMock()
resource_rooted = AsyncMock()
with (
patch.object(manager, "_fetch_issuer_anchored_oauth_metadata", new=anchored),
patch.object(manager, "_descovery_metadata", new=resource_rooted),
):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
anchored.assert_not_awaited()
resource_rooted.assert_not_awaited()
assert built.authorization_url is None
assert built.token_url is None
assert built.issuer_is_anchored is False
@pytest.mark.asyncio
async def test_urlless_obo_with_issuer_discovers_token_url(self):
"""oauth2_token_exchange is not a discovery auth type, so the plain gate relax alone
would leave a url-less OBO server undiscovered; with an issuer pinned and no configured
exchange endpoint it must resolve token_url through the issuer-anchored fetch. Kills the
mutant that drops the OBO widening from the anchor computation."""
manager = MCPServerManager()
row = self._urlless_row(
alias="obo_urlless",
auth_type=MCPAuth.oauth2_token_exchange,
issuer="https://idp.example.com",
)
resolved = MCPOAuthMetadata(token_url="https://idp.example.com/token")
resource_rooted = AsyncMock(return_value=None)
with (
patch.object(manager, "_fetch_issuer_anchored_oauth_metadata", new=AsyncMock(return_value=resolved)) as anchored,
patch.object(manager, "_descovery_metadata", new=resource_rooted),
):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
anchored.assert_awaited_once_with("https://idp.example.com", None)
resource_rooted.assert_not_awaited()
assert built.token_url == "https://idp.example.com/token"