OWASP / rbac

PHP-RBAC is an authorization library for PHP. It provides developers with NIST Level 2 Standard Role Based Access Control and more, in the fastest implementation yet.
http://phprbac.net/
Apache License 2.0
432 stars 141 forks source link

How to add Rule (such as Author Rule) to an permission #107

Open thanhnambkhn opened 6 years ago

thanhnambkhn commented 6 years ago

Hi, Do we has feature such as adding Rule to a permission? Let's images that we has a post Editor Role (with permissions as follow create/ update/ delete a post ) To pass checking permissions Update and delete a post, user needs satisfy two conditions:

thanhnambkhn commented 6 years ago

@abiusx Do you think it is useful feature?

abiusx commented 6 years ago

I'm not sure. To me, it seems like business logic of the application with respect to authorization (i.e. implementation and using the library) rather than part of the core library.

I think it'd be beneficial as a branch/usage example but not as a core feature.

-A

On Sep 17, 2018, at 2:02 AM, Nguyen Thanh Nam notifications@github.com wrote:

@abiusx https://github.com/abiusx Do you think it is useful feature?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/107#issuecomment-421897001, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVjWzqaJboRgonjZBD5fgU3yayjyspDks5ubzrZgaJpZM4WrTFx.

thanhnambkhn commented 6 years ago

I think it's a common case (edit, delete only by owner OR manager). If you separate that feature out of core, developer must hard code such that:

// To check access control to delete / update a post:
 if($identity === $post->getAuthor() && $identity->hasRole('editor')) {...}

In my opinion, it is not a good way to implement this feature.

abiusx commented 6 years ago

What you are referring to, is data-level access control. What this library does is provide function-level access control.

The difference is that functionality of a system rarely changes, whereas data changes constantly. This library is very slow on changes, and very fast on lookups.

On Sep 17, 2018, at 9:40 PM, Nguyen Thanh Nam notifications@github.com wrote:

I think it's a common case (edit, delete only by owner OR manager). If you separate that feature out of core, developer must hard code such that:

// To check access control to delete / update a post: if($identity === $post->getAuthor() && $identity->hasRole('editor')) {...} In my opinion, it is not a good way to implement this feature.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/107#issuecomment-422225138, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVjWxbbO0BjRyxJLssxMuhwIDR-2lvlks5ucE8ogaJpZM4WrTFx.

thanhnambkhn commented 6 years ago

@abiusx thank for your answer. So do you have any suggestion to implement data-level access control feature within PHPRBAC? Should I build a AccessControl class, which is extended from Rbac, and add more method suchas: AccessController->check($userId, $roleId, $DATA_need_to_check)

abiusx commented 6 years ago

You can certainly implement it in this library, it just won't be very efficient, especially if your permissions are frequently changed.

A data-level access control mechanism really depends on the business logic. If you simply want to make sure someone owns something directly, then check the ownerId. If you want to check if they belong in a certain group, you can use function-level RBAC.

On Sep 17, 2018, at 11:20 PM, Nguyen Thanh Nam notifications@github.com wrote:

@abiusx https://github.com/abiusx thank for your answer. So do you have any suggestion to implement data-level access control feature within PHPRBAC? Should I build a AccessControl class, which is extended from Rbac, and add more method suchas: AccessController->check($userId, $roleId, $DATA_need_to_check)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/107#issuecomment-422241100, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVjW3hRFeZCy-V9UZlvwTO4VTEmTSNNks5ucGaSgaJpZM4WrTFx.

thanhnambkhn commented 6 years ago

Hi, As I learned from Yii2 framework, they add a concept beside Role, Permission, that is Rule Rule is a class with piece of code which will execute logic business and return true or false. After defining Rule, you can add Rule to a permission like that:

$rule = new \app\rbac\AuthorRule;
$auth->add($rule);

// add the "updateOwnPost" permission and associate the rule with it.
$updateOwnPost = $auth->createPermission('updateOwnPost');
$updateOwnPost->description = 'Update own post';
**$updateOwnPost->ruleName = $rule->name;**
$auth->add($updateOwnPost);

You can see more here. In that way, you can define any logic business not only owner right, and no need to change the 'function-level access control' of PHPRBAC How do you think about this idea?

abiusx commented 6 years ago

It's a good security feature, however implementing it under observer pattern is not the best idea. Also if it is stored in the database, it wouldn't be very wise.

On Sep 17, 2018, at 11:36 PM, Nguyen Thanh Nam notifications@github.com wrote:

Hi, As I learned from Yii2 framework, they add a concept beside Role, Permission, that is Rule Rule is a class with piece of code which will execute logic business and return true or false. After defining Rule, you can add Rule to a permission like that:

$rule = new \app\rbac\AuthorRule; $auth->add($rule);

// add the "updateOwnPost" permission and associate the rule with it. $updateOwnPost = $auth->createPermission('updateOwnPost'); $updateOwnPost->description = 'Update own post'; $updateOwnPost->ruleName = $rule->name; $auth->add($updateOwnPost);

You can see more here https://www.yiiframework.com/doc/guide/2.0/en/security-authorization#using-rules. How do you think about this idea?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/107#issuecomment-422243650, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVjW5kHTmAgY8BCN6E2D47mBf4buYSkks5ucGo4gaJpZM4WrTFx.