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 dcf7660d002a..1535daeb01dd 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 @@ -67,9 +67,10 @@ def _is_mcp_passthrough_cold_start( spec-compliant WWW-Authenticate challenge instead of surfacing a generic admission error. - Uses "all" semantics (mirrors :meth:`MCPRequestHandler._target_servers_use_oauth2`): - one non-passthrough target in a co-targeted set must not flip the bypass - open for the others. Fails closed when any target cannot be resolved.""" + Uses "all" semantics (mirrors + :meth:`MCPRequestHandler._target_servers_delegate_auth_to_upstream`): one + non-passthrough target in a co-targeted set must not flip the bypass open + for the others. Fails closed when any target cannot be resolved.""" if not mcp_servers: return False from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( @@ -214,101 +215,64 @@ async def mock_body(): # Only OAuth metadata routes registered under /.well-known/ are public. if request_route.startswith("/.well-known/"): validated_user_api_key_auth = UserAPIKeyAuth() - elif ( - not litellm_api_key - and MCPRequestHandler._target_servers_delegate_auth_to_upstream( # noqa: E501 - path=request_route, - mcp_servers=mcp_servers, - client_ip=IPAddressUtils.get_mcp_client_ip(request), - ) - ): - # Operator opted this oauth2 server into upstream-delegated auth - # (PKCE passthrough): skip LiteLLM API-key/SSO entirely so the - # client authenticates directly with the upstream MCP server. - # Fires ONLY when neither x-litellm-api-key nor Authorization is - # present. If any LiteLLM key is supplied (primary or secondary - # header), we fall through so user_id is resolved, spend/rate - # limiting apply, and any stored OAuth token can be retrieved - # and forwarded upstream. Gated by - # _target_servers_delegate_auth_to_upstream, which only returns - # True when EVERY target is auth_type=oauth2 AND has the - # delegate_auth_to_upstream flag set — fails closed otherwise. - validated_user_api_key_auth = UserAPIKeyAuth() elif has_explicit_litellm_key: - # Explicit x-litellm-api-key provided - always validate normally + # An explicit x-litellm-api-key is always a LiteLLM credential, even + # for a delegated server, so validate it: identity / spend / rate + # limits resolve and any stored upstream token can be forwarded. validated_user_api_key_auth = await user_api_key_auth( api_key=litellm_api_key, request=request ) + elif MCPRequestHandler._target_servers_delegate_auth_to_upstream( + path=request_route, + mcp_servers=mcp_servers, + client_ip=IPAddressUtils.get_mcp_client_ip(request), + ): + # Operator opted this oauth2 server into upstream-delegated auth: the + # client authenticates directly with the upstream MCP server, so any + # Authorization bearer is an upstream token, never a LiteLLM key. Skip + # LiteLLM validation entirely — covering both the no-credential + # discovery request and the authenticated call carrying the upstream + # bearer — so a tool call that succeeds never carries a phantom 401 + # auth span; the bearer is forwarded upstream unchanged. Gated by + # _target_servers_delegate_auth_to_upstream, which returns True only + # when EVERY target is auth_type=oauth2 with delegate_auth_to_upstream + # set; fails closed otherwise. + validated_user_api_key_auth = UserAPIKeyAuth() elif oauth2_headers: - # No x-litellm-api-key, but Authorization header present. - # Could be a LiteLLM key (backward compat) OR an opaque OAuth2 token - # the operator wants forwarded to an upstream OAuth2-mode MCP server. - # Try LiteLLM auth first; on auth failure, only fall back to anonymous - # passthrough when the request actually targets a server whose operator - # configured ``auth_type=oauth2``. For any other server (api_key, - # bearer_token, basic, etc.), a failed LiteLLM auth is a real failure - # and must propagate — otherwise an attacker can exchange any garbage - # bearer for an anonymous session. + # Authorization on a non-delegated server: the bearer must be a real + # LiteLLM credential, so a failed validation is a genuine 401/403 and + # propagates. The sole anonymous fallback is the auth_type=none + # pass-through cold-start (RFC 9728 discovery return), gated on a 401 + # so a recognized-but-forbidden key still fails closed. + client_ip = IPAddressUtils.get_mcp_client_ip(request) try: validated_user_api_key_auth = await user_api_key_auth( api_key=litellm_api_key, request=request ) except (HTTPException, ProxyException) as e: - # HTTPException.status_code is int; ProxyException.code is - # normalized to str in its __init__ but can be ``"None"`` or any - # non-numeric string when the caller didn't supply a numeric - # code, so we compare against both int and str forms rather - # than coercing (``int("None")`` would raise ValueError and - # rewrite the auth error as a 500). + # ProxyException.code is normalized to str (possibly "None"), so + # compare both int and str forms rather than coercing. status = e.status_code if isinstance(e, HTTPException) else e.code - is_auth_error = status in (401, 403, "401", "403") is_unauthenticated = status in (401, "401") - client_ip = IPAddressUtils.get_mcp_client_ip(request) - if is_auth_error and MCPRequestHandler._target_servers_use_oauth2( - path=request_route, - mcp_servers=mcp_servers, - client_ip=client_ip, + mcp_servers_from_path = _parse_mcp_server_names_from_path( + request_route, mcp_servers + ) + if ( + is_unauthenticated + and mcp_servers_from_path is not None + and not _has_client_supplied_mcp_auth( + mcp_auth_header, + mcp_server_auth_headers, + ) + and _is_mcp_passthrough_cold_start( + mcp_servers_from_path, client_ip=client_ip + ) ): verbose_logger.debug( - "MCP OAuth2: target server is OAuth2-mode, treating " - "Authorization as upstream OAuth2 token passthrough" + "MCP pass-through return: forwarding Authorization as " + "upstream OAuth token for delegated auth" ) validated_user_api_key_auth = UserAPIKeyAuth() - elif is_unauthenticated: - # Pass-through cold-start return: per RFC 9728 / MCP - # Authorization spec the client completes upstream OAuth - # discovery and returns with ``Authorization: Bearer - # ``. For ``auth_type=none`` passthrough - # servers that bearer is not a LiteLLM key (auth above - # failed) but is meant to be forwarded upstream - # unchanged. Fall back to anonymous admission so the - # caller is not rejected for following the discovery - # flow without also setting ``x-litellm-api-key``. - # Only trigger on 401 (token unrecognized); a 403 means - # the key WAS recognized but is forbidden (e.g. over - # budget / rate limited) and must propagate so those - # controls are not bypassed via anonymous admission. - mcp_servers_from_path = _parse_mcp_server_names_from_path( - request_route, mcp_servers - ) - if ( - mcp_servers_from_path is not None - and not _has_client_supplied_mcp_auth( - mcp_auth_header, - mcp_server_auth_headers, - ) - and _is_mcp_passthrough_cold_start( - mcp_servers_from_path, client_ip=client_ip - ) - ): - verbose_logger.debug( - "MCP pass-through return: target server is " - "passthrough, treating Authorization as " - "upstream OAuth token for delegated auth" - ) - validated_user_api_key_auth = UserAPIKeyAuth() - else: - raise else: raise else: @@ -412,45 +376,6 @@ def _extract_target_server_names_from_path(path: str) -> List[str]: return [single_server_match.group(1)] return [servers_and_path] - @staticmethod - def _target_servers_use_oauth2( - path: str, mcp_servers: Optional[List[str]], client_ip: Optional[str] - ) -> bool: - """ - True only when EVERY MCP server the request targets is configured for - ``auth_type == oauth2``. If any target is non-OAuth2 — or if the target - cannot be resolved at all — return False so the caller fails closed. - - Used to gate the "treat Authorization as opaque OAuth2 token" fallback - in :meth:`process_mcp_request` so a failed LiteLLM-auth cannot be - exchanged for an anonymous session against a non-OAuth2 server. - """ - # Inline imports avoid a circular dependency: mcp_server_manager imports - # from this module. - from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( - global_mcp_server_manager, - ) - from litellm.types.mcp import MCPAuth - - # Resolve the same target list downstream routing will use. For - # ``/mcp/...`` routes, ``extract_mcp_auth_context`` overrides the - # ``x-mcp-servers`` header with path-derived names, so we must mirror - # that here — otherwise a caller could set the header to a permissive - # server while the path targets a stricter one (header/path TOCTOU). - target_names = MCPRequestHandler._resolve_target_server_names( - path=path, mcp_servers_header=mcp_servers - ) - if not target_names: - return False - - for name in target_names: - server = global_mcp_server_manager.get_mcp_server_by_name( - name, client_ip=client_ip - ) - if server is None or server.auth_type != MCPAuth.oauth2: - return False - return True - @staticmethod def _target_servers_delegate_auth_to_upstream( path: str, mcp_servers: Optional[List[str]], client_ip: Optional[str] @@ -472,8 +397,8 @@ def _target_servers_delegate_auth_to_upstream( ) from litellm.types.mcp import MCPAuth - # See _target_servers_use_oauth2: must mirror the downstream - # header-vs-path override or an attacker could set + # Must mirror the downstream header-vs-path override + # (``extract_mcp_auth_context``) or an attacker could set # ``x-mcp-servers`` to a delegate-enabled server while the URL path # targets a non-delegate server, skipping LiteLLM auth for it. target_names = MCPRequestHandler._resolve_target_server_names( 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 7753378ab4f0..ab42ee1e979f 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 @@ -658,12 +658,11 @@ class TestMCPOAuth2AuthFlow: async def test_oauth2_token_in_authorization_header_fallback(self): """ - When only Authorization header is present with a non-LiteLLM OAuth2 token - AND the target server is operator-configured for ``auth_type=oauth2``, - auth should fall back to permissive mode (OAuth2 passthrough). + When only the Authorization header is present with a non-LiteLLM OAuth2 + token AND the target server delegates auth to upstream, LiteLLM skips its + own validation entirely (so the upstream token is never mistaken for a + virtual key) and forwards the bearer upstream. """ - from fastapi import HTTPException - from litellm.types.mcp import MCPAuth scope = { @@ -675,17 +674,16 @@ async def test_oauth2_token_in_authorization_header_fallback(self): ], } - async def mock_user_api_key_auth_fails(api_key, request): - raise HTTPException(status_code=401, detail="Invalid API key") - oauth2_server = MagicMock() oauth2_server.auth_type = MCPAuth.oauth2 + oauth2_server.delegate_auth_to_upstream = True + oauth2_server.has_client_credentials = False with ( patch( "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth", - side_effect=mock_user_api_key_auth_fails, - ), + new_callable=AsyncMock, + ) as mock_auth, patch( "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager" ) as mock_mgr, @@ -700,10 +698,10 @@ async def mock_user_api_key_auth_fails(api_key, request): raw_headers, ) = await MCPRequestHandler.process_mcp_request(scope) - # Should succeed with default UserAPIKeyAuth (OAuth2 fallback) - assert auth_result is not None assert isinstance(auth_result, UserAPIKeyAuth) - # OAuth2 headers should contain the token for upstream forwarding + # The upstream token is never validated as a LiteLLM key ... + mock_auth.assert_not_called() + # ... and is preserved for upstream forwarding. assert ( oauth2_headers.get("Authorization") == "Bearer atlassian-oauth2-access-token-xyz" @@ -813,11 +811,12 @@ async def mock_user_api_key_auth_server_error(api_key, request): await MCPRequestHandler.process_mcp_request(scope) assert exc_info.value.status_code == 500 - async def test_proxy_exception_oauth2_fallback(self): + async def test_proxy_exception_non_delegate_oauth2_propagates(self): """ - user_api_key_auth raises ProxyException (not HTTPException) in production. - The OAuth2 fallback must catch ProxyException with code 401/403 too, - but only when the target server is operator-configured for ``auth_type=oauth2``. + Production raises ProxyException (not HTTPException) on auth failure. For + a non-delegate oauth2 server the bearer is treated as a LiteLLM credential + and a 401 must propagate as a real auth error, not be exchanged for an + anonymous upstream-passthrough session. """ from litellm.proxy._types import ProxyException from litellm.types.mcp import MCPAuth @@ -841,6 +840,8 @@ async def mock_user_api_key_auth_proxy_exception(api_key, request): oauth2_server = MagicMock() oauth2_server.auth_type = MCPAuth.oauth2 + oauth2_server.delegate_auth_to_upstream = False + oauth2_server.is_oauth_passthrough = False with ( patch( @@ -852,22 +853,9 @@ async def mock_user_api_key_auth_proxy_exception(api_key, request): ) as mock_mgr, ): mock_mgr.get_mcp_server_by_name.return_value = oauth2_server - ( - auth_result, - mcp_auth_header, - mcp_servers, - mcp_server_auth_headers, - oauth2_headers, - raw_headers, - ) = await MCPRequestHandler.process_mcp_request(scope) - - # Should succeed with default UserAPIKeyAuth (OAuth2 fallback) - assert auth_result is not None - assert isinstance(auth_result, UserAPIKeyAuth) - assert ( - oauth2_headers.get("Authorization") - == "Bearer atlassian-oauth2-access-token-xyz" - ) + with pytest.raises(ProxyException) as exc_info: + await MCPRequestHandler.process_mcp_request(scope) + assert str(exc_info.value.code) == "401" async def test_proxy_exception_non_auth_still_raises(self): """ @@ -1355,11 +1343,15 @@ async def mock_user_api_key_auth_fails(api_key, request): await MCPRequestHandler.process_mcp_request(scope) assert exc_info.value.status_code == 401 - async def test_fallback_allowed_when_target_is_oauth2_mode(self): + async def test_non_delegate_oauth2_does_not_fall_back_to_anonymous(self): """ - Operator-configured OAuth2 passthrough still works: target server has - ``auth_type=oauth2`` → failed LiteLLM auth falls back to anonymous so - the bearer can be forwarded to upstream. + An ``auth_type=oauth2`` server that has NOT opted into + ``delegate_auth_to_upstream`` must not exchange a failed LiteLLM auth for + an anonymous session: forwarding an arbitrary bearer upstream is only + allowed once the operator explicitly delegates auth. A failed validation + here is a genuine 401 and propagates (which is also what keeps the + success-path trace free of a phantom 401, since no doomed validation runs + for a delegated server). """ from fastapi import HTTPException @@ -1389,8 +1381,9 @@ async def mock_user_api_key_auth_fails(api_key, request): mock_mgr.get_mcp_server_by_name.return_value = ( TestMCPOAuth2FallbackTargetGating._make_server(MCPAuth.oauth2) ) - auth_result, *_rest = await MCPRequestHandler.process_mcp_request(scope) - assert isinstance(auth_result, UserAPIKeyAuth) + with pytest.raises(HTTPException) as exc_info: + await MCPRequestHandler.process_mcp_request(scope) + assert exc_info.value.status_code == 401 async def test_fallback_allowed_when_target_is_passthrough(self): """ @@ -1668,19 +1661,16 @@ async def test_delegate_skips_litellm_auth_with_no_authorization(self): assert isinstance(auth_result, UserAPIKeyAuth) mock_auth.assert_not_called() - async def test_delegate_with_upstream_token_in_authorization_falls_back_to_anonymous( + async def test_delegate_with_upstream_token_in_authorization_skips_litellm_auth( self, ): """ oauth2 + delegate_auth_to_upstream=True with an upstream OAuth token in - ``Authorization`` (not a LiteLLM key): LiteLLM auth is attempted first - (and fails), then the existing oauth2 fallback returns anonymous so the - bearer is forwarded upstream untouched. The delegate branch itself does - not fire when Authorization is present — that is what protects spend - tracking for callers using Authorization-style LiteLLM keys. + ``Authorization``: the delegate gate fires before any LiteLLM validation, + so ``user_api_key_auth`` is never called and the bearer is forwarded + upstream untouched. Skipping the doomed validation is what keeps a tool + call that actually succeeds from carrying a phantom 401 auth span. """ - from fastapi import HTTPException - from litellm.types.mcp import MCPAuth scope = { @@ -1690,14 +1680,11 @@ async def test_delegate_with_upstream_token_in_authorization_falls_back_to_anony "headers": [(b"authorization", b"Bearer upstream-pkce-token")], } - async def mock_user_api_key_auth_fails(api_key, request): - raise HTTPException(status_code=401, detail="Invalid API key") - with ( patch( "litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp.user_api_key_auth", - side_effect=mock_user_api_key_auth_fails, - ), + new_callable=AsyncMock, + ) as mock_auth, patch( "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager" ) as mock_mgr, @@ -1718,6 +1705,7 @@ async def mock_user_api_key_auth_fails(api_key, request): ) = await MCPRequestHandler.process_mcp_request(scope) assert isinstance(auth_result, UserAPIKeyAuth) assert oauth2_headers.get("Authorization") == "Bearer upstream-pkce-token" + mock_auth.assert_not_called() async def test_delegate_off_still_requires_litellm_auth(self): """ @@ -1912,12 +1900,15 @@ async def test_explicit_litellm_key_takes_precedence_over_delegate(self): assert auth_result.user_id == "real-user" mock_auth.assert_called_once() - async def test_litellm_key_via_authorization_header_not_bypassed(self): + async def test_authorization_bearer_on_delegate_server_treated_as_upstream(self): """ - Regression: a LiteLLM key sent via the secondary ``Authorization`` header - (e.g. ``Authorization: Bearer sk-...``) must still trigger normal auth - and not be silently swallowed by the delegate bypass — otherwise spend - tracking and rate limiting are skipped for those callers. + On a delegate server the ``Authorization`` header is, by contract, an + upstream token rather than a LiteLLM key — even when it is sk-shaped. It + is forwarded upstream without LiteLLM validation, so ``user_api_key_auth`` + is not called and no LiteLLM identity is resolved. Callers who need + LiteLLM identity / spend tracking on a delegate server must supply + ``x-litellm-api-key`` (see + test_explicit_litellm_key_takes_precedence_over_delegate). """ from litellm.types.mcp import MCPAuth @@ -1944,10 +1935,18 @@ async def test_litellm_key_via_authorization_header_not_bypassed(self): delegate_auth_to_upstream=True, ) ) - auth_result, *_rest = await MCPRequestHandler.process_mcp_request(scope) + ( + auth_result, + _, + _, + _, + oauth2_headers, + _, + ) = await MCPRequestHandler.process_mcp_request(scope) assert isinstance(auth_result, UserAPIKeyAuth) - assert auth_result.user_id == "real-user" - mock_auth.assert_called_once() + assert auth_result.user_id is None + assert oauth2_headers.get("Authorization") == "Bearer sk-1234" + mock_auth.assert_not_called() async def test_delegate_ignored_for_client_credentials_server(self): """