4 Replies Latest reply on Jul 26, 2009 1:39 AM by joblini

    Custom PermissionResolver vs PermissionStore

      I was going to create a custom implementation of PermissionStore so I started reading the included JPA based implementation to use it as reference... after reading it I realized that the methods of that implementation do not seem to be working based on the current user... that is, the listPermissions method for JpaPermissionStore does not seem to return the permission for the currently logged user, but the permissions for all the users in the system!


      Now, lets say a system has 10000 users and half of of the users
      have a particular permission, then JpaPermissionStore.listPermissions would return 5000 objects (one for each user with the permission) and then PersistentPermissionResolver.hasPermission will filter them and return true when (if) it finds that the current user has the permission?


      Here is the code in PersistentPermissionResolver:


      public boolean hasPermission(Object target, String action)
         {      
            if (permissionStore == null) return false;
            
            Identity identity = Identity.instance();
            
            if (!identity.isLoggedIn()) return false;      
            
            List<Permission> permissions = permissionStore.listPermissions(target, action);
            
            String username = identity.getPrincipal().getName();
            
            for (Permission permission : permissions)
            {
               if (permission.getRecipient() instanceof SimplePrincipal &&
                     username.equals(permission.getRecipient().getName()))
               {
                  return true;
               }
               
               if (permission.getRecipient() instanceof Role)
               {
                  Role role = (Role) permission.getRecipient();
                  
                  if (role.isConditional())
                  {
                     RuleBasedPermissionResolver resolver = RuleBasedPermissionResolver.instance();
                     if (resolver.checkConditionalRole(role.getName(), target, action)) return true;               
                  }
                  else if (identity.hasRole(role.getName()))
                  {
                     return true;
                  }
               }
            }      
            
            return false;
         }
      



      Why waste time and resources fetching the permissions for all the users? why not go straight for the permissions of the current user?


      Does this mean that if I want to avoid loading permissions of other users I should be creating a custom PermissionResolver instead of a custom PermissionStore?


      Or am I plain understanding it wrong?

        • 1. Re: Custom PermissionResolver vs PermissionStore
          asookazian

          This is a SBryzak question.  Security concerns are typically session (i.e. current user) based.
          If what you're saying is correct, it sounds like the algorithm needs to be refactored to be more performant.  Why would we care about permissions for any user other than the current user at that point in time?

          • 2. Re: Custom PermissionResolver vs PermissionStore
            joblini

            Hi Francisco,


            Yes, I have noticed this too, it is hardly optimal!


            Another problem I ran into with the code you posted is that, since Role extends SimplePrincipal, the permission will be granted if a user name happens to be the same as a role name! 


            The fix is to move


            if (permission.getRecipient() instanceof Role)



            before


             if (permission.getRecipient() instanceof SimplePrincipal



            Regards,
            Ingo


            • 3. Re: Custom PermissionResolver vs PermissionStore
              joblini

              PS Although it does not seem optimal, I am not sure how else it could be done, since the recipient of the permission can be a user or a role.


              So if we pass username to listPermissions, we don't get the roles.
              Then we would have to repeat listPermissions for each role assigned to the user.


              Possible ways to improve performance:



              • Assign permissions to roles instead of users.




              • Modify the code in JpaPermissionStore to take advantage of 2nd level cache, since currently every permission check results in a hit to the database.








              • 4. Re: Custom PermissionResolver vs PermissionStore
                joblini

                PS  The chick for


                if (role.isConditional())



                results in 2 additional requests to the database for every role based permission check, a quick way to fix this is to remove the @Conditional annonation from the Role entity.