[wp-trac] [WordPress Trac] #8764: user-edit.php - use wp_dropdown_roles() rather than duplicating its code (security)

[WordPress Trac]

#8764: user-edit.php - use wp_dropdown_roles() rather than duplicating its code
(security)
--------------------------+-------------------------------------------------
 Reporter:  jeremyclarke  |       Owner:  jeremyclarke                        
     Type:  defect (bug)  |      Status:  new                                 
 Priority:  normal        |   Milestone:  2.8                                 
Component:  Security      |     Version:                                      
 Severity:  normal        |    Keywords:  has-patch needs-testing capabilities
--------------------------+-------------------------------------------------
 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.

 Part 1 was #8760, now commited. Part 2 is #8761, which is imortant for
 this patch.

 What I want (same as #8760): 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 particular patch is very simple. All it does is change the user
 editing page (user-edit.php, the one you use to edit the details of a
 specific user) to use the function wp_dropdown_roles(), which is already
 used on the user listing page users.php. The function is general and this
 is exactly the purpose it was designed for.

 In #8761 I added a filter to wp_dropdown_roles() that is needed to secure
 the edit_user capability by limiting the visible roles in dropdowns like
 this. To keep the security effects of this filter consistent and effective
 against malicious users (hackers exploiting accounts or malcontent users)
 the wp_dropdown_roles function must be used everywhere that roles are set
 for users.

 In terms of the code, I've removed most of the redundant looping logic and
 replaced it with wp_dropdown_roles. In the old version, it would loop
 through the capabilities and add selected=selected to any that matched the
 relevant user's roles. In the new version it instead passes in the primary
 (selected) role to wp_dropdown_roles as a parameter. The primary role is
 determined in teh same way as on the user listing page (i.e. this is how
 the text in the 'role' column of users.php is determined):


 {{{
 $user_roles = $profileuser->roles;
 $user_role = array_shift($user_roles);
 }}}

 That logic should be pushed into its own function imho (as stated in the
 comment) but i wanted to keep this ticket as simple as possible.

 For normal setups this patch should have zero noticeable effect.
 Everything should work like normal but the code is cleaner and more
 modular.

 For those that use my plugin code (hopefully part of Role Manager ASAP,
 should have been a long time ago), this will allow them to delegate
 user_edit privileges to multiple people, which is vital to large WP sites.
 For more infomation on seeing this in action and to test this patch with
 it, see #8761.

 Thanks for your consideration.

-- 
Ticket URL: <http://trac.wordpress.org/ticket/8764>
WordPress Trac <http://trac.wordpress.org/>
WordPress blogging software