mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-06 08:16:43 +00:00
fix(mcp): normalize a schemed authentication_token on the v2 and OpenAPI static paths (#39345)
Co-authored-by: yassin <yassin@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
6da516e6f3
commit
35d20468cd
4 changed files with 128 additions and 6 deletions
|
|
@ -47,7 +47,7 @@ from litellm.constants import (
|
|||
MCP_TOOL_LISTING_TIMEOUT,
|
||||
)
|
||||
from litellm.exceptions import BlockedPiiEntityError, GuardrailRaisedException
|
||||
from litellm.experimental_mcp_client.client import MCPClient, MCPSigV4Auth, strip_auth_scheme
|
||||
from litellm.experimental_mcp_client.client import MCPClient, MCPSigV4Auth, strip_auth_scheme, to_basic_credentials
|
||||
from litellm.integrations.custom_guardrail import (
|
||||
_sync_guardrail_info_to_logging_obj, # pyright: ignore[reportPrivateUsage] - the same bridge @log_guardrail_information uses; reimplementing it here would fork the metadata-key logic
|
||||
)
|
||||
|
|
@ -2299,13 +2299,13 @@ class MCPServerManager:
|
|||
from litellm.types.mcp import MCPAuth
|
||||
|
||||
if server.auth_type == MCPAuth.bearer_token:
|
||||
headers["Authorization"] = f"Bearer {server.authentication_token}"
|
||||
headers["Authorization"] = f"Bearer {strip_auth_scheme(server.authentication_token, 'Bearer')}"
|
||||
elif server.auth_type == MCPAuth.api_key:
|
||||
headers["Authorization"] = f"ApiKey {server.authentication_token}"
|
||||
headers["Authorization"] = f"ApiKey {strip_auth_scheme(server.authentication_token, 'ApiKey')}"
|
||||
elif server.auth_type == MCPAuth.basic:
|
||||
headers["Authorization"] = f"Basic {server.authentication_token}"
|
||||
headers["Authorization"] = f"Basic {to_basic_credentials(server.authentication_token)}"
|
||||
elif server.auth_type == MCPAuth.token:
|
||||
headers["Authorization"] = f"token {server.authentication_token}"
|
||||
headers["Authorization"] = f"token {strip_auth_scheme(server.authentication_token, 'token')}"
|
||||
|
||||
# Add any static headers from server config.
|
||||
#
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ from fastapi import HTTPException
|
|||
from pydantic import SecretStr
|
||||
from typing_extensions import assert_never
|
||||
|
||||
from litellm.experimental_mcp_client.client import strip_auth_scheme, to_basic_credentials
|
||||
from litellm.proxy._experimental.mcp_server.oauth_utils import resolve_upstream_resource
|
||||
from litellm.proxy._experimental.mcp_server.outbound_credentials.types import (
|
||||
DEFAULT_CREDENTIAL_HEADER,
|
||||
|
|
@ -213,7 +214,9 @@ def _shared_key_spec(
|
|||
token: Final = server.authentication_token
|
||||
if not token:
|
||||
return None # no key configured -> defer to v1 (parity-safe)
|
||||
value: Final = base64.b64encode(token.encode("utf-8")).decode() if encode else token
|
||||
value: Final = (
|
||||
to_basic_credentials(token) if encode else strip_auth_scheme(token, value_prefix) if value_prefix else token
|
||||
)
|
||||
return ServerSpec(
|
||||
server_id=server.server_id,
|
||||
resource=resource,
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import pytest
|
|||
from fastapi import HTTPException
|
||||
from pydantic import ValidationError
|
||||
|
||||
from litellm.experimental_mcp_client.client import MCPClient
|
||||
from litellm.proxy._experimental.mcp_server.outbound_credentials.adapter import (
|
||||
oauth_protected_resource_path,
|
||||
raise_public,
|
||||
|
|
@ -94,6 +95,49 @@ def test_basic_scheme_base64_encodes_the_token():
|
|||
assert spec.config.key_source.value.get_secret_value() == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"auth_type, authentication_token, expected_value, expected_header",
|
||||
[
|
||||
(MCPAuth.bearer_token, "Bearer abc", "abc", ("Authorization", "Bearer abc")),
|
||||
(MCPAuth.token, "token abc", "abc", ("Authorization", "token abc")),
|
||||
(MCPAuth.basic, "user:pass", "dXNlcjpwYXNz", ("Authorization", "Basic dXNlcjpwYXNz")),
|
||||
(MCPAuth.basic, "Basic dXNlcjpwYXNz", "dXNlcjpwYXNz", ("Authorization", "Basic dXNlcjpwYXNz")),
|
||||
(MCPAuth.basic, "Basic user:pass", "dXNlcjpwYXNz", ("Authorization", "Basic dXNlcjpwYXNz")),
|
||||
],
|
||||
)
|
||||
def test_shared_key_normalizes_schemed_authentication_token(
|
||||
auth_type, authentication_token, expected_value, expected_header
|
||||
):
|
||||
spec = to_server_spec(_server(auth_type=auth_type, authentication_token=authentication_token))
|
||||
assert spec is not None and isinstance(spec.config, ApiKeyConfig)
|
||||
assert spec.config.key_source.value.get_secret_value() == expected_value
|
||||
assert spec.config.header(expected_value) == expected_header
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"auth_type, authentication_token",
|
||||
[
|
||||
(MCPAuth.bearer_token, "Bearer abc"),
|
||||
(MCPAuth.bearer_token, "abc"),
|
||||
(MCPAuth.token, "token abc"),
|
||||
(MCPAuth.token, "abc"),
|
||||
(MCPAuth.basic, "user:pass"),
|
||||
(MCPAuth.basic, "Basic dXNlcjpwYXNz"),
|
||||
(MCPAuth.basic, "Basic user:pass"),
|
||||
],
|
||||
)
|
||||
def test_shared_key_authorization_matches_v1(auth_type, authentication_token):
|
||||
spec = to_server_spec(_server(auth_type=auth_type, authentication_token=authentication_token))
|
||||
assert spec is not None and isinstance(spec.config, ApiKeyConfig)
|
||||
|
||||
client = MCPClient(server_url="https://x", auth_type=auth_type)
|
||||
client.update_auth_value(authentication_token)
|
||||
|
||||
assert spec.config.header(spec.config.key_source.value.get_secret_value())[1] == client._get_auth_headers()[
|
||||
"Authorization"
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"oauth2_flow",
|
||||
[None, "authorization_code"],
|
||||
|
|
|
|||
|
|
@ -4634,6 +4634,81 @@ class TestMCPServerManager:
|
|||
# auth_type is none here, so a 401 from this upstream must not be dressed up as a re-auth signal
|
||||
assert captured["relays_upstream_auth"] is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"auth_type, authentication_token, expected_authorization",
|
||||
[
|
||||
(MCPAuth.bearer_token, "Bearer abc", "Bearer abc"),
|
||||
(MCPAuth.bearer_token, "abc", "Bearer abc"),
|
||||
(MCPAuth.api_key, "ApiKey abc", "ApiKey abc"),
|
||||
(MCPAuth.token, "token abc", "token abc"),
|
||||
(MCPAuth.basic, "user:pass", "Basic dXNlcjpwYXNz"),
|
||||
(MCPAuth.basic, "Basic dXNlcjpwYXNz", "Basic dXNlcjpwYXNz"),
|
||||
(MCPAuth.basic, "Basic user:pass", "Basic dXNlcjpwYXNz"),
|
||||
],
|
||||
)
|
||||
async def test_register_openapi_tools_normalizes_authentication_token(
|
||||
self, tmp_path, monkeypatch, auth_type, authentication_token, expected_authorization
|
||||
):
|
||||
manager = MCPServerManager()
|
||||
spec_path = tmp_path / "openapi.json"
|
||||
spec_path.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"openapi": "3.0.0",
|
||||
"info": {"title": "Demo", "version": "1.0.0"},
|
||||
"paths": {
|
||||
"/health": {
|
||||
"get": {
|
||||
"operationId": "health_check",
|
||||
"summary": "health",
|
||||
}
|
||||
}
|
||||
},
|
||||
}
|
||||
)
|
||||
)
|
||||
server = MCPServer(
|
||||
server_id="openapi-server",
|
||||
name="openapi-server",
|
||||
server_name="openapi-server",
|
||||
url="https://example.com",
|
||||
transport=MCPTransport.http,
|
||||
auth_type=auth_type,
|
||||
authentication_token=authentication_token,
|
||||
)
|
||||
captured: dict = {}
|
||||
|
||||
def fake_create_tool_function(
|
||||
path, method, operation, base_url, headers=None, server_label=None, relays_upstream_auth=False
|
||||
):
|
||||
captured["headers"] = headers
|
||||
|
||||
async def tool_func(**kwargs):
|
||||
return "ok"
|
||||
|
||||
return tool_func
|
||||
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator.create_tool_function",
|
||||
fake_create_tool_function,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator.build_input_schema",
|
||||
lambda *args, **kwargs: {"type": "object", "properties": {}, "required": []},
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy._experimental.mcp_server.tool_registry.global_mcp_tool_registry.register_tool",
|
||||
lambda *args, **kwargs: None,
|
||||
)
|
||||
await manager._register_openapi_tools(
|
||||
spec_path=str(spec_path),
|
||||
server=server,
|
||||
base_url="https://example.com",
|
||||
)
|
||||
|
||||
assert captured["headers"]["Authorization"] == expected_authorization
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pre_call_tool_check_allowed_tools_list_allows_tool(self):
|
||||
"""Test pre_call_tool_check allows tool when it's in allowed_tools list"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue