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 33dbd233df17c9378a1f526290dca6471f65d5ca Author: Amin Ghadersohi <[email protected]> AuthorDate: Fri May 15 00:39:07 2026 +0000 fix(mcp): fix CI pre-commit failures for RBAC tool visibility - auth.py: collapse check_tool_permission signature to one line (ruff-format) - auth.py: extract _log_user_resolution_failure() helper to reduce _setup_user_context cyclomatic complexity from 11 to 10 (ruff C901) - test_middleware.py: shorten docstring to stay within 88-char limit (ruff E501) --- superset/mcp_service/auth.py | 31 ++++++++++++++----------- tests/unit_tests/mcp_service/test_middleware.py | 2 +- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/superset/mcp_service/auth.py b/superset/mcp_service/auth.py index 455634b0b81..e91d25fe414 100644 --- a/superset/mcp_service/auth.py +++ b/superset/mcp_service/auth.py @@ -88,9 +88,7 @@ class MCPPermissionDeniedError(Exception): super().__init__(message) -def check_tool_permission( - func: Callable[..., Any], *, log_denial: bool = True -) -> bool: +def check_tool_permission(func: Callable[..., Any], *, log_denial: bool = True) -> bool: """Check if the current user has RBAC permission for an MCP tool. Reads permission metadata stored on the function by the @tool decorator @@ -499,6 +497,21 @@ def check_chart_data_access(chart: Any) -> "DatasetValidationResult": return validate_chart_dataset(chart, check_access=True) +def _log_user_resolution_failure(exc: ValueError) -> None: + """Log a user-resolution ValueError at the appropriate level. + + "No authenticated user found" is expected in unauthenticated/dev + deployments (no JWT, no API key, no MCP_DEV_USERNAME configured) and + during tools/list scanning — log at DEBUG to avoid ERROR noise. + All other ValueErrors (e.g. dev username not in DB) are genuine + credential failures and are logged at ERROR. + """ + if "No authenticated user found" in str(exc): + logger.debug("MCP: no auth source configured, unauthenticated request") + else: + logger.error("MCP user resolution failed, denying request: %s", exc) + + def _setup_user_context() -> User | None: """ Set up user context for MCP tool execution. @@ -564,17 +577,7 @@ def _setup_user_context() -> User | None: # proceed as a different user in multi-tenant deployments. # Clear g.user so error/audit logging doesn't attribute # the denied request to the middleware-provided identity. - # - # "No authenticated user found" means no auth source is configured - # at all (no JWT, no API key, no MCP_DEV_USERNAME). This is the - # expected state in unauthenticated/dev deployments and at - # tools/list time — log at DEBUG to avoid ERROR noise in those - # environments. All other ValueErrors (e.g. dev username not in DB) - # are genuine credential failures and are logged at ERROR. - if "No authenticated user found" in str(e): - logger.debug("MCP: no auth source configured, unauthenticated request") - else: - logger.error("MCP user resolution failed, denying request: %s", e) + _log_user_resolution_failure(e) if has_request_context(): g.pop("user", None) raise diff --git a/tests/unit_tests/mcp_service/test_middleware.py b/tests/unit_tests/mcp_service/test_middleware.py index b7fb77063b7..00e3f9457f0 100644 --- a/tests/unit_tests/mcp_service/test_middleware.py +++ b/tests/unit_tests/mcp_service/test_middleware.py @@ -1215,7 +1215,7 @@ 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 (ValueError).""" + """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")]
