mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(mcp): forward per-server auth header for OpenAPI MCP tools
OpenAPI-backed (local-registry) MCP tools resolved the upstream auth
header only from the deprecated global mcp_auth_header, ignoring the
per-server x-mcp-{alias}-authorization header carried in
mcp_server_auth_headers. Callers configuring Authorization forwarding
therefore got no credential upstream. Resolve mcp_server_auth_headers the
same way the managed path does before local tool dispatch.
Fixes #33344
This commit is contained in:
Devin AI
parent
817582e697
commit
0b1599017c
2 changed files with 192 additions and 1 deletions
|
|
@ -2757,12 +2757,44 @@ if MCP_AVAILABLE:
|
|||
arguments = hook_result["arguments"]
|
||||
|
||||
verbose_logger.debug(f"Executing local registry tool: {name}")
|
||||
# Resolve the effective upstream auth header for this server. A per-server
|
||||
# x-mcp-{alias}-authorization header (carried in mcp_server_auth_headers)
|
||||
# takes precedence over the deprecated global x-mcp-auth / BYOK
|
||||
# mcp_auth_header, mirroring the managed path (_call_regular_mcp_tool and
|
||||
# _prepare_mcp_server_headers). Without this OpenAPI-backed tools dropped the
|
||||
# per-server credential and never authenticated against the upstream backend.
|
||||
per_server_auth_header: Optional[Union[str, dict[str, str]]] = None
|
||||
if mcp_server and mcp_server_auth_headers:
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
lookup_mcp_server_auth_in_headers,
|
||||
)
|
||||
|
||||
per_server_auth_header = lookup_mcp_server_auth_in_headers(
|
||||
mcp_server_auth_headers,
|
||||
alias=mcp_server.alias,
|
||||
server_name=mcp_server.server_name,
|
||||
)
|
||||
|
||||
# For BYOK servers the credential must be injected via a ContextVar
|
||||
# because the tool function has headers baked into its closure.
|
||||
# Pre-format the full Authorization header value using the server's
|
||||
# configured auth_type so the generator doesn't need to know the prefix.
|
||||
auth_header_value: Optional[str] = None
|
||||
if mcp_auth_header:
|
||||
per_server_forwarded_headers: Optional[dict[str, str]] = None
|
||||
if isinstance(per_server_auth_header, dict):
|
||||
# Per-server headers are already full header values; forward them
|
||||
# verbatim. Authorization becomes the ContextVar auth override and any
|
||||
# other header rides along in the forwarded extra headers.
|
||||
for header_key, header_val in per_server_auth_header.items():
|
||||
if header_key.lower() == "authorization":
|
||||
auth_header_value = header_val
|
||||
else:
|
||||
if per_server_forwarded_headers is None:
|
||||
per_server_forwarded_headers = {}
|
||||
per_server_forwarded_headers[header_key] = header_val
|
||||
elif isinstance(per_server_auth_header, str) and per_server_auth_header:
|
||||
auth_header_value = per_server_auth_header
|
||||
elif mcp_auth_header:
|
||||
server_auth_type = getattr(mcp_server, "auth_type", None) if mcp_server else None
|
||||
if server_auth_type == MCPAuth.api_key:
|
||||
auth_header_value = f"ApiKey {mcp_auth_header}"
|
||||
|
|
@ -2796,6 +2828,11 @@ if MCP_AVAILABLE:
|
|||
forwarded_headers = {}
|
||||
forwarded_headers[header_name] = value
|
||||
|
||||
# Per-server x-mcp-{alias}-* headers win over caller-forwarded extra_headers,
|
||||
# matching the managed path where the resolved server_auth_header is applied last.
|
||||
if per_server_forwarded_headers:
|
||||
forwarded_headers = {**(forwarded_headers or {}), **per_server_forwarded_headers}
|
||||
|
||||
_auth_token = _request_auth_header.set(auth_header_value)
|
||||
_extra_token = _request_extra_headers.set(forwarded_headers)
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -90,6 +90,160 @@ async def test_openapi_local_tool_runs_pre_call_tool_check():
|
|||
assert pre_call_kwargs["proxy_logging_obj"] is not None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_openapi_local_tool_forwards_per_server_auth_header():
|
||||
"""Regression for #33344: an OpenAPI-backed (local-registry) MCP server must
|
||||
forward the caller's per-server ``x-mcp-{alias}-authorization`` header to the
|
||||
upstream backend. The credential arrives in ``mcp_server_auth_headers`` (not
|
||||
the global ``mcp_auth_header``); pre-fix the local dispatch ignored it, so the
|
||||
backend received no Authorization and rejected the request."""
|
||||
from litellm.proxy._experimental.mcp_server import server as mcp_module
|
||||
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
|
||||
_request_auth_header,
|
||||
_request_extra_headers,
|
||||
)
|
||||
|
||||
user = UserAPIKeyAuth(
|
||||
api_key="sk-user",
|
||||
user_id="alice",
|
||||
user_role=LitellmUserRoles.INTERNAL_USER.value,
|
||||
)
|
||||
|
||||
fake_server = MagicMock()
|
||||
fake_server.name = "report_openapi"
|
||||
fake_server.alias = "report_openapi"
|
||||
fake_server.server_name = "report_openapi"
|
||||
fake_server.server_id = "srv-1"
|
||||
fake_server.is_byok = False
|
||||
fake_server.auth_type = None
|
||||
fake_server.mcp_info = None
|
||||
fake_server.extra_headers = None
|
||||
|
||||
fake_tool = MagicMock()
|
||||
fake_tool.name = "summary_list"
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
async def _capture_local(name, arguments):
|
||||
captured["auth"] = _request_auth_header.get()
|
||||
captured["extra"] = _request_extra_headers.get()
|
||||
return []
|
||||
|
||||
with (
|
||||
patch.object(
|
||||
mcp_module.global_mcp_server_manager,
|
||||
"_get_mcp_server_from_tool_name",
|
||||
return_value=fake_server,
|
||||
),
|
||||
patch.object(
|
||||
mcp_module.global_mcp_server_manager,
|
||||
"pre_call_tool_check",
|
||||
new=AsyncMock(return_value={}),
|
||||
),
|
||||
patch.object(
|
||||
mcp_module.global_mcp_tool_registry,
|
||||
"get_tool",
|
||||
return_value=fake_tool,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.server._handle_local_mcp_tool",
|
||||
new=_capture_local,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.server.MCPRequestHandler.is_tool_allowed",
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
await mcp_module.execute_mcp_tool(
|
||||
name="summary_list",
|
||||
arguments={},
|
||||
allowed_mcp_servers=[fake_server],
|
||||
start_time=datetime.now(timezone.utc),
|
||||
user_api_key_auth=user,
|
||||
mcp_server_auth_headers={"report_openapi": {"Authorization": "Bearer upstream-token"}},
|
||||
)
|
||||
|
||||
# The per-server Authorization must reach the OpenAPI handler verbatim (no
|
||||
# extra "Bearer " re-prefixing), so the upstream backend is authenticated.
|
||||
assert captured["auth"] == "Bearer upstream-token"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_openapi_local_tool_forwards_per_server_non_auth_header():
|
||||
"""A per-server ``x-mcp-{alias}-{header}`` that is not Authorization must ride
|
||||
along in the forwarded extra headers rather than being dropped."""
|
||||
from litellm.proxy._experimental.mcp_server import server as mcp_module
|
||||
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
|
||||
_request_auth_header,
|
||||
_request_extra_headers,
|
||||
)
|
||||
|
||||
user = UserAPIKeyAuth(
|
||||
api_key="sk-user",
|
||||
user_id="alice",
|
||||
user_role=LitellmUserRoles.INTERNAL_USER.value,
|
||||
)
|
||||
|
||||
fake_server = MagicMock()
|
||||
fake_server.name = "report_openapi"
|
||||
fake_server.alias = "report_openapi"
|
||||
fake_server.server_name = "report_openapi"
|
||||
fake_server.server_id = "srv-1"
|
||||
fake_server.is_byok = False
|
||||
fake_server.auth_type = None
|
||||
fake_server.mcp_info = None
|
||||
fake_server.extra_headers = None
|
||||
|
||||
fake_tool = MagicMock()
|
||||
fake_tool.name = "summary_list"
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
async def _capture_local(name, arguments):
|
||||
captured["auth"] = _request_auth_header.get()
|
||||
captured["extra"] = _request_extra_headers.get()
|
||||
return []
|
||||
|
||||
with (
|
||||
patch.object(
|
||||
mcp_module.global_mcp_server_manager,
|
||||
"_get_mcp_server_from_tool_name",
|
||||
return_value=fake_server,
|
||||
),
|
||||
patch.object(
|
||||
mcp_module.global_mcp_server_manager,
|
||||
"pre_call_tool_check",
|
||||
new=AsyncMock(return_value={}),
|
||||
),
|
||||
patch.object(
|
||||
mcp_module.global_mcp_tool_registry,
|
||||
"get_tool",
|
||||
return_value=fake_tool,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.server._handle_local_mcp_tool",
|
||||
new=_capture_local,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy._experimental.mcp_server.server.MCPRequestHandler.is_tool_allowed",
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
await mcp_module.execute_mcp_tool(
|
||||
name="summary_list",
|
||||
arguments={},
|
||||
allowed_mcp_servers=[fake_server],
|
||||
start_time=datetime.now(timezone.utc),
|
||||
user_api_key_auth=user,
|
||||
mcp_server_auth_headers={
|
||||
"report_openapi": {"Authorization": "Bearer tok", "X-Tenant-Id": "acme"}
|
||||
},
|
||||
)
|
||||
|
||||
assert captured["auth"] == "Bearer tok"
|
||||
assert captured["extra"] == {"X-Tenant-Id": "acme"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_openapi_local_tool_blocked_when_pre_call_check_raises():
|
||||
"""If the pre-call check raises (caller not authorized for this
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue