flask-principal Match operations with permission objects with set operators

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.

Sep 18 '13 15:09 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.

Sep 18 '13 15:09 metatoaster

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

Oct 03 '13 21:10 mattupstate

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.

Oct 03 '13 22:10 metatoaster

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.

Oct 04 '13 00:10 mattupstate

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.

Oct 04 '13 00:10 metatoaster

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?

Oct 04 '13 13:10 mattupstate

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.

Oct 05 '13 00:10 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.

Oct 07 '13 11:10 metatoaster

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

Oct 19 '13 03:10 metatoaster

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