ACLs
Scott Merrill
last night, staring at the ACL code, and trying to figure out what an
ACL implementation would look like.
A few goals for the ACL:
* Feature coverage
- Handle a broad and flexible range of permissions, including
aggregate permissions.
- Correctly handle anonymous users
- A user's UI experience should be appropriate to their permissions
(if a user cannot publish this content type they should not have a
publish button).
- The UI for creating and modifying permissions should have good usability.
* Good performance
- Avoid redundant queries
- Cache as much as possible
- Deny as early as possible
* Good reliability
- Good test coverage
From an email Michael sent to me after I gave up:
"The structure of permissions is going to be very difficult. We started
by considering parsable strings as permissions (for example
view_content_entry) but this would quickly become unworkable.
I'm not sure how the implementation would work, but I believe that
permissions need to be arrays such as these (we haven't even started
on non-post related permissions):
array('action' => 'create', 'type' => 'entry', 'status' => 'draft')
array('action' => 'create', 'type' => 'entry', 'status' => 'publish')
array('action' => 'view', 'type' => 'entry', 'group' => 'own',
'status' => 'draft')
array('action' => 'view', 'type' => 'entry', 'group' => 'all',
'status' => 'draft')
array('action' => 'view', 'type' => 'entry', 'group' => 'own',
'status' => 'published')
That's hard because you can only guarantee that 'action' will be set.
I guess you'd need one of either 'type' or 'status' set."
Here are some additional thoughts of my own:
* creating a post has nothing to do with status. You can either
create a post of a certain type, or you can't.
* when creating a new post, the list of statuses you may assign should
be filtered through ACLs, so you only see those statuses that you are
permitted to use (which may differ between content types).
* the list of statuses may be dependent on the context. If a post is
published, I might be able to edit the content of the post but not
change its status, so the only status available for me to select would
be "published". Similarly, I might be able to edit drafts, but not
publish them, so I should only see "draft" as an option.
* editing a post should be constrained by status. For example, I
might not want contributing authors to edit any posts with a status of
"published", even if they're the author.
* editing a post's content and changing a post's status are two
fundamentally different things (though it may be possible to do both
at the same time). The permission system needs to encapsulate this.
One idea we toyed with was a concatenated string to represent complex
permissions, along the lines of "view_own_entry_draft": verb + owner
+ content type + status
This would make for some pretty complex permission checks, though, we
think. It's also only applicable to actions on content, and would not
work as well for system-related permissions like activating plugins or
modifying users.
Another idea that occurred to me this morning is to pass objects to
ACL::user_can() and ACL::group_can(), so that those methods could
trigger plugins to evaluate the permission request against the object
in question. This also falls apart when dealing with non-content
permissions like activating plugins.
Finally, another idea was to focus on the primary verbs (actions) used
in Habari. These include "create", "edit", "publish", "delete',
"unpublish", etc. Rather than try to develop fine-grained composites
for each combination of content type + status + action, simply focus
on the main actions.
Maybe I've been looking at this stuff for too long, and am now missing
the forest for the trees. Any insight folks have on how to implement
a robust ACL system would be greatly appreciated.
Cheers,
Scott
ringmaster
> Micheal Harris and I spent a couple hours banging our heads together
> last night, staring at the ACL code, and trying to figure out what an
> ACL implementation would look like.
working code in trunk.
Everything in the list of requirements you posted is covered by the
existing code, or could be easily added. It seems like you're not
only looking for a set of semantics for building rule names, but also
enforcing those rules on the names that are used. This feels
impractical.
We have generated hook names for plugins by field name and content
type. We also have hook names that defy the loose rules we use to
name hooks. I think this is not a detriment to the system as long as
all of the hooks are listed and documented (*cough*) and it's well-
known how the generated rule names work.
For similar reasons, I think it's a good idea to try to standardize
the names of rules for the sake of sanity, but doing things like
the granular bits fit. From purely the perspective of what the rule
looks like, "create_draft_entry" - whether it's parsed from the string
or a set of "create_{status}_{posttype}" permissions permutations is
generated from type and status data - is much easier to understand and
code.
Why not do this?:
function create_post() {...
$perm = "create_{$status}_{$type}";
if(User::can($perm)) {
// Create the post
}
Yes, it's not as elegant as the granular array of permissions, but it
gets the job done and makes coding permissions a bit easier on
contributors.
To summarize: For what reason is the existing system inadequate?
What advantage does the direction that this message implies provide?
I'm up for changes if needed, I just want to understand what the
thinking is.
Owen
Scott Merrill
>
> On Apr 4, 10:17 am, "Scott Merrill" <ski...@skippy.net> wrote:
> > Micheal Harris and I spent a couple hours banging our heads together
> > last night, staring at the ACL code, and trying to figure out what an
> > ACL implementation would look like.
>
> I'm curious why an ACL implementation would look different from the
> working code in trunk.
>
> Everything in the list of requirements you posted is covered by the
> existing code, or could be easily added. It seems like you're not
> only looking for a set of semantics for building rule names, but also
> enforcing those rules on the names that are used. This feels
> impractical.
We struggled with trying to make a system that was self-evident as to
what the permissions meant, while still being fairly easy to implement
in code and user interface.
> To summarize: For what reason is the existing system inadequate?
> What advantage does the direction that this message implies provide?
> I'm up for changes if needed, I just want to understand what the
> thinking is.
Help me understand how you would create ACLs and implement code to
enforce them for the following configuration, one which I feel would
be fairly commonplace for a larger content producing organization:
User groups: Admin, Editor, Publisher, Author, User
Admins can do everything.
Editors can edit everyone's content, including changing the status,
but may not change site options, or manage plugins, themes, or users.
Publishers can publish other people's entries, but may not edit them.
Authors may create new entires (not pages), but may only save that
content as draft. They may see and edit only their own drafts. They
may not change statuses.
Users cannot create content of any sort.
Authors might have permissions:
create_entry
edit_own_entry_draft
Publisher might have:
change_entry_from_draft_to_published
Editor might have:
edit_all_entry_draft
edit_all_entry_published
Given these sample permissions, how can we implement the necessary
checks throughout the code? Some will be easy: if the user can create
content, show them the top-level create drop-down with the applicable
content types. Others will be more challenging, like restricting
which statuses a user may select. This is not a simple "if the user
can..." check. Rather we need to enumerate each of the status types,
checking if the user is permitted to change from any existing status
to that one.
Remember: it's not just an additive collection of "they can see
drafts" + "they can edit drafts". We need to restrict, specifically,
which drafts they can see to ensure that authors don't see one
another's drafts, while editors and publishers can. We don't want
publishers to delete entries (and we might not want them to unpublish
entries, either). We don't want publishers to change the status on
pages.
AdminHandler then needs to know how to parse each of these permissions
when accepting form submissions. When submitting a post, we need to
iterate through lots of different types of actions, to see what's
happening.
* was the status changed? Does the user have permission to use the new status?
* what was the old status? Is this user allowed to make that transition?
* was the content edited? Is that permissible for this user?
Finally, how might the user interface be implemented to assign
permissions to groups? A series of checkboxes, a la Drupal, but in a
more complex matrix for each combination of "from status X to status
Y" for each content type?
I don't think we decided that it's completely impossible, given our
current code, just that it was more challenging than either of us had
anticipated. We're not suggesting (yet) that we scrap everything and
start fresh. We're discussing the situation with others, to ensure
that what ultimately gets developed is the best that it can be for
Habari.
Cheers,
Scott
ringmaster
> Finally, how might the user interface be implemented to assign
> permissions to groups? A series of checkboxes, a la Drupal, but in a
> more complex matrix for each combination of "from status X to status
> Y" for each content type?
would provide fewer core premissions than what you've outlined, but be
sure to provide hooks for plugins to implement more detailed
permission constructs.
> I don't think we decided that it's completely impossible, given our
> current code, just that it was more challenging than either of us had
> anticipated. We're not suggesting (yet) that we scrap everything and
> start fresh. We're discussing the situation with others, to ensure
> that what ultimately gets developed is the best that it can be for
> Habari.
with what we have, although now that I see better what you're thinking
of, some ideas are percolating. I'll have to let it simmer a little
bit though, and see if there's anything worth keeping.
Owen