mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-26 01:12:21 +00:00
fix(mcp): include registration_endpoint in root /mcp OAuth discovery metadata
This commit is contained in:
parent
f318ef03bd
commit
fb14ee7fc8
3 changed files with 76 additions and 62 deletions
|
|
@ -6,11 +6,14 @@ LiteLLM runs a minimal OAuth 2.1 authorization code flow. The "authorization pa
|
|||
just a form that asks the user for their API key — not a full identity-provider OAuth.
|
||||
|
||||
Endpoints implemented here:
|
||||
GET /.well-known/oauth-authorization-server — OAuth authorization server metadata
|
||||
GET /.well-known/oauth-protected-resource — OAuth protected resource metadata
|
||||
GET /v1/mcp/oauth/authorize — Shows HTML form to collect the API key
|
||||
POST /v1/mcp/oauth/authorize — Stores temp auth code and redirects
|
||||
POST /v1/mcp/oauth/token — Exchanges code for a bearer JWT token
|
||||
|
||||
OAuth metadata discovery (`/.well-known/oauth-authorization-server` and
|
||||
`/.well-known/oauth-protected-resource`) is owned by `discoverable_endpoints.py`,
|
||||
which resolves a single OAuth2 MCP server for the root path and always emits
|
||||
`registration_endpoint` for dynamic client registration.
|
||||
"""
|
||||
|
||||
import base64
|
||||
|
|
@ -27,9 +30,6 @@ from fastapi.responses import HTMLResponse, JSONResponse, RedirectResponse
|
|||
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.proxy._experimental.mcp_server.db import store_user_credential
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
get_request_base_url,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.oauth_utils import (
|
||||
TOKEN_NO_CACHE_HEADERS,
|
||||
validate_loopback_redirect_uri,
|
||||
|
|
@ -592,39 +592,6 @@ def _build_authorize_html(
|
|||
</html>"""
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# OAuth metadata discovery endpoints
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@router.get("/.well-known/oauth-authorization-server", include_in_schema=False)
|
||||
async def oauth_authorization_server_metadata(request: Request) -> JSONResponse:
|
||||
"""RFC 8414 Authorization Server Metadata for the BYOK OAuth flow."""
|
||||
base_url = get_request_base_url(request)
|
||||
return JSONResponse(
|
||||
{
|
||||
"issuer": base_url,
|
||||
"authorization_endpoint": f"{base_url}/v1/mcp/oauth/authorize",
|
||||
"token_endpoint": f"{base_url}/v1/mcp/oauth/token",
|
||||
"response_types_supported": ["code"],
|
||||
"grant_types_supported": ["authorization_code"],
|
||||
"code_challenge_methods_supported": ["S256"],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@router.get("/.well-known/oauth-protected-resource", include_in_schema=False)
|
||||
async def oauth_protected_resource_metadata(request: Request) -> JSONResponse:
|
||||
"""RFC 9728 Protected Resource Metadata pointing back at this server."""
|
||||
base_url = get_request_base_url(request)
|
||||
return JSONResponse(
|
||||
{
|
||||
"resource": base_url,
|
||||
"authorization_servers": [base_url],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Authorization endpoint — GET (show form) and POST (process form)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -92,30 +92,6 @@ def unauthenticated_client():
|
|||
yield TestClient(_test_app, raise_server_exceptions=False)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# OAuth metadata endpoints
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_oauth_authorization_server_metadata(client):
|
||||
resp = client.get("/.well-known/oauth-authorization-server")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert "issuer" in data
|
||||
assert data["authorization_endpoint"].endswith("/v1/mcp/oauth/authorize")
|
||||
assert data["token_endpoint"].endswith("/v1/mcp/oauth/token")
|
||||
assert "S256" in data["code_challenge_methods_supported"]
|
||||
|
||||
|
||||
def test_oauth_protected_resource_metadata(client):
|
||||
resp = client.get("/.well-known/oauth-protected-resource")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert "resource" in data
|
||||
assert "authorization_servers" in data
|
||||
assert len(data["authorization_servers"]) == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Authorization GET endpoint
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -1770,6 +1770,77 @@ async def test_discovery_root_includes_server_name_prefix():
|
|||
global_mcp_server_manager.registry.clear()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_root_well_known_handlers_not_shadowed_by_byok_router():
|
||||
"""Regression: when both BYOK and discoverable routers are mounted (as in
|
||||
production via _lazy_features.py), the discoverable handler must own the
|
||||
root /.well-known/oauth-authorization-server and /.well-known/oauth-protected-resource
|
||||
paths so dynamic client registration metadata is advertised.
|
||||
|
||||
Bug: prior to the fix, byok_oauth_endpoints.py declared duplicate handlers at
|
||||
these root paths and was registered first via _lazy_features.py — so its
|
||||
trimmed payload (no `registration_endpoint`) shadowed the correct one and
|
||||
broke MCP client OAuth at the unified /mcp URL.
|
||||
"""
|
||||
try:
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.byok_oauth_endpoints import (
|
||||
router as byok_router,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||
router as discoverable_router,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
global_mcp_server_manager,
|
||||
)
|
||||
except ImportError:
|
||||
pytest.skip("MCP routers not available")
|
||||
|
||||
global_mcp_server_manager.registry.clear()
|
||||
oauth2_server = _create_oauth2_server()
|
||||
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
|
||||
|
||||
app = FastAPI()
|
||||
# Mount in the same order as _lazy_features.py: BYOK first, discoverable second.
|
||||
# If a future change reintroduces a shadowing handler in BYOK, this ordering
|
||||
# is what would mask it in production — so we reproduce it here.
|
||||
app.include_router(byok_router)
|
||||
app.include_router(discoverable_router)
|
||||
client = TestClient(app)
|
||||
|
||||
try:
|
||||
# /.well-known/oauth-authorization-server — must include registration_endpoint
|
||||
resp = client.get("/.well-known/oauth-authorization-server")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert "registration_endpoint" in data, (
|
||||
"Root oauth-authorization-server response is missing "
|
||||
"registration_endpoint — MCP clients will fail dynamic client "
|
||||
"registration. Likely a duplicate handler shadowing "
|
||||
"discoverable_endpoints.py."
|
||||
)
|
||||
# Discoverable's authorize/token endpoints — NOT BYOK's API-key-form ones.
|
||||
assert not data["authorization_endpoint"].endswith(
|
||||
"/v1/mcp/oauth/authorize"
|
||||
), "BYOK metadata is shadowing discoverable at the root path."
|
||||
assert not data["token_endpoint"].endswith(
|
||||
"/v1/mcp/oauth/token"
|
||||
), "BYOK metadata is shadowing discoverable at the root path."
|
||||
|
||||
# /.well-known/oauth-protected-resource — resource must point at /mcp.
|
||||
resp = client.get("/.well-known/oauth-protected-resource")
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["resource"].endswith("/mcp"), (
|
||||
"BYOK protected-resource handler is shadowing discoverable at the "
|
||||
"root path (resource should be base_url/mcp, not base_url)."
|
||||
)
|
||||
finally:
|
||||
global_mcp_server_manager.registry.clear()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discovery_root_does_not_expose_private_server_for_external_client():
|
||||
"""Root discovery must use caller visibility before adding server-specific metadata."""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue