Match operations with permission objects with set operators

[metatoaster]

Correct the operator terms to match what one might expect if they were working with permission objects as an encapsulation of sets of roles.

Test cases enclosed to demonstrate how this compares to working with the sets directly.

[metatoaster]

I am doing this to make way for a proper and (also &) operator that will create a Permission object that matches all RoleNeeds that are present within. I probably will create a new set (say needs_all) and a method that checks for all. However as I have mentioned in another issue (also demonstrated in test cases) that matching all permissions required can already be done by nesting them in any order.

[mattupstate]

At risk of sounding stupid, is this basically to get operators between permissions to behave like set operators?

[metatoaster]

I guess I should have included some more examples.

Yes, the overridden operators wasn't documented, and then if the permissions are read out in English it didn't make sense.

Previously, given this:

Permission(RoleNeed('manager') & Permission(RoleNeed('reviewer'))

Implies that a new permission object will be constructed such that both the manager and reviewer permissions will be required, i.e. the Identity must also have both the 'manager' and 'reviewer' role in order to access the resource. However, what actually happened was that union was used, resulted in this

Permission(RoleNeed('manager'), RoleNeed('reviewer'))

Which, as per the documentations, results in a Permission object that checks for either the 'manager' or the 'reviewer' role, which the or operator would actually fit. Hence this implementation.

Now as for and. That maps to intersection, but then it wouldn't work in the way that user would expect these to work, hence my suggestion of a needs_all, but then after thinking even more about it, I think if we want to make this work a more primitive Permission object that can make these checks more intuitive needs to be provided. That does make the implementation more complex. I didn't end up finishing that due to time, but it really depends if you think whether an intuitive logical operations on Permission objects is needed/useful for a "barebone" package like this.

[mattupstate]

What about something like:

Permission(RoleNeed('manager') and Permission(RoleNeed('reviewer'))

or

Permission(RoleNeed('manager') or Permission(RoleNeed('reviewer'))

That, read out in English, makes sense to me.

[metatoaster]

While they read out the same in English, the two operators are not quite the same thing (boolean operator vs. set operator), given that we are working with sets:

>>> set([1,2,3]) and set([2,67])
set([2, 67])
>>> set([1,2,3]) & set([2,67])
set([2])
>>> set([1,2,3]) or set([2,67])
set([1, 2, 3])
>>> set([1,2,3]) | set([2,67])
set([67, 1, 2, 3])

I guess this is why I am hesitant in implementing this because this might be too complicated for a simple library to handle.

[mattupstate]

I totally hear you. Though, I think a problem has arisen here.

I believe this is a significant API change due to the fundamental change in operator overloading. One must think differently, in terms of sets, instead of conditionals. However, if it is agreed that this is a significant API change then I'm afraid that anyone that uses Flask-Principal and uses conditional operators to compose permission schemes will experience some problems if they want to upgrade.

That being said, I am also aware of the fact that there is no documentation on using operators to compose permissions schemes. Anyone who happens to be using this approach most likely looked into the code or hacked something together a little differently.

I believe the set operators make more sense. However, they finally need to be documented, thoroughly, with some easy to follow examples. Are you willing to contribute a draft?

[metatoaster]

I'll take a shot, might take me a while. I have a few ideas that also includes the 'and' and 'not' operators, but having these may require a radical change (i.e. parent class for the Permission object). Part of the inspiration for this comes from the sqlalchemy ORM filters, so aside from the two implemented operators the others may differ from the "real" set operators.

I will continue work on this branch unless there are objections. It may take me a while to fully formalize this as I have limited spare time until early next year.

[metatoaster]

Might as well show you what my intention is. Note my implementation of and - with this change it should be possible to construct much more expressive permission checks.

I will still write the docs, but I figured I should have what I really wanted to do coded out before I go and write the docs.

[metatoaster]

First cut of documenting of what I added (bitwise operator) was done at 680a474.

[noirbizarre]

It seems very interesting and useful !
Any plan to merge and release this ?

[klinkin]

+1 with @noirbizarre

[ericcong]

This makes much more sense to me, is there a plan of merging this?

[indera]

This seems to be ready for a merge...