WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#8760 closed defect (bug) (fixed)

wp-admin user listing: hide checkboxes unless user is editable

Reported by: jeremyclarke Owned by: jeremyclarke
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch needs-testing capabilities
Focuses: Cc:

Description

For history see: #6014

I'm updating that patch so it can be added to 2.8, but i'm splitting up the various parts so they can be added more easily.

What I want: To add security to the capabilities system because right now edit_users can't be delegated to non-admins (in our case our content editors). If someone has 'edit_users' they can make themself admin because nothing stops them from editing themselves or others to be admin. I think it should be integrated into core but don't care enough to fight. It can be done with a plugin so my priority is to make sure that my plugin (and Role Manager plugin) can hook into the appropriate places and add a role comparison such that wp only lets people edit users/roles "lower" than them (i.e. users that don't have any powers that the editor don't have).

This patch is for one bug in the system. If you have edit_users cap at all then you are given access to users.php, the user listing screen. Because you have that privilege and because normally edit_users means edit *any* user right now the page just shows a checkbox next to every single user. Instead it should determine the presence of a checkbox based on a current_user_can('edit_user', $user_id) check for the user in that row.

Usually this would do nothing, but the current_user_can('edit_user', $user_id) check is where my plugin does the checking, and so it will block the checkbox for users with more caps than you. It's also just plain logical and good API practice to add this check.

Luckily the user_row() function is already doing that check to determine whether to make the username a link and whether to show the edit link on hover. This patch just adds the $checkbox definition inside the if ( current_user_can( 'edit_user', $user_object->ID ) ) so that both elements are determined by the check.

This is pretty non-controversial imho, and can be implemented easily even if my other changes are denied.

Feedback welcome, thanks.

Attachments (1)

just-checkbox-changes.diff (1.9 KB) - added by jeremyclarke 5 years ago.
checkbox patch 2.8 trunk dec30-08

Download all attachments as: .zip

Change History (2)

jeremyclarke5 years ago

checkbox patch 2.8 trunk dec30-08

comment:1 ryan5 years ago

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

(In [10268]) Show actions and checkbox only if user is editable by current user. Props jeremyclarke. fixes #8760

Note: See TracTickets for help on using tickets.