Yii2: Use case for yii\rbac

[SamMousa]

This is intended to be a discussion after which the docs may need updating / additional examples.

1. Implementation of yii\rbac\Rule must be done in code.
2. When saving the rule to the database, it saves the name of the rule and a serialized object.

I have not found a clear use case for the serialization of the object into the database. I understand that it could be used to have the rule which contain logic be reused; but it is unclear when I would want that.

Alternatively, would it not make more sense to just save the class name, any additional data, and recreate the rule and then pass the data to the execute method?

[mdmunir]

Interesting.

When saving the rule to the database, it saves the name of the rule and a serialized object.

I agree if we should only save the name, instead of name and object.

[Sammaye]

I have not found a clear use case for the serialization of the object into the database

Migration/moving of data for one.

Also just like session data it creates a separate storage of roles outside of the application servers

[Sammaye]

Alternatively, would it not make more sense to just save the class name, any additional data, and recreate the rule and then pass the data to the execute method?

For role defined by class yes, but not all are

[SamMousa]

All rules are defined by a class though, in yii2:
https://github.com/yiisoft/yii2/blob/master/framework/rbac/DbManager.php#L608

And the interface:
https://github.com/yiisoft/yii2/blob/master/framework/rbac/ManagerInterface.php#L121

So a rule item is always related to an implementing class, however due to object serialization it is not just generally related to the implementing class, but to a specific instance.

[samdark]

It is to be able to set rule config.

[SamMousa]

So a rule would be reusable for multiple different authitems. I realized that but I have trouble coming up with a situation where this would actually work well.

Do you have any concrete example(s) @samdark ?

In examples that I can think of using a rule in the first place seems to be inappropriate and also causes me to lose the ability to enumerate permissions.
(If configuration inside a rule decides permissions there is no way to list all users with some permission)

[samdark]

Well, for example, users have rating and certain permissions are given when users are reaching certain rating scores. That's how StackOverflow works. To implement it you may create RatingRule which will accept minimum rating to match.

[lynicidn]

it bad - except dynamical rules, example if i manage rules via form

$form = new SuperRuleForm($name);
//...
$rule = SuperRule::createFromRequest($form);
//add or remove or update

in this case 1 form 1 rule
if you remove serialize - need use other storage for save this data

[SamMousa]

Okay, so consider the rating rule.
For it to be useful it must be related to an authorization item.
Authorization items have data; I can store the minimum required rating there.

Since each authorization item has at most one related rule, and the only code that can use this data during access checking is in the rule, why would I ever need to store data in the rule itself?

[SamMousa]

Also there are some practical issues.

Rule naming.

• Rule names must be unique.
• I could create a unique name based on the class name combined with all the instance data: RatingRule50
• What happens if I change the required rating? Should the rule name change?
• What permissions would be impacted by such a change?
• If I choose a name unrelated to the instance how do I keep this workable when the number of rules increases?
• What if the required rating for a specific permission / action changes? If I change the rule then it could change other permissions.

[klimov-paul]

Same rule class can be reused for different permission roles.
Rule class represents the logic, while this logic may have some particular configuration. Just in a way Behavior carry some logic, but have a configuration for particular attachment.
The Rule instance saved into database in order to save this configuration.

The particular example is usage of defaultRoles with custom Rule attached. In this case the actual check of the permission is performed by the rule logic.
Imagine we grant user some role in case he has its name in some array attribute of the identity record.
Rule class:

class IdentityAttributeRule extends Rule
{
    public $name = 'default';

    public function execute($user, $item, $params)
    {
        $webUser = Yii::$app->getUser();
        if ($webUser->getIsGuest()) {
            return false;
        }
        /* @var $identity \app\models\User */
        $identity = $webUser->getIdentity();
        return in_array($this->name, $identity->rbacRoles);
    }
}

RBAC setup script:

$authManager = Yii::$app->get('authManager');

foreach ($authManager->defaultRoles as $roleName) {
    $ownerRule = new IdentityAttributeRule();
    $ownerRule->name = $roleName; // rule name is dynamic!
    $authManager->add($ownerRule);
    $ownerRole = $authManager->createRole($roleName);
    $ownerRole->ruleName = $ownerRule->name;
    $ownerRole->description = $roleName;
    $authManager->add($ownerRole);
}

There is nothing to discuss here.

[SamMousa]

There is something to discuss @klimov-paul .
In the scenario you describe you can just as easily store the $name in $ownerRole->data.
There is nothing that is added by serializing the rule instance to the database.

Please re-open, even if you think there is nothing to discuss then you could instead go the polite way by saying:
"I think there is nothing more to discuss, please close this issue unless you have more comments."

That way your issue tracker still gets cleaned up without you acting like someone who knows better; history has shown that you are not always right, so stop acting like it.

[klimov-paul]

In the scenario you describe you can just as easily store the $name in $ownerRole->data.

I do not time for composition of the huge complex example of Rule internal configuration usage. I suppose you have enough imagination to do this on your own.

The point is: there is a featur, whiche requires storage of the seriazed object in the database. Someone may rely on it, and thus it can not be changed.

there is nothing to discuss then you could instead go the polite way by saying:

I admit: I am not a diplomat, and thus my answers may not always sound nice and appropriate for everyone. Still, I mean no offence to anyone.

Note: you may always call for GitHub arbitration, if you think there is any disrespective or insult from my behalf.

Please re-open, even if you think there is nothing to discuss

The rest of team members participating this disscussion share opinion, that feature should remain, even while they can not give coherit example of it, which I have just provided. Thus I can see not reason to keep issue open any longer.
Any core team member can reopen the issue, if he think otherwise - I will not object in this case.

Also you may still continue your disscussion if you like event at the closed issue.

That way your issue tracker still gets cleaned up without you acting like someone who knows better; history has shown that you are not always right, so stop acting like it.

At the present state, me and my coleagues have 834 open issues, which need attention. Thus I have to discard some of them to be able to focus on more crucial ones.
Of course, for you your particular problem or idea seem to be the most important one - it is normal. But we can not satisfy everyone demands and wishes.
Perhaps if the number of opened issues will be less then 100, we can reconsider this policy.

[SamMousa]

Thanks for your response!
As you may have noticed from the number of issues I create, my goal is to make Yii better, which is something we share.

I would never suggest removing a feature that is being relied upon by other developers, I just hoped that at least 1 of them would respond with a proper example. This example could then be added to the documentation.

I understand you have to prioritize, however binary priorities don't really work (open / closed). How about just adding a label that says low priority? I agree that this issue is not high priority and I am in no rush to get underlying code / documentation updated. Closing the issue however means it is essentially lost, no way for you to even have another look at it if ever the number of open issues goes below 100 ;-).

My goal with this discussion is not to solve my own problem, obviously if there is a field that (in my opinion) adds no functionality, I can just not use it. My goal was to see if I missed something, or if maybe the documentation could be improved or the code simplified.

Also, I'm available to help out fixing bugs if you want!

[lynicidn]

@SamMousa

Beyond the above advice, note that you can also hook into the serialization and unserialization events on an object using the __sleep() and __wakeup() methods. Using __sleep() also allows you to only serialize a subset of the object's properties. 

(c) http://php.net/manual/en/language.oop5.serialization.php

[SamMousa]

@lynicidn I know about that; not sure how it is relevant to this discussion though.
I understand how I could use it, I just don't see what it adds versus using the data field in the AuthItem.

[samdark]

Reopened for docs purpose. The intended usage of the feature was as I and Paul have described. @SamMousa if you have time, you can help us with docs about that.

[SamMousa]

I'd love to. Can you explain why you would prefer to store the information in the rule instance instead of in the authorization item?
Since that is the main question I'd like to answer in docs:
Why / when should I use a rule instance vs data in the item:
Rule instance:

class Rule ... {
$minRating = 50;
    public function execute($user, $item, $params) {
        return $user->rating > $this->minRating;
    }
}

vs

class Rule ... {
    public function execute($user, $item, $params) {
        return $user->rating > $item['minRating'];
    }
}

[TerraSkye]

@SamMousa i think u should define a property when its the same application wide, and item when its defers on user. ( my 2 cents)

[SamMousa]

@TerraSkye isn't that an arbitrary choice?
@samdark, could you elaborate in which cases you think the first is better and in which cases you think the last is better?

[TerraSkye]

It is while i u would prefer the second version for example a db value. And the first for an controller action with static data

[samdark]

@SamMousa it's all about different levels of configuration. By using data in auth_rule you can provide global setting which is configurable from admin UI. You can't do so via code because, well, it's hardcoded then. If you'll use item's data in this case, you'd have to re-configure each item in order to change value to check.

[SamMousa]

@samdark Can you show a short example? As far as I know the rule gets serialized so even if you change default values in code the "stored" values will be used when unserialized.

For example:

class OfficeHourRule {

public $start = '09:00';
public $end = '17:00';

public function execute(...) {

}

}

Will not behave differently when changed after it has been saved to the database, since it is saved using serialize.
Maybe you could show me some rules you have implemented in your project(s)?

[samdark]

I don't have any :)

[Ziggizag]

Well,

IMHO, current documentation simply does not explain in any meaningful way the concept of a rule attached directly to role (not to a permission!). Please comment!

(I mean 'ruleName' property of a role)

[samdark]

For example, user could be assigned a "night shift" role which is active at night only.

[Ziggizag]

Thank you, Alexandr.
Perhaps it is also worth the usage (besides AccessFilter):

Yii::$app->user->can("worker");

If the user role name is "worker" and the "nightShift" rule is attached directly to this role then this rule will be checked using "can" method of a yiiwebUser.

Documentation is unclear about this.

[samdark]

The thing is that all the RBAC hierarchy items i.e. both roles and permissions are basically the same and are named differently for the sake of easier thinking in authorization terms. That means what could be applied to permission also applies to a role and vice versa.

[Ziggizag]

Alexandr - you know that - but when one starts with RBAC system - he/she knows nothing!
Please - put it explicit in the docs, I know what I am saying!

[dynasource]

I'm experiencing issues with this approach of saving a PHP class to the DB. When the Class of a Rule changes because of namespace changes (for instance because of refactoring project folder structures) all affected Rules will throw errors.

This should be improved to an architecture in which PHP v.s. DB are held in sync out of the box.

[SamMousa]

That's not technically possible, what if the implementation of the class changes, what does unserialization yield then?

[dynasource]

architecture & implementation should be separated. This can be done by using mapping & only serializing data.

[SamMousa]

Yes but if the data we're serializing contains objects we have a problem anyway. So that basically reduces the type of data we can serialize to basic types, for which something like json paired with custom serializers on the rule classes is a better fit imo.

[dynasource]

to Rule should have an array of it's state stored and not be polluted with class references

[SamMousa]

The only one who knows its state is the object itself, so it will need to be a custom serializer.