Yii2: AccessControl behavior "role" could be confusing

[gvasilopulos]

What steps will reproduce the problem?

there is no problem just a litte confusing implementation

What is the expected result?

What do you get instead?

Additional info

taken from the Yii2 guide:

public function behaviors()
{
    return [
        'access' => [
            'class' => AccessControl::className(),
            'rules' => [
                [
                    'allow' => true,
                    'actions' => ['index'],
                    'roles' => ['managePost'],
                ],

It seems that 'roles'=>['managePost'] really refers to permission
so it may be 'permissions'=>['managePost']
to avoid confusion.

| Q | A
| ---------------- | ---
| Yii version | 2.0.10
| PHP version | 5.6
| Operating system |

[cebe]

Yeah, according to RBAC best practice, checks should only be made against permissions so it would make sense to rename that for better user education.

[samdark]

Agree that it should be renamed.

[samdark]

I'm for supporting both keys in 2.0.x and dropping roles in 2.1.

[bizley]

Maybe some other name is needed here, something that means (or concerns) "roles+permissions" because in 2.1 expect issue like:

It seems that 'permissions'=>['?', 'admin'] really refers to roles

I suggest "applyTo" or "check".

[samdark]

only?

[cebe]

"only" is too generic, we have "actions", "controller", "ips"... then "only"-what?

[SilverFire]

How about can? It is alike Yii::$app->user->can()

I'm for supporting both keys in 2.0.x and dropping roles in 2.1.

IMHO it's not the change that requires such a massive BC breaking. I'm OK even with having both permanently.

[samdark]

can doesn't sound descriptive. The access rule itself has allow which is very close to can.

[bizley]

What about privileges? It sounds descriptive for both roles and permissions: "admin privileges", "editing privileges".

[thyseus]

Actually, you can pass an array of roles, of permissions or even mixed here. If the developer chooses to not follow the RBAC best practice for whatever reason, its his choice. The best practice should be highlighted in the documentation, and there may be cases where a security check according to a role may make sense.

So i suggest that 'roles' and 'permissions' should be aliases and be both valid even in yii 2.1+, as long as only one of them are specified. Or possibly they may me merged together for maximum flexibility.

Roles and Permissions are both valid entities here.

This way we also stay backward-compatible.

[cebe]

sure that is the point of the issue. find a name that fits both because both are allowed. I do not think we should go with two separate properties, it will only make the implementation too complicated.

[thyseus]

The Implementation is straightforward in my opinion:

if($roles && $permissions)
  return array_merge($roles, $permissions);
else if (!$roles && !$permissions)
  return null;
else 
  return $roles || $permissions;

or even prettier if there is some code logic guru :)

[thyseus]

This way we could do something like:

'roles' => ['role_1', 'role_2'],
'permissions' => ['permission_1', 'permission_2'],

which seems quite readable for me.

[samdark]

Yes, indeed it looks OK.

[thyseus]

Perhaps https://github.com/yiisoft/yii2/pull/13403 may already be the solution? Please Code Review, thanks !

[bizley]

The point is that you can do

'roles' => ['permission_1', 'permission_2'],
'permissions' => ['role_1', 'role_2'],

and it's perfectly fine. That is why we think about one new name for this key.

[Kolyunya]

Roles and permissions already have a general name. Why don't we stick to the existing naming convention of the RBAC framework and call it just items?

[samdark]

Item is a general internal name which is not really exposed in the docs or in RBAC papers.

[Kolyunya]

@samdark it is indeed exposed: 1, 2, 3, 4.

[samdark]

Well, that is API, not docs and not RBAC original paper. Anyway, thanks for suggestion, it got handy for a pull request.