WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 5 years ago

#2787 closed enhancement (wontfix)

New Method of storing and calculating cap2user and user2cap

Reported by: markjaquith Owned by: jacobsantos
Milestone: Priority: normal
Severity: normal Version: 2.1
Component: Role/Capability Keywords: roles capabilities
Focuses: Cc:

Description (last modified by markjaquith)

This idea came from a conversation I had with Ryan in #wordpress-dev

The Problem

The role/cap system is hindered by having much of its data buried in arrays. User2cap is ridiculous.

The Solution

Roles are meaningless. Getting users who have role X proves nothing, because they could have extra capabilities. Capabilities are the thing you want. 'Roles are just a capability container... a short way of granting a bunch of capabilities to a user.' Once you realize that, you see that what you really need is a cap2users table, that could double as a users2cap table. That gives you one-query access to "what users can do X?" and "What can user X do?"

How Roles Fit In

In order for the role2cap table to be correct, it would need to be updated whenever:

  • A user is given an extra capability
  • A user is stripped of an extra capability
  • A role is given an extra capability
  • A role is stripped of an extra capability
  • A user is moved to a different role

So we'd need a solid API for this. This is the heavy lifting... done only when something is changed on the back end (infrequent).

Schema

  • $wpdb->user2cap (per blog)
    • u2cid
    • user_id
    • cap_id
    • extra_cap

extra_cap would be a binary flag. Basically, it would say whether or not this cap is associated with a role or not. It would be used on the backend. The scenario is this:

User gets granted an extra cap, foo_bar. Then, they get upgrade to a new role that includes foo_bar. The "extra_cap" flag stays on, because they were granted this capability specifically. If they are then subsequently downgraded in roles, or if the role is edited to now have foo_bar cap anymore, they keep foo_bar cap, because it is an extra role. To strip users of extra roles, you should have to specifically take it away. Changing their role, or editing the role they have shouldn't mess with their extra caps.

  • $wpdb->usermeta (multiple blogs can share this)
    • wp_role => Administrator
    • wp_otherblog_role => Garbage Collector
    • wp_otherblog_role => Little League Coach

Note that a usermeta table can have roles for that user on different blogs and that there can be multiple roles for each user, even on the same blog. That just means that cap2user has all the capabilities of all the roles that the user has, along with all extra caps (marked with extra_cap = 1).

The array that stores the Role => Cap information could stay as an array. It would only be used in API functions on the back end.

Original conversation

MarkJaquith: what do you think about doing all the heavy lifting on the back end?
[6:32pm] MarkJaquith: like... just have user2cap
[6:33pm] MarkJaquith: don't grab the roles live when doing current_user_can() or users_with_cap()
[6:33pm] MarkJaquith: calculate that when a user is changed or when roles are changed, and populate user2cap
[6:34pm] MarkJaquith: who really cares if editing a user takes 4 queries... it's seldom done.
[6:34pm] rboren: That's fine.
[6:34pm] MarkJaquith: Roles are meaningless anyway
[6:34pm] MarkJaquith: this tells people to focus on capabilities
[6:34pm] MarkJaquith: define their role in usermeta.  keep role2caps in an array, it's only used when updating the user.
[6:35pm] rboren: We should be able to retrieve all users with a given cap though.
[6:35pm] rboren: We don't do that very well now.
[6:35pm] MarkJaquith: user2cap doubles as cap2user, right?
[6:36pm] rboren: It can.
[6:36pm] MarkJaquith: SELECT DISTINCT user_id FROM $wpdb->cap2user WHERE cap = 'foo';
[6:37pm] rboren: As long as cap2user contains the caps inherited from the role.
[6:37pm] MarkJaquith: yes... calculated when things are changed
[6:37pm] rboren: We'd have to make sure that a role change updated all affected users too.
[6:37pm] MarkJaquith: yep.  we'd need that to be solid
[6:38pm] MarkJaquith: And we'd need to deactivate older versions of Owen's Role Manager... they could really screw things up with that
[6:38pm] rboren: Okay.  That sounds like a good possibility.

Paging Owen... your feedback is important on this one.

Change History (22)

comment:1 markjaquith8 years ago

  • Description modified (diff)
  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

comment:2 markjaquith8 years ago

Hm, Usermeta API doesn't support multiple entries with the same Key, like postmeta does. That's a bummer, but it'd not a big deal in this case. Since the lifting is done on the backend, we could just have the roles in an array in usermeta, one for each blog.

comment:3 ringmaster8 years ago

Using this system, which does not explicitly deny caps:

  1. User X, is assigned role A containing cap C.
  2. Remove ("Deny") cap C from user X.
  3. Switch user to use role B having cap C.
  4. User X is once again granted cap C, in spite of previous denial.

Maybe add a new column, "grant", that could be set to 0 to deny a cap on a per-user basis?

Also- Using the current system, you can be assinged more than one role. We'd need to determine what cap came from what role, perhaps rebuilding all of a user's caps whenever caps are set. Assuming we'd like to keep that functionality.

FYI, I started moving the Role Manager as a core patch last week, but got hung up trying to use mdawaffe's admin ajax madness. The only thing working at this point is the display of role caps.

comment:4 mdawaffe8 years ago

All of the following is off topic.

ringmaster, madness is a good description. There's some sketchy documentation here: http://codex.wordpress.org/User:MDAWaffe/List_Manipulation , but it doesn't really cover the stuff you actually need to *know* in order to make stuff work.

One thing you need to keep in mind is the way the JS interacts with the PHP. You need to give explicit IDs to each od your list items of the form: what-# (e.g. post-12 or category-17). Then, when you run ajaxAdder, ajaxDimmer, ajaxDelete or ajaxUpdate the PHP script looks for the add-what, dim-what, delete-what, update-what action case. You've mentioned before that this should all be controlled by hook firing. You're right.

All that said, does Role Manager need to be in the core at all?

comment:5 markjaquith8 years ago

Owen,
I like your "grant" column idea.

I don't think that it matters that users can have multiple roles. I don't think we need to know which caps came from which role.

Check out this example:

Role "Admin" has caps "Apple," "Banana" and "Tomato"
Role "Editor" has caps "Banana," "Pear" and "Tomato"

Lets give a user both roles, so they get caps of "Apple," "Banana," "Pear" and "Tomato"

Now, let's grant them the extra cap of "Mango", and deny them the cap of "Tomato"

so right now, they have 3 role-generated caps: Apple, Banana and Pear. They have one extra cap of "Mango," and one denied cap of "Tomato"

Now, we delete the Editor role. All we do is recalculate the caps, and we end up only losing the "Pear" cap.

The pseudo code process works like this:

  • Add all caps associated with the user's roles, with no duplicates
  • If the user is granted an extra cap, add it with the extra_cap flag
  • If the user is denied a cap that they already have, switch the "grant" column to "0"
  • If the user is denied a cap that they don't have, add the cap with the extra_cap flag and the "grant" column set to "0"

That's the initial setup. When deleting a role, you merely flush out all non-extra_cap capabilities, and then re-apply the user's new role-associated caps (not overwriting any extra_caps).

We don't need to know which caps came from which roles... only if they came from outside the role system. Does this make sense?

And yeah, I'd like to see at least some components of the Role Manager in core. Specifically, the ability to add a new capability and grant it to a role or user. Adding new capabilities or renaming capabilities doesn't even have to be in there. Plugin authors aren't using their own special capabilities, like they should, because it requires people to install a third party plugin to add that capability to a user or role. So they're either using an existing capability, which reduces the granularity of the capabilities system, or they're (and this is really bad), hardcoding it to a role (i.e. one of the default roles, that may or may not exist!!)

comment:6 ringmaster8 years ago

The p-code process you've describe seems reasonable, with one exception.

If a user is denied a cap that they already have, then the extra_cap flag should also be set. This will make it easier to query for caps that are applied only to that user, and not those coming from a role.

This system should value user-granted caps over role-granted ones. If a cap is granted or denied explicitly to a user, it should be retained if their roles are changed.

The patch I was writing did not include the form for creating new caps (since that's mostly a developer tool, and not really useful for end-users), and there isn't a form for renaming caps, but I think you meant renaming roles. Assuming it was upgraded to your specs (er, made to work), it's probably useful.

comment:7 markjaquith8 years ago

Yeah, agreed on the extra_cap flag for denials. That denial should stay even if you are later upgrade to a role that has that cap.

So basically, order of operations:

ROLE_1_CAPS + ROLE_2_CAPS + USER_GRANTED_CAPS - USER_DENIED_CAPS

And yes, I meant renaming roles.

Sounds perfect. For people who need more flexibility than that (few), they can download the full Role Manager plugin.

comment:8 ringmaster8 years ago

The p-code process you've describe seems reasonable, with one exception.

If a user is denied a cap that they already have, then the extra_cap flag should also be set. This will make it easier to query for caps that are applied only to that user, and not those coming from a role.

This system should value user-granted caps over role-granted ones. If a cap is granted or denied explicitly to a user, it should be retained if their roles are changed.

The patch I was writing did not include the form for creating new caps (since that's mostly a developer tool, and not really useful for end-users), and there isn't a form for renaming caps, but I think you meant renaming roles. Assuming it was upgraded to your specs (er, made to work), it's probably useful.

comment:9 ryan8 years ago

usermeta supports multiple keys except for update_usermeta(). That can be remedied by adding a $prev_value arg like we don for update_post_meta().

comment:10 markjaquith8 years ago

Unless there are any objections, I might take a stab at this later tonight.

comment:11 matt8 years ago

  • Milestone changed from 2.1 to 2.2

This is way too much for 2.1, pushing to 2.2. Regardless, it seems convoluted.

comment:12 markjaquith8 years ago

As much as I want this, I agree with 2.2 milestone. Convoluted? No, it's simplicity! simple user2cap relationship table. If you want convolution, you'd stick capabilities and roles into a serialized usermeta array so that cap2user requires unserializing each user's cap/role array, figuring out which are roles and which are capabilities, mapping capabilities from the roles to the capabilities that the role has, and then searching each user's capabilities array (at a huge memory cost) to see who has the specified capability. You know, like we're currently doing. :-)

comment:13 rob1n7 years ago

  • Milestone changed from 2.2 to 2.3

comment:14 Nazgul7 years ago

  • Milestone changed from 2.3 to 2.5 (future)
  • Type changed from defect to enhancement

comment:15 santosj6 years ago

  • Owner changed from markjaquith to jacobsantos
  • Status changed from assigned to new

comment:16 FFEMTcJ5 years ago

  • Milestone changed from 2.9 to Future Release

comment:17 jacobsantos5 years ago

  • Component changed from Administration to Role/Capability

comment:20 Denis-de-Bernardy5 years ago

err... sorry: #5255 even :-)

comment:22 Denis-de-Bernardy5 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

see #10201

Note: See TracTickets for help on using tickets.