bito-code-review[bot] commented on PR #41858:
URL: https://github.com/apache/superset/pull/41858#issuecomment-4909671976
<!-- Bito Reply -->
The reviewer's suggestion to add a negative test case for non-admin users is
appropriate and would improve the test suite's rigor. Adding a parametrized
test that verifies `check_tool_permission` returns `False` for a user lacking
the required permissions (e.g., a viewer) would explicitly confirm the negative
path for these specific tools, complementing the existing positive test for the
Admin role.
**tests/unit_tests/mcp_service/test_auth_rbac.py**
```
@pytest.mark.parametrize(
"module_path,func_name",
[
("superset.mcp_service.user.tool.list_users", "list_users"),
("superset.mcp_service.user.tool.get_user_info", "get_user_info"),
("superset.mcp_service.role.tool.list_roles", "list_roles"),
("superset.mcp_service.role.tool.get_role_info", "get_role_info"),
],
)
def test_user_role_tools_denied_for_viewer(app_context, module_path,
func_name) -> None:
"""A viewer (holding no permissions) must be denied access to user/role
tools."""
import importlib
func = getattr(importlib.import_module(module_path), func_name)
g.user = MagicMock(username="viewer")
mock_sm = MagicMock()
mock_sm.can_access = MagicMock(return_value=False)
with patch("superset.mcp_service.auth.security_manager", mock_sm):
assert check_tool_permission(func) is False
```
--
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]
