Ordering of user roles in SimpleAuthorizationInfo

[Dan Haywood <da...@haywood-associates.co.uk>]

Working on integrating Shiro with Isis, I have the basic IniRealm working
nicely, mapping users to roles though to permissions, using the basic
WildcardPermission.  What a given user can do is the sum of the
permissions... in other words all very standard.

What I would like to do now is introduce the concept of "negative" or
"vetoing" permissions.  Using a custom subclass of WildcardPermission, I
have defined the syntax:

[!][permGroup/]packageName:className:memberName:r,w

where the "!" implies a veto, and the permGroup implies a bunch of
permissions to be evaluated together.

For example:

fred = pass, user
joe = pass, read-only, user
bill = admin

read-only = !group1/*:*:*:w
user          =  group1/*:ToDoItem:*:*
admin = *

means that fred has access to r/w for ToDoItem, but joe only has r
permissions because of the read-only veto and is in the same permission
group "group1".

I've almost got this working using a bit of ThreadLocal hackery, however it
depends on the ordering, with the vetos being applied before the non-vetos.
 Looking at the implementation of the IniRealm, I see that the role holds
permissions in order (through use of a LinkedHashSet), however the order of
a users roles are non-deterministic (because it uses a HashSet).  This is a
problem for me.

~~~
My question is therefore: could SimpleAuthorizationInfo be changed so that,
when it lazily creates a HashSet (in addRole(...) or addRoles(...)), that
it uses a LinkedHashSet?   This would only change the internal
implementaition, so perhaps could go into a point release?

~~~
A larger change to remove my threadlocal hackery might be to change the API
so that there is a "PermissionEvaluationContext", acting a bit like a
collecting parameter and allowing permissions to indicate whether they
represent an "allow", a "veto", or neither.  Then, the
PermissionEvaluationContext could be asked what the outcome was.


Thx
Dan

Re: Ordering of user roles in SimpleAuthorizationInfo

[Dan Haywood <da...@haywood-associates.co.uk>]

On 6 January 2013 01:02, Les Hazlewood <lh...@apache.org> wrote:

> read-only = !group1/*:*:*:w
>> user          =  group1/*:ToDoItem:*:*
>> admin = *
>>
>> means that fred has access to r/w for ToDoItem, but joe only has r
>> permissions because of the read-only veto and is in the same permission
>> group "group1".
>>
>
> I'm not saying the following is better or not since I'm unaware of your
> particular use cases, but another technique I've seen is the following:
>
> - you create an AuthorizingRealm subclass (or in your case, subclass
> IniRealm) and override the relevant isPermitted implementations.  These
> implementations would check for a specific implementation of
> AuthorizationInfo that you create:
>

I did explore doing this, however I found that when I subclassed IniRealm,
that my implementation didn't get injected with an Ini.  Looking at the
1.2.1 code, it seems that there is special case processing for the
defaults, see for example IniSecurityManagerFactory#createRealm(...).

I wonder whether there should be an IniAware interface that can be
implemented so that you could inject the Ini automatically to any custom
realms that declared that dependency?



>
> - your AuthorizationInfo implementation would have some methods:
>  getPermissions() and getNegativePermissions() (or getDeniedPermissions()
> or whatever).  Then the realm implementation can check both collections (in
> the order you choose) to see if the operation is permitted or not.  The
> trick here is in determining what overrides the other:  does a granted
> permission override a negative permission?  Or vice versa?
>

I took the view that a negative permission overrides a granted permission,
but only if in the same permissionGroup.  Thus read-only, admin would have
all permissions, because the admin permission "*" is not in the same
permission group as the read-only.




>
>
>
>>
>> ~~~
>> My question is therefore: could SimpleAuthorizationInfo be changed so
>> that, when it lazily creates a HashSet (in addRole(...) or addRoles(...)),
>> that it uses a LinkedHashSet?   This would only change the internal
>> implementaition, so perhaps could go into a point release?
>>
>
> I see no problem with this - sounds fine to me.  Could you please create a
> Jira issue with a patch so it can be tracked/recorded?
>

I've raised SHIRO-405, with a patch attached.



>
>
>> ~~~
>> A larger change to remove my threadlocal hackery might be to change the
>> API so that there is a "PermissionEvaluationContext", acting a bit like a
>> collecting parameter and allowing permissions to indicate whether they
>> represent an "allow", a "veto", or neither.  Then, the
>> PermissionEvaluationContext could be asked what the outcome was.
>>
>
> This is indeed a more significant change - but if a LinkedHashSet is used
> as mentioned above, is this still necessary?
>

No, I can make do without.  As I say, my code is a bit hacky, that's all.


Cheers
Dan



>
> Cheers,
>
> Les
>

Re: Ordering of user roles in SimpleAuthorizationInfo

[Les Hazlewood <lh...@apache.org>]

>
> read-only = !group1/*:*:*:w
> user          =  group1/*:ToDoItem:*:*
> admin = *
>
> means that fred has access to r/w for ToDoItem, but joe only has r
> permissions because of the read-only veto and is in the same permission
> group "group1".
>

I'm not saying the following is better or not since I'm unaware of your
particular use cases, but another technique I've seen is the following:

- you create an AuthorizingRealm subclass (or in your case, subclass
IniRealm) and override the relevant isPermitted implementations.  These
implementations would check for a specific implementation of
AuthorizationInfo that you create:

- your AuthorizationInfo implementation would have some methods:
 getPermissions() and getNegativePermissions() (or getDeniedPermissions()
or whatever).  Then the realm implementation can check both collections (in
the order you choose) to see if the operation is permitted or not.  The
trick here is in determining what overrides the other:  does a granted
permission override a negative permission?  Or vice versa?

If anyone has any general purpose recommendations for how Shiro might
incorporate these things into a future release, I think we're all ears. :)


> I've almost got this working using a bit of ThreadLocal hackery, however
> it depends on the ordering, with the vetos being applied before the
> non-vetos.  Looking at the implementation of the IniRealm, I see that the
> role holds permissions in order (through use of a LinkedHashSet), however
> the order of a users roles are non-deterministic (because it uses a
> HashSet).  This is a problem for me.
>
> ~~~
> My question is therefore: could SimpleAuthorizationInfo be changed so
> that, when it lazily creates a HashSet (in addRole(...) or addRoles(...)),
> that it uses a LinkedHashSet?   This would only change the internal
> implementaition, so perhaps could go into a point release?
>

I see no problem with this - sounds fine to me.  Could you please create a
Jira issue with a patch so it can be tracked/recorded?


> ~~~
> A larger change to remove my threadlocal hackery might be to change the
> API so that there is a "PermissionEvaluationContext", acting a bit like a
> collecting parameter and allowing permissions to indicate whether they
> represent an "allow", a "veto", or neither.  Then, the
> PermissionEvaluationContext could be asked what the outcome was.
>

This is indeed a more significant change - but if a LinkedHashSet is used
as mentioned above, is this still necessary?

Cheers,

Les