mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(mcp/oauth): rediscover interactive endpoints when authorize is empty
Interactive PKCE should resolve authorize/token from upstream OAuth discovery (RFC 9728/8414) without admins pasting URLs. A transient discovery miss on oauth/session was cached and /authorize hard-400'd forever. Re-run discovery before failing authorize/register, retry on temp session cache, surface discovered fields on the session response, stamp oauth2_flow on temp records, and cover the Figma-style GET 405 without WWW-Authenticate path
This commit is contained in:
parent
cbeaf86c8d
commit
58f5998f0f
6 changed files with 302 additions and 0 deletions
|
|
@ -755,6 +755,12 @@ async def authorize_with_server(
|
|||
ephemeral_dcr_client: "EphemeralDcrClient | None" = None,
|
||||
):
|
||||
_raise_if_not_oauth2(mcp_server)
|
||||
if mcp_server.authorization_url is None:
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
|
||||
await global_mcp_server_manager.ensure_oauth_endpoints_resolved(mcp_server)
|
||||
if mcp_server.authorization_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
@ -1552,6 +1558,12 @@ async def resolve_ephemeral_dcr_client(
|
|||
usable to generate orphan IdP clients)."""
|
||||
if not (mcp_server.is_true_passthrough or (mcp_server.is_oauth_delegate and not mcp_server.is_dcr_bridge)):
|
||||
return None
|
||||
if mcp_server.authorization_url is None:
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
|
||||
await global_mcp_server_manager.ensure_oauth_endpoints_resolved(mcp_server)
|
||||
if mcp_server.authorization_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
@ -1596,6 +1608,12 @@ async def register_client_with_server(
|
|||
):
|
||||
return dummy_return
|
||||
|
||||
if mcp_server.authorization_url is None or mcp_server.registration_url is None:
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( # noqa: PLC0415
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
|
||||
await global_mcp_server_manager.ensure_oauth_endpoints_resolved(mcp_server)
|
||||
if mcp_server.authorization_url is None:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
|
|
|||
|
|
@ -3797,6 +3797,15 @@ class MCPServerManager:
|
|||
),
|
||||
)
|
||||
if metadata is None and resource_scopes:
|
||||
verbose_logger.warning(
|
||||
"MCP OAuth discovery for %s found resource scopes %s but no authorization/token "
|
||||
"endpoints. Attempts: %s. Interactive OAuth will fail until discovery resolves "
|
||||
"the authorization server (RFC 9728 / RFC 8414), or Authorization URL and Token "
|
||||
"URL are set manually",
|
||||
origin,
|
||||
resource_scopes,
|
||||
"; ".join(attempts) if attempts else "none recorded",
|
||||
)
|
||||
return MCPOAuthMetadata(scopes=resource_scopes), attempts
|
||||
if metadata is not None and resource_scopes:
|
||||
metadata.scopes = resource_scopes
|
||||
|
|
@ -3882,12 +3891,72 @@ class MCPServerManager:
|
|||
|
||||
preferred_scopes = scopes or resource_scopes
|
||||
if metadata is None and preferred_scopes:
|
||||
verbose_logger.warning(
|
||||
"MCP OAuth discovery for %s found resource scopes %s but no authorization/token "
|
||||
"endpoints. Attempts: %s. Interactive OAuth will fail until discovery resolves "
|
||||
"the authorization server (RFC 9728 / RFC 8414), or Authorization URL and Token "
|
||||
"URL are set manually",
|
||||
origin,
|
||||
preferred_scopes,
|
||||
"; ".join(attempts) if attempts else "none recorded",
|
||||
)
|
||||
return MCPOAuthMetadata(scopes=preferred_scopes), attempts
|
||||
if metadata is not None and preferred_scopes:
|
||||
metadata.scopes = preferred_scopes
|
||||
|
||||
return metadata, attempts
|
||||
|
||||
def _apply_discovered_oauth_metadata(self, server: MCPServer, metadata: MCPOAuthMetadata) -> None:
|
||||
"""Fill blank OAuth fields on ``server`` from discovery without overwriting admin pins."""
|
||||
if not server.authorization_url and metadata.authorization_url:
|
||||
server.authorization_url = metadata.authorization_url
|
||||
if not server.token_url and metadata.token_url:
|
||||
server.token_url = metadata.token_url
|
||||
if not server.registration_url and metadata.registration_url:
|
||||
server.registration_url = metadata.registration_url
|
||||
if not server.scopes and metadata.scopes:
|
||||
server.scopes = list(metadata.scopes)
|
||||
if (
|
||||
not server.issuer
|
||||
and metadata.discovered_issuer
|
||||
and not metadata.from_origin_fallback
|
||||
):
|
||||
server.issuer = metadata.discovered_issuer
|
||||
|
||||
async def ensure_oauth_endpoints_resolved(self, server: MCPServer) -> bool:
|
||||
"""Re-run OAuth discovery when interactive endpoints are missing on a live server object.
|
||||
|
||||
Discovery normally runs only at build/cache time (``build_mcp_server_from_table`` /
|
||||
``oauth/session``). A transient upstream failure leaves ``authorization_url`` /
|
||||
``token_url`` unset, and ``/authorize`` would otherwise hard-400 for the life of that
|
||||
cache entry even though the upstream advertises valid well-known metadata (e.g. Figma
|
||||
MCP returns HTTP 405 without ``WWW-Authenticate`` on GET, then resolves via
|
||||
``/.well-known/oauth-protected-resource``). Call this before failing authorize/register
|
||||
so a live rediscovery can recover without the admin pasting URLs.
|
||||
"""
|
||||
if not _oauth_endpoints_unresolved(server):
|
||||
return True
|
||||
if not server.url:
|
||||
return False
|
||||
if server.auth_type not in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES:
|
||||
return False
|
||||
|
||||
if server.issuer_is_anchored and server.issuer:
|
||||
metadata = await self._fetch_issuer_anchored_oauth_metadata(server.issuer, server.url)
|
||||
else:
|
||||
metadata = await self._descovery_metadata(
|
||||
server.url,
|
||||
allow_origin_fallback=True,
|
||||
warn_when_no_metadata=True,
|
||||
)
|
||||
if metadata is None:
|
||||
self._record_oauth_discovery_outcome(server)
|
||||
return False
|
||||
|
||||
self._apply_discovered_oauth_metadata(server, metadata)
|
||||
self._record_oauth_discovery_outcome(server)
|
||||
return not _oauth_endpoints_unresolved(server)
|
||||
|
||||
def _parse_www_authenticate_header(self, header_value: str | None) -> tuple[str | None, list[str] | None]:
|
||||
if not header_value:
|
||||
return None, None
|
||||
|
|
|
|||
|
|
@ -741,6 +741,7 @@ if MCP_AVAILABLE:
|
|||
authorization_url=payload.authorization_url,
|
||||
token_url=payload.token_url,
|
||||
registration_url=payload.registration_url,
|
||||
oauth2_flow=payload.oauth2_flow,
|
||||
allow_all_keys=payload.allow_all_keys,
|
||||
available_on_public_internet=payload.available_on_public_internet,
|
||||
timeout=payload.timeout,
|
||||
|
|
@ -1550,6 +1551,23 @@ if MCP_AVAILABLE:
|
|||
temp_record,
|
||||
credentials_are_encrypted=False,
|
||||
)
|
||||
# Interactive OAuth needs authorize/token endpoints. Discovery can fail once
|
||||
# (transient upstream / 405 without WWW-Authenticate then well-known race); retry
|
||||
# before caching so /authorize does not inherit a permanently empty session.
|
||||
await global_mcp_server_manager.ensure_oauth_endpoints_resolved(temporary_server)
|
||||
# Surface discovered endpoints on the response so the UI can show what was resolved
|
||||
# without requiring the admin to paste Authorization URL / Token URL by hand.
|
||||
temp_record.authorization_url = temporary_server.authorization_url
|
||||
temp_record.token_url = temporary_server.token_url
|
||||
temp_record.registration_url = temporary_server.registration_url
|
||||
temp_record.issuer = temporary_server.issuer
|
||||
if temporary_server.scopes and isinstance(temp_record.credentials, dict):
|
||||
temp_record.credentials = {
|
||||
**temp_record.credentials,
|
||||
"scopes": list(temporary_server.scopes),
|
||||
}
|
||||
elif temporary_server.scopes and temp_record.credentials is None:
|
||||
temp_record.credentials = {"scopes": list(temporary_server.scopes)}
|
||||
_cache_temporary_mcp_server(
|
||||
temporary_server,
|
||||
ttl_seconds=TEMPORARY_MCP_SERVER_TTL_SECONDS,
|
||||
|
|
|
|||
|
|
@ -8265,6 +8265,79 @@ async def test_authorize_wall_names_the_fix_for_urlless_servers():
|
|||
assert "Issuer" in detail_text
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_authorize_rediscovers_endpoints_when_authorization_url_missing():
|
||||
"""Interactive PKCE must not require the admin to paste Authorization URL when the upstream
|
||||
advertises it via OAuth discovery. If the session/cache build left authorization_url empty,
|
||||
/authorize re-runs discovery once and proceeds with the discovered redirect."""
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
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="rediscover-authorize",
|
||||
name="figma_like",
|
||||
server_name="figma_like",
|
||||
url="https://mcp.figma.example/mcp",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
oauth2_flow="authorization_code",
|
||||
authorization_url=None,
|
||||
token_url=None,
|
||||
client_id="figma-client",
|
||||
)
|
||||
mock_request = MagicMock()
|
||||
mock_request.base_url = "https://litellm.example.com/"
|
||||
mock_request.headers = {}
|
||||
mock_request.url = MagicMock()
|
||||
mock_request.url.scheme = "https"
|
||||
mock_request.url.netloc = "litellm.example.com"
|
||||
mock_request.cookies = {}
|
||||
|
||||
async def _fake_ensure(s):
|
||||
s.authorization_url = "https://www.figma.example/oauth/mcp"
|
||||
s.token_url = "https://api.figma.example/v1/oauth/token"
|
||||
s.scopes = ["mcp:connect"]
|
||||
return True
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager.ensure_oauth_endpoints_resolved",
|
||||
new=AsyncMock(side_effect=_fake_ensure),
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.encrypt_value_helper",
|
||||
return_value="mocked_encrypted_state",
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.validate_trusted_redirect_uri",
|
||||
return_value=None,
|
||||
),
|
||||
):
|
||||
response = await authorize_with_server(
|
||||
request=mock_request,
|
||||
mcp_server=server,
|
||||
client_id="figma-client",
|
||||
redirect_uri="http://127.0.0.1:60108/callback",
|
||||
state="s",
|
||||
code_challenge="chal",
|
||||
code_challenge_method="S256",
|
||||
scope="mcp:connect",
|
||||
)
|
||||
|
||||
assert response.status_code in (302, 307)
|
||||
location = response.headers["location"]
|
||||
assert location.startswith("https://www.figma.example/oauth/mcp")
|
||||
query = parse_qs(urlparse(location).query)
|
||||
assert query["client_id"] == ["figma-client"]
|
||||
assert query["scope"] == ["mcp:connect"]
|
||||
assert server.authorization_url == "https://www.figma.example/oauth/mcp"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_token_wall_names_the_fix_for_urlless_servers():
|
||||
"""The /token wall is the second stop on the same misconfiguration (LIT-4629): after an admin
|
||||
|
|
|
|||
|
|
@ -1431,6 +1431,119 @@ class TestMCPServerManager:
|
|||
assert built.token_url == "https://idp.example.com/token"
|
||||
assert built.scopes == ["read"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_oauth_endpoints_resolved_rediscovers_when_authorization_url_missing(self):
|
||||
"""A temp oauth/session or registry entry that was built while discovery failed must recover
|
||||
on the next ensure call without the admin pasting Authorization URL / Token URL. This is the
|
||||
recovery path used by /authorize for Figma-style upstreams (GET 405, well-known still works)."""
|
||||
manager = MCPServerManager()
|
||||
server = MCPServer(
|
||||
server_id="ensure-rediscover-1",
|
||||
name="figma_like",
|
||||
url="https://mcp.example.com/mcp",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
oauth2_flow="authorization_code",
|
||||
authorization_url=None,
|
||||
token_url=None,
|
||||
)
|
||||
metadata = MCPOAuthMetadata(
|
||||
authorization_url="https://www.example.com/oauth/mcp",
|
||||
token_url="https://api.example.com/v1/oauth/token",
|
||||
registration_url="https://api.example.com/v1/oauth/mcp/register",
|
||||
scopes=["mcp:connect"],
|
||||
discovered_issuer="https://api.example.com",
|
||||
)
|
||||
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)) as mock_discovery:
|
||||
ok = await manager.ensure_oauth_endpoints_resolved(server)
|
||||
|
||||
mock_discovery.assert_awaited_once()
|
||||
assert ok is True
|
||||
assert server.authorization_url == "https://www.example.com/oauth/mcp"
|
||||
assert server.token_url == "https://api.example.com/v1/oauth/token"
|
||||
assert server.registration_url == "https://api.example.com/v1/oauth/mcp/register"
|
||||
assert server.scopes == ["mcp:connect"]
|
||||
assert server.issuer == "https://api.example.com"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_oauth_endpoints_resolved_noops_when_endpoints_present(self):
|
||||
manager = MCPServerManager()
|
||||
server = MCPServer(
|
||||
server_id="ensure-noop-1",
|
||||
name="already_resolved",
|
||||
url="https://mcp.example.com/mcp",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=MCPAuth.oauth2,
|
||||
authorization_url="https://idp.example.com/authorize",
|
||||
token_url="https://idp.example.com/token",
|
||||
)
|
||||
with patch.object(manager, "_descovery_metadata", new=AsyncMock()) as mock_discovery:
|
||||
ok = await manager.ensure_oauth_endpoints_resolved(server)
|
||||
|
||||
mock_discovery.assert_not_awaited()
|
||||
assert ok is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discover_metadata_figma_style_405_without_www_authenticate_uses_well_known(self):
|
||||
"""Figma remote MCP answers GET /mcp with HTTP 405 and no WWW-Authenticate. Discovery must
|
||||
still resolve authorize/token/registration via protected-resource + authorization-server
|
||||
well-known documents so interactive PKCE does not require manual endpoint paste."""
|
||||
manager = MCPServerManager()
|
||||
resource_payload = {
|
||||
"resource": "https://mcp.figma.example/mcp",
|
||||
"authorization_servers": ["https://api.figma.example"],
|
||||
"scopes_supported": ["mcp:connect"],
|
||||
}
|
||||
as_payload = {
|
||||
"issuer": "https://api.figma.example",
|
||||
"authorization_endpoint": "https://www.figma.example/oauth/mcp",
|
||||
"token_endpoint": "https://api.figma.example/v1/oauth/token",
|
||||
"registration_endpoint": "https://api.figma.example/v1/oauth/mcp/register",
|
||||
"scopes_supported": ["mcp:connect"],
|
||||
}
|
||||
|
||||
def _response(url: str, status: int, payload: dict | None = None) -> httpx.Response:
|
||||
request = httpx.Request("GET", url)
|
||||
return httpx.Response(
|
||||
status,
|
||||
request=request,
|
||||
json=payload,
|
||||
headers={"content-type": "application/json"} if payload is not None else {},
|
||||
)
|
||||
|
||||
async def fake_client_get(url: str, **kwargs):
|
||||
if url.rstrip("/").endswith("/mcp") and "well-known" not in url:
|
||||
return _response(url, 405)
|
||||
raise AssertionError(f"unexpected client.get url: {url}")
|
||||
|
||||
async def fake_fetch_discovery(url: str, server_url: str):
|
||||
if "oauth-protected-resource" in url:
|
||||
return _response(url, 200, resource_payload)
|
||||
if "oauth-authorization-server" in url or url.rstrip("/") == "https://api.figma.example":
|
||||
return _response(url, 200, as_payload)
|
||||
return _response(url, 404)
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.get = AsyncMock(side_effect=fake_client_get)
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.mcp_server_manager.get_async_httpx_client",
|
||||
return_value=mock_client,
|
||||
),
|
||||
patch.object(manager, "_fetch_oauth_discovery_url", new=AsyncMock(side_effect=fake_fetch_discovery)),
|
||||
):
|
||||
metadata = await manager._descovery_metadata(
|
||||
"https://mcp.figma.example/mcp",
|
||||
warn_when_no_metadata=True,
|
||||
)
|
||||
|
||||
assert metadata is not None
|
||||
assert metadata.authorization_url == "https://www.figma.example/oauth/mcp"
|
||||
assert metadata.token_url == "https://api.figma.example/v1/oauth/token"
|
||||
assert metadata.registration_url == "https://api.figma.example/v1/oauth/mcp/register"
|
||||
assert metadata.scopes == ["mcp:connect"]
|
||||
assert metadata.discovered_issuer == "https://api.figma.example"
|
||||
|
||||
@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
|
||||
|
|
|
|||
|
|
@ -192,6 +192,17 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
url,
|
||||
transport: transport === TRANSPORT.OPENAPI ? "http" : transport,
|
||||
auth_type: isClientForwardedTokenMode(values.auth_type) ? values.auth_type : AUTH_TYPE.OAUTH2,
|
||||
// Interactive (PKCE) is the create-form default for oauth2; stamp it so temp-session
|
||||
// discovery/authorize treat the server as authorization_code rather than inferring M2M from
|
||||
// a prefilled client_id/secret + token_url shape. Client-forwarded modes leave it unset.
|
||||
...(isClientForwardedTokenMode(values.auth_type)
|
||||
? {}
|
||||
: {
|
||||
oauth2_flow:
|
||||
values.oauth_flow_type === OAUTH_FLOW.M2M
|
||||
? MCP_OAUTH2_FLOW_M2M
|
||||
: MCP_OAUTH2_FLOW_INTERACTIVE,
|
||||
}),
|
||||
// Mirror getCredentials: merge the ref-held DCR client for oauth2 so a re-authorize reuses the
|
||||
// registered client (useMcpOAuthFlow keys reuse off credentials.client_id) instead of re-DCRing;
|
||||
// the client-forwarded modes carry only the declared app.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue