Optimized user permission checks on resource. (#406)

* Optimized user permission checks on resource.

* Removed unused fields.

src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/SecurityDAO.java

@@ -41,22 +41,18 @@ public interface SecurityDAO extends RestrictedGenericDAO<SecurityRule> {
      * @param resourceId
      * @return List<SecurityRule>
      */
-    public List<SecurityRule> findUserSecurityRule(String userName, long resourceId);
+    List<SecurityRule> findUserSecurityRule(String userName, long resourceId);
 
     /**
      * @param groupNames
      * @param resourceId
      * @return
      */
-    public List<SecurityRule> findGroupSecurityRule(List<String> groupNames, long resourceId);
+    List<SecurityRule> findGroupSecurityRule(List<String> groupNames, long resourceId);
 
     /**
      * @param resourceId
      * @return List<SecurityRule>
      */
-    public List<SecurityRule> findResourceSecurityRules(long resourceId);
-
-    List<SecurityRule> findUserSecurityRules(long userId);
-
-    List<SecurityRule> findUserGroupSecurityRules(long userGroupId);
+    List<SecurityRule> findResourceSecurityRules(long resourceId);
 }

src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/UserDAO.java

@@ -20,10 +20,15 @@
 package it.geosolutions.geostore.core.dao;
 
 import it.geosolutions.geostore.core.model.User;
+import java.util.List;
 
 /**
  * Interface UserDAO.
  *
  * @author Tobia di Pisa (tobia.dipisa at geo-solutions.it)
  */
-public interface UserDAO extends RestrictedGenericDAO<User> {}
+public interface UserDAO extends RestrictedGenericDAO<User> {
+    default List<User> findFavoritedBy(Long resourceId) {
+        return null;
+    }
+}

src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/impl/SecurityDAOImpl.java

@@ -246,39 +246,6 @@ public List<SecurityRule> findResourceSecurityRules(long resourceId) {
         return super.search(searchCriteria);
     }
 
-    /**
-     * @param userId
-     * @return List<SecurityRule>
-     */
-    @Override
-    public List<SecurityRule> findUserSecurityRules(long userId) {
-        Search searchCriteria = new Search(SecurityRule.class);
-
-        Filter securityFilter = Filter.equal("user.id", userId);
-
-        searchCriteria.addFilter(securityFilter);
-
-        return super.search(searchCriteria);
-    }
-
-    /**
-     * @param userGroupId
-     * @return List<SecurityRule>
-     */
-    @Override
-    public List<SecurityRule> findUserGroupSecurityRules(long userGroupId) {
-        Search searchCriteria = new Search(SecurityRule.class);
-
-        Filter securityFilter = Filter.equal("group.id", userGroupId);
-
-        searchCriteria.addFilter(securityFilter);
-
-        return super.search(searchCriteria);
-    }
-
-    /* (non-Javadoc)
-     * @see it.geosolutions.geostore.core.dao.ResourceDAO#findGroupSecurityRule(java.lang.String, long)
-     */
     @Override
     public List<SecurityRule> findGroupSecurityRule(List<String> groupNames, long resourceId) {
         List<SecurityRule> rules = findResourceSecurityRules(resourceId);

src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/impl/UserDAOImpl.java

@@ -19,7 +19,9 @@
  */
 package it.geosolutions.geostore.core.dao.impl;
 
+import com.googlecode.genericdao.search.Filter;
 import com.googlecode.genericdao.search.ISearch;
+import com.googlecode.genericdao.search.Search;
 import it.geosolutions.geostore.core.dao.UserDAO;
 import it.geosolutions.geostore.core.model.User;
 import it.geosolutions.geostore.core.model.UserAttribute;
@@ -167,4 +169,14 @@ public User[] save(User... entities) {
 
         return super.save(entities);
     }
+
+    @Override
+    public List<User> findFavoritedBy(Long resourceId) {
+
+        Search searchCriteria = new Search(User.class);
+
+        searchCriteria.addFilter(Filter.some("favorites", Filter.equal("id", resourceId)));
+
+        return super.search(searchCriteria);
+    }
 }

src/core/services-api/src/main/java/it/geosolutions/geostore/services/ResourcePermissionService.java

@@ -6,48 +6,35 @@
 public interface ResourcePermissionService {
 
     /**
-     * Verifies whether the user or any of their groups is the owner of the resource and has read
+     * Verifies whether the user or any of its groups is the owner of the resource and has read
      * permissions on it.
      *
-     * <p>Be aware to fetch the user security rules prior to call this method.
+     * <p>Be aware to fetch the resource security rules prior to call this method.
      *
+     * @param resource
      * @param user
-     * @param resourceId
-     * @return <code>true</code> if the user can read the resource, <code>false</code> otherwise
-     * @throws IllegalArgumentException if the user security rules have not been initialized
+     * @return <code>true</code> if the resource can be read by the user, <code>false</code>
+     *     otherwise
+     * @throws IllegalArgumentException if the resource security rules have not been initialized
      *     properly
      */
-    boolean canUserReadResource(User user, Long resourceId);
+    boolean canResourceBeReadByUser(Resource resource, User user);
 
     /**
-     * Verifies whether the user or any of their groups is the owner of the resource and has write
+     * Verifies whether the user or any of its groups is the owner of the resource and has write
      * permissions on it.
      *
      * <p>GUEST users can not access to the delete and edit (resource, data blob is editable)
      * services, so only admins and authenticated users with write permissions can.
      *
-     * <p>Be aware to fetch the user security rules prior to call this method.
+     * <p>Be aware to fetch the resource security rules prior to call this method.
      *
-     * @param user
      * @param resource
-     * @return <code>true</code> if the user can write the resource, <code>false</code> otherwise
-     * @throws IllegalArgumentException if the user security rules have not been initialized
-     *     properly
-     */
-    boolean canUserWriteResource(User user, Resource resource);
-
-    /**
-     * Verifies whether the user or any of their groups is the owner of the resource and has both
-     * read and write permissions on it.
-     *
-     * <p>Be aware to fetch the user security rules prior to call this method.
-     *
      * @param user
-     * @param resource
-     * @return <code>true</code> if the user can read and write the resource, <code>false</code>
+     * @return <code>true</code> if the resource can be written by the user, <code>false</code>
      *     otherwise
-     * @throws IllegalArgumentException if the user security rules have not been initialized
+     * @throws IllegalArgumentException if the resource security rules have not been initialized
      *     properly
      */
-    boolean canUserReadAndWriteResource(User user, Resource resource);
+    boolean canResourceBeWrittenByUser(Resource resource, User user);
 }

src/core/services-api/src/main/java/it/geosolutions/geostore/services/ResourceService.java

@@ -253,4 +253,23 @@ long count(SearchFilter filter, User user, boolean favoritesOnly)
 
     long insertAttribute(long id, String name, String value, DataType type)
             throws InternalErrorServiceEx;
+
+    /**
+     * Update the resource entity by fetching its security rules from the database.
+     *
+     * @param resource
+     */
+    default void fetchSecurityRules(Resource resource) {
+        /* no-op */
+    }
+
+    /**
+     * Update the resource entity by fetching from the database the users who marked it as a
+     * favorite.
+     *
+     * @param resource
+     */
+    default void fetchFavoritedBy(Resource resource) {
+        /* no-op */
+    }
 }

src/core/services-api/src/main/java/it/geosolutions/geostore/services/UserService.java

@@ -129,16 +129,6 @@ List<User> getAll(Integer page, Integer entries, String nameLike, boolean includ
 
     Collection<User> getByGroup(UserGroup group);
 
-    /**
-     * Update the user entity by fetching its security rules and group security rules from the
-     * database.
-     *
-     * @param user
-     */
-    default void fetchSecurityRules(User user) {
-        /* no-op */
-    }
-
     /**
      * Update the user entity by fetching its favorites resources from the database.
      *

src/core/services-impl/src/main/java/it/geosolutions/geostore/services/ResourcePermissionServiceImpl.java

@@ -3,102 +3,107 @@
 import it.geosolutions.geostore.core.model.Resource;
 import it.geosolutions.geostore.core.model.SecurityRule;
 import it.geosolutions.geostore.core.model.User;
+import it.geosolutions.geostore.core.model.UserGroup;
 import it.geosolutions.geostore.core.model.enums.Role;
 import java.util.List;
 import java.util.function.BiFunction;
 
 public class ResourcePermissionServiceImpl implements ResourcePermissionService {
 
-    private final BiFunction<SecurityRule, Resource, Boolean> resourceOwnership =
-            (rule, resource) -> resource.getId().equals(rule.getResource().getId());
-    private final BiFunction<SecurityRule, Resource, Boolean> resourceOwnershipWithReadPermission =
-            (rule, resource) -> resourceOwnership.apply(rule, resource) && rule.isCanRead();
-    private final BiFunction<SecurityRule, Resource, Boolean> resourceOwnershipWithWritePermission =
-            (rule, resource) -> resourceOwnership.apply(rule, resource) && rule.isCanWrite();
-    private final BiFunction<SecurityRule, Resource, Boolean>
-            resourceOwnershipWithReadAndWritePermission =
-                    (rule, resource) ->
-                            resourceOwnershipWithWritePermission.apply(rule, resource)
-                                    && resourceOwnershipWithReadPermission.apply(rule, resource);
+    private final BiFunction<SecurityRule, User, Boolean> resourceUserOwnership =
+            (rule, user) -> user.getId().equals(rule.getUser().getId());
+
+    private final BiFunction<SecurityRule, UserGroup, Boolean> resourceGroupOwnership =
+            (rule, group) -> group.getId().equals(rule.getGroup().getId());
+
+    private final BiFunction<SecurityRule, User, Boolean> resourceUserOwnershipWithReadPermission =
+            (rule, user) ->
+                    rule.getUser() != null
+                            && resourceUserOwnership.apply(rule, user)
+                            && rule.isCanRead();
+
+    private final BiFunction<SecurityRule, UserGroup, Boolean>
+            resourceGroupOwnershipWithReadPermission =
+                    (rule, group) ->
+                            rule.getGroup() != null
+                                    && resourceGroupOwnership.apply(rule, group)
+                                    && rule.isCanRead();
+
+    private final BiFunction<SecurityRule, User, Boolean> resourceUserOwnershipWithWritePermission =
+            (rule, user) ->
+                    rule.getUser() != null
+                            && resourceUserOwnership.apply(rule, user)
+                            && rule.isCanWrite();
+
+    private final BiFunction<SecurityRule, UserGroup, Boolean>
+            resourceGroupOwnershipWithWritePermission =
+                    (rule, group) ->
+                            rule.getGroup() != null
+                                    && resourceGroupOwnership.apply(rule, group)
+                                    && rule.isCanWrite();
 
     @Override
-    public boolean canUserReadResource(User user, Long resourceId) {
-        Resource resource = new Resource();
-        resource.setId(resourceId);
-
+    public boolean canResourceBeReadByUser(Resource resource, User user) {
         return user.getRole().equals(Role.ADMIN)
                 || isUserOwnerWithReadPermission(user, resource)
                 || haveUserGroupsOwnershipWithReadPermission(user, resource);
     }
 
     private boolean isUserOwnerWithReadPermission(User user, Resource resource) {
-        checkUserSecurityRules(user);
-        return checkSecurityRulesAgainstResource(
-                user.getSecurity(), resource, resourceOwnershipWithReadPermission);
+        checkResourceSecurityRules(resource);
+        return checkSecurityRulesAgainstUser(
+                resource.getSecurity(), user, resourceUserOwnershipWithReadPermission);
     }
 
     private boolean haveUserGroupsOwnershipWithReadPermission(User user, Resource resource) {
-        return checkUserGroupsSecurityRulesAgainstResource(
-                user, resource, resourceOwnershipWithReadPermission);
+        return checkResourceSecurityRulesAgainstUserGroup(
+                user, resource, resourceGroupOwnershipWithReadPermission);
     }
 
     @Override
-    public boolean canUserWriteResource(User user, Resource resource) {
+    public boolean canResourceBeWrittenByUser(Resource resource, User user) {
         return !user.getRole().equals(Role.GUEST)
                 && (user.getRole().equals(Role.ADMIN)
                         || isUserOwnerWithWritePermission(user, resource)
                         || haveUserGroupsOwnershipWithWritePermission(user, resource));
     }
 
     private boolean isUserOwnerWithWritePermission(User user, Resource resource) {
-        checkUserSecurityRules(user);
-        return checkSecurityRulesAgainstResource(
-                user.getSecurity(), resource, resourceOwnershipWithWritePermission);
-    }
-
-    private boolean haveUserGroupsOwnershipWithWritePermission(User user, Resource resource) {
-        return checkUserGroupsSecurityRulesAgainstResource(
-                user, resource, resourceOwnershipWithWritePermission);
+        checkResourceSecurityRules(resource);
+        return checkSecurityRulesAgainstUser(
+                resource.getSecurity(), user, resourceUserOwnershipWithWritePermission);
     }
 
-    @Override
-    public boolean canUserReadAndWriteResource(User user, Resource resource) {
-        return user.getRole().equals(Role.ADMIN)
-                || isUserOwnerWithReadAndWritePermission(user, resource)
-                || haveUserGroupOwnershipWithReadAndWritePermission(user, resource);
-    }
-
-    private boolean isUserOwnerWithReadAndWritePermission(User user, Resource resource) {
-        checkUserSecurityRules(user);
-        return checkSecurityRulesAgainstResource(
-                user.getSecurity(), resource, resourceOwnershipWithReadAndWritePermission);
-    }
-
-    private void checkUserSecurityRules(User user) {
-        if (user.getSecurity() == null) {
+    private void checkResourceSecurityRules(Resource resource) {
+        if (resource.getSecurity() == null) {
             throw new IllegalArgumentException(
-                    "set user security rules prior checking for permissions");
+                    "set resource security rules prior checking for permissions");
         }
     }
 
-    private boolean haveUserGroupOwnershipWithReadAndWritePermission(User user, Resource resource) {
-        return checkUserGroupsSecurityRulesAgainstResource(
-                user, resource, resourceOwnershipWithReadAndWritePermission);
+    private boolean haveUserGroupsOwnershipWithWritePermission(User user, Resource resource) {
+        return checkResourceSecurityRulesAgainstUserGroup(
+                user, resource, resourceGroupOwnershipWithWritePermission);
     }
 
-    private boolean checkUserGroupsSecurityRulesAgainstResource(
-            User user, Resource resource, BiFunction<SecurityRule, Resource, Boolean> check) {
+    private boolean checkResourceSecurityRulesAgainstUserGroup(
+            User user, Resource resource, BiFunction<SecurityRule, UserGroup, Boolean> check) {
         return user.getGroups().stream()
                 .anyMatch(
                         group ->
-                                checkSecurityRulesAgainstResource(
-                                        group.getSecurity(), resource, check));
+                                checkSecurityRulesAgainstUserGroup(
+                                        resource.getSecurity(), group, check));
+    }
+
+    private boolean checkSecurityRulesAgainstUser(
+            List<SecurityRule> rules, User user, BiFunction<SecurityRule, User, Boolean> check) {
+        return rules.stream().anyMatch(rule -> check.apply(rule, user));
     }
 
-    private boolean checkSecurityRulesAgainstResource(
+    private boolean checkSecurityRulesAgainstUserGroup(
             List<SecurityRule> rules,
-            Resource resource,
-            BiFunction<SecurityRule, Resource, Boolean> check) {
-        return rules.stream().anyMatch(rule -> check.apply(rule, resource));
+            UserGroup group,
+            BiFunction<SecurityRule, UserGroup, Boolean> check) {
+        return rules.stream().anyMatch(rule -> check.apply(rule, group));
     }
 }