feat: Restrict superuser attributes to permission #29604
Conversation
Add the 'users.admin' permission and restrict superuser-style mutations on users to superusers which have this permission.
wedamija
left a comment
wedamija
left a comment
There was a problem hiding this comment.
I'm wondering if there's a way we can just build this into is_active_superuser so that we don't have to add these has_permission calls everywhere.
| # This is customizable for sentry.io, but generally should only be additive | ||
| # (currently the values not used anymore so this is more for documentation purposes) | ||
| SENTRY_USER_PERMISSIONS = ("broadcasts.admin",) | ||
| SENTRY_USER_PERMISSIONS = ("broadcasts.admin", "users.admin") |
There was a problem hiding this comment.
Wondering if we should just delete this? Unless you're planning on using it as part of your upcoming changes
There was a problem hiding this comment.
@wedamija i wanted to keep it as a way of documenting perms. it could have uses, though I dont know that it does yet today
|
@wedamija yeah probably good idea. i can f/u with refactoring and adding that in with another branch. probably need some test helpers to reduce boilerplate too. |
markstory
left a comment
markstory
left a comment
There was a problem hiding this comment.
Looks good to me. We don't use the isStaff, isSuperuser flags in the admin UI so they should be safe to hide from most staff users.
Would be nice if this were part of the permissions checks that endpoints do automatically. |
|
Also f/u need more audit trail on these changes |
Add the 'users.admin' permission and restrict superuser-style mutations on users to superusers which have this permission.