From 110018b03f8f7f7ab8f2be0a3c59b1cedadd9b50 Mon Sep 17 00:00:00 2001 From: mbarto Date: Thu, 18 Mar 2021 11:41:59 +0100 Subject: [PATCH] #219: improvements to the LDAP compatible DAOs to fully replace the standard ones in a MapStore standard security configuration (#220) * #219: improvements to the LDAP compatible DAOs to fully replace the standard ones in a MapStore stadanrd security configuration * Update src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserDAOImpl.java Co-authored-by: Lorenzo Natali Co-authored-by: Lorenzo Natali --- .../core/model/enums/GroupReservedNames.java | 23 ++ .../core/model/GroupReservedNamesTest.java | 42 +++ .../core/dao/ldap/impl/LdapBaseDAOImpl.java | 25 +- .../core/dao/ldap/impl/UserDAOImpl.java | 117 +++++++- .../core/dao/ldap/impl/UserGroupDAOImpl.java | 12 +- .../geostore/core/dao/ldap/BaseDAOTest.java | 278 ++++++++++++++++++ .../geostore/core/dao/ldap/UserDAOTest.java | 148 +++------- .../core/dao/ldap/UserGroupDAOTest.java | 67 +---- .../geostore/services/UserServiceImpl.java | 13 +- .../services/UserServiceImplTest.java | 45 +++ .../SessionTokenAuthenticationFilter.java | 13 +- .../UserLdapAuthenticationProvider.java | 23 +- .../SessionTokenAuthenticationFilterTest.java | 30 +- 13 files changed, 613 insertions(+), 223 deletions(-) create mode 100644 src/core/model/src/test/java/it/geosolutions/geostore/core/model/GroupReservedNamesTest.java create mode 100644 src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/BaseDAOTest.java diff --git a/src/core/model/src/main/java/it/geosolutions/geostore/core/model/enums/GroupReservedNames.java b/src/core/model/src/main/java/it/geosolutions/geostore/core/model/enums/GroupReservedNames.java index 438c6024..d5bf1ba9 100644 --- a/src/core/model/src/main/java/it/geosolutions/geostore/core/model/enums/GroupReservedNames.java +++ b/src/core/model/src/main/java/it/geosolutions/geostore/core/model/enums/GroupReservedNames.java @@ -19,6 +19,13 @@ */ package it.geosolutions.geostore.core.model.enums; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import it.geosolutions.geostore.core.model.UserGroup; /** * @author DamianoG @@ -51,4 +58,20 @@ public static boolean isAllowedName(String groupNameToCheck){ } return true; } + + /** + * Utility method to remove Reserved group (for example EVERYONE) from a group list + * + * @param groups + * @return + */ + public static Set checkReservedGroups(Collection groups) { + Set result = new HashSet(); + for(UserGroup ug : groups){ + if(GroupReservedNames.isAllowedName(ug.getGroupName())){ + result.add(ug); + } + } + return result; + } } diff --git a/src/core/model/src/test/java/it/geosolutions/geostore/core/model/GroupReservedNamesTest.java b/src/core/model/src/test/java/it/geosolutions/geostore/core/model/GroupReservedNamesTest.java new file mode 100644 index 00000000..98a5cc62 --- /dev/null +++ b/src/core/model/src/test/java/it/geosolutions/geostore/core/model/GroupReservedNamesTest.java @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2021 - 2011 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 . + */ +package it.geosolutions.geostore.core.model; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.junit.Test; +import it.geosolutions.geostore.core.model.enums.GroupReservedNames; +import static org.junit.Assert.assertEquals; + +public class GroupReservedNamesTest { + @Test + public void testRemoveReserved() { + List groups = new ArrayList(); + UserGroup everyOne = new UserGroup(); + everyOne.setGroupName(GroupReservedNames.EVERYONE.groupName()); + groups.add(everyOne); + UserGroup sample = new UserGroup(); + sample.setGroupName("sample"); + groups.add(sample); + + Set result = GroupReservedNames.checkReservedGroups(groups); + + assertEquals(1, result.size()); + assertEquals("sample", result.iterator().next().getGroupName()); + } +} diff --git a/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/LdapBaseDAOImpl.java b/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/LdapBaseDAOImpl.java index 317c03b9..2c4b7181 100644 --- a/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/LdapBaseDAOImpl.java +++ b/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/LdapBaseDAOImpl.java @@ -23,7 +23,10 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; + import javax.naming.directory.DirContext; + +import org.apache.commons.lang.StringUtils; import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -31,6 +34,7 @@ import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.DirContextProcessor; import org.springframework.ldap.core.LdapTemplate; + import com.googlecode.genericdao.search.Filter; import com.googlecode.genericdao.search.ISearch; @@ -197,6 +201,7 @@ private String getLdapFilter(Filter filter, Map propertyMapper) mapper = (Map)propertyMapper.get(property); } } + // we support the minimum set of operators used by GeoStore user and group services switch(filter.getOperator()) { case Filter.OP_EQUAL: return property + "=" + filter.getValue().toString(); @@ -204,12 +209,30 @@ private String getLdapFilter(Filter filter, Map propertyMapper) return getLdapFilter((Filter)filter.getValue(), mapper); case Filter.OP_ILIKE: return property + "=" + filter.getValue().toString().replaceAll("[%]", "*"); + case Filter.OP_IN: + return getInLdapFilter(property, (List)filter.getValue()); //TODO: implement all operators } return ""; } /** + * Builds a filter for property in (values) search type. + * This is done by creating a list of property=value combined by or (|). + * + * @param property + * @param values + * @return + */ + private String getInLdapFilter(String property, List values) { + List filters = new ArrayList(); + for(Object value : values) { + filters.add("(" + property + "=" + value.toString() + ")"); + } + return StringUtils.join(filters, "|"); + } + + /** * Returns true if the given search has one or more filters on a nested object. * * @param search @@ -263,7 +286,7 @@ protected ISearch getNestedSearch(ISearch search) { * @param filter * @return */ - private Filter getNestedFilter(Filter filter) { + protected Filter getNestedFilter(Filter filter) { if (filter.getOperator() == Filter.OP_SOME || filter.getOperator() == Filter.OP_ALL) { return (Filter)filter.getValue(); } diff --git a/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserDAOImpl.java b/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserDAOImpl.java index f372e990..b2b3adef 100644 --- a/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserDAOImpl.java +++ b/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserDAOImpl.java @@ -21,20 +21,28 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; + import javax.naming.directory.SearchControls; + import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DirContextProcessor; import org.springframework.ldap.core.support.AbstractContextMapper; + +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; import it.geosolutions.geostore.core.model.UserGroup; +import it.geosolutions.geostore.core.model.enums.Role; /** * Class UserDAOImpl. @@ -46,6 +54,7 @@ public class UserDAOImpl extends LdapBaseDAOImpl implements UserDAO { protected Map attributesMapper = new HashMap(); private Pattern memberPattern = Pattern.compile("^(.*)$"); + private String adminRoleGroup = "ADMIN"; UserGroupDAOImpl userGroupDAO = null; @@ -60,7 +69,21 @@ public void setUserGroupDAO(UserGroupDAOImpl userGroupDAO) { } } + public String getAdminRoleGroup() { + return adminRoleGroup; + } + /** + * Case insensitive name of the group associated to the ADMIN role. + * This is used to assign ADMIN role to users belonging to a specific LDAP group. + * + * @param adminRoleGroup ADMIN role group name (default to ADMIN) + */ + public void setAdminRoleGroup(String adminRoleGroup) { + this.adminRoleGroup = adminRoleGroup; + } + + /** * Sets regular expression used to extract the member user name from a member LDAP attribute. * The LDAP attribute can contain a DN, so this is useful to extract the real member name from it. * @@ -91,8 +114,10 @@ public void setAttributesMapper(Map attributesMapper) { */ @Override public void persist(User... entities) { - throw new UnsupportedOperationException(); - // NOT SUPPORTED + // we don't want to throw an exception on write operations, because + // some authentication providers try to persist stuff for synchronization + // purposes and they don't know DAOs can be readonly + // TODO: make readonly behaviour explicit } /* @@ -114,6 +139,7 @@ public List findAll() { @Override public List search(ISearch search) { if (isNested(search)) { + // users belonging to a group List users = new ArrayList(); for(UserGroup group : userGroupDAO.search(getNestedSearch(search))) { users.addAll(group.getUsers()); @@ -152,21 +178,68 @@ protected User doMapFromContext(DirContextOperations ctx) { user.setId((long)counter++); // TODO: optionally map an attribute to the id user.setEnabled(true); user.setName(ctx.getStringAttribute(nameAttribute)); - List attributes = new ArrayList(); - for (String ldapAttr : attributesMapper.keySet()) { - String value = ctx.getStringAttribute(ldapAttr); - String userAttr = attributesMapper.get(ldapAttr); - UserAttribute attr = new UserAttribute(); - attr.setName(userAttr); - attr.setValue(value); - attributes.add(attr); - } - user.setAttribute(attributes); + user.setAttribute(fetchAttributes(ctx)); + assignGroupsAndRole(ctx, user); return user; } - }, processor); } + + /** + * Gets all the attributes defined in AttributeMapper. + * + * @param ctx + * @return + */ + private List fetchAttributes(DirContextOperations ctx) { + List attributes = new ArrayList(); + for (String ldapAttr : attributesMapper.keySet()) { + String value = ctx.getStringAttribute(ldapAttr); + String userAttr = attributesMapper.get(ldapAttr); + UserAttribute attr = new UserAttribute(); + attr.setName(userAttr); + attr.setValue(value); + attributes.add(attr); + } + return attributes; + } + + /** + * If UserGroupDAO is defined, fetches all the groups + * using a membership filter (member=) on groups. + * + * Assigns the ADMIN role to users belonging to the adminRoleGroup group. + * + * @param ctx + * @param user + */ + private void assignGroupsAndRole(DirContextOperations ctx, User user) { + // defaults to no groups and USER role + user.setGroups(new HashSet()); + user.setRole(Role.USER); + if (userGroupDAO != null) { + Search searchCriteria = new Search(UserGroup.class); + searchCriteria.addFilterSome("user", + new Filter("name", ctx.getNameInNamespace(), Filter.OP_EQUAL)); + for (UserGroup ug : userGroupDAO.search(searchCriteria)) { + if (isAdminGroup(ug)) { + user.setRole(Role.ADMIN); + } + user.getGroups().add(ug); + } + + } + } + + /** + * Returns true if the given group is the adminRoleGroup group. + * + * @param ug + * @return + */ + private boolean isAdminGroup(UserGroup ug) { + return ug.getGroupName().equalsIgnoreCase(adminRoleGroup); + } /* * (non-Javadoc) @@ -175,7 +248,11 @@ protected User doMapFromContext(DirContextOperations ctx) { */ @Override public User merge(User entity) { - throw new UnsupportedOperationException(); + // we don't want to throw an exception on write operations, because + // some authentication providers try to persist stuff for synchronization + // purposes and they don't know DAOs can be readonly + // TODO: make readonly behaviour explicit + return entity; } /* @@ -185,7 +262,11 @@ public User merge(User entity) { */ @Override public boolean remove(User entity) { - throw new UnsupportedOperationException(); + // we don't want to throw an exception on write operations, because + // some authentication providers try to persist stuff for synchronization + // purposes and they don't know DAOs can be readonly + // TODO: make readonly behaviour explicit + return true; } /* @@ -195,7 +276,11 @@ public boolean remove(User entity) { */ @Override public boolean removeById(Long id) { - throw new UnsupportedOperationException(); + // we don't want to throw an exception on write operations, because + // some authentication providers try to persist stuff for synchronization + // purposes and they don't know DAOs can be readonly + // TODO: make readonly behaviour explicit + return true; } /* diff --git a/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserGroupDAOImpl.java b/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserGroupDAOImpl.java index 1ebdf22e..75f1232e 100644 --- a/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserGroupDAOImpl.java +++ b/src/core/persistence/src/main/java/it/geosolutions/geostore/core/dao/ldap/impl/UserGroupDAOImpl.java @@ -28,6 +28,8 @@ import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DirContextProcessor; import org.springframework.ldap.core.support.AbstractContextMapper; + +import com.googlecode.genericdao.search.Filter; import com.googlecode.genericdao.search.ISearch; import it.geosolutions.geostore.core.dao.UserGroupDAO; import it.geosolutions.geostore.core.model.UserGroup; @@ -130,8 +132,16 @@ public UserGroup find(Long id) { @SuppressWarnings("unchecked") @Override public List search(ISearch search) { + String filter; + if (isNested(search)) { + // membership filter (member = ) + Filter nested = getNestedFilter(search.getFilters().get(0)); + filter = memberAttribute + "=" + nested.getValue().toString(); + } else { + filter = getLdapFilter(search, getPropertyMapper()); + } return addEveryOne( - ldapSearch(combineFilters(baseFilter, getLdapFilter(search, getPropertyMapper())), getProcessorForSearch(search)), + ldapSearch(combineFilters(baseFilter, filter), getProcessorForSearch(search)), search ); } diff --git a/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/BaseDAOTest.java b/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/BaseDAOTest.java new file mode 100644 index 00000000..39daa422 --- /dev/null +++ b/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/BaseDAOTest.java @@ -0,0 +1,278 @@ +/* + * Copyright (C) 2021 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 . + */ +package it.geosolutions.geostore.core.dao.ldap; + +import java.util.Arrays; +import java.util.Collections; +import javax.naming.NamingEnumeration; +import javax.naming.NamingException; +import javax.naming.directory.BasicAttributes; +import javax.naming.directory.DirContext; +import javax.naming.directory.SearchControls; +import javax.naming.directory.SearchResult; +import org.springframework.ldap.core.DirContextAdapter; +import it.geosolutions.geostore.core.ldap.IterableNamingEnumeration; +import it.geosolutions.geostore.core.ldap.MockDirContextOperations; + +public abstract class BaseDAOTest { + protected DirContext buildContextForUsers() { + return new DirContextAdapter() { + @Override + public NamingEnumeration search(String name, String filter, SearchControls cons) + throws NamingException { + if ("ou=users".equals(name)) { + if("cn=*".equals(filter)) { + SearchResult sr1 = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=username,ou=users"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "username"; + } + return ""; + } + + }, new BasicAttributes()); + SearchResult sr2 = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=username2,ou=users"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "username2"; + } + return ""; + } + + }, new BasicAttributes()); + return new IterableNamingEnumeration(Arrays.asList(sr1, sr2)); + } else if ("(& (cn=*) (cn=username))".equals(filter)) { + SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=username,ou=users"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "username"; + } + return ""; + } + + }, new BasicAttributes()); + return new IterableNamingEnumeration(Collections.singletonList(sr)); + } else if ("(& (cn=*) (cn=username2))".equals(filter)) { + SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=username2,ou=users"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "username2"; + } + return ""; + } + + }, new BasicAttributes()); + return new IterableNamingEnumeration(Collections.singletonList(sr)); + } + } + return new IterableNamingEnumeration(Collections.EMPTY_LIST); + } + }; + } + + protected DirContext buildContextForGroupsMembership(final String memberString) { + return new DirContextAdapter() { + @Override + public NamingEnumeration search(String name, String filter, SearchControls cons) + throws NamingException { + if ("ou=groups".equals(name)) { + if ("(& (cn=*) (cn=group))".equals(filter)) { + SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=group,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "group"; + } + return ""; + } + + @Override + public String[] getStringAttributes(String name) { + if ("member".equals(name)) { + return new String[] {memberString == null ? "username" : memberString}; + } + return new String[] {}; + } + + + }, new BasicAttributes()); + return new IterableNamingEnumeration(Collections.singletonList(sr)); + } + } + return new IterableNamingEnumeration(Collections.EMPTY_LIST); + } + }; + } + + protected DirContext buildContextForGroups() { + return new DirContextAdapter() { + @Override + public NamingEnumeration search(String name, String filter, SearchControls cons) + throws NamingException { + if ("ou=groups".equals(name)) { + if ("cn=*".equals(filter)) { + SearchResult sr1 = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=group,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "group"; + } + return ""; + } + + }, new BasicAttributes()); + SearchResult sr2 = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=group2,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "group2"; + } + return ""; + } + + }, new BasicAttributes()); + return new IterableNamingEnumeration(Arrays.asList(sr1, sr2)); + } else if ("(& (cn=*) (cn=group))".equals(filter)) { + SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=group,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "group"; + } + return ""; + } + + }, new BasicAttributes()); + return new IterableNamingEnumeration(Collections.singletonList(sr)); + } else if("(& (cn=*) (member=cn=username,ou=users))".contentEquals(filter)) { + SearchResult sr1 = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=group,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "group"; + } + return ""; + } + + }, new BasicAttributes()); + SearchResult sr2 = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=admin,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "admin"; + } + return ""; + } + + }, new BasicAttributes()); + return new IterableNamingEnumeration( + Arrays.asList(new SearchResult[] {sr1, sr2})); + } else if("(& (cn=*) (member=cn=username2,ou=users))".contentEquals(filter)) { + SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { + + @Override + public String getNameInNamespace() { + return "cn=group,ou=groups"; + } + + @Override + public String getStringAttribute(String name) { + if ("cn".equals(name)) { + return "group"; + } + return ""; + } + + }, new BasicAttributes()); + + return new IterableNamingEnumeration( + Collections.singletonList(sr)); + } + } + return new IterableNamingEnumeration(Collections.EMPTY_LIST); + } + }; + } + +} diff --git a/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserDAOTest.java b/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserDAOTest.java index 77538ae4..105c5c61 100644 --- a/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserDAOTest.java +++ b/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserDAOTest.java @@ -41,113 +41,10 @@ import it.geosolutions.geostore.core.ldap.MockContextSource; import it.geosolutions.geostore.core.ldap.MockDirContextOperations; import it.geosolutions.geostore.core.model.User; +import it.geosolutions.geostore.core.model.enums.Role; -public class UserDAOTest { +public class UserDAOTest extends BaseDAOTest { - DirContext buildContextForUsers() { - return new DirContextAdapter() { - @Override - public NamingEnumeration search(String name, String filter, SearchControls cons) - throws NamingException { - if ("ou=users".equals(name)) { - if("cn=*".equals(filter)) { - SearchResult sr1 = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=username,ou=users"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "username"; - } - return ""; - } - - }, new BasicAttributes()); - SearchResult sr2 = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=username2,ou=users"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "username2"; - } - return ""; - } - - }, new BasicAttributes()); - return new IterableNamingEnumeration(Arrays.asList(sr1, sr2)); - } else if ("(& (cn=*) (cn=username))".equals(filter)) { - SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=username,ou=users"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "username"; - } - return ""; - } - - }, new BasicAttributes()); - return new IterableNamingEnumeration(Collections.singletonList(sr)); - } - } - return new IterableNamingEnumeration(Collections.EMPTY_LIST); - } - }; - } - - DirContext buildContextForGroups(final String memberString) { - return new DirContextAdapter() { - @Override - public NamingEnumeration search(String name, String filter, SearchControls cons) - throws NamingException { - if ("ou=groups".equals(name)) { - if ("(& (cn=*) (cn=group))".equals(filter)) { - SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=group,ou=groups"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "group"; - } - return ""; - } - - @Override - public String[] getStringAttributes(String name) { - if ("member".equals(name)) { - return new String[] {memberString == null ? "username" : memberString}; - } - return new String[] {}; - } - - - }, new BasicAttributes()); - return new IterableNamingEnumeration(Collections.singletonList(sr)); - } - } - return new IterableNamingEnumeration(Collections.EMPTY_LIST); - } - }; - } @Test public void testFindAll() { @@ -170,6 +67,43 @@ public void testSearchByname() { assertEquals("username", user.getName()); } + @Test + public void testGroupsAreFetched() { + UserDAOImpl userDAO = new UserDAOImpl(new MockContextSource(buildContextForUsers())); + userDAO.setSearchBase("ou=users"); + UserGroupDAOImpl userGroupDAO = new UserGroupDAOImpl(new MockContextSource(buildContextForGroups())); + userGroupDAO.setSearchBase("ou=groups"); + userDAO.setUserGroupDAO(userGroupDAO); + + Search search = new Search(User.class); + List users = userDAO.search(search.addFilter(Filter.equal("name", "username"))); + + User user = users.get(0); + assertEquals(2, user.getGroups().size()); + } + + @Test + public void testRolesAreAssigned() { + UserDAOImpl userDAO = new UserDAOImpl(new MockContextSource(buildContextForUsers())); + userDAO.setSearchBase("ou=users"); + userDAO.setAdminRoleGroup("admin"); + UserGroupDAOImpl userGroupDAO = new UserGroupDAOImpl(new MockContextSource(buildContextForGroups())); + userGroupDAO.setSearchBase("ou=groups"); + userDAO.setUserGroupDAO(userGroupDAO); + + Search search = new Search(User.class); + List users = userDAO.search(search.addFilter(Filter.equal("name", "username"))); + + User user = users.get(0); + assertEquals(Role.ADMIN, user.getRole()); + + search = new Search(User.class); + users = userDAO.search(search.addFilter(Filter.equal("name", "username2"))); + + user = users.get(0); + assertEquals(Role.USER, user.getRole()); + } + @Test public void testAttributesMapper() { UserDAOImpl userDAO = new UserDAOImpl(new MockContextSource(buildContextForUsers())); @@ -186,7 +120,7 @@ public void testAttributesMapper() { @Test public void testSearchByGroup() { - UserGroupDAOImpl userGroupDAO = new UserGroupDAOImpl(new MockContextSource(buildContextForGroups(null))); + UserGroupDAOImpl userGroupDAO = new UserGroupDAOImpl(new MockContextSource(buildContextForGroupsMembership(null))); userGroupDAO.setSearchBase("ou=groups"); UserDAOImpl userDAO = new UserDAOImpl(new MockContextSource(buildContextForUsers())); userDAO.setUserGroupDAO(userGroupDAO); @@ -201,7 +135,7 @@ public void testSearchByGroup() { @Test public void testMemberPattern() { - UserGroupDAOImpl userGroupDAO = new UserGroupDAOImpl(new MockContextSource(buildContextForGroups("uid=username,ou=users"))); + UserGroupDAOImpl userGroupDAO = new UserGroupDAOImpl(new MockContextSource(buildContextForGroupsMembership("uid=username,ou=users"))); userGroupDAO.setSearchBase("ou=groups"); UserDAOImpl userDAO = new UserDAOImpl(new MockContextSource(buildContextForUsers())); userDAO.setUserGroupDAO(userGroupDAO); diff --git a/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserGroupDAOTest.java b/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserGroupDAOTest.java index 2c8b1bf9..81d6f111 100644 --- a/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserGroupDAOTest.java +++ b/src/core/persistence/src/test/java/it/geosolutions/geostore/core/dao/ldap/UserGroupDAOTest.java @@ -40,71 +40,8 @@ import it.geosolutions.geostore.core.model.User; import it.geosolutions.geostore.core.model.UserGroup; -public class UserGroupDAOTest { - DirContext buildContextForGroups() { - return new DirContextAdapter() { - @Override - public NamingEnumeration search(String name, String filter, SearchControls cons) - throws NamingException { - if ("ou=groups".equals(name)) { - if ("cn=*".equals(filter)) { - SearchResult sr1 = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=group,ou=groups"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "group"; - } - return ""; - } - - }, new BasicAttributes()); - SearchResult sr2 = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=group2,ou=groups"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "group2"; - } - return ""; - } - - }, new BasicAttributes()); - return new IterableNamingEnumeration(Arrays.asList(sr1, sr2)); - } else if ("(& (cn=*) (cn=group))".equals(filter)) { - SearchResult sr = new SearchResult("cn=*", null, new MockDirContextOperations() { - - @Override - public String getNameInNamespace() { - return "cn=group,ou=groups"; - } - - @Override - public String getStringAttribute(String name) { - if ("cn".equals(name)) { - return "group"; - } - return ""; - } - - }, new BasicAttributes()); - return new IterableNamingEnumeration(Collections.singletonList(sr)); - } - } - return new IterableNamingEnumeration(Collections.EMPTY_LIST); - } - }; - } +public class UserGroupDAOTest extends BaseDAOTest { + @Test public void testFindAll() { diff --git a/src/core/services-impl/src/main/java/it/geosolutions/geostore/services/UserServiceImpl.java b/src/core/services-impl/src/main/java/it/geosolutions/geostore/services/UserServiceImpl.java index 2d5c1e14..3f98e4df 100644 --- a/src/core/services-impl/src/main/java/it/geosolutions/geostore/services/UserServiceImpl.java +++ b/src/core/services-impl/src/main/java/it/geosolutions/geostore/services/UserServiceImpl.java @@ -167,18 +167,23 @@ public long insert(User user) throws BadRequestServiceEx, NotFoundServiceEx { */ @Override public long update(User user) throws NotFoundServiceEx, BadRequestServiceEx { - User orig = userDAO.find(user.getId()); + User orig = get(user.getId()); if (orig == null) { - throw new NotFoundServiceEx("User not found " + user.getId()); + orig = get(user.getName()); + user.setId(orig.getId()); } + + if (orig == null) { + throw new NotFoundServiceEx("User not found " + user.getId()); + } // // Checking User Group // Set groups = user.getGroups(); List groupNames = new ArrayList(); - List existingGroups = new ArrayList(); + Set existingGroups = new HashSet(); if (groups != null && groups.size() > 0) { for(UserGroup group : groups){ String groupName = group.getGroupName(); @@ -193,7 +198,7 @@ public long update(User user) throws NotFoundServiceEx, BadRequestServiceEx { Search searchCriteria = new Search(UserGroup.class); searchCriteria.addFilterIn("groupName", groupNames); - existingGroups = userGroupDAO.search(searchCriteria); + existingGroups = GroupReservedNames.checkReservedGroups(userGroupDAO.search(searchCriteria)); if (existingGroups == null || (existingGroups != null && groups.size() != existingGroups.size())) { throw new NotFoundServiceEx("At least one User group not found; review the groups associated to the user you want to insert" + user.getId()); diff --git a/src/core/services-impl/src/test/java/it/geosolutions/geostore/services/UserServiceImplTest.java b/src/core/services-impl/src/test/java/it/geosolutions/geostore/services/UserServiceImplTest.java index 2480eef2..12c97683 100644 --- a/src/core/services-impl/src/test/java/it/geosolutions/geostore/services/UserServiceImplTest.java +++ b/src/core/services-impl/src/test/java/it/geosolutions/geostore/services/UserServiceImplTest.java @@ -136,5 +136,50 @@ public void testGetByGroupName() throws Exception { Collection users = userService.getByGroup(group); assertEquals(1, users.size()); } + + @Test + public void testUpdateByUserId() throws Exception { + final String NAME = "name1"; + + long userId = createUser(NAME, Role.USER, "testPW"); + + assertEquals(1, userService.getCount(null)); + + User loaded = userService.get(userId); + assertNotNull(loaded); + assertEquals(NAME, loaded.getName()); + assertTrue( PwEncoder.isPasswordValid(loaded.getPassword(),"testPW")); + assertEquals(Role.USER, loaded.getRole()); + + loaded.setNewPassword("testPW2"); + userService.update(loaded); + + loaded = userService.get(userId); + assertNotNull(loaded); + assertTrue(PwEncoder.isPasswordValid(loaded.getPassword(),"testPW2")); + } + + @Test + public void testUpdateByUserName() throws Exception { + final String NAME = "name1"; + + long userId = createUser(NAME, Role.USER, "testPW"); + + assertEquals(1, userService.getCount(null)); + + User loaded = userService.get(userId); + assertNotNull(loaded); + assertEquals(NAME, loaded.getName()); + assertTrue( PwEncoder.isPasswordValid(loaded.getPassword(),"testPW")); + assertEquals(Role.USER, loaded.getRole()); + + loaded.setNewPassword("testPW2"); + loaded.setId(-1L); + userService.update(loaded); + + loaded = userService.get(userId); + assertNotNull(loaded); + assertTrue(PwEncoder.isPasswordValid(loaded.getPassword(),"testPW2")); + } } diff --git a/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/SessionTokenAuthenticationFilter.java b/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/SessionTokenAuthenticationFilter.java index d85354d6..7134812f 100644 --- a/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/SessionTokenAuthenticationFilter.java +++ b/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/SessionTokenAuthenticationFilter.java @@ -65,26 +65,25 @@ protected Authentication checkToken(String token) { if(ud != null) { User user = null; if (validateUserFromService) { + // we search user by id first, if available if(ud.getId() != null) { user = userService.get((Long) ud.getId()); - } else if (ud.getName() != null){ + } + // then by name if no id is available or the service cannot search by id (e.g. LDAP) + if (user == null && ud.getName() != null){ try { - // in case of external authentication provider the user may not come from the UserService - // so try to use the name to retrieve from the userservice (that should be populated in the meanwhile). user = userService.get(ud.getName()); } catch (NotFoundServiceEx e) { LOGGER.error("User " + ud.getName() + " not found on the database because of an exception", e); } - } else { - LOGGER.error("User login success, but couldn't retrieve a session. Probably auth user and and userService are out of sync."); } - - } else { user = ud; } if (user != null) { return createAuthenticationForUser(user); + } else { + LOGGER.error("User login success, but couldn't retrieve a session. Probably auth user and and userService are out of sync."); } } return null; diff --git a/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/UserLdapAuthenticationProvider.java b/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/UserLdapAuthenticationProvider.java index 98977903..2c054d83 100644 --- a/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/UserLdapAuthenticationProvider.java +++ b/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/UserLdapAuthenticationProvider.java @@ -133,7 +133,7 @@ public Authentication authenticate(Authentication authentication) Set groups = new HashSet(); Role role = extractUserRoleAndGroups(user.getRole(), authorities, groups); user.setRole(role); - user.setGroups(checkReservedGroups(groups)); + user.setGroups(GroupReservedNames.checkReservedGroups(groups)); if (userService != null) userService.update(user); @@ -159,7 +159,7 @@ public Authentication authenticate(Authentication authentication) Set groups = new HashSet(); Role role = extractUserRoleAndGroups(null, authorities, groups); user.setRole(role); - user.setGroups(checkReservedGroups(groups)); + user.setGroups(GroupReservedNames.checkReservedGroups(groups)); if(userMapper != null) { userMapper.mapUser(ldapUser, user); } @@ -248,24 +248,5 @@ private UserGroup synchronizeGroup(GrantedAuthority a) } else { return group; } - } - - /** - * Utility method to remove Reserved group (for example EVERYONE) from a group list - * - * @param groups - * @return - */ - private Set checkReservedGroups(Set groups){ - List reserved = new ArrayList(); - for(UserGroup ug : groups){ - if(!GroupReservedNames.isAllowedName(ug.getGroupName())){ - reserved.add(ug); - } - } - for(UserGroup ug : reserved){ - groups.remove(ug); - } - return groups; } } diff --git a/src/modules/rest/impl/src/test/java/it/geosolutions/geostore/rest/security/SessionTokenAuthenticationFilterTest.java b/src/modules/rest/impl/src/test/java/it/geosolutions/geostore/rest/security/SessionTokenAuthenticationFilterTest.java index e5bec9d1..69a867db 100644 --- a/src/modules/rest/impl/src/test/java/it/geosolutions/geostore/rest/security/SessionTokenAuthenticationFilterTest.java +++ b/src/modules/rest/impl/src/test/java/it/geosolutions/geostore/rest/security/SessionTokenAuthenticationFilterTest.java @@ -133,5 +133,33 @@ public void userWorksWithNameOnlyTest() throws IOException, ServletException, Ba assertNotNull(authUser.getId()); } - + @Test + public void userWorksWithFakeIdTest() throws IOException, ServletException, BadRequestServiceEx, NotFoundServiceEx { + User user = new User(); + user.setName(SAMPLE_USER); + user.setRole(Role.USER); + filter.setCacheExpiration(1); + // different users, same name + User sessionUser = new User(); + sessionUser.setId(-1L); + sessionUser.setName(SAMPLE_USER); + sessionUser.setRole(Role.USER); + // here the mock service sets the id. + filter.getUserService().insert(user); + + Mockito.when(request.getHeader(DEFAULT_HEADER)).thenReturn(DEFAULT_PREFIX + SAMPLE_TOKEN); + + Calendar expires = new GregorianCalendar(); + expires.add(Calendar.SECOND, (int) 60* 60 * 24* 15); + + filter.getUserSessionService().registerNewSession(SAMPLE_TOKEN, new UserSessionImpl(sessionUser, expires)); + + filter.doFilter(request, response, chain); + + assertNotNull(SecurityContextHolder.getContext().getAuthentication()); + User authUser = (User)SecurityContextHolder.getContext().getAuthentication().getPrincipal(); + assertEquals(SAMPLE_USER, authUser.getName()); + assertNotNull(authUser.getId()); + } + }