map_meta_cap use "manage_network_users" instead of is_super_admin for edit_users

[sboisvert]

I find that for a multi-site setup, the ability to edit users is inconsistent. there is a capability called "manage_network_users", but this cap if given to "regular" admin's does not give them the power to edit users.
The current code in map_meta_cap in wp-includes/capabilities.php is:

case 'edit_users':
	// If multisite these caps are allowed only for super admins.
	if ( is_multisite() && !is_super_admin( $user_id ) )
		$caps[] = 'do_not_allow';

removing any flexibility, I would suggest having this based on the "manage_network_users" capability, giving us much more flexibility in terms of assigning power to control user administration.

[duck_]

Note that in terms of flexibility you can always hook into the map_meta_cap filter.

[chriscct7]

I agree with the idea here. Marking for core team to take a look at

[jeremyfelt]

I like the thought of this as well. I would much rather have explicit capabilities than rely on is_super_admin() for many things.

[jeremyfelt]

This could also be attached to delete_user and delete_users at the same time. create_users already uses the add_new_users network option rather than is_super_admin(), though it may benefit from a manage_network_users capability. Any work here helps to define a path for a sort of network admin role that is not a super admin.

[daniellandau]

Fixes issue for me

[daniellandau]

Show the "no confirmation" checkbox when user can manage_network_users

[daniellandau]

I need to give administrators in my site access to creating and editing users, but I don't want to give them super admin rights (e.g. I don't want them editing theme functions). I understand the need for a more thorough "network admin role", but for the time being these simple fixes do just the trick for me. If there is some security concern or similar that didn't cross my mind, I'd be happy to revise until something achieving the same effects is in acceptable form.

[jeremyfelt]

We should take a closer look at this for 4.3.

[johnbillion]

Related: #32394

[obenland]

@jeremyfelt, have you had a chance to take a closer look?

[daniellandau]

It would really be nice to get this moving forward. The patches are really not that big. It's okay if it doesn't make the 4.3 release, but some indication of wheels turning would be appreciated.

[jeremyfelt]

Thanks for the patch @daniellandau, I think we can make this happen in 4.4. It looks like the two patches should be combined into one for easy application/testing.

[daniellandau]

a single patch containing all modifications

[daniellandau]

Thanks for taking a look at this!

I uploaded a single patch containing all modifications.

[sboisvert]

Thanks for all the work on this everyone, especially Daniellandau, greatly appreciated.

[chriscct7]

Removed rogue line remove in the media-upload.php file from combined patch (the media-upload.php file was otherwise untouched by the patch)

[chriscct7]

Assignments to a milestone must be owned by someone

[chriscct7]

This should be documented in the Codex whenever this gets merged in.

[chriscct7]

Good to commit. Post-commit, someone should update the permissions docs to note the change in case people want to start utilizing manage_network_users

[jeremyfelt]

I'll take back some of what I said previously. Let's focus on edit_user/edit_users only for this ticket. We can cover create and delete elsewhere. I want to make sure we focus on what this changeset will be doing.

There's a security concern with the current patch in that any user given the manage_network_users capability can also manage super admins, including their passwords. Once that's changed...ouch.

We can check the specific user ID that is passed into map_meta_cap() and bail if that user ID itself is a super admin. I've done this in 16860.diff, though the conditional is pretty crazy.

I'd like to have unit tests attached to this as well. We'll get this in, but it definitely needs to be vetted quite a bit. I read through the other uses of edit_user and edit_users caps and things seem okay. What other possible consequences does this have?

[daniellandau]

Incorporates @jeremyfelt's security improvement and unit tests

[daniellandau]

I added a unit test (with a couple of assertions).

[jeremyfelt]

In 33987:

Multisite: Test edit_user capabilities for multisite administrators

An administrator in multisite can not edit users other than itself.

Props daniellandau for the initial patch.
See #16860.

[jeremyfelt]

In 33988:

Multisite: Allow users with manage_network_users to edit network users.

Other users in a network can now be given capabilities to manage users without also having global super admin privileges.

• Users with manage_network_users can not edit super admins.
• Users with manage_network_users can not promote users to super admin.
• Uses of is_super_admin() in user-new.php are now updated to manage_network_users.

Props daniellandau, chriscct7.
Fixes #16860.