Make WordPress Core

Opened 14 years ago

Last modified 23 months ago

#14986 new enhancement

Make WordPress roles/capabilities more secure (edit_users related)

Reported by: otto42's profile Otto42 Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Role/Capability Keywords: needs-patch
Focuses: Cc:

Description

We've discussed this before, but after some thought, I think we can do this and make it work.

Right now, the edit_users capability is the key to the kingdom. Anybody with edit_users can change their role to anything, including to something with more capabilities than they already have.

Back in #6908 and #6014, this was thought about in terms of the old user levels system, which of course shouldn't be used and makes no sense.

In #8770, an editable_roles filter was introduced, which allows a plugin to limit the roles that a user can change themselves too. This works for one aspect of the problem, but a) it doesn't solve the passwords problem(1), and b) it assumes that the roles are still only in one chain of command. That is to say, that all the roles have an ordering, where each role has all the capabilities of the role "below" it.

To solve these, I think we need a capability comparison system. To wit, code that implements these two rules:

  1. No user can change the role of another user to a role that has capabilities that the changing user does not also have.
  1. No user can change either the role or the password of another user who has any capability that the changing user does not also have.

For rule 1, this means that if Adam was to try to assign Bob a role, he would only be given the choice of assigning roles that have a subset of the capabilities Adam himself had.

For rule 2, if Adam was to try to change the role or the password of Bob, he would not be able to change either unless Bob already had a subset of Adam's own capabilities.

This makes the roles have a sort of definable hierarchy, where roles with lesser capabilities can be multi-faceted. I can define more than one chain of roles which are "above" each other in hierarchy, allowing me to build a tree of groups and users capable of different things.

For things like bbPress-as-a-plugin, this sort of enforcement is going to be a must-have feature.

Note 1: The "passwords problem" is that any user who can change another users password can easily change the password of somebody of "higher" rank, log in as them, and then have their capabilities. It's detectable, but in some cases, may not be so detectable. Admins who don't log in often, say.

Note 2: We also need role management in core, but that's a topic for a separate day. I'm speaking only of enforcement of a security model here for now.

Change History (8)

#1 @scribu
14 years ago

  • Milestone changed from Awaiting Review to Future Release

We already have a promote_users capability, so we should try to make use of it somehow.

Anyway, I think this will be easier to handle once we do the role cleanup we keep talking about.

#2 @nacin
14 years ago

Agreed with scribu.

The end all, be all capability is currently remove_users. That is, that is how we define is_super_admin() for non-multisite installs.

#3 @RavanH
14 years ago

Wow, has this or something like it been introduced in 3.0.4 maybe?

Since upgrading a Multi-site install, I noticed that as site admin one cannot edit other users accounts (Editors and Authors) anymore while super-admin has no problems.

Same goes for managing plugins like User Role Editor and Adminimize ...

#4 @designbymerovingi
14 years ago

My solution to this issue is to add && current_user_can('promote_users') on line 229 (wp 3.0.3) to solve the "editors should not promote" problem. Why are we not utilizing promote_users more?

#5 @scribu
14 years ago

Line 229 of which file?

#6 @designbymerovingi
14 years ago

sorry about that user-edit.php in /wp-admin/

#7 @boxcarpress
10 years ago

Is it still the case that users with 'edit_users' capability can elevate themselves? Does WP rely on 'promote_users' except for multisite?

#8 @chriscct7
9 years ago

  • Keywords needs-patch added
Note: See TracTickets for help on using tickets.