aminghadersohi commented on code in PR #40098:
URL: https://github.com/apache/superset/pull/40098#discussion_r3277397509
##########
superset/mcp_service/server.py:
##########
@@ -403,38 +400,23 @@ def _summary_serializer(tools: Sequence[Any]) ->
list[dict[str, Any]]:
def _tool_allowed_for_current_user(tool: Any) -> bool:
"""Return whether the current Flask user can see this tool in search
results."""
try:
- from flask import current_app, g
-
- if not current_app.config.get("MCP_RBAC_ENABLED", True):
- return True
+ from flask import g
- from superset import security_manager
from superset.mcp_service.auth import (
- CLASS_PERMISSION_ATTR,
get_user_from_request,
- METHOD_PERMISSION_ATTR,
- PERMISSION_PREFIX,
+ is_tool_visible_to_current_user,
)
- tool_func = getattr(tool, "fn", None)
- if tool_requires_data_model_metadata_access(tool_func) and not (
- user_can_view_data_model_metadata()
- ):
- return False
-
- class_permission_name = getattr(tool_func, CLASS_PERMISSION_ATTR, None)
- if not class_permission_name:
- return True
-
if not getattr(g, "user", None):
try:
g.user = get_user_from_request()
- except ValueError:
- return False
+ except (ValueError, PermissionError):
+ # Can't resolve user; only hide protected tools. Public tools
+ # (no _class_permission_name) pass through regardless.
+ func = getattr(tool, "fn", tool)
+ return not getattr(func, "_class_permission_name", None)
Review Comment:
Fixed in 9ca3f8ec01. Split the combined `except (ValueError,
PermissionError)` into two handlers: `PermissionError` now returns `False`
(deny-all, matching `RBACToolVisibilityMiddleware`'s fail-closed path), while
`ValueError` retains the public-tool fallback for the no-auth-configured case.
Added a regression test
`test_tool_search_permission_filter_denies_all_on_invalid_credentials` to cover
this path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]