+1 It does look like there is some optimization we could do here. Even when there is multiple realms, we could check only the "failed" permissions on each subsequent realm. Same for `isPermittedAll` and any of the role or permission checks that take an array/collection.
Thoughts? On Tue, Mar 31, 2020 at 4:49 AM Modanese, Riccardo < riccardo.modan...@eurotech.com> wrote: > 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 > >
