API critique wanted for my RBAC/ACL

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

API critique wanted for my RBAC/ACL

Post by koen.h »

How would you improve this API? I'll give some basic examples of how it works:

note: aco = access controled object (eg a resource), aro = access request object (eg a role), it's actually an aco so it extends 'role based'.

Code: Select all

 
$acl = new PHPRBAC_AccessControl();
$storage = new PHPRBAC_Storage_NonPersistentStorage();
$acl->setStorage($storage);
 
/**
 * example 1
 */
$storage->addAco('role');
$storage->addAco('resource');
$storage->allow('role', 'action', 'resource');
 
$result = $acl->isAllowed('role', 'action', 'resource');
var_dump($result->isAllowed()); // true;
 
/**
 * example 2
 */
$storage->addAco('parentRole');
$storage->addParent('role', 'parentRole'); // note: awkward name 'addParent'?
$storage->addAco('parentResource');
$storage->addParent('resource', 'parentResource');
$storage->allow('parentRole', 'action', 'parentResource');
 
$result = $acl->isAllowed('role', 'action', 'parentResource');
var_dump($result->isAllowed()); // true;
 
/**
 * example 3
 */
$storage->addAco('post');
$storage->addAco('published', array($acl->criteria()->eq('status', 'published')));
$storage->addParent('published', 'post');
$storage->addAco('visitor');
$storage->deny('visitor', 'view', 'post');
$storage->allow('visitor', 'view', 'published');
 
$result = $acl->isAllowed('visitor', 'view', 'post');
var_dump($result->isAllowed()); // false;
var_dump($result->isDenied()); // false;
var_dump($result->isCriterial()); // true;
$result->setRewriter(new PHPRBAC_Criteria_Rewriter_MysqlExpressionRewriter()); // note: $acl->setCriteriaRewriter() to set default rewriter
// SELECT * FROM posts WHERE
echo $result->getCriteria(); // status = published 
 
I haven't yet implemented the following so these are not in the examples:
-checking an actual aco (only the returning criteria part for now, as in example 3).
-caching the results
-binding of variables (so you can have dynamic criteria, eg as in $acl->criteria()->eq('user_id', ':user_id:'), for things like allowing users to edit their own posts).
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

Update on the variable parameters:

Code: Select all

$exampleVariableCriteria = $acl->criteria()->eq('user_id', ':variable_user_id');
 
$variablesHandler = new PHPRBAC_Criteria_Visitor_DynamicVariablesVisitor(); // visitor pattern
$variablesHandler->addVariable('variable_user_id', 23769);
$acl->attach($variablesHandler);
When the result is ready to be returned from $acl->isAllowed(), all observers are notified through the 'result' event. The DynamicVariablesVisitor *can* (note 1) be added as an observer (implements SplObserver (note2)) and if 'updated' will act as visitor of Criteria->accept($visitor), replacing all variable values.

note
(1) I'm thinking about not giving this choice as it makes client code need to do more work for low added value
(2) don't like the specific Observer-pattern implementation in Spl library though
User avatar
VladSun
DevNet Master
Posts: 4313
Joined: Wed Jun 27, 2007 9:44 am
Location: Sofia, Bulgaria

Re: API critique wanted for my RBAC/ACL

Post by VladSun »

Could you post a more real-world like example :)
Including all of the MVC components?
There are 10 types of people in this world, those who understand binary and those who don't
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: API critique wanted for my RBAC/ACL

Post by pickle »

:arrow: Moved to Coding Critique
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

VladSun wrote:Could you post a more real-world like example :)
Including all of the MVC components?
What do you mean with the MVC components? In my view the authorization check will mostly take place in the controllers.

The blog post example above is a realistic one I think. Lets build up a simple example:

Code: Select all

$acl->addAco('guests');
$acl->addAco('admin');
$acl->addAco('post');
$acl->allow('*', 'view', 'post'); // anyone can view posts
 
// we don't want to allow guests to view posts still in draft: a draft is a post with property 'status' = 'draft'
$acl->addAco('draft', array($acl->criteria()->eq('status', 'draft'));
$acl->addParent('draft', 'post');
$acl->deny('user', 'view', 'draft');
 
// let's say we are on the front page and need to retrieve the latest post a user is allowed to view
$result = $acl->authorize('user', 'view', 'post'); // isAllowed is now for checking concrete Aco objects
if ($result->isCriterial()) {
    $where = $result->getCriteria($acl->ciriteria()->asSql()); // small API change here, return 'status = "draft"'
}
You could check a concrete $postObject with $acl->isAllowed($aro, $action, $postObject) which returns a true or false.

On second thought, it's not very clear what's the best way to implement the access check in the MVC triad.

I remember a discussion with you long time ago about what you were trying to achieve with your access control. Maybe my reply here is a step in that direction: viewtopic.php?p=554537#p554537

Anyway, thanks for looking at this post.
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

There's a clear influence from Zend_Acl. My implementation builds upon the foundation:
-resources are now Access Controled Object (ACOs)
-roles are now Access Request Object (AROs)
these two names come frop PHPGACL. An ACO can be a resource but doesn't need to be
-AROs are ACOs (roles are resources too IMO)
-ACOs can inherit from more than one parent
-ACOs can be specified by attributes, ag a Post that has status = 'draft', a Post in category X. This means that in its simplest form you have something like Zend_Acl (resources just have names). You can extend this by using Criteria (ACOs can have a Is A relationship). This probably needs a explanation in itself.
-Avoidance of needing to duplicate resources (more needs to be said about this too).
-caching of results (calculations are more processor intensive, so better do it once and cache; keep ability to cache all access checks for one request => one db query for all access requests)
-access rules with variables (allows for relationships like 'owner')
-storage: can choose storage medium like database for rules and ACOs (or combine them, not yet implemented, probably chain of responsability, ties with avoidance to duplicate resources because modules don't need to add ACOs but can add storage)

I haven't implemented assertions.
Haven't yet implemented everything listed.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: API critique wanted for my RBAC/ACL

Post by josh »

The "criterial result" stuff makes my head hurt. you gonna release the source?
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

I don't know how the criterial stuff evolved but while researching that direction I found this:

http://docs.jboss.org/teiid/6.0/apidocs ... teria.html

I've drawn a lot of inspiration from it. The criterial stuff seems to be more widespread in the Java world. I think people will understand it faster if I rename it to 'attribute' or something like that.
If I say 'Something is allowed to do an action on objects named "post" that have a property "status" which equals "published"', that may sound more natural to grasp. That could translate into code like:

Code: Select all

$acl->createAco('published', array($acl->property()->eq('status', 'published'));
Maybe this is more intuitive:

Code: Select all

$aco = $acl->createAco('published_post');
$aco->hasProperty('status')->equals('published');
There's a lot I can apply from posts on this blog:

http://misko.hevery.com/

I'm going to release the code from the moment I'm going to use it in my own projects. For now I only have a sort of 'proof of concept' working.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: API critique wanted for my RBAC/ACL

Post by Christopher »

Looks interesting. Let us know when here is something we can download and try.
(#10850)
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

josh wrote:The "criterial result" stuff makes my head hurt. you gonna release the source?
josh, what do you find confusing about the criterial stuff?
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: API critique wanted for my RBAC/ACL

Post by josh »

koen.h wrote:josh, what do you find confusing about the criterial stuff?
Sorry for the tardy reply:

# $result = $acl->authorize('user', 'view', 'post'); // isAllowed is now for checking concrete Aco objects
# if ($result->isCriterial()) {
# $where = $result->getCriteria($acl->ciriteria()->asSql()); // small API change here, return 'status = "draft"'
# }

I just thought having to remember to do an additional test on $result was something that could be considered a learning curve for the API, I have no idea what isCriterial() does though.
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

josh wrote:I have no idea what isCriterial() does though.
I've changed it to isConditional(). Maybe that's more intuitive. Since English is not my first language it's sometimes hard to find the best words.

IsConditional() returns true if an action on a type of objects (eg a 'post') is allowed under certain conditions (eg view only posts that have 'status = published').
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: API critique wanted for my RBAC/ACL

Post by josh »

Could this "context" be passed to the method, so you just get a boolean back?
koen.h
Forum Contributor
Posts: 268
Joined: Sat May 03, 2008 8:43 am

Re: API critique wanted for my RBAC/ACL

Post by koen.h »

josh wrote:Could this "context" be passed to the method, so you just get a boolean back?
What do you mean with 'context'? You get a boolean back if you check access on individual objects.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: API critique wanted for my RBAC/ACL

Post by josh »

As in, pass the post's status to the ACL, the ACL would handle checking if it is "allowed" for a given status ( rather then getting back an object you have to "poke at" further to get a simple true/false )

So you would have a 4th optional "context" parameter that could be anything ( a post's status, the request method GET vs POST, etc.. any "context" you needed to be conditional )... these contexts would work like a second "permission" ( the resource, role, permission, AND context would all have to match )
Post Reply