From 638a0667e9d45cfafb4b96182b34cc1de2ab88fb Mon Sep 17 00:00:00 2001 From: etj Date: Thu, 14 Apr 2022 13:45:15 +0200 Subject: [PATCH] #265 LDAP groups and roles concepts are mixed --- .../SimpleGrantedAuthoritiesMapper.java | 65 +++++++--- .../GeoStoreLdapAuthoritiesPopulator.java | 112 ++++++++++++------ 2 files changed, 121 insertions(+), 56 deletions(-) diff --git a/src/core/security/src/main/java/it/geosolutions/geostore/core/security/SimpleGrantedAuthoritiesMapper.java b/src/core/security/src/main/java/it/geosolutions/geostore/core/security/SimpleGrantedAuthoritiesMapper.java index ebdd38df..11c33711 100644 --- a/src/core/security/src/main/java/it/geosolutions/geostore/core/security/SimpleGrantedAuthoritiesMapper.java +++ b/src/core/security/src/main/java/it/geosolutions/geostore/core/security/SimpleGrantedAuthoritiesMapper.java @@ -32,40 +32,69 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.GrantedAuthorityImpl; /** * Map based implementation of GrantedAuthoritiesMapper. - * * + * * * @author Mauro Bartolomeoli * */ public class SimpleGrantedAuthoritiesMapper implements GrantedAuthoritiesMapper { - private Map mappings = new HashMap(); - + private Map mappings = new HashMap<>(); + + /** + * Do not consider authorities that do not exist in the mapping + **/ + private boolean dropUnmapped = false; + + private static final Log logger = LogFactory.getLog(SimpleGrantedAuthoritiesMapper.class); + public SimpleGrantedAuthoritiesMapper(Map mappings) { super(); this.mappings = mappings; } - @Override - public Collection mapAuthorities( - Collection authorities) { - if(mappings.isEmpty()) { - return authorities; - } - List result = new ArrayList(); - for(GrantedAuthority authority : authorities) { - if(mappings.containsKey(authority.getAuthority())) { - result.add(new GrantedAuthorityImpl(mappings.get(authority.getAuthority()))); - } else { - result.add(authority); + @Override + public Collection mapAuthorities( + Collection authorities) { + if (mappings.isEmpty()) { + return authorities; + } + + List result = new ArrayList<>(); + + for (GrantedAuthority authority : authorities) { + String src = authority.getAuthority(); + if (mappings.containsKey(src)) { + String dst = mappings.get(authority.getAuthority()); + result.add(new GrantedAuthorityImpl(dst)); + if (logger.isDebugEnabled()) { + logger.debug("Mapping authority: " + src + " --> " + dst ); } - } - return result; - } + } else if(dropUnmapped) { + if (logger.isDebugEnabled()) { + logger.debug("Dropping unmapped authority: " + src); + } + } + else + { + result.add(authority); + if (logger.isDebugEnabled()) { + logger.debug("Adding unmapped authority: " + src ); + } + } + } + return result; + } + + public void setDropUnmapped(boolean dropUnmapped) { + this.dropUnmapped = dropUnmapped; + } } diff --git a/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/GeoStoreLdapAuthoritiesPopulator.java b/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/GeoStoreLdapAuthoritiesPopulator.java index 17ea870e..b09efcfb 100644 --- a/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/GeoStoreLdapAuthoritiesPopulator.java +++ b/src/modules/rest/impl/src/main/java/it/geosolutions/geostore/services/rest/security/GeoStoreLdapAuthoritiesPopulator.java @@ -124,7 +124,8 @@ public Authority(String name, String dn) { */ private boolean convertToUpperCase = true; - private GrantedAuthoritiesMapper authoritiesMapper = null; + private GrantedAuthoritiesMapper roleMapper = null; + private GrantedAuthoritiesMapper groupMapper = null; /** * @param contextSource @@ -156,64 +157,89 @@ public GeoStoreLdapAuthoritiesPopulator(ContextSource contextSource, String grou } } + @Deprecated public void setAuthoritiesMapper(GrantedAuthoritiesMapper authoritiesMapper) { - this.authoritiesMapper = authoritiesMapper; + logger.error("AuthoritiesMapper is deprecated, please set roleMapper and groupMapper separately"); + this.roleMapper = authoritiesMapper; + this.groupMapper = authoritiesMapper; } + public void setRoleMapper(GrantedAuthoritiesMapper roleMapper) { + this.roleMapper = roleMapper; + } + public void setGroupMapper(GrantedAuthoritiesMapper groupMapper) { + this.groupMapper = groupMapper; + } @Override public Set getGroupMembershipRoles(String userDn, String username) { - return getGroupsOrRoles(userDn, username, true, true); + // TODO: double check if we really want to return groups+roles + Set ret = new HashSet<>(); + ret.addAll(getGroups(userDn, username)); + ret.addAll(getRoles(userDn, username)); + return ret; } - private Set getGroupsOrRoles(String userDn, String username, boolean groups, boolean roles) { - if (roleSearchBase == null && groupSearchBase == null) { - return new HashSet(); + private Set getRoles(String userDn, String username) { + if (roleSearchBase == null) { + return new HashSet<>(); } - Set authorities = new HashSet(); + Set authorities = new HashSet<>(); + String[] searchParams = username == null ? new String[] {} : new String[] {userDn, username}; - if(roles) { - // Searching for ROLES - if (logger.isDebugEnabled()) { - logger.debug("Searching for roles for user '" + username + "', DN = " + "'" + userDn + "', with filter " - + roleSearchFilter + " in search base '" + roleSearchBase + "'"); - } - - String[] rolesRoots = roleSearchBase.split(";"); - String filter = username == null ? allRolesSearchFilter : roleSearchFilter; - - for(String rolesRoot : rolesRoots) { - addAuthorities(searchParams, authorities, rolesRoot, filter, rolePrefix, false); - } + + // Searching for ROLES + if (logger.isDebugEnabled()) { + logger.debug("Searching for roles for user '" + username + "', DN = " + "'" + userDn + "', with filter " + + roleSearchFilter + " in search base '" + roleSearchBase + "'"); + } + + String[] rolesRoots = roleSearchBase.split(";"); + String filter = username == null ? allRolesSearchFilter : roleSearchFilter; + + for(String rolesRoot : rolesRoots) { + addAuthorities(searchParams, authorities, rolesRoot, filter, rolePrefix, false); + } + + if(roleMapper != null) { + authorities = new HashSet<>(roleMapper.mapAuthorities(authorities)); } - - if(groups) { - // Searching for Groups - if (logger.isDebugEnabled()) { - logger.debug("Searching for groups for user '" + username + "', DN = " + "'" + userDn + "', with filter " - + groupSearchFilter + " in search base '" + groupSearchBase + "'"); - } - String[] groupsRoots = groupSearchBase.split(";"); - String filter = username == null ? allGroupsSearchFilter : groupSearchFilter; - for(String groupsRoot : groupsRoots) { - addAuthorities(searchParams, authorities, groupsRoot, filter, null, enableHierarchicalGroups); - } + return authorities; + } + + private Set getGroups(String userDn, String username) { + if (groupSearchBase == null) { + return new HashSet<>(); } + + Set authorities = new HashSet<>(); + String[] searchParams = username == null ? new String[] {} : new String[] {userDn, username}; + + // Searching for Groups + if (logger.isDebugEnabled()) { + logger.debug("Searching for groups for user '" + username + "', DN = " + "'" + userDn + "', with filter " + + groupSearchFilter + " in search base '" + groupSearchBase + "'"); + } + String[] groupsRoots = groupSearchBase.split(";"); + String filter = username == null ? allGroupsSearchFilter : groupSearchFilter; + for(String groupsRoot : groupsRoots) { + addAuthorities(searchParams, authorities, groupsRoot, filter, null, enableHierarchicalGroups); + } - if(authoritiesMapper != null) { - return new HashSet(authoritiesMapper.mapAuthorities(authorities)); + if(groupMapper != null) { + authorities = new HashSet<>(groupMapper.mapAuthorities(authorities)); } return authorities; } public Set getAllGroups() { - return getGroupsOrRoles(null, null, true, false); + return getGroups(null, null); } public Set getAllRoles() { - return getGroupsOrRoles(null, null, false, true); + return getRoles(null, null); } private void addAuthorities(String[] params, Set authorities, @@ -233,7 +259,7 @@ protected Object doMapFromContext(DirContextOperations ctx) { }); if (logger.isDebugEnabled()) { - logger.debug("Authorities from search: " + ldapAuthorities); + logger.debug("Found " + ldapAuthorities.size() + " authorities from search"); } for (Object authority : ldapAuthorities) { Authority ldapAuthority = (Authority)authority; @@ -248,17 +274,27 @@ protected Object doMapFromContext(DirContextOperations ctx) { private boolean addAuthority(Set authorities, String authorityPrefix, String authority) { + + if (logger.isDebugEnabled()) { + logger.debug("Adding authority: " + authorityPrefix + "::" + authority); + } + if (convertToUpperCase) { authority = authority.toUpperCase(); } String prefix = (authorityPrefix != null && !authority.startsWith(authorityPrefix) ? authorityPrefix : ""); - GrantedAuthorityImpl role = new GrantedAuthorityImpl(prefix + authority); + String rolename = prefix + authority; + GrantedAuthorityImpl role = new GrantedAuthorityImpl(rolename); if (!authorities.contains(role)) { authorities.add(role); + if (logger.isDebugEnabled()) { + logger.debug("Authority added: " + rolename); + } return true; } + logger.debug("Authority not added: " + rolename); return false; }