Replace is_super_admin() check with current_user_can( 'upgrade_database' )

[dhanendran]

This is part of the #37616 task. There is an is_super_admin() check which needs to be replaced with current_user_can( 'upgrade_database' ) in wp-admin/includes/ms.php (line 776)

[flixos90]

Thanks for the patch @dhanendran, looks good!

We'll need to have some discussion on how this new capability should be handled: Should we not handle it at all (meaning only the super admin has that capability) or should we add a clause in map_meta_cap() to grant that capability to regular admins on non-multisite setups as well? Is there another place in core where this capability could be used?

We definitely need to document the new capability in a dev note and add it to the unit tests array in Tests_User_Capabilities::_getMultiSiteMetaCaps().

[johnjamesjacoby]

Should we add a clause in map_meta_cap() to grant that capability to regular admins on non-multisite setups as well?

This is what I recommend we work towards. It's the most backwards compatible, and provides a filterable funnel for all database upgrade checks.

Is there another place in core where this capability could be used?

• Maybe wp_should_upgrade_global_tables()?
• Maybewp_upgrade()?
• Maybe some additional context could/should be passed in, so plugins can come to their own conclusions

[flixos90]

@johnjamesjacoby: I like the idea of having this capability check before upgrade routines. I think we should only check for this when upgrades happen via the UI though as I'm not quite sure about the consequences if we put such a check into functions like wp_upgrade() directly (for example, how does WP-CLI handle this?).

My approach would be to put current_user_can( 'upgrade_database' ) into wp-admin/network/upgrade.php, it could replace the existing manage_network check in order to be more precise. We might also be able to insert the same check into wp-admin/upgrade.php - the latter would depend on our map_meta_cap() approach for the function though: Currently everyone can access the wp-admin/upgrade.php screen and perform a database upgrade as far as I can see. We can either change that (probably not though), or we could say that on a non-multisite setup the capability maps to exist which everyone has. It could look like this (in map_meta_cap()):

case 'upgrade_database':
    if ( is_multisite() ) {
        $caps[] = 'manage_network';
    } else {
        $caps[] = 'exist';
    }

The only thing that would technically change is that in a multisite setup only the network administrator could access the wp-admin/upgrade.php page while currently everyone can. I don't really think the current (pretty much open) implementation makes sense, but I might be missing something.

Anyways, if we do anything with that capability beyond the existing patch (and unit tests), we should handle this in a separate ticket.

[flixos90]

As the general update routines in WordPress are publicly accessible (see #34200), let's only work in network upgrade scope here. I uploaded 39205.diff​ which I think is good to go, except for one major question: Since we're only dealing with upgrading the network, should we rather be calling the capability upgrade_network instead of the general upgrade_database?

[johnjamesjacoby]

Checking upgrade_network and passing in the network ID is an interesting idea. I like it.

[flixos90]

39205.2.diff​ uses a more specific upgrade_network capability, as mentioned above. That capability is also used for the menu page in the network administration panel, which was missed in the previous patch.

@johnjamesjacoby I like the idea of passing an ID to it, however we should follow these efforts in a separate ticket in the future where we can investigate more granular network capabilities in general. It should not be a problem to adjust the existing capabilities in a backward-compatible way, since we can always pre-fill with the current network if none is passed to it. Feel free to open one (it would fit well into this list: https://core.trac.wordpress.org/query?status=accepted&status=assigned&status=new&status=reopened&status=reviewing&id=29213%2C31020%2C33240%2C38652%2C39083%2C39084%2C39156%2C39677&col=id&col=summary&col=status&col=owner&col=priority&col=milestone&col=version&order=priority)!

[johnbillion]

I reviewed and tested this yesterday and it all looks good.

At WordCamp Torino we briefly discussed the idea of naming the cap update_network but soon started down a rabbit hole. upgrade_network is good.

[flixos90]

In 40404:

Multisite: Introduce an upgrade_network capability.

Prior to this change, a mix of is_super_admin() calls and manage_network capability checks was used to determine whether the current user could upgrade the network. With this changeset a dedicated capability is introduced that allows more granular handling.

Props dhanendran for the original patch.
Fixes #39205. See #37616.

[desrosj]

This was detailed in the following dev note: ​https://make.wordpress.org/core/2017/05/22/multisite-focused-changes-in-4-8/