mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix: check all auth headers in _try_resolve_mcp_oauth_broker_user
Previously _try_resolve_mcp_oauth_broker_user only inspected the 'Authorization' header to decide whether to run the auth pipeline. user_api_key_auth_from_request_headers supports six auth headers: Authorization, API-Key, x-api-key, x-goog-api-key, Ocp-Apim-Subscription-Key, and x-litellm-api-key. Callers authenticating via any of the alternative headers (e.g. Azure's API-Key) had their credentials silently ignored, causing them to be treated as unauthenticated. This would bypass admin/allowlist access control for temp-cache servers, and result in a 403 for global-registry servers they otherwise have permission to use. Fix: iterate over all six header names before deciding whether to skip the full auth pipeline call. Co-authored-by: Sameer Kankute <Sameerlite@users.noreply.github.com>
This commit is contained in:
parent
057b2438bd
commit
be04f99288
2 changed files with 81 additions and 2 deletions
|
|
@ -1458,8 +1458,12 @@ if MCP_AVAILABLE:
|
|||
servers only (browser OAuth). When present, global-registry access
|
||||
follows admin / allowlist rules via ``_get_cached_temporary_mcp_server_or_404``.
|
||||
|
||||
Only non-empty **string** auth header values trigger a full auth
|
||||
pipeline import (tests and mocks may attach MagicMock headers).
|
||||
Only non-empty **string** values in any recognised auth header trigger a
|
||||
full auth pipeline import (tests and mocks may attach MagicMock headers).
|
||||
The recognised headers match those checked by
|
||||
``user_api_key_auth_from_request_headers``: ``Authorization``,
|
||||
``API-Key``, ``x-api-key``, ``x-goog-api-key``,
|
||||
``Ocp-Apim-Subscription-Key``, and ``x-litellm-api-key``.
|
||||
"""
|
||||
try:
|
||||
headers = request.headers
|
||||
|
|
|
|||
|
|
@ -2007,6 +2007,81 @@ class TestTemporaryMCPSessionEndpoints:
|
|||
assert result is None
|
||||
|
||||
|
||||
class TestTryResolveMcpOAuthBrokerUser:
|
||||
"""Unit tests for _try_resolve_mcp_oauth_broker_user."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_none_when_no_auth_headers_present(self):
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
_try_resolve_mcp_oauth_broker_user,
|
||||
)
|
||||
|
||||
request = MagicMock()
|
||||
request.headers = {}
|
||||
result = await _try_resolve_mcp_oauth_broker_user(request)
|
||||
assert result is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_auth_for_standard_authorization_header(self):
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
_try_resolve_mcp_oauth_broker_user,
|
||||
)
|
||||
|
||||
request = MagicMock()
|
||||
request.headers = {"authorization": "Bearer sk-test"}
|
||||
mock_auth = generate_mock_user_api_key_auth()
|
||||
|
||||
with patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.user_api_key_auth_from_request_headers",
|
||||
AsyncMock(return_value=mock_auth),
|
||||
):
|
||||
result = await _try_resolve_mcp_oauth_broker_user(request)
|
||||
|
||||
assert result is mock_auth
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"header_name,header_value",
|
||||
[
|
||||
("api-key", "sk-azure-key"),
|
||||
("x-api-key", "sk-anthropic-key"),
|
||||
("x-goog-api-key", "sk-google-key"),
|
||||
("ocp-apim-subscription-key", "sk-apim-key"),
|
||||
("x-litellm-api-key", "sk-litellm-key"),
|
||||
],
|
||||
)
|
||||
async def test_returns_auth_for_alternative_auth_headers(
|
||||
self, header_name: str, header_value: str
|
||||
):
|
||||
"""Callers using non-Authorization auth headers must not be treated as unauthenticated."""
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
_try_resolve_mcp_oauth_broker_user,
|
||||
)
|
||||
|
||||
request = MagicMock()
|
||||
request.headers = {header_name: header_value}
|
||||
mock_auth = generate_mock_user_api_key_auth()
|
||||
|
||||
with patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.user_api_key_auth_from_request_headers",
|
||||
AsyncMock(return_value=mock_auth),
|
||||
):
|
||||
result = await _try_resolve_mcp_oauth_broker_user(request)
|
||||
|
||||
assert result is mock_auth
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_returns_none_when_auth_header_value_is_empty_string(self):
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
_try_resolve_mcp_oauth_broker_user,
|
||||
)
|
||||
|
||||
request = MagicMock()
|
||||
request.headers = {"authorization": " ", "api-key": ""}
|
||||
result = await _try_resolve_mcp_oauth_broker_user(request)
|
||||
assert result is None
|
||||
|
||||
|
||||
class TestUpdateMCPServer:
|
||||
"""Test suite for update MCP server functionality"""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue