scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-07 08:26:10 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 89

fix(mcp): normalize blank OAuth endpoint fields to None at build entry points

Browse source 
A whitespace-only authorization_url was truthy to the row/config merges and
has_all check but blank to the corroboration gate, so discovery and
carry-forward adopted token_url/registration_url/scopes as if unpinned while
the broken whitespace value was still used for redirects. Rather than add
another strip() at each site, the pinned authorization_url/token_url/
registration_url are normalized once per build path (DB and config) via
_blank_to_none, so the merge, has_all gate, discovery gate, persist hook, and
carry-forward all see a single notion of blank. Empty and whitespace pins now
behave identically to an omitted field.
This commit is contained in:
Tin Chi Lo 2026-07-15 11:12:54 -07:00
parent 8650f6c7d3
commit feedab214e
2 changed files with 75 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -186,6 +186,21 @@ _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES: tuple[MCPAuth, ...] = (
)
def _blank_to_none(value: str | None) -> str | None:
"""Collapse an absent, empty, or whitespace-only string to ``None``.
OAuth endpoint fields are consumed by truthiness-based merges (``row or discovered``) and by the
corroboration gate. A whitespace-only value is truthy to ``or`` but is not a usable endpoint, so
without this the merge would keep the blank value for redirects while the gate treats it as
unpinned and backfills the other fields, yielding a broken half-discovered config. Normalizing
the pinned fields once, at each build entry point, gives every downstream consumer a single
notion of "blank" so those code paths cannot disagree.
"""
if not isinstance(value, str):
return None
return value.strip() or None
def _normalized_authorize_endpoint(url: str) -> str:
"""Compare authorize endpoints on scheme, host, and path only. The default port is elided and
the host is lowercased so ``https://IDP.example.com:443/authorize/`` and
@ -1120,12 +1135,15 @@ class MCPServerManager:
)
auth_type = server_config.get("auth_type", None)
manual_authorization_url = _blank_to_none(server_config.get("authorization_url"))
manual_token_url = _blank_to_none(server_config.get("token_url"))
manual_registration_url = _blank_to_none(server_config.get("registration_url"))
if server_url and (
auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES
or self._obo_needs_endpoint_discovery(
auth_type,
server_config.get("token_exchange_endpoint"),
server_config.get("token_url"),
manual_token_url,
)
):
mcp_oauth_metadata = await self._descovery_metadata(
@ -1138,7 +1156,7 @@ class MCPServerManager:
gated_oauth_metadata = (
_restrict_discovery_to_corroborated_authorization_server(
mcp_oauth_metadata,
server_config.get("authorization_url"),
manual_authorization_url,
server_name or server_id,
bool(server_config.get("dcr_bridge")),
)
@ -1152,13 +1170,11 @@ class MCPServerManager:
resolved_scopes = self._extract_scopes(server_config.get("scopes")) or (
gated_oauth_metadata.scopes if gated_oauth_metadata else None
)
resolved_authorization_url = server_config.get("authorization_url") or (
resolved_authorization_url = manual_authorization_url or (
gated_oauth_metadata.authorization_url if gated_oauth_metadata else None
)
resolved_token_url = server_config.get("token_url") or (
gated_oauth_metadata.token_url if gated_oauth_metadata else None
)
resolved_registration_url = server_config.get("registration_url") or (
resolved_token_url = manual_token_url or (gated_oauth_metadata.token_url if gated_oauth_metadata else None)
resolved_registration_url = manual_registration_url or (
gated_oauth_metadata.registration_url if gated_oauth_metadata else None
)
@ -1552,14 +1568,17 @@ class MCPServerManager:
auth_type = cast(MCPAuthType, mcp_server.auth_type)
server_url = mcp_server.url
has_all_upstream_oauth_fields = bool(mcp_server.authorization_url and mcp_server.token_url and scopes)
manual_authorization_url = _blank_to_none(mcp_server.authorization_url)
manual_token_url = _blank_to_none(mcp_server.token_url)
manual_registration_url = _blank_to_none(mcp_server.registration_url)
has_all_upstream_oauth_fields = bool(manual_authorization_url and manual_token_url and scopes)
needs_discovery = bool(server_url) and (
(auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES and not has_all_upstream_oauth_fields)
or self._obo_needs_endpoint_discovery(
auth_type,
mcp_server.token_exchange_endpoint
or (credentials_dict.get("token_exchange_endpoint") if credentials_dict else None),
mcp_server.token_url,
manual_token_url,
)
)
mcp_oauth_metadata = (
@ -1580,7 +1599,7 @@ class MCPServerManager:
gated_oauth_metadata = (
_restrict_discovery_to_corroborated_authorization_server(
mcp_oauth_metadata,
mcp_server.authorization_url,
manual_authorization_url,
mcp_server.server_id,
bool(getattr(mcp_server, "dcr_bridge", None)),
)
@ -1608,9 +1627,9 @@ class MCPServerManager:
client_secret=client_secret_value or getattr(mcp_server, "client_secret", None),
oauth2_flow=self._explicit_oauth2_flow(getattr(mcp_server, "oauth2_flow", None)),
scopes=resolved_scopes,
authorization_url=mcp_server.authorization_url or getattr(gated_oauth_metadata, "authorization_url", None),
token_url=mcp_server.token_url or getattr(gated_oauth_metadata, "token_url", None),
registration_url=mcp_server.registration_url or getattr(gated_oauth_metadata, "registration_url", None),
authorization_url=manual_authorization_url or getattr(gated_oauth_metadata, "authorization_url", None),
token_url=manual_token_url or getattr(gated_oauth_metadata, "token_url", None),
registration_url=manual_registration_url or getattr(gated_oauth_metadata, "registration_url", None),
token_endpoint_auth_method=(
credentials_dict.get("token_endpoint_auth_method") if credentials_dict else None
),
@ -1661,14 +1680,14 @@ class MCPServerManager:
await self._persist_discovered_obo_token_url(
server_id=mcp_server.server_id,
auth_type=auth_type,
existing_token_url=mcp_server.token_url,
existing_token_url=manual_token_url,
discovered_token_url=new_server.token_url,
)
await self._persist_discovered_oauth_endpoints(
server_id=mcp_server.server_id,
auth_type=auth_type,
existing_authorization_url=mcp_server.authorization_url,
existing_token_url=mcp_server.token_url,
existing_authorization_url=manual_authorization_url,
existing_token_url=manual_token_url,
existing_scopes=scopes,
metadata=gated_oauth_metadata,
)

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

@ -409,11 +409,13 @@ class TestMCPServerManager:
assert server.scopes == ["read"]
@pytest.mark.asyncio
async def test_load_servers_from_config_blank_authorization_url_is_not_a_pin(self):
"""A blank (empty-string) authorization_url is not a trust anchor, so discovery backfills
the whole set — authorize endpoint, token_url, and its resource-preferred scopes — from the
same chain, exactly as if the field had been omitted. The corroboration gate must treat
empty-string as unpinned so it does not strand the token_url the merge still fills."""
@pytest.mark.parametrize("blank_authorization_url", ["", " "])
async def test_load_servers_from_config_blank_authorization_url_is_not_a_pin(self, blank_authorization_url):
"""A blank authorization_url — empty or whitespace-only — is not a trust anchor, so discovery
backfills the whole set (authorize endpoint, token_url, and its resource-preferred scopes)
from the same chain, exactly as if the field had been omitted. The merge and the corroboration
gate must agree that blank means unpinned; a whitespace value that the merge kept for redirects
while the gate treated as unpinned would strand a broken half-discovered config."""
manager = MCPServerManager()
metadata = MCPOAuthMetadata(
@ -424,7 +426,7 @@ class TestMCPServerManager:
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
authorization_url="",
authorization_url=blank_authorization_url,
token_url=None,
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)):
@ -1165,6 +1167,38 @@ class TestMCPServerManager:
assert built.token_url == "https://idp.example.com/token"
assert built.scopes == ["read", "write"]
@pytest.mark.asyncio
async def test_build_from_table_whitespace_authorization_url_is_not_a_pin(self):
"""A whitespace-only authorization_url on the row must not be kept for redirects while the
gate treats it as unpinned. It is normalized to unpinned everywhere, so the built server
takes the discovered authorize endpoint, token_url, and scopes as one consistent group
rather than serving the whitespace value with half-discovered fields."""
manager = MCPServerManager()
row = LiteLLM_MCPServerTable(
server_id="whitespace-auth-url",
alias="whitespace_auth_url",
description="whitespace authorization_url is not a pin",
url="https://up.example.com/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
authorization_url=" ",
created_at=datetime.now(),
updated_at=datetime.now(),
)
metadata = MCPOAuthMetadata(
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read"],
authorization_server_scopes=["read"],
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
assert built.authorization_url == "https://idp.example.com/authorize"
assert built.token_url == "https://idp.example.com/token"
assert built.scopes == ["read"]
@pytest.mark.asyncio
async def test_build_from_table_fills_endpoints_when_metadata_corroborates_manual_authorization_url(self):
"""A discovered token_url is only trusted next to a manual authorization_url when the same