diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index 4bac65125b4..7e6de474b0b 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -179,16 +179,7 @@ def _gateway_dcr_challenge_target( mcp_servers: list[str] | None, client_ip: str | None, ) -> str | None: - """The single path-named server this request targets, iff it resolves to a - gateway-managed oauth2 server — the one per-server shape the gateway's own keyless - DCR flow serves end to end, so the 401 challenge may advertise the per-server - protected-resource metadata (whose ``authorization_servers`` names the gateway). - - Multi-server CSV paths, header/path mismatches, unknown names, and every - client-forwarded or delegated mode return ``None``: those cells keep their existing - challenge (or absence of one), and a challenge is never emitted for a name the - public discovery routes would 404, so this reveals exactly the server set the - per-server protected-resource metadata already reveals.""" + """Resolve a single path target whose sign-in metadata advertises the gateway.""" from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( global_mcp_server_manager, ) @@ -217,7 +208,7 @@ def _is_gateway_dcr_challenge_scope( the caller is not a cold-start DCR client), on the scopes the gateway's keyless flow serves: the aggregate ``/mcp`` endpoint, an ``x-mcp-servers``-scoped request (the resource the client configured is still ``/mcp``), or a per-server path whose - single target is a gateway-managed oauth2 server. Every other named target keeps + single target advertises gateway-owned sign-in. Every other named target keeps its existing behavior, failing closed to the original admission error.""" if not _is_litellm_auth_admission_error(exc): return False @@ -236,7 +227,7 @@ def _gateway_dcr_challenge( ) -> HTTPException: """The RFC 9728 challenge pointing the client at the protected-resource metadata matching the scope it requested: the per-server document (same URL spelling the - request arrived on) when the single target is a gateway-managed oauth2 server, + request arrived on) when the single target advertises gateway-owned sign-in, else the gateway's aggregate document. Either way the client discovers the gateway as its authorization server and starts the same sign-in flow. diff --git a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py index cab4b6c161a..f18acb7d88d 100644 --- a/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py +++ b/litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py @@ -2310,8 +2310,7 @@ async def _build_oauth_protected_resource_response( it. Only the legacy ``is_oauth_passthrough`` opt-in rewrites ``resource`` to the gateway's own URL so clients present the bearer token back to the gateway. - An explicitly named gateway-managed oauth2 server (interactive with - gateway-vaulted per-user tokens, or M2M) advertises the gateway's own + An explicitly named server with gateway-owned sign-in advertises the gateway's own authorization server (``{base}/mcp``): a keyless DCR client that configured the per-server URL completes the same sign-in flow the aggregate ``/mcp`` endpoint supports and is admitted with a gateway session bearer. The per-server relay @@ -2401,11 +2400,6 @@ async def _build_oauth_protected_resource_response( if obo_response is not None: return obo_response - # An OBO server with no configured issuer falls through to the gateway default so discovery still - # returns metadata; every other non-oauth2 named server 404s to avoid enumeration. - if mcp_server is None or mcp_server.auth_type != MCPAuth.oauth2_token_exchange: - _raise_unless_oauth2_discovery_server(mcp_server, mcp_server_name, "not an OAuth-protected resource") - if explicitly_named and mcp_server is not None and mcp_server.advertises_gateway_authorization_server: return { "authorization_servers": [f"{request_base_url}/mcp"], @@ -2413,6 +2407,9 @@ async def _build_oauth_protected_resource_response( "scopes_supported": (mcp_server.scopes if mcp_server.scopes else []), } + if mcp_server is None or mcp_server.auth_type != MCPAuth.oauth2_token_exchange: + _raise_unless_oauth2_discovery_server(mcp_server, mcp_server_name, "not an OAuth-protected resource") + return { "authorization_servers": [ (f"{request_base_url}/{mcp_server_name}" if mcp_server_name else f"{request_base_url}") diff --git a/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py b/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py index 3d94fa345d0..f4889008e94 100644 --- a/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py +++ b/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py @@ -411,7 +411,7 @@ def relative_request_url(request: Request) -> str: def resolve_scoped_resource_server(request: Request, resource: str | None) -> MCPServer | None: - """Resolve an RFC 8707 ``resource`` value to the single gateway-managed oauth2 server it + """Resolve an RFC 8707 ``resource`` value to the single gateway-owned server it names, or ``None`` for every other shape: absent, the aggregate resource, a foreign host, an unparseable value, a multi-server path, an unknown name, or any server mode the keyless gateway flow does not serve (whose protected-resource metadata never directs a @@ -443,7 +443,7 @@ def resolve_scoped_resource_server(request: Request, resource: str | None) -> MC if len(names) != 1: return None server: Final = global_mcp_server_manager.get_mcp_server_by_name(names[0]) - if server is None or not server.is_gateway_managed_oauth2: + if server is None or not (server.is_gateway_managed_oauth2 or server.advertises_gateway_authorization_server): return None return server @@ -729,11 +729,15 @@ async def _flow_target( server: Final = global_mcp_server_manager.get_mcp_server_by_id(flow.resource_server_id) if ( server is None - or not server.is_gateway_managed_oauth2 + or not (server.is_gateway_managed_oauth2 or server.advertises_gateway_authorization_server) or not await lookup_server_reachability(flow.user_id, server.server_id) ): return "stale", None - state: Final = "m2m" if MCPServerManager.effective_oauth2_flow(server) == "client_credentials" else "interactive" + state: Final = ( + "interactive" + if server.is_gateway_managed_oauth2 and MCPServerManager.effective_oauth2_flow(server) != "client_credentials" + else "m2m" + ) return state, server diff --git a/litellm/types/mcp_server/mcp_server_manager.py b/litellm/types/mcp_server/mcp_server_manager.py index 84ffd50eea1..ab2e8a70754 100644 --- a/litellm/types/mcp_server/mcp_server_manager.py +++ b/litellm/types/mcp_server/mcp_server_manager.py @@ -250,7 +250,23 @@ class MCPServer(BaseModel): @property def advertises_gateway_authorization_server(self) -> bool: """Whether named discovery should advertise the aggregate gateway authorization server.""" - return self.is_gateway_managed_oauth2 and not self.uses_per_server_oauth_relay + if self.auth_type == MCPAuth.oauth2: + return self.is_gateway_managed_oauth2 and not self.uses_per_server_oauth_relay + if self.auth_type not in ( + None, + MCPAuth.none, + MCPAuth.api_key, + MCPAuth.bearer_token, + MCPAuth.basic, + MCPAuth.authorization, + MCPAuth.token, + MCPAuth.aws_sigv4, + ): + return False + return not any( + header.lower() in ("authorization", "x-api-key", "api-key", "apikey") + for header in (self.extra_headers or ()) + ) @property def is_true_passthrough(self) -> bool: diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py index 20f4719e4bf..e6c8d4ee039 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py @@ -7193,14 +7193,13 @@ class TestAggregateGatewayDcrChallenge: www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"] assert www_authenticate == f"Bearer {self._EXPECTED_RESOURCE_METADATA}" - async def test_per_server_challenge_for_gateway_managed_oauth2(self): - """Anonymous request to a per-server path whose single target is a gateway-managed - oauth2 server: 401 plus the RFC 9728 challenge advertising the PER-SERVER - protected-resource metadata in the same URL spelling the request used, so a keyless - DCR client configured with either per-server spelling discovers the gateway as the - authorization server (LIT-4864). Covers interactive and M2M, which the gateway can - both serve end to end.""" - from litellm.types.mcp import MCPAuth + @pytest.mark.parametrize( + "auth_type", + (None, "none", "api_key", "bearer_token", "basic", "aws_sigv4", "authorization", "token", "oauth2"), + ) + @pytest.mark.parametrize("bearer_presented", (False, True)) + async def test_per_server_challenge_for_gateway_owned_auth(self, auth_type, bearer_presented): + """Gateway admission challenges are independent of upstream authentication.""" from litellm.types.mcp_server.mcp_server_manager import MCPServer server = MCPServer( @@ -7209,7 +7208,7 @@ class TestAggregateGatewayDcrChallenge: server_name="github", url="https://upstream.example/mcp", transport="http", - auth_type=MCPAuth.oauth2, + auth_type=auth_type, ) for path, expected_metadata_path in ( ("/mcp/github", "/.well-known/oauth-protected-resource/mcp/github"), @@ -7223,10 +7222,16 @@ class TestAggregateGatewayDcrChallenge: ): mock_mgr.get_mcp_server_by_name.return_value = server with pytest.raises(HTTPException) as exc_info: - await MCPRequestHandler.process_mcp_request(self._scope(path=path)) + await MCPRequestHandler.process_mcp_request( + self._scope( + path=path, + extra_headers=((b"authorization", b"Bearer invalid-key"),) if bearer_presented else (), + ) + ) assert exc_info.value.status_code == 401 www_authenticate = (exc_info.value.headers or {})["WWW-Authenticate"] - assert www_authenticate == f'Bearer resource_metadata="http://testserver{expected_metadata_path}"' + error = 'error="invalid_token", ' if bearer_presented else "" + assert www_authenticate == f'Bearer {error}resource_metadata="http://testserver{expected_metadata_path}"' async def test_per_server_challenge_keeps_spelling_under_server_root_path(self): """On a sub-path deployment the challenge must still advertise the spelling the client @@ -7303,10 +7308,7 @@ class TestAggregateGatewayDcrChallenge: ) def test_challenge_target_excludes_every_non_gateway_managed_mode(self): - """Unit pin of the challenge-target owner: only a resolved gateway-managed oauth2 - target (interactive or M2M) yields a per-server challenge; delegate-auth oauth2 - (whose keyless flow is upstream PKCE via the relay), every client-forwarded auth - type, OBO, api_key, unknown names, and CSV paths yield None (LIT-4864).""" + """Gateway challenges exclude unresolved, delegated, and client-forwarded targets.""" from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import ( _gateway_dcr_challenge_target, ) @@ -7333,7 +7335,11 @@ class TestAggregateGatewayDcrChallenge: (_server(MCPAuth.true_passthrough), None), (_server(MCPAuth.oauth_delegate), None), (_server(MCPAuth.oauth_delegate, dcr_bridge=True), None), - (_server(MCPAuth.api_key), None), + (_server(MCPAuth.api_key), "srv"), + (_server(MCPAuth.none, extra_headers=["Authorization"]), None), + (_server(None, extra_headers=["X-API-Key"]), None), + (_server(MCPAuth.none, extra_headers=["Authorization"], oauth_passthrough=True), None), + (_server(MCPAuth.oauth2_id_jag), None), (None, None), ] for resolved, expected in cases: 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 763200c3709..a7a65bfe466 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 @@ -7651,12 +7651,6 @@ async def test_token_endpoint_client_secret_basic_without_secret_returns_400(): assert exc_info.value.status_code == 400 -# ------------------------------------------------------------------- -# Non-oauth2 (auth_type=none, access-group gated) servers must not be -# driven through the gateway OAuth authorize/token/register/discovery -# flow, and must not be advertised as OAuth-protected in discovery docs. -# ------------------------------------------------------------------- - def _access_group_none_server(server_name="access_group_server"): """A non-oauth2, access-group gated MCP server: no client_id, no OAuth.""" @@ -7794,35 +7788,38 @@ async def test_register_client_rejects_non_oauth2_server(): @pytest.mark.asyncio -async def test_oauth_protected_resource_404_for_non_oauth2_server(): - """Discovery must not advertise a none-auth server as an OAuth-protected resource.""" - try: - from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( - _build_oauth_protected_resource_response, - ) - from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( - global_mcp_server_manager, - ) - except ImportError: - pytest.skip("MCP discoverable endpoints not available") +@pytest.mark.parametrize( + "auth_type", (None, "none", "api_key", "bearer_token", "basic", "aws_sigv4", "authorization", "token") +) +@pytest.mark.parametrize("use_standard_pattern", (False, True)) +async def test_oauth_protected_resource_for_gateway_owned_auth(auth_type, use_standard_pattern): + from starlette.requests import Request + from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( + _build_oauth_protected_resource_response, + ) + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + global_mcp_server_manager, + ) + + server = _access_group_none_server().model_copy(update={"auth_type": auth_type}) + request = Request( + {"type": "http", "scheme": "https", "path": "/", "headers": [(b"host", b"litellm.example.com")]} + ) global_mcp_server_manager.registry.clear() - server = _access_group_none_server() global_mcp_server_manager.registry[server.server_id] = server - - mock_request = MagicMock() - mock_request.base_url = "https://litellm.example.com/" - mock_request.headers = {} - try: - with pytest.raises(HTTPException) as exc_info: - await _build_oauth_protected_resource_response( - request=mock_request, - mcp_server_name="access_group_server", - use_standard_pattern=False, - ) - assert exc_info.value.status_code == 404 - assert "not an OAuth-protected resource" in str(exc_info.value.detail) + response = await _build_oauth_protected_resource_response( + request=request, + mcp_server_name="access_group_server", + use_standard_pattern=use_standard_pattern, + ) + resource_path = "/mcp/access_group_server" if use_standard_pattern else "/access_group_server/mcp" + assert response == { + "resource": f"https://litellm.example.com{resource_path}", + "authorization_servers": ["https://litellm.example.com/mcp"], + "scopes_supported": [], + } finally: global_mcp_server_manager.registry.clear() @@ -7914,9 +7911,7 @@ async def test_oauth_protected_resource_passthrough_none_auth_not_404(): @pytest.mark.asyncio async def test_oauth_protected_resource_404_for_unknown_server_name(): - """A discovery request for an unknown server name returns the same 404 as a non-oauth2 - server (not a 200 metadata doc with broken URLs), so the well-known paths cannot be used - to enumerate non-OAuth server names.""" + """Unknown server names must not produce metadata advertising nonexistent resources.""" try: from litellm.proxy._experimental.mcp_server.discoverable_endpoints import ( _build_oauth_protected_resource_response, diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py index 73a52a8d2e8..8aa4cfc5619 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py @@ -833,7 +833,7 @@ async def test_manual_delivery_page_renders_the_url_as_data_never_as_a_shell_com assert 'value="' in body -def _scoped_mcp_server(name="github", **kw): +def _scoped_mcp_server(name="github", auth_type="oauth2", **kw): from litellm.types.mcp import MCPAuth from litellm.types.mcp_server.mcp_server_manager import MCPServer @@ -844,7 +844,7 @@ def _scoped_mcp_server(name="github", **kw): alias=name, url="https://upstream.example/mcp", transport="http", - auth_type=MCPAuth.oauth2, + auth_type=MCPAuth(auth_type) if auth_type is not None else None, **kw, ) @@ -2044,3 +2044,45 @@ async def test_introspect_fails_closed_on_dead_user_and_503s_on_outage(): status, body = await _introspect(minted.token.get_secret_value(), master_key=None) assert (status, body["error"]) == (500, "server_error") + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "auth_type", [None, "none", "api_key", "bearer_token", "basic", "authorization", "token", "aws_sigv4"] +) +@pytest.mark.parametrize("resource", ["https://llm.example.com/mcp/github", "https://llm.example.com/github/mcp"]) +async def test_gateway_owned_resource_stays_scoped_through_consent_and_refresh(auth_type, resource): + from unittest.mock import patch + + client_id = (await _register([REDIRECT_URI]))["client_id"] + server = _scoped_mcp_server(auth_type=auth_type) + vendor = _VendorCredential("absent") + with patch(_MANAGER_PATCH) as manager: + manager.get_mcp_server_by_name.return_value = server + response = _scoped_authorize(client_id, resource) + described = await _describe_page(response, scoped_server=server, vendor=vendor) + assert json.loads(described.body) == { + "state": "m2m", + "client_origin": "https://claude.ai", + "server_id": "github-id", + "server_name": "github", + "connected": True, + } + unreachable = await _complete_page(response, scoped_server=server, reachable=_ServerReachability(False)) + assert unreachable.status_code == 400 + cache = DualCache() + completed = await _complete_page(response, scoped_server=server, vendor=vendor, cache=cache) + assert completed.status_code == 303 + assert vendor.calls == [] + code = parse_qs(urlparse(completed.headers["location"]).query)["code"][0] + with patch(_MANAGER_PATCH) as manager: + manager.get_mcp_server_by_name.return_value = server + redeemed = await _redeem(code, client_id, cache=cache, resource=resource) + assert redeemed.status_code == 200 + payload = json.loads(redeemed.body) + assert _opened_principal(payload).resource_server_id == "github-id" + renewed = await _redeem( + None, client_id, cache=cache, grant_type="refresh_token", refresh_token=payload["refresh_token"] + ) + assert renewed.status_code == 200 + assert _opened_principal(json.loads(renewed.body)).resource_server_id == "github-id"