aminghadersohi opened a new pull request, #36546:
URL: https://github.com/apache/superset/pull/36546
### SUMMARY
This POC adds RBAC (Role-Based Access Control) permission enforcement to MCP
tools, addressing the gap where MCP commands bypass the action-level permission
checks that the REST API enforces via Flask-AppBuilder's `@protect()` decorator.
**The Problem:**
The MCP service checks data access permissions (via
`security_manager.can_access_datasource()` and
`security_manager.can_access_database()`), but it does NOT enforce action-level
RBAC permissions like the REST API does. This means:
- A user with database access but NO "can execute queries on SQLLab"
permission would:
- ❌ Be blocked by the REST API (403 Forbidden)
- ✅ Be allowed by MCP (query executes)
**The Solution:**
This PR introduces:
- **`MCPPermissionDeniedError`**: Custom exception for permission failures
- **`MCP_TOOL_PERMISSIONS`**: Registry mapping MCP tools to required
permissions (aligned with REST API's `class_permission_name`)
- **`check_tool_permission()`** and **`require_tool_permission()`**:
Functions to check/enforce permissions
- **Integration into `mcp_auth_hook`**: Permission checking now happens
automatically for all protected tools
- **`check_permissions` parameter**: Control to skip permission checking
when needed
The permission registry maps tools to the same permission/view combinations
used by Flask-AppBuilder:
- `execute_sql` → `("can_execute_sql", "SQLLab")`
- `list_charts` → `("can_read", "Chart")`
- `generate_chart` → `("can_write", "Chart")`
- etc.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Backend security feature
### TESTING INSTRUCTIONS
1. Run the new unit tests:
```bash
pytest tests/unit_tests/mcp_service/test_auth_rbac.py -v
```
2. To manually verify RBAC enforcement:
- Create a user with database access but NO SQLLab permissions
- Attempt to use `execute_sql` via MCP
- Should receive `MCPPermissionDeniedError` instead of successful
execution
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
**Note:** The complexity warnings from ruff are for the existing
`mcp_auth_hook` function structure, not new code. The function was already
complex; this PR adds permission checking to it.
--
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]
