fix(mcp): allow team access-group grants in OAuth authorize/token access check (#30041)
* fix(mcp): honor team access-group grants in OAuth authorize/token access check * test(mcp): mock build_effective_auth_contexts in non-admin authorize tests for isolation
This commit is contained in:
parent
d8fe091938
commit
5b7063d194
@ -21,7 +21,7 @@ import json
|
||||
import os
|
||||
from dataclasses import dataclass
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import Any, Dict, Iterable, List, Literal, Optional
|
||||
from typing import Any, Dict, Iterable, List, Literal, Optional, Set
|
||||
|
||||
from fastapi import (
|
||||
APIRouter,
|
||||
@ -1722,11 +1722,13 @@ if MCP_AVAILABLE:
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={"error": f"Access denied to MCP server {server_id}"},
|
||||
)
|
||||
allowed_server_ids = (
|
||||
await global_mcp_server_manager.get_allowed_mcp_servers(
|
||||
user_api_key_dict
|
||||
allowed_server_ids: Set[str] = set()
|
||||
for auth_context in await build_effective_auth_contexts(user_api_key_dict):
|
||||
allowed_server_ids.update(
|
||||
await global_mcp_server_manager.get_allowed_mcp_servers(
|
||||
auth_context
|
||||
)
|
||||
)
|
||||
)
|
||||
if server.server_id not in allowed_server_ids:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
|
||||
@ -1482,6 +1482,10 @@ class TestTemporaryMCPSessionEndpoints:
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
|
||||
mock_manager,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.build_effective_auth_contexts",
|
||||
AsyncMock(return_value=[non_admin]),
|
||||
),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await _get_cached_temporary_mcp_server_or_404("server-x", non_admin)
|
||||
@ -1514,6 +1518,10 @@ class TestTemporaryMCPSessionEndpoints:
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
|
||||
mock_manager,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.build_effective_auth_contexts",
|
||||
AsyncMock(return_value=[non_admin]),
|
||||
),
|
||||
):
|
||||
result = await _get_cached_temporary_mcp_server_or_404(
|
||||
"server-x", non_admin
|
||||
@ -1521,6 +1529,58 @@ class TestTemporaryMCPSessionEndpoints:
|
||||
|
||||
assert result is registry_server
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_cached_temporary_mcp_server_non_admin_allowed_via_team_access_group(
|
||||
self,
|
||||
):
|
||||
"""Internal user whose only grant to the server flows through a team
|
||||
access-group must pass the authorize/token access check. The check has to
|
||||
expand the UI session into per-team contexts (build_effective_auth_contexts),
|
||||
the same way the server-list grid does; checking only the bare session
|
||||
context leaves the team grant invisible and 403s the user."""
|
||||
from litellm.constants import UI_SESSION_TOKEN_TEAM_ID
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
_get_cached_temporary_mcp_server_or_404,
|
||||
)
|
||||
|
||||
registry_server = generate_mock_mcp_server_config_record(server_id="server-x")
|
||||
ui_session_auth = generate_mock_user_api_key_auth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
team_id=UI_SESSION_TOKEN_TEAM_ID,
|
||||
)
|
||||
team_context = ui_session_auth.model_copy()
|
||||
team_context.team_id = "team-with-mcp-grant"
|
||||
|
||||
mock_manager = MagicMock()
|
||||
mock_manager.get_mcp_server_by_id.return_value = registry_server
|
||||
mock_manager.get_mcp_server_by_name.return_value = None
|
||||
|
||||
def allowed_for(auth):
|
||||
return ["server-x"] if auth.team_id == "team-with-mcp-grant" else []
|
||||
|
||||
mock_manager.get_allowed_mcp_servers = AsyncMock(side_effect=allowed_for)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.get_cached_temporary_mcp_server",
|
||||
return_value=None,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
|
||||
mock_manager,
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.build_effective_auth_contexts",
|
||||
AsyncMock(return_value=[ui_session_auth, team_context]),
|
||||
),
|
||||
):
|
||||
result = await _get_cached_temporary_mcp_server_or_404(
|
||||
"server-x", ui_session_auth
|
||||
)
|
||||
|
||||
assert result is registry_server
|
||||
assert mock_manager.get_allowed_mcp_servers.await_count == 2
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_cached_temporary_mcp_server_temp_cache_non_admin_denied(self):
|
||||
"""Servers resolved from the admin-only temp cache reject non-admins."""
|
||||
|
||||
Loading…
Reference in New Issue
Block a user