diff --git a/litellm/proxy/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/mcp_server/auth/user_api_key_auth_mcp.py index c788a0f5e22..db640b6d689 100644 --- a/litellm/proxy/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/mcp_server/auth/user_api_key_auth_mcp.py @@ -363,7 +363,7 @@ async def _upstream_bearer_identity(token: str, litellm_api_key: str, request: R return None try: return await user_api_key_auth(api_key=litellm_api_key, request=request) - except (HTTPException, ProxyException) as exc: + except (HTTPException, ProxyException, OAuth2TokenValidationError) as exc: if _caused_by(exc, UserNotFoundError) or _caused_by(exc, OAuth2TokenValidationError): return None raise diff --git a/litellm/proxy/mcp_server/server.py b/litellm/proxy/mcp_server/server.py index 6c79bc2d937..0fb1e6bbd3e 100644 --- a/litellm/proxy/mcp_server/server.py +++ b/litellm/proxy/mcp_server/server.py @@ -2214,12 +2214,18 @@ if MCP_AVAILABLE: mcp_servers=mcp_servers, client_ip=client_ip, ) - # A legacy delegate is probed only on a single-server route, so one rejected token cannot 401 a - # multi-server connect; its challenge echoes the requested name, matching the tokenless challenge. + # A legacy delegate is probed only when the single requested name resolves to it the way admission + # resolves it, so neither a multi-server connect nor an access group can be failed by one rejected + # token; its challenge echoes the requested name, matching the tokenless challenge. requested_scope: Final = requested_servers if requested_servers is not None else mcp_servers single_requested_name: Final = ( requested_scope[0] if requested_scope is not None and len(requested_scope) == 1 else None ) + requested_server: Final = ( + operations.global_mcp_server_manager.get_mcp_server_by_name(single_requested_name, client_ip=client_ip) + if single_requested_name is not None + else None + ) prepared_headers: Final = tuple( ( srv, @@ -2234,7 +2240,12 @@ if MCP_AVAILABLE: ), ) for srv in allowed_servers - if srv.is_oauth_passthrough or (single_requested_name is not None and is_legacy_upstream_delegate(srv)) + if srv.is_oauth_passthrough + or ( + requested_server is not None + and srv.server_id == requested_server.server_id + and is_legacy_upstream_delegate(srv) + ) ) probe_headers: Final = tuple( ( diff --git a/tests/unit/proxy/mcp_server/auth/test_user_api_key_auth_mcp.py b/tests/unit/proxy/mcp_server/auth/test_user_api_key_auth_mcp.py index 68f309b4926..d249e5cb525 100644 --- a/tests/unit/proxy/mcp_server/auth/test_user_api_key_auth_mcp.py +++ b/tests/unit/proxy/mcp_server/auth/test_user_api_key_auth_mcp.py @@ -3023,6 +3023,25 @@ class TestMCPDelegateAuthToUpstream: assert auth_result.mcp_forwards_caller_authorization is True assert oauth2_headers == {"Authorization": "Bearer opaque-idp-token"} + async def test_delegate_oauth2_rejection_admits_anonymously_whether_or_not_auth_wraps_it(self) -> None: + """A provider rejection is the provider declining to vouch, so the bearer is forwarded anonymously + even when it reaches admission as the bare validation error rather than a wrapped auth error.""" + from litellm.proxy.auth.oauth2_check import OAuth2TokenValidationError + + with ( + TestMCPDelegateAuthToUpstream._idp_auth_enabled("oauth2", AsyncMock()), + TestMCPDelegateAuthToUpstream._delegate_admission( + AsyncMock(side_effect=OAuth2TokenValidationError("Token is not active")) + ), + ): + auth_result, _, _, _, oauth2_headers, _ = await MCPRequestHandler.process_mcp_request( + TestMCPDelegateAuthToUpstream._bearer_scope("opaque-idp-token") + ) + + assert auth_result.user_id is None + assert auth_result.mcp_forwards_caller_authorization is True + assert oauth2_headers == {"Authorization": "Bearer opaque-idp-token"} + @pytest.mark.parametrize("mechanism", ["oauth2", "oauth2_proxy"]) async def test_delegate_idp_user_refusal_propagates(self, mechanism: Literal["oauth2", "oauth2_proxy"]) -> None: """Once the provider vouches for the caller, LiteLLM's budget and permission refusals apply.""" diff --git a/tests/unit/proxy/mcp_server/test_mcp_server_tool_calls_and_headers.py b/tests/unit/proxy/mcp_server/test_mcp_server_tool_calls_and_headers.py index 77526095f21..a56a51d5d87 100644 --- a/tests/unit/proxy/mcp_server/test_mcp_server_tool_calls_and_headers.py +++ b/tests/unit/proxy/mcp_server/test_mcp_server_tool_calls_and_headers.py @@ -7355,6 +7355,7 @@ async def test_legacy_delegate_dual_credentials_probe_only_the_upstream_token() ) with ( + _patch_delegate_resolver(server, "delegate_test"), patch( # test-quality-ok: isolate authorized-server resolution so this test targets the preflight boundary "litellm.proxy.mcp_server.operations._get_allowed_mcp_servers", new=AsyncMock(return_value=[server]), @@ -7397,10 +7398,12 @@ def _expired_delegate_token_preflight(mcp_servers: Sequence[str]) -> Coroutine[o @contextlib.contextmanager def _rejecting_delegate_upstream() -> Iterator[None]: + server: Final = _delegate_auth_mcp_server() with ( + _patch_delegate_resolver(server, "delegate_test"), patch( # test-quality-ok: isolate authorized-server resolution so this test targets the preflight boundary "litellm.proxy.mcp_server.operations._get_allowed_mcp_servers", - new=AsyncMock(return_value=[_delegate_auth_mcp_server()]), + new=AsyncMock(return_value=[server]), ), patch( # test-quality-ok: stands in for the upstream rejecting an expired token "litellm.proxy.mcp_server.server._probe_upstream_auth", @@ -7451,6 +7454,7 @@ async def test_legacy_delegate_probe_sends_the_headers_a_tool_call_sends( server: Final = _delegate_auth_mcp_server().model_copy(update={"static_headers": static_headers}) with ( + _patch_delegate_resolver(server, "delegate_test"), patch( # test-quality-ok: isolate authorized-server resolution so this test targets the preflight boundary "litellm.proxy.mcp_server.operations._get_allowed_mcp_servers", new=AsyncMock(return_value=[server]), @@ -7480,6 +7484,7 @@ async def test_legacy_delegate_probe_steps_aside_when_static_headers_need_unset_ } ) with ( + _patch_delegate_resolver(server, "delegate_test"), patch( # test-quality-ok: isolate authorized-server resolution so this test targets the preflight boundary "litellm.proxy.mcp_server.operations._get_allowed_mcp_servers", new=AsyncMock(return_value=[server]), @@ -7543,6 +7548,7 @@ async def test_modern_interaction_probes_legacy_delegate_only_for_a_single_reque } ).encode() with ( + _patch_delegate_resolver(target, "delegate_test"), patch( # test-quality-ok: isolate authorized-server resolution so this test targets the preflight boundary "litellm.proxy.mcp_server.operations._get_allowed_mcp_servers", new=AsyncMock(return_value=[target]), @@ -7791,39 +7797,47 @@ async def test_delegate_not_probed_when_named_only_via_server_id(): @pytest.mark.asyncio -async def test_delegate_probe_not_fanned_out_to_access_group_members(): - """A single access-group name passes the one-target route gate but must not fan - the delegate probe out to group-expanded member servers; the group name resolves - to no server under admission's resolver, so no probe fires.""" +@pytest.mark.usefixtures("httpx_transport") +async def test_delegate_probe_not_fanned_out_to_access_group_members() -> None: + """A single access-group name resolves to no server under admission's resolver, so the delegate probe is + not fanned out to the group's members and one member rejecting the token cannot fail the whole group.""" + import respx + from litellm.proxy._types import UserAPIKeyAuth from litellm.proxy.mcp_server.server import ( _check_passthrough_upstream_auth, ) - group_member = _delegate_auth_mcp_server() - + rejecting: Final = _delegate_auth_mcp_server() + accepting: Final = _delegate_auth_mcp_server("delegate-2").model_copy( + update={"name": "delegate_two", "url": "http://upstream-two:9402/mcp"} + ) with ( - _patch_delegate_resolver(group_member, "delegate_test"), - patch( + _patch_delegate_resolver(rejecting, "delegate_test"), + patch( # test-quality-ok: isolate authorized-server resolution so this test targets the preflight boundary "litellm.proxy.mcp_server.operations._get_allowed_mcp_servers", - new=AsyncMock(return_value=[group_member]), + new=AsyncMock(return_value=[rejecting, accepting]), ), - patch( - "litellm.proxy.mcp_server.server._probe_upstream_auth", - new=AsyncMock(return_value=(401, None)), - ) as probe, + respx.mock as upstream, ): - await _check_passthrough_upstream_auth( - scope=_delegate_scope([(b"authorization", b"Bearer bogus-token")]), - user_api_key_auth=UserAPIKeyAuth(), - mcp_servers=["prod_tools_group"], - client_ip=None, - oauth2_headers=None, - mcp_server_auth_headers=None, - raw_headers=None, + upstream.post(rejecting.url).respond(401, headers={"www-authenticate": 'Bearer error="invalid_token"'}) + upstream.post(accepting.url).respond(200) + assert ( + await _check_passthrough_upstream_auth( + scope=_delegate_scope( + [(b"x-litellm-api-key", b"sk-litellm-proxy-key"), (b"authorization", b"Bearer upstream-token")] + ), + user_api_key_auth=UserAPIKeyAuth(user_id="admitted-user"), + mcp_servers=["prod_tools_group"], + client_ip=None, + oauth2_headers={"Authorization": "Bearer upstream-token"}, + mcp_server_auth_headers=None, + raw_headers={"x-litellm-api-key": "sk-litellm-proxy-key", "Authorization": "Bearer upstream-token"}, + ) + is None ) - probe.assert_not_awaited() + assert upstream.calls.call_count == 0 @pytest.mark.asyncio