You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/05/20 21:19:46 UTC
(superset) 13/13: refactor(mcp): promote local imports to module level in __main__.py and test_middleware.py
This is an automated email from the ASF dual-hosted git repository.
aminghadersohi pushed a commit to branch mcp-rbac-tool-visibility
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 094cbf69726f055f8e5c9602d3746b5197eaf1d6
Author: Amin Ghadersohi <am...@gmail.com>
AuthorDate: Wed May 20 21:14:22 2026 +0000
refactor(mcp): promote local imports to module level in __main__.py and test_middleware.py
Move create_response_size_guard_middleware and build_middleware_list from
function body to module level in __main__.py (no circular import issue).
Move MCPPermissionDeniedError and RBACToolVisibilityMiddleware from
repeated local imports to module-level imports in test_middleware.py.
Co-Authored-By: Claude Sonnet 4.6 <no...@anthropic.com>
---
superset/mcp_service/__main__.py | 5 ++---
tests/unit_tests/mcp_service/test_middleware.py | 20 ++------------------
2 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/superset/mcp_service/__main__.py b/superset/mcp_service/__main__.py
index 1c9f906b96e..968a5f9cb67 100644
--- a/superset/mcp_service/__main__.py
+++ b/superset/mcp_service/__main__.py
@@ -42,6 +42,8 @@ if os.environ.get("FASTMCP_TRANSPORT", "stdio") == "stdio":
click.echo = lambda *args, **kwargs: click.echo(*args, file=sys.stderr, **kwargs)
from superset.mcp_service.app import init_fastmcp_server, mcp
+from superset.mcp_service.middleware import create_response_size_guard_middleware
+from superset.mcp_service.server import build_middleware_list
def _add_default_middlewares() -> None:
@@ -56,9 +58,6 @@ def _add_default_middlewares() -> None:
``build_middleware_list()`` already returns middlewares in the correct
outermost-first order.
"""
- from superset.mcp_service.middleware import create_response_size_guard_middleware
- from superset.mcp_service.server import build_middleware_list
-
for middleware in build_middleware_list():
mcp.add_middleware(middleware)
diff --git a/tests/unit_tests/mcp_service/test_middleware.py b/tests/unit_tests/mcp_service/test_middleware.py
index e2285f77515..122c5b926a7 100644
--- a/tests/unit_tests/mcp_service/test_middleware.py
+++ b/tests/unit_tests/mcp_service/test_middleware.py
@@ -34,11 +34,13 @@ from superset.commands.exceptions import (
)
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetException, SupersetSecurityException
+from superset.mcp_service.auth import MCPPermissionDeniedError
from superset.mcp_service.mcp_config import MCP_RESPONSE_SIZE_CONFIG
from superset.mcp_service.middleware import (
_is_user_error,
create_response_size_guard_middleware,
GlobalErrorHandlerMiddleware,
+ RBACToolVisibilityMiddleware,
ResponseSizeGuardMiddleware,
)
@@ -1044,8 +1046,6 @@ class TestGlobalErrorHandlerLogLevels:
@pytest.mark.asyncio
async def test_mcp_permission_denied_error_becomes_tool_error(self) -> None:
"""MCPPermissionDeniedError must convert to ToolError, not a generic error."""
- from superset.mcp_service.auth import MCPPermissionDeniedError
-
middleware = GlobalErrorHandlerMiddleware()
context = MagicMock()
@@ -1073,8 +1073,6 @@ class TestGlobalErrorHandlerLogLevels:
@pytest.mark.asyncio
async def test_mcp_permission_denied_error_is_user_error(self) -> None:
"""MCPPermissionDeniedError must be classified as a user error (WARNING)."""
- from superset.mcp_service.auth import MCPPermissionDeniedError
-
error = MCPPermissionDeniedError(
permission_name="can_write",
view_name="Chart",
@@ -1084,8 +1082,6 @@ class TestGlobalErrorHandlerLogLevels:
@pytest.mark.asyncio
async def test_mcp_permission_denied_error_logs_at_warning(self) -> None:
"""MCPPermissionDeniedError should log at WARNING, not ERROR."""
- from superset.mcp_service.auth import MCPPermissionDeniedError
-
middleware = GlobalErrorHandlerMiddleware()
context = MagicMock()
@@ -1124,8 +1120,6 @@ class TestRBACToolVisibilityMiddleware:
@pytest.mark.asyncio
async def test_fails_open_on_exception(self) -> None:
"""Returns all tools when get_flask_app raises (fail open)."""
- from superset.mcp_service.middleware import RBACToolVisibilityMiddleware
-
tools = [self._make_tool("list_charts"), self._make_tool("generate_chart")]
call_next = AsyncMock(return_value=tools)
middleware = RBACToolVisibilityMiddleware()
@@ -1141,8 +1135,6 @@ class TestRBACToolVisibilityMiddleware:
@pytest.mark.asyncio
async def test_fails_open_when_user_is_none(self, app) -> None:
"""Returns all tools when get_user_from_request returns None."""
- from superset.mcp_service.middleware import RBACToolVisibilityMiddleware
-
tools = [self._make_tool("list_charts"), self._make_tool("generate_chart")]
call_next = AsyncMock(return_value=tools)
middleware = RBACToolVisibilityMiddleware()
@@ -1163,8 +1155,6 @@ class TestRBACToolVisibilityMiddleware:
@pytest.mark.asyncio
async def test_filters_tools_by_rbac(self, app) -> None:
"""Tools denied by is_tool_visible_to_current_user are removed."""
- from superset.mcp_service.middleware import RBACToolVisibilityMiddleware
-
read_tool = self._make_tool("list_charts")
write_tool = self._make_tool("generate_chart")
tools = [read_tool, write_tool]
@@ -1197,8 +1187,6 @@ class TestRBACToolVisibilityMiddleware:
@pytest.mark.asyncio
async def test_fails_closed_on_permission_error(self, app) -> None:
"""Returns empty list when credentials are invalid (PermissionError)."""
- from superset.mcp_service.middleware import RBACToolVisibilityMiddleware
-
tools = [self._make_tool("list_charts"), self._make_tool("generate_chart")]
call_next = AsyncMock(return_value=tools)
middleware = RBACToolVisibilityMiddleware()
@@ -1219,8 +1207,6 @@ class TestRBACToolVisibilityMiddleware:
@pytest.mark.asyncio
async def test_fails_closed_on_bad_credentials_value_error(self, app) -> None:
"""Returns empty list when auth was attempted but user not found."""
- from superset.mcp_service.middleware import RBACToolVisibilityMiddleware
-
tools = [self._make_tool("list_charts"), self._make_tool("generate_chart")]
call_next = AsyncMock(return_value=tools)
middleware = RBACToolVisibilityMiddleware()
@@ -1241,8 +1227,6 @@ class TestRBACToolVisibilityMiddleware:
@pytest.mark.asyncio
async def test_fails_open_when_no_auth_configured(self, app) -> None:
"""Returns all tools when no auth source is configured at all."""
- from superset.mcp_service.middleware import RBACToolVisibilityMiddleware
-
tools = [self._make_tool("list_charts"), self._make_tool("generate_chart")]
call_next = AsyncMock(return_value=tools)
middleware = RBACToolVisibilityMiddleware()