WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 3 years ago

#37836 closed defect (bug) (fixed)

Cannot disable 'Add existing user'

Reported by: bseddon Owned by: johnbillion
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.1
Component: Users Keywords: needs-testing has-patch
Focuses: ui, administration, multisite Cc:

Description

The 'Add existing user' form on the Add New User page should only show if the user has been granted the 'promote_users' capability. At least I am inferring this is the intended purpose of the 'promote_users' capability because it's only used in wp-admin/users.php to restrict features relating to the addition of existing users.

However, it doesn't work because of the misplaced use of the variable $do_both.

$do_both is set to true if the installation is multi-site and the user has been granted both 'create_users' and 'promote_users' capabilities.

This variable is used incorrectly on line 307. Here it only prevents the display of the title of the form to add an existing user. I believe the variable $do_both should be added to the test on line 306 so the whole form, not just the heading, is displayed only when the user has been granted the 'promote_users' capability.

Attachments (3)

37836.1.patch (1.5 KB) - added by Mista-Flo 5 years ago.
Fix wrong display of "Add existing user" form when user has user_promote cap
37836.2.patch (1.4 KB) - added by Mista-Flo 5 years ago.
Better fix
37836.3.patch (2.3 KB) - added by Mista-Flo 5 years ago.
Fix if state with promote user cap test

Download all attachments as: .zip

Change History (15)

#1 @swissspidy
5 years ago

  • Keywords needs-patch needs-testing added
  • Version changed from 4.6 to 3.1

Introduced in [16294]

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


5 years ago

@Mista-Flo
5 years ago

Fix wrong display of "Add existing user" form when user has user_promote cap

#3 @Mista-Flo
5 years ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core-multisite by florian-tiar. View the logs.


5 years ago

#5 @Mista-Flo
5 years ago

Here's a second patch. Thanks to @fperdaan for the review.

Since the conditional test of $do_both is already made line 13 with the is_multisite, and user cap test, it's not necessary to test again with $both var, we only need to test if it's multisite.

By the way the code of this file is really hard to read, a refactoring would be a nice thing to do to avoid DRY code.

Last edited 5 years ago by Mista-Flo (previous) (diff)

@Mista-Flo
5 years ago

Better fix

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


5 years ago

#7 @flixos90
5 years ago

  • Keywords needs-refresh added

I think the problem here is only related to the usage of the promote_users capability, not the $do_both variable. As @bseddon says on the ticket description, the "Add Existing User" form should only show when the user has the promote_users capability, but it currently shows regardless.

Of course the headings missing is weird, but it's not the actual bug, since when the additional capability check is added, the headings are displayed correctly under all circumstances.

@Mista-Flo
5 years ago

Fix if state with promote user cap test

#8 @Mista-Flo
5 years ago

  • Keywords needs-refresh removed

It should be good now.

This ticket was mentioned in Slack in #core-multisite by florian-tiar. View the logs.


5 years ago

#10 @johnbillion
4 years ago

  • Milestone changed from Awaiting Review to 4.9
  • Owner set to johnbillion
  • Status changed from new to reviewing

#11 @johnbillion
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 41122:

Users: Don't show the 'Add Existing User' form on the 'Add New User' screen to users without the promote_users capability.

Props bseddon, Mista-Flo

Fixes #37836

#12 @jeremyfelt
3 years ago

#42249 was marked as a duplicate.

Note: See TracTickets for help on using tickets.