GitHub user EnxDev added a comment to the discussion: Restriction of User and 
Role Views to Admins Only

Hey @jon-coffey, thanks for raising this and sorry for the confusion! Let me 
share some context.

The Admin-only restriction on these views actually predates the React 
migration. It's enforced in the backend via 
[`ADMIN_ONLY_VIEW_MENUS`](https://github.com/apache/superset/blob/master/superset/security/manager.py#L439)
 in `SupersetSecurityManager`, which strips the relevant PVMs (`List Users, 
List Roles, List Groups, UsersListView, RoleModelView,` etc.) from Alpha/Gamma 
roles during `superset init`. 
The original FAB views were already gated this way through `@has_access ` they 
weren't accessible to non-admins via granular permissions either. 
The granular permissions still exist as PVM rows in the DB, but they're 
effectively unassignable to non-admin roles because of that filter.

What [PR #32432](https://github.com/apache/superset/pull/32432) (and the 
follow-up Users/Groups migrations) added was a frontend `isAdmin` guard at 
routes.tsx#L351 for route registration, mirroring the existing backend 
behavior. 
The new Python views still gate correctly via `@has_access` + 
`permission_name("read")` on class_permission_name = "security", so the backend 
story is consistent  but you're absolutely right that the user-facing behavior 
is "Admin-only."

To enable what you're describing (and what @rusackas suggested), we'd need 
changes in two places:

1. **Backend** — selectively remove only the read PVMs from 
ADMIN_ONLY_VIEW_MENUS / ADMIN_ONLY_PERMISSIONS, keeping all write/mutation PVMs 
admin-only:

- Allow `can read` on `User`, `Role`, `Group`, `UsersListView`, 
`RoleModelView`, `UserGroupModelView` to be assignable to non-admin roles (so a 
"User Manager" role can list users/roles/groups).
- **Keep admin-only**: `can write`, `can edit`, `can delete`, `can add`, `can 
update_role`, `update_roles_users`, `list_roles` on these same view-menus, plus 
the REST API write endpoints. 
Without constraint logic preventing privilege escalation (e.g. a sub-admin 
granting permissions they don't hold, or assigning the Admin role), write 
access stays gated.
- Add the now-removable read PVMs to `GAMMA_EXCLUDED_PVMS` (and the alpha 
equivalent) so they aren't auto-granted to Alpha/Gamma on `superset ini`t — 
they should only land on roles where an admin explicitly assigns them.

2. **Frontend** — replace the `isAdmin` route guard in `routes.tsx#L351` with a 
permission check like `findPermission('can_read', 'security', user)` (or 
whichever granularity we land on with maintainers). 
Same swap in the page-level guards in UsersList/index.tsx, RolesList/index.tsx, 
GroupsList/index.tsx. 
Crucially, the page components also need to conditionally hide write actions 
(Add Role, Edit, Delete, Add User, etc.) based on the corresponding can_write / 
can_edit permissions  so a read-only user manager sees the lists but no 
mutation buttons.

Out of scope for this first pass (worth flagging in the PR description): full 
write access for sub-admins. That requires new constraint logic in 
`SupersetSecurityManager` e.g. preventing a non-admin from assigning 
permissions they don't themselves hold, or from granting the Admin role — which 
is a separate design discussion.

Happy to collaborate on a PR if you want to take a stab at it I'd be glad to 
review. 
We should probably also align with maintainers e.g. @dpgaspar  on whether 
`can_read` on security is the right granularity, or whether we want separate 
perms for users / roles / groups views.

GitHub link: 
https://github.com/apache/superset/discussions/44409#discussioncomment-18489842

----
This is an automatically sent email for [email protected].
To unsubscribe, please send an email to: 
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to