#39206 closed enhancement (fixed)
Replace `is_super_admin()` with `current_user_can( 'manage_options' )` in wp-admin/network.php
Reported by: | ashokkumar24 | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 4.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Role/Capability | Keywords: | has-patch has-unit-tests has-dev-note |
Focuses: | multisite | Cc: |
Description (last modified by )
This is part of the #37616 task. There is 1 is_super_admin()
check in wp-admin/network.php that should be replaced with current_user_can( 'manage_options' )
.
Attachments (3)
Change History (15)
#1
@
8 years ago
- Description modified (diff)
- Keywords has-patch needs-refresh added
- Milestone changed from Awaiting Review to 4.8
- Owner set to flixos90
- Status changed from new to reviewing
- Summary changed from Replace current_user_can( 'manage_options' ) in wp-admin/network.php to Replace `is_super_admin()` with `current_user_can( 'manage_options' )` in wp-admin/network.php
- Type changed from defect (bug) to enhancement
#4
@
8 years ago
- Keywords 2nd-opinion needs-refresh added
I noticed that we might need to rethink our actions here. With the current patch, the capability required is always manage_options
. That means when a user on an already-setup multisite tries to access wp-admin/network/setup.php
, they can do so even if they are not a network administrator. So we need to change the check the following way:
- If a multisite, the user must have
manage_network
. - If not a multisite, the user must have
manage_options
.
There's a small room for discussion on how we implement this. I see two possible ways:
- Just make the clause a little more complex (as in
is_multisite() && ! current_user_can( 'manage_network' ) || ! is_multisite() && ! current_user_can( 'manage_options' )
). - Introduce a meta capability
setup_network
and handle the above check inmap_meta_cap()
.
I think the latter approach would be cleaner. Just as a reminder, we would need to add the new meta capability to the unit tests as well.
This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.
8 years ago
#7
@
8 years ago
- Keywords needs-patch needs-unit-tests added; has-patch 2nd-opinion needs-refresh removed
#8
@
8 years ago
I'd prefer a setup_network
meta capability (handled in map_meta_cap()
) which maps to the logic you mentioned.
Needs a patch, and the cap needs adding to Tests_User_Capabilities::_getSingleSiteMetaCaps()
and Tests_User_Capabilities::_getMultiSiteMetaCaps()
.
#9
@
8 years ago
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
39206.diff introduces a setup_network
cap as discussed. In single site, it falls back to manage_options
; in multisite, it falls back to manage_network_options
. While I originally proposed manage_network
as fallback, manage_network_options
is a better choice since that has been previously used as a capability for the menu item in multisite and aligns better with the single site fallback.
#10
@
8 years ago
- Keywords needs-dev-note added
Adding needs-dev-note
since this is a new capability that should be documented in the multisite dev note for 4.8.
#12
@
5 years ago
- Keywords has-dev-note added; needs-dev-note removed
This was detailed in the following dev note: https://make.wordpress.org/core/2017/05/22/multisite-focused-changes-in-4-8/
Thanks for the patch @ashokkumar24! You are using the
manage_network
capability there although it should bemanage_options
(see "Ticket 15" in https://core.trac.wordpress.org/ticket/37616#comment:28 for reference) - could you update the patch accordingly?