gkneighb opened a new pull request, #41858: URL: https://github.com/apache/superset/pull/41858
### SUMMARY Fixes a bug that makes four shipped MCP tools — `list_users`, `get_user_info`, `list_roles`, `get_role_info` — unusable for **every** principal, including Admin, on a stock instance. **Root cause.** The tools declare `class_permission_name="User"/"Role"` and rely on the `@tool` decorator's default method permission, `read`, so `check_tool_permission` demands `can_read on User` / `can_read on Role`. But those permission-view pairs can never exist: FAB's security API views (`UserApi`, `RoleApi`) register only `can_get / can_info / can_post / can_put / can_delete`. Superset's own `ModelRestApi` subclasses remap their methods onto `read`/`write` — which is why `can_read on Chart` exists — but the FAB security views get no such remapping. Since Admin only holds permissions that are *registered*, even Admin is denied (probe evidence from a stock instance: `"Permission denied: can_read on Role for user admin"`; the metadata DB holds 14 Admin grants on those views, none of them `can_read`). The failure is closed (deny), so this is not a security issue — just four dead tools. **Fix.** - Declare `method_permission_name="get"` on the four tools, matching the permission FAB actually registers for their views (`can_get` covers both the list and single-item GET endpoints). - Add `"get": "superset:read"` to `_METHOD_TO_REQUIRED_SCOPE` in `superset/mcp_service/auth.py` — that map fails closed for unmapped method permissions (by design), so without this entry scoped-JWT deployments would still see the tools denied. Its own doc comment mandates the addition: "When introducing a new method permission, add it here." `find_users` was also suspected but is **not** affected — it declares no `class_permission_name`, so it takes the documented allow-by-default path. Found via an automated REST↔MCP capability probe run against master `1c3b070d34`. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A (MCP service; no UI changes) ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/mcp_service/test_auth_rbac.py ``` New regression tests: a simulated stock Admin holding exactly the FAB-registered permission set (`can_get/can_info/can_post/can_put/can_delete`) must pass `check_tool_permission` for each of the four tools (fails before this change with `can_read`), plus an assertion that the `get` method permission is scope-mapped. Manual: on any instance with `MCP_RBAC_ENABLED = True` (the default), call `list_users` as Admin — before this change it returns "Permission denied: can_read on User"; after, it succeeds. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_014z1fQxTadTEvpCJLx9WnB8 -- 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]
