Fix #180. Filter properly user rules in RESTServiceImpl

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

@@ -119,7 +119,9 @@ public List<SecurityRule> findUserSecurityRule(String userName, long resourceId)
                 Filter.and(Filter.equal("resource.id", resourceId),
                         Filter.equal("user.name", userName)));
         searchCriteria.addFilter(securityFilter);
-
+        // now rules are not properly filtered. 
+        // so no user rules have to be removed externally (see RESTServiceImpl > ResourceServiceImpl)
+        // TODO: apply same worakaround of findGroupSecurityRule or fix searchCriteria issue (when this unit is well tested).
         return super.search(searchCriteria);
     }
 

src/modules/rest/api/src/main/java/it/geosolutions/geostore/services/rest/RESTResourceService.java

@@ -281,7 +281,7 @@ long updateAttribute(@Context SecurityContext sc, @PathParam("id") long id,
      */
     @POST
     @Path("/resource/{id}/permissions")
-    @Consumes({ MediaType.APPLICATION_XML, MediaType.TEXT_XML })
+    @Consumes({ MediaType.APPLICATION_XML, MediaType.TEXT_XML, MediaType.APPLICATION_JSON  })
     @Secured({ "ROLE_USER", "ROLE_ADMIN" })
     void updateSecurityRules(@Context SecurityContext sc, @PathParam("id") long id, @Multipart("rules") SecurityRuleList securityRules);
 

src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/impl/RESTServiceImpl.java

@@ -138,10 +138,12 @@ public boolean resourceAccessWrite(User authUser, long resourceId) {
                     authUser.getName(), resourceId);
 
             if (userSecurityRules != null && userSecurityRules.size() > 0){
-                SecurityRule sr = userSecurityRules.get(0);
-                if (sr.isCanWrite()){
-                    return true;
-                }
+            	for(SecurityRule sr : userSecurityRules){
+            		// the getUserSecurityRules returns all rules instead of user rules. So the user name check is necessary until problem with DAO is solved
+	                if (sr.isCanWrite() && sr.getUser() != null && sr.getUser().getName() == authUser.getName()){
+	                    return true;
+	                }
+            	}
             }
 
             List<String> groupNames = extratcGroupNames(authUser.getGroups());
@@ -181,9 +183,11 @@ public boolean resourceAccessRead(User authUser, long resourceId) {
                     authUser.getName(), resourceId);
 
             if (userSecurityRules != null && userSecurityRules.size() > 0){
-                SecurityRule sr = userSecurityRules.get(0);
-                if (sr.isCanRead()){
-                    return true;
+            	// the getUserSecurityRules returns all rules instead of user rules. So the user name check is necessary until problem with DAO is solved
+                for(SecurityRule sr : userSecurityRules){
+	                if (sr.isCanRead() && sr.getUser() != null && sr.getUser().getName() == authUser.getName()){
+	                    return true;
+	                }
                 }
             }
 

src/modules/rest/impl/src/test/java/it/geosolutions/geostore/rest/service/impl/RESTServiceImplTest.java

@@ -0,0 +1,203 @@
+/*
+ *  Copyright (C) 2018 GeoSolutions S.A.S.
+ *  http://www.geo-solutions.it
+ * 
+ *  GPLv3 + Classpath exception
+ * 
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ * 
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ * 
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package it.geosolutions.geostore.rest.service.impl;
+
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+
+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 it.geosolutions.geostore.services.SecurityService;
+import it.geosolutions.geostore.services.rest.impl.RESTServiceImpl;
+
+/**
+ * This test checks that the resourceAccessRead and resourceAccessWrite
+ * methods properly check and return expected canEdit and canWrite for a mock SecurityService. 
+ * 
+ * @author Lorenzo Natali, GeoSolutions S.a.s.
+ *
+ */
+public class RESTServiceImplTest {
+	TESTRESTServiceImpl restService;
+	TestSecurityService securityService;
+	User user;
+	UserGroup group;
+	UserGroup everyone;
+	UserGroup extGroup;
+	private class TestSecurityService implements SecurityService {
+		private List<SecurityRule> userSecurityRules = null;
+		private List<SecurityRule> groupSecurityRules = null;
+		public void setUserSecurityRules(List<SecurityRule> userSecurityRules) {
+			this.userSecurityRules = userSecurityRules;
+		}
+
+		public void setGroupSecurityRules(List<SecurityRule> groupSecurityRules) {
+			this.groupSecurityRules = groupSecurityRules;
+		}
+
+		@Override
+		public List<SecurityRule> getUserSecurityRule(String userName, long entityId) {
+			return userSecurityRules;
+		}
+
+		@Override
+		public List<SecurityRule> getGroupSecurityRule(List<String> groupNames, long entityId) {
+			return groupSecurityRules;
+		}
+
+	}
+    private class TESTRESTServiceImpl extends RESTServiceImpl {
+    	private SecurityService securityService = null;
+    	public void setSecurityService(SecurityService s) {
+    		securityService = s;
+    	}
+		@Override
+		protected SecurityService getSecurityService() {
+			return securityService;
+		}
+
+    }
+
+    private SecurityRule createSecurityRule(long id,User user,UserGroup group,boolean canRead,boolean canWrite) {
+    	SecurityRule sr = new SecurityRule();
+    	sr.setId(id);
+    	sr.setUser(user);
+    	sr.setGroup(group);
+    	sr.setCanRead(canRead);
+    	sr.setCanWrite(canWrite);
+    	return sr;
+    }
+
+    @Before
+    public void setUp () {
+    	// set up services
+    	 restService = new TESTRESTServiceImpl();
+    	securityService = new TestSecurityService();
+        restService.setSecurityService(securityService);
+
+        // set up users and groups 
+        user = new User();
+        user.setName("TEST_USER");
+        user.setId(new Long(100));
+        user.setRole(Role.USER);
+        everyone = new UserGroup();
+        everyone.setId(new Long(200));
+        everyone.setGroupName("everyone");
+        group = new UserGroup();
+        group.setGroupName("TEST_GROUP");
+        group.setId(new Long(201));
+        HashSet<UserGroup> groups = new HashSet<UserGroup>();
+        groups.add(everyone);
+        groups.add(group);
+        user.setGroups(groups);
+        user.setGroups(groups);
+
+    }
+
+    @Test
+    public void testRulesReadWrite() {
+
+        // set up rules for group write access
+        List<SecurityRule> groupSecurityRules = new ArrayList<SecurityRule>();
+        groupSecurityRules.add(createSecurityRule(1, null, group, true, true));
+
+        List<SecurityRule> userSecurityRules = new ArrayList<SecurityRule>();
+        userSecurityRules.add(createSecurityRule(1, user, null, true, false));
+        securityService.setGroupSecurityRules(groupSecurityRules);
+        securityService.setUserSecurityRules(userSecurityRules);
+
+        assertTrue(restService.resourceAccessRead(user, 1));
+        assertTrue(restService.resourceAccessWrite(user, 1));
+
+        groupSecurityRules = new ArrayList<SecurityRule>();
+        groupSecurityRules.add(createSecurityRule(1, null, group, true, false));
+
+        userSecurityRules = new ArrayList<SecurityRule>();
+        userSecurityRules.add(createSecurityRule(1, user, null, true, true));
+        securityService.setGroupSecurityRules(groupSecurityRules);
+        securityService.setUserSecurityRules(userSecurityRules);
+
+        assertTrue(restService.resourceAccessRead(user, 1));
+        assertTrue(restService.resourceAccessWrite(user, 1));
+
+    }
+    @Test
+    public void testRulesReadOnly() {
+        // set up rules 
+        List<SecurityRule> groupSecurityRules = new ArrayList<SecurityRule>();
+        groupSecurityRules.add(createSecurityRule(1, null, group, true, false));
+
+        List<SecurityRule> userSecurityRules = new ArrayList<SecurityRule>();
+        userSecurityRules.add(createSecurityRule(1, user, null, true, false));
+        securityService.setGroupSecurityRules(groupSecurityRules);
+        securityService.setUserSecurityRules(userSecurityRules);
+
+        assertTrue(restService.resourceAccessRead(user, 1));
+        assertFalse(restService.resourceAccessWrite(user, 1));
+    }
+
+    @Test
+    public void testRulesAccessDenied() {
+        // set up rules 
+        List<SecurityRule> groupSecurityRules = new ArrayList<SecurityRule>();
+        groupSecurityRules.add(createSecurityRule(1, null, group, false, false));
+
+        List<SecurityRule> userSecurityRules = new ArrayList<SecurityRule>();
+        userSecurityRules.add(createSecurityRule(1, user, null, false, false));
+        securityService.setGroupSecurityRules(groupSecurityRules);
+        securityService.setUserSecurityRules(userSecurityRules);
+
+        assertFalse(restService.resourceAccessRead(user, 1));
+        assertFalse(restService.resourceAccessWrite(user, 1));
+    }
+
+    @Test
+    public void testIgnoreNotValidUserRules () {
+    	// set up rules 
+        List<SecurityRule> groupSecurityRules = new ArrayList<SecurityRule>();
+        groupSecurityRules.add(createSecurityRule(1, null, group, false, false));
+
+        List<SecurityRule> userSecurityRules = new ArrayList<SecurityRule>();
+        userSecurityRules.add(createSecurityRule(1, null, group, true, false)); // this should be skipped
+        userSecurityRules.add(createSecurityRule(1, user, null, true, true));
+        securityService.setGroupSecurityRules(groupSecurityRules);
+        securityService.setUserSecurityRules(userSecurityRules);
+
+        assertTrue(restService.resourceAccessRead(user, 1));
+        assertTrue(restService.resourceAccessWrite(user, 1));
+    }
+
+
+
+
+
+
+}
+;