BeatSwitch/lock

Logic of results of allowing or denying permissions

Opened this issue · 6 comments

This is an important one. Currently it's so that a top level allow or deny clears any lower level privilege or restriction. So for example, when you allow only an an action of create you basically get permission to create everything (every resource). It will also clear any restrictions in place for any resource where you wouldn't be allowed to create anything.

Practical example:

// Deny a caller from creating users and events.
$lock->deny('create', 'users');
$lock->deny('create', 'events');

// Allow the caller to create everything. After this he can both create users and events again.
$lock->allow('create');

This is the way I deem it to be logic but I don't know if the majority of you also thinks this is the logical way to do something.

Any thoughts or concerns that may arise? This is something I really want to tackle as early as possible so please give enough feedback on this.

Also: if you at any point find the current Acl not to behave in the way that makes sense for you then please tell me here so we can clear things out and improve it or make it more clear in the docs how things work.

I feel like this makes sense.
Though I do have a question. What's the default behavior when doing a can on a caller that doesn't have the permission explicitly set? I haven't seen this anywhere, did I overlook it?

I feel like this makes sense.
Though I do have a question. What's the default behavior when doing a can on a caller that doesn't have the permission explicitly set? I haven't seen this anywhere, did I overlook it?

Hey @Sxderp, default behavior is false. Permissions need to be explicitly set.

Here's a test which checks that behavior: https://github.com/BeatSwitch/lock/blob/master/tests/LockTestCase.php#L67-L70

Although remember that when you allow an action you allow that action for every type of resource.

I would have thought that deny should always overwrite allow.

More importantly, though, I think that specific rules ("create events") should always overrule more general ones (just "create"). What dod you think?

I take that statement back, at least partially. Order should matter, but I am still pondering whether specific rules should overwrite generic ones.

I think the current logic and tests describe the behavior well enough. I'm going to leave this thread open for future discussion for now.

After have a long time to think about this (it's been a couple months). I feel like order should NOT matter. What should happen is specific conditions should always override general. However, under certain circumstances what you described in the initial post my be favorable. Under these cases another function should be used in order to allow the general condition to override the specific conditions (or just delete the specific condition permissions and leave the general condition).

To use your example, after the:

$lock->allow('create')

The caller should be allowed to create everything except users and events.

Further, an alternative function would implement an overriding functionality for general conditions.

// Allow the caller to create everything.
$lock->allowOverride('create')

// Additional restrictions
$lock->deny('create', 'candy', 4);

// Allow creation of all candy but 4.
$lock->allow('create', 'candy')

// Allow creation of all candy
$lock->allowOverride('create', 'candy')

For overriding a deny or allow you can just delete any permission that is stricter than the current one.


One last thing, and correct me if I'm wrong here, there is no reason to store a deny permission unless there's a more general allow permission?
When checking if someone can do something they need an explicit allow in order to be able to do the action. If that does not exist then it defaults to 'denyed.' As a result, you don't need to store denies unless there is a more general 'allow' that you want to override.

The reason I bring this up is because currently if you allow something and then deny it (the same thing) afterwards an explicit deny is created in your database (or whatever you're using). If you have an application were allowing and denying could potentially happen a lot, back and forth (example below), then your database may get cluttered with a bunch of redundant denies.

Ex.
By default a user cannot edit another user's post. However, userA could give permission (allow) userB to edit their post. Later on they may decide they no longer want userB to edit their post and therefore prevents them from doing so (deny). This would create a specific 'deny' permission but that deny permission is redundant because by default userB cannot edit userA's post.