diff --git a/litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py b/litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py index 2f5973ca371..de275c94c12 100644 --- a/litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/byok_oauth_endpoints.py @@ -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( """ -# --------------------------------------------------------------------------- -# 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) # --------------------------------------------------------------------------- diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_byok_oauth_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_byok_oauth_endpoints.py index 89992c510f5..de8eb7484ec 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_byok_oauth_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_byok_oauth_endpoints.py @@ -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 # --------------------------------------------------------------------------- diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index 85d5d6ba466..a00514fcba7 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -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."""