mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-26 01:12:21 +00:00
fix(mcp): skip JWT injection when per-user mcp_auth_header is set
MCPClient._get_auth_headers() applies extra_headers AFTER writing Authorization from auth_value, so an injected JWT silently overwrites the user's per-server OAuth token. Guard the JWT signer with 'not mcp_auth_header' so per-user OAuth (and any dict-form per-user auth) takes precedence, mirroring the existing static_headers guard. Adds a regression test that the signer's inject helper is not called when mcp_auth_header is supplied.
This commit is contained in:
parent
1015d8fbd6
commit
8da32a51a7
2 changed files with 59 additions and 5 deletions
|
|
@ -1436,10 +1436,13 @@ class MCPServerManager:
|
|||
|
||||
# MCPJWTSigner: inject signed JWT for tools/list (list path skips pre_call_hook).
|
||||
# Skip entirely when the signer is not configured (avoid an unnecessary
|
||||
# dict copy on every list call) and when the server has its own
|
||||
# static Authorization header — admin-configured static auth must
|
||||
# take precedence per-server so the signer doesn't silently
|
||||
# overwrite e.g. an upstream API key.
|
||||
# dict copy on every list call), when the server has its own static
|
||||
# Authorization header, or when a per-user mcp_auth_header has already
|
||||
# been resolved — admin-configured static auth and per-user OAuth must
|
||||
# take precedence so the signer doesn't silently overwrite e.g. an
|
||||
# upstream API key or a user's OAuth token (MCPClient._get_auth_headers
|
||||
# applies extra_headers after writing Authorization from auth_value, so
|
||||
# an injected JWT would otherwise clobber the per-user token).
|
||||
if user_api_key_auth is not None and not server.spec_path:
|
||||
from litellm.proxy.guardrails.guardrail_hooks.mcp_jwt_signer.mcp_jwt_signer import (
|
||||
get_mcp_jwt_signer,
|
||||
|
|
@ -1452,7 +1455,11 @@ class MCPServerManager:
|
|||
for k in static_headers.keys()
|
||||
)
|
||||
|
||||
if get_mcp_jwt_signer() is not None and not has_static_authorization:
|
||||
if (
|
||||
get_mcp_jwt_signer() is not None
|
||||
and not has_static_authorization
|
||||
and not mcp_auth_header
|
||||
):
|
||||
extra_headers = await inject_mcp_jwt_headers_for_upstream(
|
||||
user_api_key_dict=user_api_key_auth,
|
||||
extra_headers=extra_headers,
|
||||
|
|
|
|||
|
|
@ -1805,6 +1805,53 @@ class TestMCPServerManager:
|
|||
assert len(tools_unprefixed) == 1
|
||||
assert tools_unprefixed[0].name == "send_email"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_tools_from_server_jwt_skipped_when_mcp_auth_header_set(self):
|
||||
"""When a per-user mcp_auth_header is resolved, JWT injection must be skipped.
|
||||
|
||||
MCPClient._get_auth_headers() applies extra_headers AFTER writing
|
||||
Authorization from auth_value, so an injected JWT would clobber the
|
||||
user's per-server OAuth token. Regression test for that interaction.
|
||||
"""
|
||||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
|
||||
manager = MCPServerManager()
|
||||
server = MCPServer(
|
||||
server_id="zapier",
|
||||
name="zapier",
|
||||
transport=MCPTransport.http,
|
||||
)
|
||||
|
||||
manager._create_mcp_client = AsyncMock(return_value=object())
|
||||
manager._fetch_tools_with_timeout = AsyncMock(return_value=[])
|
||||
|
||||
user_auth = UserAPIKeyAuth(api_key="sk-test", user_id="alice")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.guardrails.guardrail_hooks.mcp_jwt_signer.mcp_jwt_signer.get_mcp_jwt_signer",
|
||||
return_value=MagicMock(),
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.guardrails.guardrail_hooks.mcp_jwt_signer.mcp_jwt_signer.inject_mcp_jwt_headers_for_upstream",
|
||||
new=AsyncMock(return_value={"Authorization": "Bearer signed-jwt"}),
|
||||
) as mock_inject,
|
||||
):
|
||||
# Case A: mcp_auth_header present -> JWT must NOT be injected
|
||||
await manager._get_tools_from_server(
|
||||
server,
|
||||
mcp_auth_header="oauth-user-token",
|
||||
user_api_key_auth=user_auth,
|
||||
)
|
||||
mock_inject.assert_not_called()
|
||||
|
||||
# Case B: no mcp_auth_header -> JWT injection runs as before
|
||||
await manager._get_tools_from_server(
|
||||
server,
|
||||
user_api_key_auth=user_auth,
|
||||
)
|
||||
mock_inject.assert_awaited_once()
|
||||
|
||||
def test_create_prefixed_tools_updates_mapping_for_both_forms(self):
|
||||
"""_create_prefixed_tools should populate mapping for prefixed and original names even when not adding prefix in output."""
|
||||
manager = MCPServerManager()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue