diff --git a/src/services/core/services-api/src/main/java/org/geoserver/geofence/services/dto/RuleFilter.java b/src/services/core/services-api/src/main/java/org/geoserver/geofence/services/dto/RuleFilter.java index dbdaa738..ac46dccd 100644 --- a/src/services/core/services-api/src/main/java/org/geoserver/geofence/services/dto/RuleFilter.java +++ b/src/services/core/services-api/src/main/java/org/geoserver/geofence/services/dto/RuleFilter.java @@ -8,6 +8,10 @@ import org.geoserver.geofence.core.model.Rule; import java.io.Serializable; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; /** @@ -25,7 +29,7 @@ */ public class RuleFilter implements Serializable, Cloneable { - private static final long serialVersionUID = 5629211135629700042L; + private static final long serialVersionUID = 5629211135629700043L; public enum FilterType { @@ -159,6 +163,7 @@ public RuleFilter setUser(SpecialFilterType type) { public RuleFilter setRole(String name) { if(name == null) throw new NullPointerException(); + name = sortNames(name); // force ordering to preserve hashing role.setText(name); return this; } @@ -276,6 +281,17 @@ public TextFilter getWorkspace() { // return this; // } + + private String sortNames(String s) { + if(s.contains(",")) { + s = Arrays.asList(s.split(",")).stream() + .map(n -> n.trim()) + .sorted() + .collect(Collectors.joining(",")); + } + return s; + } + @Override public boolean equals(Object obj) { if (obj == null) { diff --git a/src/services/core/services-api/src/test/java/org/geoserver/geofence/services/util/RuleFilterTest.java b/src/services/core/services-api/src/test/java/org/geoserver/geofence/services/util/RuleFilterTest.java new file mode 100644 index 00000000..d8249120 --- /dev/null +++ b/src/services/core/services-api/src/test/java/org/geoserver/geofence/services/util/RuleFilterTest.java @@ -0,0 +1,30 @@ +package org.geoserver.geofence.services.util; + +import org.geoserver.geofence.services.dto.RuleFilter; +import static org.junit.Assert.*; +import org.junit.Test; + +/** + * + */ +public class RuleFilterTest { + @Test + public void testRole() { + RuleFilter f = new RuleFilter(); + f.setRole("pippo"); + assertEquals("pippo", f.getRole().getText()); + + f.setRole("a,b"); + assertEquals("a,b", f.getRole().getText()); + + f.setRole("b,a"); + assertEquals("a,b", f.getRole().getText()); + + f.setRole("a, b"); + assertEquals("a,b", f.getRole().getText()); + + f.setRole(" b , a "); + assertEquals("a,b", f.getRole().getText()); + } + +} diff --git a/src/services/core/services-impl/src/main/java/org/geoserver/geofence/services/RuleReaderServiceImpl.java b/src/services/core/services-impl/src/main/java/org/geoserver/geofence/services/RuleReaderServiceImpl.java index 8c5c4571..f5ac39af 100644 --- a/src/services/core/services-impl/src/main/java/org/geoserver/geofence/services/RuleReaderServiceImpl.java +++ b/src/services/core/services-impl/src/main/java/org/geoserver/geofence/services/RuleReaderServiceImpl.java @@ -21,7 +21,6 @@ import org.geoserver.geofence.services.dto.AccessInfo; import org.geoserver.geofence.services.dto.AuthUser; import org.geoserver.geofence.services.dto.RuleFilter; -import org.geoserver.geofence.services.dto.RuleFilter.FilterType; import org.geoserver.geofence.services.dto.RuleFilter.IdNameFilter; import org.geoserver.geofence.services.dto.RuleFilter.SpecialFilterType; import org.geoserver.geofence.services.dto.RuleFilter.TextFilter; @@ -33,9 +32,6 @@ import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.opengis.referencing.operation.MathTransform; import org.opengis.referencing.operation.TransformException; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.context.SecurityContextHolder; import java.util.*; import java.util.Map.Entry; @@ -364,14 +360,19 @@ private String validateUsername(TextFilter filter) { } } - private String validateRolename(TextFilter filter) { + private List validateRolenames(TextFilter filter) { switch(filter.getType()) { case NAMEVALUE: - String name = filter.getText(); - if(StringUtils.isBlank(name) ) - throw new BadRequestServiceEx("Blank role name"); - return name.trim(); + String names = filter.getText(); + List roles = new ArrayList<>(); + for(String name : names.split(",")) { + if(StringUtils.isBlank(name) ) + throw new BadRequestServiceEx("Blank role name"); + roles.add(name.trim()); + } + return roles; + case DEFAULT: case ANY: return null; @@ -533,7 +534,7 @@ protected static CatalogMode getLarger(CatalogMode m1, CatalogMode m2) { * username assigned and rolename:ANY -> should consider all the roles the user belongs to * username:ANY and rolename assigned -> should consider all the users belonging to the given role * - * + * @param filter a RuleFilter for rule selection. side effect May be changed by the method * @return a Map having role names as keys, and the list of matching Rules as values. The NULL key holds the rules for the DEFAULT group. */ protected Map> getRules(RuleFilter filter) throws BadRequestServiceEx { @@ -547,7 +548,8 @@ protected Map> getRules(RuleFilter filter) throws BadRequestS Map> ret = new HashMap<>(); if(finalRoleFilter.isEmpty()) { - List found = getRuleAux(filter, filter.getRole()); + TextFilter roleFilter = new TextFilter(filter.getRole().getType(), filter.getRole().isIncludeDefault()); + List found = getRuleAux(filter, roleFilter); ret.put(null, found); } else { for (String role : finalRoleFilter) { @@ -593,65 +595,45 @@ protected Set validateUserRoles(RuleFilter filter) throws BadRequestServ String username = validateUsername(filter.getUser()); // rolename can be null if the group filter asks for ANY or DEFAULT - String rolename = validateRolename(filter.getRole()); + List requestedRoles = validateRolenames(filter.getRole()); // CSV rolenames - // filtering by both user and role is pointless - if(username != null && rolename != null) { - throw new BadRequestServiceEx("You can filter either by user or role"); - } - - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - Collection authorities; - if (authentication == null) { - authorities = new HashSet<>(); - } else { - authorities = authentication.getAuthorities(); - } Set finalRoleFilter = new HashSet<>(); // If both user and group are defined in filter - // if user doensn't belong to group, no rule is returned + // if user doesn't belong to group, no rule is returned // otherwise assigned or default rules are searched for - if(username != null) { - Set assignedRoles = userResolver.getRoles(username); - if (authorities != null) { - for (GrantedAuthority authority : authorities) { - assignedRoles.add(authority.getAuthority()); - } - } - if(rolename != null) { - if( assignedRoles.contains(rolename)) { - finalRoleFilter = Collections.singleton(rolename); + + switch(filter.getRole().getType()) { + case NAMEVALUE: + if(username != null) { + Set resolvedRoles = userResolver.getRoles(username); + for(String role: requestedRoles) { + if(resolvedRoles.contains(role)) { + finalRoleFilter.add(role); + } else { + LOGGER.warn("User does not belong to role [User:"+filter.getUser()+"] [Role:"+role+"] [ResolvedRoles:"+resolvedRoles+"]"); + } + } } else { - LOGGER.warn("User does not belong to role [User:"+filter.getUser()+"] [Role:"+filter.getRole()+"] [Roles:"+assignedRoles+"]"); - return null; + finalRoleFilter.addAll(requestedRoles); } - } else { - // User set and found, role (ANY, DEFAULT or notfound): + break; - if(filter.getRole().getType() == FilterType.ANY) { - if( ! assignedRoles.isEmpty()) { - finalRoleFilter = assignedRoles; + case ANY: + if(username != null) { + Set resolvedRoles = userResolver.getRoles(username); + if( ! resolvedRoles.isEmpty()) { + finalRoleFilter = resolvedRoles; } else { filter.setRole(SpecialFilterType.DEFAULT); } } else { - // role is DEFAULT or not found: - // if role filter request DEFAULT -> ok, apply the filter - // if role does not exists, just apply the filter, even if probably no rule will match + // no changes, use requested filtering } - } - } else { - // user is null: then either: - // 1) no filter on user was requested (ANY or DEFAULT) - // 2) user has not been found - if(rolename != null) { - finalRoleFilter.add(rolename); - } else if(filter.getUser().getType() != FilterType.ANY) { - filter.setRole(SpecialFilterType.DEFAULT); - } else { - // group is ANY, DEFAULT or not found: - // no grouping, use requested filtering - } + break; + + case DEFAULT: + // no changes + break; } return finalRoleFilter; diff --git a/src/services/core/services-impl/src/test/java/org/geoserver/geofence/services/RuleReaderServiceImplTest.java b/src/services/core/services-impl/src/test/java/org/geoserver/geofence/services/RuleReaderServiceImplTest.java index f4da6c23..8713acbb 100644 --- a/src/services/core/services-impl/src/test/java/org/geoserver/geofence/services/RuleReaderServiceImplTest.java +++ b/src/services/core/services-impl/src/test/java/org/geoserver/geofence/services/RuleReaderServiceImplTest.java @@ -17,6 +17,8 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -719,4 +721,113 @@ public void testAdminRules() { assertTrue(accessInfo.getAdminRights()); } + @Test + public void testMultiRoles() { + + RuleFilter filter; + + filter = new RuleFilter(RuleFilter.SpecialFilterType.ANY); + assertEquals(0, ruleAdminService.count(filter)); + + UserGroup p1 = createRole("p1"); + UserGroup p2 = createRole("p2"); + UserGroup p3 = createRole("p3"); + + String u1 = "TestUser1"; + String u2 = "TestUser2"; + String u3 = "TestUser3"; + + GSUser user1 = new GSUser(); + user1.setName(u1); + user1.getGroups().add(p1); + + GSUser user2 = new GSUser(); + user2.setName(u2); + user2.getGroups().add(p2); + + GSUser user12 = new GSUser(); + user12.setName(u3); + user12.getGroups().add(p1); + user12.getGroups().add(p2); + + userAdminService.insert(user1); + userAdminService.insert(user2); + userAdminService.insert(user12); + + ruleAdminService.insert(new Rule(10, u1, "p1", null, null, "s1", "r1", "w1", "l1", GrantType.ALLOW)); + ruleAdminService.insert(new Rule(20, u2, "p2", null, null, "s1", "r2", "w2", "l2", GrantType.ALLOW)); + ruleAdminService.insert(new Rule(30, u1, null, null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(40, u2, null, null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(50, u3, null, null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(51, u3, "p1", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(52, u3, "p2", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(60, null,"p1", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(70, null,"p2", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(80, null,"p3", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(901, u1, "p2", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(902, u2, "p1", null, null, null, null, null, null, GrantType.ALLOW)); + ruleAdminService.insert(new Rule(999, null, null, null, null, null, null, null, null, GrantType.ALLOW)); + + + assertRules(createFilter("*", "*"), new Integer[]{10,20,30,40,50,51,52,60,70,80,901,902,999}); + assertRules(createFilter("*", null), new Integer[]{30,40,50,999}); + assertRules(createFilter("*", "NO"), new Integer[]{30,40,50,999}); + assertRules(createFilter("*", "p1"), new Integer[]{10,30,40,50,51,60,902,999}); + assertRules(createFilter("*", "p1,NO"), new Integer[]{10,30,40,50,51,60,902,999}); + assertRules(createFilter("*", "p1,p2"), new Integer[]{10,20,30,40,50,51,52,60,70,901,902,999}); + assertRules(createFilter("*", "p1,p2,NO"), new Integer[]{10,20,30,40,50,51,52,60,70,901,902,999}); + + assertRules(createFilter(null, "*"), new Integer[]{60,70,80,999}); + assertRules(createFilter(null, null), new Integer[]{999}); + assertRules(createFilter(null, "NO"), new Integer[]{999}); + assertRules(createFilter(null, "p1"), new Integer[]{60,999}); + assertRules(createFilter(null, "p1,NO"), new Integer[]{60,999}); + assertRules(createFilter(null, "p1,p2"), new Integer[]{60,70,999}); + assertRules(createFilter(null, "p1,p2,NO"), new Integer[]{60,70,999}); + + assertRules(createFilter("NO", "*"), new Integer[]{999}); + assertRules(createFilter("NO", null), new Integer[]{999}); + assertRules(createFilter("NO", "NO"), new Integer[]{999}); + assertRules(createFilter("NO","p1"), new Integer[]{999}); + assertRules(createFilter("NO","p1,NO"), new Integer[]{999}); + assertRules(createFilter("NO","p1,p2"), new Integer[]{999}); + assertRules(createFilter("NO","p1,p2,NO"), new Integer[]{999}); + + assertRules(createFilter(u1, "*"), new Integer[]{10,30,60,999}); + assertRules(createFilter(u1, null), new Integer[]{30,999}); + assertRules(createFilter(u1, "NO"), new Integer[]{30,999}); + assertRules(createFilter(u1, "p1"), new Integer[]{10,30,60,999}); + assertRules(createFilter(u1, "p1,NO"), new Integer[]{10,30,60,999}); + assertRules(createFilter(u1, "p1,p2"), new Integer[]{10,30,60,999}); + assertRules(createFilter(u1, "p1,p2,NO"), new Integer[]{10,30,60,999}); + + assertRules(createFilter(u3, "*"), new Integer[]{50,51,52,60,70,999}); + assertRules(createFilter(u3, null), new Integer[]{50,999}); + assertRules(createFilter(u3, "NO"), new Integer[]{50,999}); + assertRules(createFilter(u3, "p1"), new Integer[]{50,51,60,999}); + assertRules(createFilter(u3, "p2"), new Integer[]{50,52,70,999}); + assertRules(createFilter(u3, "p1,NO"), new Integer[]{50,51,60,999}); + assertRules(createFilter(u3, "p1,p2"), new Integer[]{50,51,52,60,70,999}); + assertRules(createFilter(u3, "p1,p2,p3"), new Integer[]{50,51,52,60,70,999}); + assertRules(createFilter(u3, "p1,p2,NO"), new Integer[]{50,51,52,60,70,999}); + } + + + private RuleFilter createFilter(String userName, String groupName) { + return new RuleFilter(userName, groupName, "*", "*", "*", "*", "*", "*"); + } + + private void assertRules(RuleFilter filter, Integer[] expectedPriorities) { + RuleFilter origFilter = filter.clone(); + List rules = ruleReaderService.getMatchingRules(filter); + + Set pri = rules.stream() + .map(r -> r.getPriority()) + .collect(Collectors.toSet()); + Set exp = Arrays.asList(expectedPriorities).stream() + .map(i -> i.longValue()) + .collect(Collectors.toSet()); + assertEquals("Bad rule set selected for filter " + origFilter, exp, pri); + } + }