scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-11-09 08:18:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 67

fix: preserve per-user OAuth Authorization over MCP signer JWT on tools/call

Browse source
This commit is contained in:
Rajan Chavada 2026-07-03 19:40:31 +00:00
parent 4e53ae98a7
commit d55e32b4f9
2 changed files with 45 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -3352,27 +3352,31 @@ class MCPServerManager:
if hook_extra_headers:
if extra_headers is None:
extra_headers = {}
if "Authorization" in hook_extra_headers:
if "Authorization" in extra_headers:
verbose_logger.warning(
"MCPServerManager: hook_extra_headers 'Authorization' will overwrite "
"the existing Authorization header from static_headers. "
"The hook JWT will take precedence."
)
elif server_auth_header is not None:
# server_auth_header is passed separately to _create_mcp_client as
# auth_value. Both will reach the upstream server — warn so admins
# know two Authorization credentials are being sent.
verbose_logger.warning(
"MCPServerManager: hook_extra_headers injects 'Authorization' while "
"server '%s' already has a configured authentication_token. "
"Both credentials will be sent; the hook header is in extra_headers "
"and the server token is in auth_value — the upstream server decides "
"which one wins. Consider unsetting authentication_token if you want "
"the hook JWT to be the sole credential.",
mcp_server.server_name or mcp_server.name,
)
extra_headers.update(hook_extra_headers)
# Per-user OAuth and admin-configured static Authorization must take
# precedence over the signer's JWT (matches the tools/list guard) — skip the
# hook Authorization when extra_headers already carries one so the signed JWT
# doesn't clobber the resolved per-user token.
has_existing_authorization = any(
isinstance(k, str) and k.lower() == "authorization" for k in extra_headers.keys()
)
for header, value in hook_extra_headers.items():
if isinstance(header, str) and header.lower() == "authorization":
if has_existing_authorization:
continue
if server_auth_header is not None:
# server_auth_header is passed separately to _create_mcp_client as
# auth_value. Both will reach the upstream server — warn so admins
# know two Authorization credentials are being sent.
verbose_logger.warning(
"MCPServerManager: hook_extra_headers injects 'Authorization' while "
"server '%s' already has a configured authentication_token. "
"Both credentials will be sent; the hook header is in extra_headers "
"and the server token is in auth_value — the upstream server decides "
"which one wins. Consider unsetting authentication_token if you want "
"the hook JWT to be the sole credential.",
mcp_server.server_name or mcp_server.name,
)
extra_headers[header] = value
# Reset to None if no headers were actually added
if extra_headers is not None and len(extra_headers) == 0:

27
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_hook_extra_headers.py
View file

@ -536,14 +536,15 @@ class TestHookHeaderMergePriority:
)
@pytest.mark.asyncio
async def test_hook_headers_override_static_headers(self):
"""Hook headers should take precedence over static_headers."""
async def test_hook_non_auth_headers_merge_but_static_authorization_wins(self):
"""Hook non-Authorization headers merge in, but an existing static
Authorization must survive the hook (JWT) merge (matches tools/list)."""
manager = MCPServerManager()
server = self._make_server(
static_headers={"Authorization": "Bearer static-token", "X-Static": "yes"}
)
hook_headers = {"Authorization": "Bearer hook-signed-jwt"}
hook_headers = {"Authorization": "Bearer hook-signed-jwt", "X-Trace-Id": "t1"}
captured_extra_headers: Dict[str, Any] = {}
@ -576,8 +577,9 @@ class TestHookHeaderMergePriority:
pass
headers = captured_extra_headers.get("value", {})
assert headers["Authorization"] == "Bearer hook-signed-jwt"
assert headers["Authorization"] == "Bearer static-token"
assert headers["X-Static"] == "yes"
assert headers["X-Trace-Id"] == "t1"
@pytest.mark.asyncio
async def test_no_hook_headers_preserves_existing_behavior(self):
@ -619,8 +621,18 @@ class TestHookHeaderMergePriority:
assert headers == {"X-Static": "static-value"}
@pytest.mark.asyncio
async def test_hook_headers_merge_with_oauth2(self):
"""Hook headers merge on top of OAuth2 headers."""
async def test_hook_headers_do_not_overwrite_per_user_oauth_authorization(self):
"""Regression for #31977: a per-user OAuth Authorization already resolved
into extra_headers must survive the MCP JWT signer (hook) merge on the
tools/call path, matching the precedence enforced on tools/list. The
hook's non-Authorization headers still merge in.
Uses delegate_auth_to_upstream so the server stays on the v1 path
(to_server_spec is None) where the caller's OAuth token is NOT stripped
and therefore legitimately lives in extra_headers at the hook-merge
point — the exact scenario the reporter hits (tools/list works because
that guard already skips the signer when Authorization exists; tools/call
was clobbering it)."""
manager = MCPServerManager()
server = MCPServer(
server_id="test-id",
@ -629,6 +641,7 @@ class TestHookHeaderMergePriority:
url="https://example.com",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
delegate_auth_to_upstream=True,
)
captured_extra_headers: Dict[str, Any] = {}
@ -668,7 +681,7 @@ class TestHookHeaderMergePriority:
pass
headers = captured_extra_headers.get("value", {})
assert headers["Authorization"] == "Bearer hook-jwt"
assert headers["Authorization"] == "Bearer oauth2-token"
assert headers["X-OAuth"] == "yes"
assert headers["X-Trace-Id"] == "trace-123"