Opened 7 years ago
Last modified 6 weeks ago
#47338 new defect (bug)
is_super_admin() should check a different capability
| Reported by: | lllor | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Role/Capability | Version: | |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: | administration, multisite |
Description
Currently is_super_admin() returns true in case the user has the delete_users cap (in case of a single site).
Since admins may want to delegate users managemente capability, IMHO a more appropriate capability to check is 'activate_plugins' or, better, check more than a single capability.
Attachments (1)
Change History (12)
#3
@
7 years ago
@SergeyBiryukov
While I get why you assigned this ticket to multisite, in fact this ticket isn't multisite related.
For multisite installations is_super_admin doesn't check capabilities. This only happens on single installations.
This ticket was mentioned in PR #7797 on WordPress/wordpress-develop by @geekofshire.
22 months ago
#4
- Keywords has-patch added; needs-patch removed
This PR updates the is_super_admin function by replacing the delete_users capability check with manage_network_options. This change provides a more accurate way to verify super admin status in multisite setups, as manage_network_options is exclusively available to super admins. This improves role accuracy and ensures the function's logic aligns better with WordPress’s intended permissions structure.
Trac ticket: https://core.trac.wordpress.org/ticket/47338
#5
@
6 weeks ago
Our team runs a number of WordPress websites, both single site and multisite installs. On all of these, we have a custom user role ("editor plus") that inherits all capabilities an editor has, and adds user management (with some restrictions).
On single site installs, this issue causes is_super_admin to return true for users with our custom role. This can cause security issues when the check is not combined with is_multisite or some other additional check.
I've added a patch that simply changes the checked capability from delete_users to manage_options. This is the most commonly recommended (and most logical) capability to check for to see if someone is an administrator on single site installs.
The documentation for is_super_admin says "Determines whether user is a site admin.", so on single sites, it should return true for regular administrators. Checking for manage_network_options as previously suggested does this in my testing, but I feel it makes more sense to check for a "single site-native" capability here.
This is a very small patch that provides a solution until #37616 is perhaps eventually revived.
#6
follow-up:
↓ 7
@
6 weeks ago
- Focuses administration added
Might even be an option to check both. When you can manage all site options but are still not allowed to edit/delete users I wouldn't consider you a super admin. I maintain several instances where this is the case for security reasons.
#7
in reply to: ↑ 6
;
follow-up:
↓ 8
@
6 weeks ago
Replying to keraweb:
Might even be an option to check both. When you can manage all site options but are still not allowed to edit/delete users I wouldn't consider you a super admin. I maintain several instances where this is the case for security reasons.
This is obviously debatable, but I think I would consider such a user to be an administrator. And I would expect is_super_admin to return true for them on single site installs. From there, in your case, I'd check for more specific capabilities when determining access to user management functionality.
For me, if you're able change things like a site's name, design, language, etc, that makes you a site's administrator.
#8
in reply to: ↑ 7
;
follow-up:
↓ 9
@
6 weeks ago
Replying to bdwebteam006:
Replying to keraweb:
Might even be an option to check both. When you can manage all site options but are still not allowed to edit/delete users I wouldn't consider you a super admin. I maintain several instances where this is the case for security reasons.
This is obviously debatable, but I think I would consider such a user to be an administrator. And I would expect
is_super_adminto return true for them on single site installs. From there, in your case, I'd check for more specific capabilities when determining access to user management functionality.
For me, if you're able change things like a site's name, design, language, etc, that makes you a site's administrator.
If a user cannot manage access then it's not a super admin. A super admin, by design, has all access (just like in a multisite). Therefore I wouldn't remove the delete_users cap check as this is a vital part of having all access, same as activate_plugins, which makes even more sense than manage_options.
We also need to consider backwards compatibility on plugins/scri[ts that expect the delete_users cap.
In any case, this is up to the core maintainer to decide.
#9
in reply to: ↑ 8
@
6 weeks ago
Replying to keraweb:
If a user cannot manage access then it's not a super admin. A super admin, by design, has all access (just like in a multisite).
I think it's important to remember that this change only affects the single site fallback of the is_super_admin function. Single site does not have the concept of a "super admin". On single site it (by design) simply checks for regular admins. The recommended way to do this is to check for manage_options.
This ticket was mentioned in Slack in #core by roytanck. View the logs.
6 weeks ago
#11
@
6 weeks ago
Hi @bdwebteam006
I fully understand that this is single site only and therefore I think it's even more important to think this through carefully. This change could introduce a security loophole for users with the manage_options capability.
There can be situations where a user has the manage_options capability but not the delete_users capability, perhaps not even the administrator role.
By applying your patch it could unintentionally create a loophole to manage other users even though the user isn't an admin (other roles can also have the manage_options capability).
For this reason I think a multi-cap validation is the best and most secure way forward.
Personally I would keep the delete_users cap and add both manage_options and activate_plugins.
This combination of capabilities would IMO reflect a very super-admin-"like" combination and are all by default applied for the administrator role so no backwards compatibility issues would arise.
Other scripts/plugins/core-code what uses is_super_admin() (perhaps wrongly) won't suddenly return very different results, it would only be more secure.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Related: #37616