I agree with your analysis.

The goal is if there is a way to avoid multiple calls to the 
doGetAuthorizationInfo (at least with our use case).
So changing the loop can avoid too many calls since, regardless of the 
permissions checked, there is just one call per realm. On the other hand, if 
there are few realms, as you said, the risk is to execute checks also if the 
result is already determined.

Then, assuming to have one realm, do you think our solution could be right?

> Il giorno 30 mar 2020, alle ore 12:35, Benjamin Marwell <bmarw...@gmail.com> 
> ha scritto:
> 
> I think you "just" changed the loop:
> 
> The current ModularRealmAuthorizer checks:
> 
> boolean permission[]
> For every permission
>   for every realm
>      permission[i] = isPermitted
> 
> But your loop does:
> 
> boolean permission[]
> For every realm
>   for every permission
>     permission[i] = isPermitted
>     if (permission = true); break
> 
> i.e. changing the loop enables to shortcircuit.
> 
> Additionally: In every case, we could skip those which are already
> permitted (i.e. set to true):
> 
> for every permission
>  if (permission[i] = true); continue
> 
> Did I get this right?
> 
> Am Mo., 30. März 2020 um 08:52 Uhr schrieb Modanese, Riccardo
> <riccardo.modan...@eurotech.com>:
>> 
>> Hi all,
>> 
>>   I have a question about the ModularRealmAuthorizer implementation (Shiro 
>> version 1.3.2).
>> There are 2 methods to check multiple permissions:
>>  public boolean[] isPermitted(PrincipalCollection principals, String... 
>> permissions)
>>  public boolean[] isPermitted(PrincipalCollection principals, 
>> List<Permission> permissions)
>> 
>> Both of these implementations does a loop to call the isPermitted method 
>> with a single permission.
>> So the AuthorizingRealm method doGetAuthorizationInfo is called at each 
>> iteration. (we aren’t using cache)
>> 
>> Since the AuthorizingRealm has a specific implementation for the isPermitted 
>> method with multiple permissions we tried to use it customizing the 
>> ModularRealmAuthorizer.
>> In Kapua project we wrote a custom ModularRealmAuthorizer implementation 
>> (see [1]) to reduce the doGetAuthorizationInfo calls count (with the "at 
>> least one realm” as result aggregation strategy).
>> 
>> In the ModularRealmAuthorizer did you implement the isPermitted method with 
>> the for loop to use the realm aggregation strategy configuration for the 
>> realms results?
>> If not, is it possible to change the implementation to make it more 
>> performant (avoiding multiple doGetAuthorizationInfo)?
>> 
>> Thank you
>> 
>> Riccardo
>> 
>> [1] 
>> https://github.com/eclipse/kapua/blob/develop/broker-core/src/main/java/org/eclipse/kapua/broker/core/security/EnhModularRealmAuthorizer.java#L47

Reply via email to