mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-12 08:42:18 +00:00
fix(mcp): probe only the delegate a single requested name resolves to
The legacy delegate preflight resolves the single requested name the way admission does, through get_mcp_server_by_name, and probes only that server, as v1.102 did. A name that expands to several delegates, such as an access group, is no longer fanned out, so one rejected token cannot fail the whole group Admission also reads a bare OAuth2TokenValidationError as the provider declining to vouch, so the anonymous fallback no longer depends on user_api_key_auth wrapping it
This commit is contained in:
parent
0676e437d7
commit
8fc1d45a4d
4 changed files with 71 additions and 27 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
(
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue