New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move permissions-type conditions to a Capabilities API (Trac #4888) #4888
Comments
trac user mrclay wrote on 42800481-09-08 Examples: Drupal user_access; Moodle has_capability. WordPress current_user_can. I rather like sound of "user_can". The hard part is determining what/how much context info to pass on to whatever underlying systems may make the decision. I'd suggest the context where this is occurring (group|user|site), and optionally the object being worked (usually an entity). Most The core would have a built-in capabilities hook handler that had a look-up table for the core capabilities. This handler would, at a low level, call functions like |
trac user mrclay wrote on 42813883-01-01 Evan suggested
Also I don't like at this point in the code that |
trac user mrclay wrote on 42849864-01-15 Added more info to [https://docs.google.com/a/elgg.org/document/d/1aIQqRgu3bICTqSXn9GURwNjZdigVGGwXy18_Q_LJKvo/edit this doc]. |
Thought about this more. We could have |
@mrclay, I've written a gist with my proposal, including proposed documentation. We may have to add end-to-end permissions tests before we can reasonably hope to refactor the permissions system, though... |
Here's the core of a plugin implementation I'm putting in production soon. https://gist.github.com/mrclay/d277b311402f1f37f2b5 ufcoe_grant_capabilities() is a shortcut to setting site-wide global capabilities, which would be mostly useful for plugin-specific capabilities. I used closures to bind the handlers to a particular location in an array, so you can set many capabilities at priority 400 but they will all be resolved in a single hook handler for performance. Of course we don't necessarily need to provide this feature. |
A hurdle I see ahead is race conditions between plugins that check permissions and those that "set" them. Is there a logical boot up event before init/system that plugins should use for setting perms? |
Looks good. I think this has a bug as it relates to creating new content, can('post', new Blog ()) If the first returns true, so will the second because of the caching layer,
|
A different direction that I like more and more the longer I think about it: consolidate further around "actions". $actions->can('action/name', $params, $user = null); // defaults to logged in user After all, what you need to be able to do at the end of the day is POST to an action endpoint, right? |
What I like about this approach is that it doesn't require forcing all permissions into a subject/object/target model; the activitystreams-inspired model is nice and clean but might not end up working very well in real life for unforseen reasons/actions/capabilities. It also feels very much like consolidation around a concept and not some new thing we're adding to the platform that further adds to what devs have to learn. There is a one-to-one correspondence b/w permission and action. |
In practice I have permissions for viewing pages as well so not all will be POST operations. Agree with freeing the model, and I do like the tie in with actions. Registering an action could be an implicit grant in the capabilities system and have all actions check for hook-provided capabilities. Here's an init function from one of my plugins: function init_user_permissions(\ElggUser $user = null) {
if (!$user) {
return;
}
if ($user->isAdmin()) {
ufcoe_grant_capabilities(array(
'delete group',
));
}
if ($user->isAdmin() || $user->is_ufcoe_mentor) {
ufcoe_grant_capabilities(array(
'download user stats',
'create group',
'edit profile tags',
'view others activity',
));
}
// users can edit own tags
elgg_register_plugin_hook_handler('ufcoe_can', 'edit profile tags', function ($h, $t, $v, $p) {
/* @var \ElggUser[] $p */
if ($p['object'] && $p['subject'] && ($p['object']->guid == $p['subject']->guid)) {
return true;
}
});
} This model is working well for me, though I would prefer to pass in a useful object instead of an array. E.g. this would be nicer: elgg_register_plugin_hook_handler('ufcoe_can', 'edit profile tags', function ($h, $t, $v, CanParams $p) {
if ($p->subjectIsObject()) {
return true;
}
}); |
Hadn't thought about the view permissions. Seems like that complicates
|
Say we have a dashboard page made for special users, with various interfaces. If ACLs is all we have, we either have to make a corresponding object for that page/each bit of UI that might vary or at the very least associate the page with an ACL and test if the current user is in it. |
OK agreed this seems like a good feature but do we need a unified model? I guess the current way we get what you're asking for is just admin/logged Something to consider is what this would look like for web services. How
|
Sorry, I meant to suggest that having to bring in ACLs in that case was messy. Expecting developers to know about ACLs and capabilities (when they need them) is not onerous IMO. I don't understand why the JSON API would add difficulty. |
Tl;dr: Can we return a collection of users instead of a boolean for a given user? That would make for a much more flexible system in a way that I think people can still understand. Thinking about this more today, it's pretty common to want to ask "who can do X?" in addition to "can a given user do X? I.e. you want to be able to audit permissions without looping through all users in the system and running some code for each one (prohibitively expensive in general). This, I think, would provide our currently complex permissions system with some much needed transparency which I have not really seen anywhere else. I think this is worth pursuing because it provides some really nifty advantages beyond just a unified API signature. If you can answer "who can do X", you can just check if the current user is in that set of users to check if the current user can do X, so no functionality is lost. A nice property that comes out of such a system is that you can also have users who do not have permission to perform certain actions submit a request for that action to be done. The system can then search for everyone who can do that and notify them about the request, or notify some subset, or let the requester choose who to notify, whatever. Think "request to join" and "request to transfer ownership" and even "invite to join" or "suggest an edit". In general these are all proposals to take an action. They generally come with some justification or explanation and can be rejected, approved, or countered with an alternate proposal. I also mentioned audits. For any action, it is possible to get a master list of people who can do that action. That is frankly incredible! I think this would really make the system feel more approachable and understandable to everyone, and certainly less mysterious and frustrating. Proposed API: Permissions {
function whoCan(Action $action): Set<User>;
}
Session {
public function can(Action $action) {
return $permissions->whoCan($action)->contains($this->user);
}
} By default this returns the empty set (no one can do anything, secure by default). We would need a sentinel value of some kind for "current user" in order to be able to handle logged out users. Or else another API method specifically for the logged out case. Thoughts? One more very nice use-case for collections, IMO. |
I like it a lot. This definitely changes how we think of permissions in Elgg, and I think
|
Yes, I was thinking the action could be what we discussed above: 1:1 relationship with current meaning of action in elgg. Accept arbitrary params. Or we could restrict the meaning. Haven't thought about that as much yet. |
Alternatively, we could move the whoCan onto the action... Action { Or if we go to restful controllers: whoCanPost, whoCanGet, whoCanDelete, whoCanPut, etc. I'm not sure what the right level is to put this... |
FWIW in Drupal users either have or don't have a permission, and they happen to be granted by roles. You can query ppl who have a permission or a role. Having the permission purely depend on the DB makes the system easy to reason about and design caching around. We can bind these permissions within "contexts" (eg a group) to get more flexibility, but key is to not guarantee arbitrary query-time logic (no hooks). |
I guess I see how to persist the permissions data in a reasonable way as being the bigger question. Also how often does the system really have to query all users who can do something? The system is constantly trying determine what the logged in user can do. Maybe I'm missing something; I'll reread this ticket again later. |
I don't know how often this would need to happen because it isn't even remotely possible right now. But I suspect it would be a nice feature once it is possible. I listed two examples already:
I'm sure we could come up with more. |
Original ticket http://trac.elgg.org/ticket/4888 on 42797668-09-16 by trac user mrclay, assigned to unknown.
Elgg version: 1.8
Elgg is currently full of code like:
I propose we refactor this into an API that looks something like:
In the short term we'll have just moved the condition code into a permissions hook handler, but 1) this will open the door for more granular permissions control, and 2) anyone who wants to create a roles implementation can simply tie roles to the named capabilities.
The text was updated successfully, but these errors were encountered: