'Add Existing' button should be dependent on is_multisite

[nacin]

if ( current_user_can( 'create_users' ) ) { ?>
	<a href="user-new.php" class="button add-new-h2"><?php echo esc_html_x( 'Add New', 'user' ); ?></a>
<?php } elseif ( current_user_can( 'promote_users' ) ) { ?>
	<a href="user-new.php" class="button add-new-h2"><?php echo esc_html_x( 'Add Existing', 'user' ); ?></a>
<?php }

I think there's an is_multisite() check missing on promote_users.

Noticed this while looking into ​http://wordpress.org/support/topic/create_users-capability-not-working?replies=5. I'm going to do some more investigating to make sure there aren't any other inconsistencies, especially when a user has create_users but *not* promote_users.

Speaking of, is there ever a time when a user should have create but not promote? Seems like a user with create_users should get promote_users automatically.

Moving to 3.1 for my own investigation, but low priority.

[linuxologos]

promote_users is somehow problematic (on single-site installs at least).

Let's say we'd like to give an editor some extra capabilities concerning users.

• list_users does just what it says. However there is something that could be enhanced: "Bulk Actions" and "Change role to..." dropdowns are visible to him, though he cannot use them.

• edit_users allows him to edit user accounts with an inferior role (author/contributor/subscriber).

• create_users does not work at all without promote_users (returns "You do not have sufficient permissions to access this page.")

• But giving promote_users grants the editor the capability to change the role of ANY account, admins included. He cannot edit the account directly, but the "Change role to..." dropdown is now fully functional and does not respect superior roles.

create_users currently needs promote_users to work, but promote_users has the above mentioned problem. These caps should be clarified and that issue taken into consideration.

I have not tested how add/delete/remove_users work.

[nacin]

Okay, found at least one regression.

• In 3.0 single site, you used to be able to add a user when you had create_users. Indeed you still can except for the user_can_access_admin_page() fallthrough. Adding a check in wp-admin/menu.php handles that.

• promote_users indeed is a very powerful capability. It shouldn't change.

• Some of the dropdowns and stuff should be hidden. Don't think that's a regression.

Working on a patch. Gonna test the various possibilities.

[nacin]

If you have create_users, then you should be able to have promote_users automatically. Do we agree on that? promote_users is a subset. I can't think of a situation where you should have create_users but not promote_users. I suppose the exception would be that if you can't promote users, then you shouldn't be able to set user roles in user-new, and instead the default one should be used.

Continuing to work on a patch to make this all work.

[westi]

Replying to linuxologos:

• But giving promote_users grants the editor the capability to change the role of ANY account, admins included. He cannot edit the account directly, but the "Change role to..." dropdown is now fully functional and does not respect superior roles.

There is no such thing as a superior role (apart from maybe Super Admin).

The ability to change the role has to be capability based - you shouldn't be giving this power to people you don't trust :)

[westi]

In any reworking of this you need to remember that even for a single site install users can exist in the users table that have no role on the current site - think shared user tables between multiple WordPresses or with bbPress, GlotPress, ...

[nacin]

Okay. Let's approach this a bit differently.

There are three distinct bugs here.

1. You can't create a user unless you have promote_users. This is not by design, and is a regression. Patch attached, the wp-admin/menu.php diff handles this.

2. The bulk role dropdown doesn't work if you don't have promote_users, but we show it anyway. This isn't a regression, but I suggest we fix it two-fold: One, add the cap check on display. Two, add promote_users to the promote action handler. (We check promote_user on each individual user, which maps to promote_users, but we should also check the main cap.)

3. You can change the role of a user -- either on creation (changing it from default_role), or on editing the individual user. This is not a regression, and I don't want to fix this at this point in the cycle. The problem is, anyone who didn't account for promote_users in a custom role in 3.0 would have had a broken bulk role dropdown, but everything else would have worked fine. Suddenly hiding role dropdowns on user edit or making them read-only could be a very bad idea. Especially in RC4 stage.

Incoming patch addresses points 1 and 2.

Next up, looking into what happens when you have promote_users but not create_users. I think we're good here with one exception, that the 'Add Existing' button shouldn't show up in single-site, because all existing users -- even those without a role -- are displayed when not running multisite.

[nacin]

4. Indeed, when you deny create_users, the 'Add Existing' button appears and you get a link to user-new.php. Only, you only see 'Add Existing User' -- the rest of it a blank form, as it is cap-checked, and the other form is multisite-checked.

Second patch also drops in the appropriate is_multisite() checks.

[nacin]

Flip-side now: Should the remove_user(s) branches in users.php also have multisite checks? Doubt there's anything dangerous there, but still.

[linuxologos]

Replying to nacin:

1. You can't create a user unless you have promote_users. This is not by design, and is a regression. Patch attached, the wp-admin/menu.php diff handles this.

Excellent. That's what I was more concerned about, since promote_users currently exposes Admins without good reason.

[nacin]

I gave MarkJaquith a walkthrough in IRC (posted below). Want ryan and westi to as well.

wp-admin/user.php: First piece, add an additional promote_users cap check before the promote_user meta cap checks in the post handler for bulk action promotions.

Second piece, don't show the 'Add Existing' button in single-site mode when you can't create new users. (All users are shown even if they don't have a blog, such as shared user tables.)

wp-users-list-table: Don't show the bulk actions if you can't promote_users. It doesn't work post-side.

wp-admin/user-new.php: Check my logic, but — deny if you don't have create_users. Exception is that on multisite, you can be missing create_users but have promote_users.

wp-admin/menu.php: We need user-new.php accessible for those who have create_users but not promote_users. Trick is to do a cap check then pass the right cap to $submenu.

[ryan]

(In [17431]) Add additional promote_users checks. Show Add existing button only for multisite. Don't show bulk actions if user can't promote users. Show add new user menu if user has create_users but not promote_users. Props nacin. fixes #16501 for trunk

[ryan]

(In [17432]) Add additional promote_users checks. Show Add existing button only for multisite. Don't show bulk actions if user can't promote users. Show add new user menu if user has create_users but not promote_users. Props nacin. fixes #16501 for 3.1

[linuxologos]

Replying to nacin:

Flip-side now: Should the remove_user(s) branches in users.php also have multisite checks? Doubt there's anything dangerous there, but still.

In single-site installs, if a user is given the remove_users cap but not the delete_users one, if he tries to remove a user through the Bulk Actions dropdown (does this really need to be visible?), he is sent to the "Remove Users from Site" screen. I don't know if this already is something that should not happen, but if he then presses "Confirm Removal" he gets an error like:

Fatal error: Call to undefined function remove_user_from_blog() in F:\xampplite\htdocs\31rc4\wp-admin\users.php on line 224

This applies to WP 3.0.5 too.

Fatal error: Call to undefined function remove_user_from_blog() in F:\xampplite\htdocs\305\wp-admin\users.php on line 238

(If the user has both remove_users and delete_users, the latter "overrides" the former and the problem is not obvious).

[nacin]

I think in single site mode, remove_user(s) should map to delete_user(s). Unlike promote/create, that seems clear cut.

Running some tests now.

[nacin]

16501.3.diff​ is an untested, more conservative approach that simply ignores the removal aspect in non-multisite. Realistically, it's a cap that a single-site admin doesn't need.

[ryan]

Looks good.

[ryan]

(In [17439]) Disallow and hide user removal (as opposed to deletion) for single site installs. Removal is a multisite concept. Props nacin. fixes #16501 for trunk

[ryan]

(In [17440]) Disallow and hide user removal (as opposed to deletion) for single site installs. Removal is a multisite concept. Props nacin. fixes #16501 for 3.1

[johnjamesjacoby]

Probably beating a dead horse (or the wrong one) but doesn't 'promote_users' insinuate the existence of 'demote_users'?

1. I have promote_users. I can take any user of a lesser role than mine, and bump them up to and including mine. I cannot demote any user.
2. I have demote_users. I can take any user equal to or lesser than mine, and bump them down to any lesser role. I cannot promote any user.
3. I have both. I can adjust the role of any user equal or lesser than mine either up or down.
4. Some new override capability would need to exist to allow promoting of users above your own role.

[nacin]

Keep in mind roles are not necessarily hierarchical, so that won't work, save actually comparing individual caps.

promote_users just means you can change the role of the user in general. You can change them to anything you'd like. The only restriction is you can't bulk-promote/demote yourself to a role that no longer has promote_users.