This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 7943af359c2 feat(mcp): implement RBAC permission checking for MCP
tools (#38407)
7943af359c2 is described below
commit 7943af359c2ec8bcc1dbd215278571a89eac4a5c
Author: Amin Ghadersohi <[email protected]>
AuthorDate: Thu Mar 12 17:35:07 2026 +0100
feat(mcp): implement RBAC permission checking for MCP tools (#38407)
Co-authored-by: Claude Opus 4.6 <[email protected]>
---
superset-core/src/superset_core/mcp/decorators.py | 22 +-
superset/core/mcp/core_mcp_injection.py | 24 ++-
superset/mcp_service/auth.py | 136 +++++++++++--
superset/mcp_service/chart/tool/generate_chart.py | 2 +-
superset/mcp_service/chart/tool/get_chart_data.py | 2 +-
superset/mcp_service/chart/tool/get_chart_info.py | 2 +-
.../mcp_service/chart/tool/get_chart_preview.py | 2 +-
superset/mcp_service/chart/tool/list_charts.py | 2 +-
superset/mcp_service/chart/tool/update_chart.py | 2 +-
.../mcp_service/chart/tool/update_chart_preview.py | 2 +-
.../tool/add_chart_to_existing_dashboard.py | 2 +-
.../dashboard/tool/generate_dashboard.py | 2 +-
.../dashboard/tool/get_dashboard_info.py | 2 +-
.../mcp_service/dashboard/tool/list_dashboards.py | 2 +-
.../mcp_service/dataset/tool/get_dataset_info.py | 2 +-
superset/mcp_service/dataset/tool/list_datasets.py | 2 +-
.../explore/tool/generate_explore_link.py | 2 +-
superset/mcp_service/mcp_config.py | 5 +
superset/mcp_service/sql_lab/tool/execute_sql.py | 6 +-
.../sql_lab/tool/open_sql_lab_with_context.py | 2 +-
tests/unit_tests/mcp_service/conftest.py | 19 +-
tests/unit_tests/mcp_service/test_auth_rbac.py | 225 +++++++++++++++++++++
22 files changed, 431 insertions(+), 36 deletions(-)
diff --git a/superset-core/src/superset_core/mcp/decorators.py
b/superset-core/src/superset_core/mcp/decorators.py
index d8c814a1acd..6ad29c9adc5 100644
--- a/superset-core/src/superset_core/mcp/decorators.py
+++ b/superset-core/src/superset_core/mcp/decorators.py
@@ -48,11 +48,14 @@ def tool(
description: str | None = None,
tags: list[str] | None = None,
protect: bool = True,
+ class_permission_name: str | None = None,
+ method_permission_name: str | None = None,
) -> Any: # Use Any to avoid mypy issues with dependency injection
"""
Decorator to register an MCP tool with optional authentication.
- This decorator combines FastMCP tool registration with optional
authentication.
+ This decorator combines FastMCP tool registration with optional
authentication
+ and RBAC permission checking.
Can be used as:
@tool
@@ -69,6 +72,11 @@ def tool(
description: Tool description (defaults to function docstring)
tags: List of tags for categorizing the tool (defaults to empty list)
protect: Whether to require Superset authentication (defaults to True)
+ class_permission_name: FAB view/resource name for RBAC checking
+ (e.g., "Chart", "Dashboard", "SQLLab"). When set, enables
+ permission checking via security_manager.can_access().
+ method_permission_name: FAB action name (e.g., "read", "write").
+ Defaults to "write" if tags includes "mutate", else "read".
Returns:
Decorator function that registers and wraps the tool, or the wrapped
function
@@ -90,6 +98,18 @@ def tool(
def public_tool() -> str:
'''Public tool accessible without auth'''
return "Hello world"
+
+ @tool(class_permission_name="Chart") # RBAC: requires can_read on
Chart
+ def list_charts() -> list:
+ '''List charts the user can access'''
+ return []
+
+ @tool( # RBAC: can_write on Chart
+ tags=["mutate"], class_permission_name="Chart",
+ )
+ def create_chart(name: str) -> dict:
+ '''Create a new chart'''
+ return {"name": name}
"""
raise NotImplementedError(
"MCP tool decorator not initialized. "
diff --git a/superset/core/mcp/core_mcp_injection.py
b/superset/core/mcp/core_mcp_injection.py
index ad4a7b1211d..fa2ec23738a 100644
--- a/superset/core/mcp/core_mcp_injection.py
+++ b/superset/core/mcp/core_mcp_injection.py
@@ -60,12 +60,14 @@ def create_tool_decorator(
description: Optional[str] = None,
tags: Optional[list[str]] = None,
protect: bool = True,
+ class_permission_name: Optional[str] = None,
+ method_permission_name: Optional[str] = None,
) -> Callable[[F], F] | F:
"""
Create the concrete MCP tool decorator implementation.
- This combines FastMCP tool registration with optional Superset
authentication,
- replacing the need for separate @mcp.tool and @mcp_auth_hook decorators.
+ This combines FastMCP tool registration with optional Superset
authentication
+ and RBAC permission checking.
Supports both @tool and @tool() syntax.
@@ -76,6 +78,10 @@ def create_tool_decorator(
description: Tool description (defaults to function docstring)
tags: List of tags for categorization (defaults to empty list)
protect: Whether to apply Superset authentication (defaults to True)
+ class_permission_name: FAB view/resource name for RBAC
+ (e.g., "Chart", "Dashboard", "SQLLab"). Enables permission
checking.
+ method_permission_name: FAB action name (e.g., "read", "write").
+ Defaults to "write" if tags has "mutate", else "read".
Returns:
Decorator that registers and wraps the tool with optional
authentication,
@@ -95,6 +101,20 @@ def create_tool_decorator(
# Get prefixed ID based on ambient context
tool_name, context_type =
_get_prefixed_id_with_context(base_tool_name)
+ # Store RBAC permission metadata on the function so
+ # mcp_auth_hook can read them at call time.
+ if class_permission_name:
+ from superset.mcp_service.auth import (
+ CLASS_PERMISSION_ATTR,
+ METHOD_PERMISSION_ATTR,
+ )
+
+ setattr(func, CLASS_PERMISSION_ATTR, class_permission_name)
+ actual_method = method_permission_name
+ if actual_method is None:
+ actual_method = "write" if "mutate" in tool_tags else
"read"
+ setattr(func, METHOD_PERMISSION_ATTR, actual_method)
+
# Conditionally apply authentication wrapper
if protect:
from superset.mcp_service.auth import mcp_auth_hook
diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py
index 8675947097f..77544f224cb 100644
--- a/superset/mcp_service/auth.py
+++ b/superset/mcp_service/auth.py
@@ -16,15 +16,16 @@
# under the License.
"""
-Minimal authentication hooks for MCP tools.
-This is a placeholder implementation that provides basic user context.
-
-Future enhancements (to be added in separate PRs):
-- JWT token authentication and validation
-- User impersonation support
-- Permission checking with scopes
-- Comprehensive audit logging
-- Field-level permissions
+Authentication and authorization hooks for MCP tools.
+
+This module provides:
+- User authentication from JWT or configured dev user
+- RBAC permission checking aligned with Superset's REST API permissions
+- Dataset access validation
+- Session lifecycle management
+
+The RBAC enforcement mirrors Flask-AppBuilder's @protect() decorator,
+ensuring MCP tools respect the same permission model as the REST API.
"""
import logging
@@ -42,6 +43,90 @@ F = TypeVar("F", bound=Callable[..., Any])
logger = logging.getLogger(__name__)
+# Constants for RBAC permission attributes (mirrors FAB conventions)
+PERMISSION_PREFIX = "can_"
+CLASS_PERMISSION_ATTR = "_class_permission_name"
+METHOD_PERMISSION_ATTR = "_method_permission_name"
+
+
+class MCPPermissionDeniedError(Exception):
+ """Raised when user lacks required RBAC permission for an MCP tool."""
+
+ def __init__(
+ self,
+ permission_name: str,
+ view_name: str,
+ user: str | None = None,
+ tool_name: str | None = None,
+ ):
+ self.permission_name = permission_name
+ self.view_name = view_name
+ self.user = user
+ self.tool_name = tool_name
+ message = (
+ f"Permission denied: {permission_name} on {view_name}"
+ + (f" for user {user}" if user else "")
+ + (f" (tool: {tool_name})" if tool_name else "")
+ )
+ super().__init__(message)
+
+
+def check_tool_permission(func: Callable[..., Any]) -> bool:
+ """Check if the current user has RBAC permission for an MCP tool.
+
+ Reads permission metadata stored on the function by the @tool decorator
+ and uses Superset's security_manager to verify access.
+
+ Controlled by the ``MCP_RBAC_ENABLED`` config flag (default True).
+ Set to False in superset_config.py to disable RBAC checking.
+
+ Args:
+ func: The tool function with optional permission attributes.
+
+ Returns:
+ True if user has permission or no permission is required.
+ """
+ try:
+ from flask import current_app
+
+ if not current_app.config.get("MCP_RBAC_ENABLED", True):
+ return True
+
+ from superset import security_manager
+
+ if not hasattr(g, "user") or not g.user:
+ logger.warning(
+ "No user context for permission check on tool: %s",
func.__name__
+ )
+ return False
+
+ class_permission_name = getattr(func, CLASS_PERMISSION_ATTR, None)
+ if not class_permission_name:
+ # No RBAC configured for this tool; allow by default.
+ return True
+
+ method_permission_name = getattr(func, METHOD_PERMISSION_ATTR, "read")
+ permission_str = f"{PERMISSION_PREFIX}{method_permission_name}"
+
+ has_permission = security_manager.can_access(
+ permission_str, class_permission_name
+ )
+
+ if not has_permission:
+ logger.warning(
+ "Permission denied for user %s: %s on %s (tool: %s)",
+ g.user.username,
+ permission_str,
+ class_permission_name,
+ func.__name__,
+ )
+
+ return has_permission
+
+ except (AttributeError, ValueError, RuntimeError) as e:
+ logger.warning("Error checking tool permission: %s", e)
+ return False
+
def load_user_with_relationships(
username: str | None = None, email: str | None = None
@@ -219,14 +304,15 @@ def mcp_auth_hook(tool_func: F) -> F: # noqa: C901
"""
Authentication and authorization decorator for MCP tools.
- This decorator pushes Flask application context and sets up g.user
- for MCP tool execution.
+ This decorator pushes Flask application context, sets up g.user,
+ and enforces RBAC permission checks for MCP tool execution.
- Supports both sync and async tool functions.
+ Permission metadata (class_permission_name, method_permission_name) is
+ stored on tool_func by the @tool decorator in core_mcp_injection.py.
+ If present, check_tool_permission() verifies the user has the required
+ FAB permission before the tool function runs.
- TODO (future PR): Add permission checking
- TODO (future PR): Add JWT scope validation
- TODO (future PR): Add comprehensive audit logging
+ Supports both sync and async tool functions.
"""
import contextlib
import functools
@@ -264,6 +350,16 @@ def mcp_auth_hook(tool_func: F) -> F: # noqa: C901
)
return await tool_func(*args, **kwargs)
+ # RBAC permission check
+ if not check_tool_permission(tool_func):
+ method_name = getattr(tool_func, METHOD_PERMISSION_ATTR,
"read")
+ raise MCPPermissionDeniedError(
+ permission_name=f"{PERMISSION_PREFIX}{method_name}",
+ view_name=getattr(tool_func, CLASS_PERMISSION_ATTR,
"unknown"),
+ user=user.username,
+ tool_name=tool_func.__name__,
+ )
+
try:
logger.debug(
"MCP tool call: user=%s, tool=%s",
@@ -294,6 +390,16 @@ def mcp_auth_hook(tool_func: F) -> F: # noqa: C901
)
return tool_func(*args, **kwargs)
+ # RBAC permission check
+ if not check_tool_permission(tool_func):
+ method_name = getattr(tool_func, METHOD_PERMISSION_ATTR,
"read")
+ raise MCPPermissionDeniedError(
+ permission_name=f"{PERMISSION_PREFIX}{method_name}",
+ view_name=getattr(tool_func, CLASS_PERMISSION_ATTR,
"unknown"),
+ user=user.username,
+ tool_name=tool_func.__name__,
+ )
+
try:
logger.debug(
"MCP tool call: user=%s, tool=%s",
diff --git a/superset/mcp_service/chart/tool/generate_chart.py
b/superset/mcp_service/chart/tool/generate_chart.py
index f09d8f89fdf..7cec6e9e71f 100644
--- a/superset/mcp_service/chart/tool/generate_chart.py
+++ b/superset/mcp_service/chart/tool/generate_chart.py
@@ -118,7 +118,7 @@ def _compile_chart(
return CompileResult(success=False, error=str(exc))
-@tool(tags=["mutate"])
+@tool(tags=["mutate"], class_permission_name="Chart")
@parse_request(GenerateChartRequest)
async def generate_chart( # noqa: C901
request: GenerateChartRequest, ctx: Context
diff --git a/superset/mcp_service/chart/tool/get_chart_data.py
b/superset/mcp_service/chart/tool/get_chart_data.py
index 9239a278d9e..96e7b1845ad 100644
--- a/superset/mcp_service/chart/tool/get_chart_data.py
+++ b/superset/mcp_service/chart/tool/get_chart_data.py
@@ -63,7 +63,7 @@ def _get_cached_form_data(form_data_key: str) -> str | None:
return None
-@tool(tags=["data"])
+@tool(tags=["data"], class_permission_name="Chart")
@parse_request(GetChartDataRequest)
async def get_chart_data( # noqa: C901
request: GetChartDataRequest, ctx: Context
diff --git a/superset/mcp_service/chart/tool/get_chart_info.py
b/superset/mcp_service/chart/tool/get_chart_info.py
index 95d0af23ff3..6c60214f705 100644
--- a/superset/mcp_service/chart/tool/get_chart_info.py
+++ b/superset/mcp_service/chart/tool/get_chart_info.py
@@ -56,7 +56,7 @@ def _get_cached_form_data(form_data_key: str) -> str | None:
return None
-@tool(tags=["discovery"])
+@tool(tags=["discovery"], class_permission_name="Chart")
@parse_request(GetChartInfoRequest)
async def get_chart_info(
request: GetChartInfoRequest, ctx: Context
diff --git a/superset/mcp_service/chart/tool/get_chart_preview.py
b/superset/mcp_service/chart/tool/get_chart_preview.py
index 6ee3ece4df4..579a2d748f5 100644
--- a/superset/mcp_service/chart/tool/get_chart_preview.py
+++ b/superset/mcp_service/chart/tool/get_chart_preview.py
@@ -2065,7 +2065,7 @@ async def _get_chart_preview_internal( # noqa: C901
)
-@tool(tags=["data"])
+@tool(tags=["data"], class_permission_name="Chart")
@parse_request(GetChartPreviewRequest)
async def get_chart_preview(
request: GetChartPreviewRequest, ctx: Context
diff --git a/superset/mcp_service/chart/tool/list_charts.py
b/superset/mcp_service/chart/tool/list_charts.py
index a3a72ad8166..3dacb3a7ec8 100644
--- a/superset/mcp_service/chart/tool/list_charts.py
+++ b/superset/mcp_service/chart/tool/list_charts.py
@@ -61,7 +61,7 @@ SORTABLE_CHART_COLUMNS = [
]
-@tool(tags=["core"])
+@tool(tags=["core"], class_permission_name="Chart")
@parse_request(ListChartsRequest)
async def list_charts(request: ListChartsRequest, ctx: Context) -> ChartList:
"""List charts with filtering and search.
diff --git a/superset/mcp_service/chart/tool/update_chart.py
b/superset/mcp_service/chart/tool/update_chart.py
index b0a099888e1..7abc1ef9da3 100644
--- a/superset/mcp_service/chart/tool/update_chart.py
+++ b/superset/mcp_service/chart/tool/update_chart.py
@@ -45,7 +45,7 @@ from superset.utils import json
logger = logging.getLogger(__name__)
-@tool(tags=["mutate"])
+@tool(tags=["mutate"], class_permission_name="Chart")
@parse_request(UpdateChartRequest)
async def update_chart(
request: UpdateChartRequest, ctx: Context
diff --git a/superset/mcp_service/chart/tool/update_chart_preview.py
b/superset/mcp_service/chart/tool/update_chart_preview.py
index 7ff3d2bc84a..117a92a6331 100644
--- a/superset/mcp_service/chart/tool/update_chart_preview.py
+++ b/superset/mcp_service/chart/tool/update_chart_preview.py
@@ -44,7 +44,7 @@ from superset.mcp_service.utils.schema_utils import
parse_request
logger = logging.getLogger(__name__)
-@tool(tags=["mutate"])
+@tool(tags=["mutate"], class_permission_name="Chart",
method_permission_name="write")
@parse_request(UpdateChartPreviewRequest)
def update_chart_preview(
request: UpdateChartPreviewRequest, ctx: Context
diff --git
a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
index 2ffec57f267..19f17c95a5e 100644
--- a/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
+++ b/superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
@@ -304,7 +304,7 @@ def _ensure_layout_structure(
layout["DASHBOARD_VERSION_KEY"] = "v2"
-@tool(tags=["mutate"])
+@tool(tags=["mutate"], class_permission_name="Dashboard")
@parse_request(AddChartToDashboardRequest)
def add_chart_to_existing_dashboard(
request: AddChartToDashboardRequest, ctx: Context
diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py
b/superset/mcp_service/dashboard/tool/generate_dashboard.py
index 8126ee387ca..61285c1a874 100644
--- a/superset/mcp_service/dashboard/tool/generate_dashboard.py
+++ b/superset/mcp_service/dashboard/tool/generate_dashboard.py
@@ -177,7 +177,7 @@ def _generate_title_from_charts(chart_objects: List[Any])
-> str:
return title
-@tool(tags=["mutate"])
+@tool(tags=["mutate"], class_permission_name="Dashboard")
@parse_request(GenerateDashboardRequest)
def generate_dashboard(
request: GenerateDashboardRequest, ctx: Context
diff --git a/superset/mcp_service/dashboard/tool/get_dashboard_info.py
b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
index de688ee8d8d..fa0df2bee14 100644
--- a/superset/mcp_service/dashboard/tool/get_dashboard_info.py
+++ b/superset/mcp_service/dashboard/tool/get_dashboard_info.py
@@ -59,7 +59,7 @@ def _get_permalink_state(permalink_key: str) ->
DashboardPermalinkValue | None:
return None
-@tool(tags=["discovery"])
+@tool(tags=["discovery"], class_permission_name="Dashboard")
@parse_request(GetDashboardInfoRequest)
async def get_dashboard_info(
request: GetDashboardInfoRequest, ctx: Context
diff --git a/superset/mcp_service/dashboard/tool/list_dashboards.py
b/superset/mcp_service/dashboard/tool/list_dashboards.py
index eaab375d6c8..78bfec2794a 100644
--- a/superset/mcp_service/dashboard/tool/list_dashboards.py
+++ b/superset/mcp_service/dashboard/tool/list_dashboards.py
@@ -62,7 +62,7 @@ SORTABLE_DASHBOARD_COLUMNS = [
]
-@tool(tags=["core"])
+@tool(tags=["core"], class_permission_name="Dashboard")
@parse_request(ListDashboardsRequest)
async def list_dashboards(
request: ListDashboardsRequest, ctx: Context
diff --git a/superset/mcp_service/dataset/tool/get_dataset_info.py
b/superset/mcp_service/dataset/tool/get_dataset_info.py
index 07333e837a5..59c3603a185 100644
--- a/superset/mcp_service/dataset/tool/get_dataset_info.py
+++ b/superset/mcp_service/dataset/tool/get_dataset_info.py
@@ -42,7 +42,7 @@ from superset.mcp_service.utils.schema_utils import
parse_request
logger = logging.getLogger(__name__)
-@tool(tags=["discovery"])
+@tool(tags=["discovery"], class_permission_name="Dataset")
@parse_request(GetDatasetInfoRequest)
async def get_dataset_info(
request: GetDatasetInfoRequest, ctx: Context
diff --git a/superset/mcp_service/dataset/tool/list_datasets.py
b/superset/mcp_service/dataset/tool/list_datasets.py
index 9404b93179d..1bdbcb381fd 100644
--- a/superset/mcp_service/dataset/tool/list_datasets.py
+++ b/superset/mcp_service/dataset/tool/list_datasets.py
@@ -61,7 +61,7 @@ SORTABLE_DATASET_COLUMNS = [
]
-@tool(tags=["core"])
+@tool(tags=["core"], class_permission_name="Dataset")
@parse_request(ListDatasetsRequest)
async def list_datasets(request: ListDatasetsRequest, ctx: Context) ->
DatasetList:
"""List datasets with filtering and search.
diff --git a/superset/mcp_service/explore/tool/generate_explore_link.py
b/superset/mcp_service/explore/tool/generate_explore_link.py
index 81e12974824..d60c0fd4d42 100644
--- a/superset/mcp_service/explore/tool/generate_explore_link.py
+++ b/superset/mcp_service/explore/tool/generate_explore_link.py
@@ -39,7 +39,7 @@ from superset.mcp_service.chart.schemas import (
from superset.mcp_service.utils.schema_utils import parse_request
-@tool(tags=["explore"])
+@tool(tags=["explore"], class_permission_name="Explore")
@parse_request(GenerateExploreLinkRequest)
async def generate_explore_link(
request: GenerateExploreLinkRequest, ctx: Context
diff --git a/superset/mcp_service/mcp_config.py
b/superset/mcp_service/mcp_config.py
index dab549b9b06..75221f854ef 100644
--- a/superset/mcp_service/mcp_config.py
+++ b/superset/mcp_service/mcp_config.py
@@ -45,6 +45,10 @@ MCP_SERVICE_PORT = 5008
# MCP Debug mode - shows suppressed initialization output in stdio mode
MCP_DEBUG = False
+# MCP RBAC - when True, tools with class_permission_name are checked
+# against the FAB security_manager before execution.
+MCP_RBAC_ENABLED = True
+
# MCP JWT Debug Errors - controls server-side JWT debug logging.
# When False (default), uses the default JWTVerifier with minimal logging.
# When True, uses DetailedJWTVerifier with tiered logging:
@@ -319,6 +323,7 @@ def get_mcp_config(app_config: Dict[str, Any] | None =
None) -> Dict[str, Any]:
"MCP_SERVICE_HOST": MCP_SERVICE_HOST,
"MCP_SERVICE_PORT": MCP_SERVICE_PORT,
"MCP_DEBUG": MCP_DEBUG,
+ "MCP_RBAC_ENABLED": MCP_RBAC_ENABLED,
**MCP_SESSION_CONFIG,
**MCP_CSRF_CONFIG,
}
diff --git a/superset/mcp_service/sql_lab/tool/execute_sql.py
b/superset/mcp_service/sql_lab/tool/execute_sql.py
index b9a79b54b28..88fad7d4414 100644
--- a/superset/mcp_service/sql_lab/tool/execute_sql.py
+++ b/superset/mcp_service/sql_lab/tool/execute_sql.py
@@ -50,7 +50,11 @@ from superset.mcp_service.utils.schema_utils import
parse_request
logger = logging.getLogger(__name__)
-@tool(tags=["mutate"])
+@tool(
+ tags=["mutate"],
+ class_permission_name="SQLLab",
+ method_permission_name="execute_sql_query",
+)
@parse_request(ExecuteSqlRequest)
async def execute_sql(request: ExecuteSqlRequest, ctx: Context) ->
ExecuteSqlResponse:
"""Execute SQL query against database using the unified Database.execute()
API."""
diff --git a/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py
b/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py
index d27b970d609..d9dc6bcd564 100644
--- a/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py
+++ b/superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py
@@ -38,7 +38,7 @@ from superset.mcp_service.utils.url_utils import
get_superset_base_url
logger = logging.getLogger(__name__)
-@tool(tags=["explore"])
+@tool(tags=["explore"], class_permission_name="SQLLab",
method_permission_name="read")
@parse_request(OpenSqlLabRequest)
def open_sql_lab_with_context(
request: OpenSqlLabRequest, ctx: Context
diff --git a/tests/unit_tests/mcp_service/conftest.py
b/tests/unit_tests/mcp_service/conftest.py
index 92868309552..fbb14f270df 100644
--- a/tests/unit_tests/mcp_service/conftest.py
+++ b/tests/unit_tests/mcp_service/conftest.py
@@ -18,6 +18,21 @@
"""
MCP service test configuration.
-Tool imports are handled by app.py, not here.
-This conftest is empty to prevent test pollution.
+Disables RBAC permission checks for integration tests.
+RBAC logic is tested directly in test_auth_rbac.py.
"""
+
+import pytest
+
+
[email protected](autouse=True)
+def disable_mcp_rbac(app):
+ """Disable RBAC permission checks for MCP integration tests.
+
+ The RBAC permission logic is tested directly in test_auth_rbac.py.
+ Integration tests use mock users that do not have real FAB roles,
+ so we disable RBAC to let them exercise tool logic.
+ """
+ app.config["MCP_RBAC_ENABLED"] = False
+ yield
+ app.config.pop("MCP_RBAC_ENABLED", None)
diff --git a/tests/unit_tests/mcp_service/test_auth_rbac.py
b/tests/unit_tests/mcp_service/test_auth_rbac.py
new file mode 100644
index 00000000000..3abc87be500
--- /dev/null
+++ b/tests/unit_tests/mcp_service/test_auth_rbac.py
@@ -0,0 +1,225 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Tests for MCP RBAC permission checking (auth.py)."""
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import g
+
+from superset.mcp_service.auth import (
+ check_tool_permission,
+ CLASS_PERMISSION_ATTR,
+ MCPPermissionDeniedError,
+ METHOD_PERMISSION_ATTR,
+ PERMISSION_PREFIX,
+)
+
+
[email protected](autouse=True)
+def enable_mcp_rbac(app):
+ """Re-enable RBAC for dedicated RBAC tests.
+
+ The shared conftest disables RBAC for integration tests. This fixture
+ overrides that so we can test the actual permission checking logic.
+ """
+ app.config["MCP_RBAC_ENABLED"] = True
+ yield
+ app.config.pop("MCP_RBAC_ENABLED", None)
+
+
+class _ToolFunc:
+ """Minimal callable stand-in for a tool function in tests.
+
+ Unlike MagicMock, this does NOT auto-create attributes on access,
+ so ``getattr(func, ATTR, None)`` properly returns None when the
+ attribute hasn't been set.
+ """
+
+ def __init__(self, name: str = "test_tool"):
+ self.__name__ = name
+
+ def __call__(self, *args, **kwargs):
+ pass
+
+
+def _make_tool_func(
+ class_perm: str | None = None,
+ method_perm: str | None = None,
+) -> _ToolFunc:
+ """Create a tool function stub with optional permission attributes."""
+ func = _ToolFunc()
+ if class_perm is not None:
+ setattr(func, CLASS_PERMISSION_ATTR, class_perm)
+ if method_perm is not None:
+ setattr(func, METHOD_PERMISSION_ATTR, method_perm)
+ return func
+
+
+# -- MCPPermissionDeniedError --
+
+
+def test_permission_denied_error_message_basic() -> None:
+ err = MCPPermissionDeniedError(
+ permission_name="can_read",
+ view_name="Chart",
+ )
+ assert "can_read" in str(err)
+ assert "Chart" in str(err)
+ assert err.permission_name == "can_read"
+ assert err.view_name == "Chart"
+
+
+def test_permission_denied_error_message_with_user_and_tool() -> None:
+ err = MCPPermissionDeniedError(
+ permission_name="can_write",
+ view_name="Dashboard",
+ user="alice",
+ tool_name="generate_dashboard",
+ )
+ assert "alice" in str(err)
+ assert "generate_dashboard" in str(err)
+ assert "Dashboard" in str(err)
+
+
+# -- check_tool_permission --
+
+
+def test_check_tool_permission_no_class_permission_allows(app_context) -> None:
+ """Tools without class_permission_name should be allowed by default."""
+ g.user = MagicMock(username="admin")
+ func = _make_tool_func() # no class_permission_name
+ assert check_tool_permission(func) is True
+
+
+def test_check_tool_permission_no_user_denies(app_context) -> None:
+ """If no g.user, permission check should deny."""
+ g.user = None
+ func = _make_tool_func(class_perm="Chart")
+ assert check_tool_permission(func) is False
+
+
+def test_check_tool_permission_granted(app_context) -> None:
+ """When security_manager.can_access returns True, permission is granted."""
+ g.user = MagicMock(username="admin")
+ func = _make_tool_func(class_perm="Chart", method_perm="read")
+
+ mock_sm = MagicMock()
+ mock_sm.can_access = MagicMock(return_value=True)
+ with patch("superset.security_manager", mock_sm):
+ result = check_tool_permission(func)
+
+ assert result is True
+ mock_sm.can_access.assert_called_once_with("can_read", "Chart")
+
+
+def test_check_tool_permission_denied(app_context) -> None:
+ """When security_manager.can_access returns False, permission is denied."""
+ g.user = MagicMock(username="viewer")
+ func = _make_tool_func(class_perm="Dashboard", method_perm="write")
+
+ mock_sm = MagicMock()
+ mock_sm.can_access = MagicMock(return_value=False)
+ with patch("superset.security_manager", mock_sm):
+ result = check_tool_permission(func)
+
+ assert result is False
+ mock_sm.can_access.assert_called_once_with("can_write", "Dashboard")
+
+
+def test_check_tool_permission_default_method_is_read(app_context) -> None:
+ """When no method_permission_name is set, defaults to 'read'."""
+ g.user = MagicMock(username="admin")
+ func = _make_tool_func(class_perm="Dataset")
+ # No method_perm set - should default to "read"
+
+ mock_sm = MagicMock()
+ mock_sm.can_access = MagicMock(return_value=True)
+ with patch("superset.security_manager", mock_sm):
+ result = check_tool_permission(func)
+
+ assert result is True
+ mock_sm.can_access.assert_called_once_with("can_read", "Dataset")
+
+
+def test_check_tool_permission_disabled_via_config(app_context, app) -> None:
+ """When MCP_RBAC_ENABLED is False, permission checks are skipped."""
+ g.user = MagicMock(username="viewer")
+ func = _make_tool_func(class_perm="Chart", method_perm="read")
+
+ app.config["MCP_RBAC_ENABLED"] = False
+ try:
+ assert check_tool_permission(func) is True
+ finally:
+ app.config["MCP_RBAC_ENABLED"] = True
+
+
+# -- Permission constants --
+
+
+def test_permission_prefix() -> None:
+ assert PERMISSION_PREFIX == "can_"
+
+
+def test_class_permission_attr() -> None:
+ assert CLASS_PERMISSION_ATTR == "_class_permission_name"
+
+
+def test_method_permission_attr() -> None:
+ assert METHOD_PERMISSION_ATTR == "_method_permission_name"
+
+
+# -- create_tool_decorator permission metadata --
+
+
+def test_permission_attrs_read_tag() -> None:
+ """Tags with class_permission_name set method_permission to read."""
+ func = _make_tool_func(class_perm="Chart", method_perm="read")
+ assert getattr(func, CLASS_PERMISSION_ATTR) == "Chart"
+ assert getattr(func, METHOD_PERMISSION_ATTR) == "read"
+
+
+def test_permission_attrs_write_tag() -> None:
+ """mutate tag convention → method_permission = 'write'."""
+ func = _make_tool_func(class_perm="Chart", method_perm="write")
+ assert getattr(func, CLASS_PERMISSION_ATTR) == "Chart"
+ assert getattr(func, METHOD_PERMISSION_ATTR) == "write"
+
+
+def test_permission_attrs_custom_method() -> None:
+ """Explicit method_permission_name overrides tag-based default."""
+ func = _make_tool_func(class_perm="SQLLab", method_perm="execute_sql")
+ assert getattr(func, CLASS_PERMISSION_ATTR) == "SQLLab"
+ assert getattr(func, METHOD_PERMISSION_ATTR) == "execute_sql"
+
+
+def test_no_class_permission_means_no_attrs() -> None:
+ """Without class_permission_name, no permission attrs are set."""
+ func = _make_tool_func()
+ assert not hasattr(func, CLASS_PERMISSION_ATTR)
+ assert not hasattr(func, METHOD_PERMISSION_ATTR)
+
+
+# -- Fixture --
+
+
[email protected]
+def app_context(app):
+ """Provide Flask app context for tests needing g.user."""
+ with app.app_context():
+ yield
