From 4a9878403158aa1d16817bf78fc56e5e8a26e8cd Mon Sep 17 00:00:00 2001 From: Martin Stockhammer Date: Fri, 24 Jan 2020 21:58:09 +0100 Subject: [PATCH] [MRM-2008] Fix for group names with slashes Changing the group name retrieval to attribute read. Using CompositeName and LdapName to retrieve the result. Slashes are treated special in JNDI. --- .../redback/common/ldap/LdapUtils.java | 43 ++ .../ldap/role/DefaultLdapRoleMapper.java | 374 ++++++++++-------- .../common/ldap/role/TestLdapRoleMapper.java | 30 +- .../src/test/security.properties | 3 +- .../configuration/UserConfigurationKeys.java | 4 + 5 files changed, 275 insertions(+), 179 deletions(-) diff --git a/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java b/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java index 1131cee2..529a2d9d 100644 --- a/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java +++ b/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/LdapUtils.java @@ -19,10 +19,14 @@ package org.apache.archiva.redback.common.ldap; * under the License. */ +import javax.naming.CompositeName; +import javax.naming.InvalidNameException; import javax.naming.NamingEnumeration; import javax.naming.NamingException; import javax.naming.directory.Attribute; import javax.naming.directory.Attributes; +import javax.naming.ldap.LdapName; +import javax.naming.ldap.Rdn; /** * @@ -129,4 +133,43 @@ public final class LdapUtils return null; } + + /** + * Returns a LDAP name from a given RDN string. The name parameter must be a string + * representation of a composite name (as returned by ldapsearch result getName()) + * @param name The string of the RDN (may be escaped) + * @return The LdapName that corresponds to this string + * @throws InvalidNameException If the string cannot be parsed as LDAP name + */ + public static LdapName getLdapNameFromString(final String name) throws InvalidNameException + { + CompositeName coName = new CompositeName( name ); + LdapName ldapName = new LdapName( "" ); + ldapName.addAll( coName ); + return ldapName; + } + + /** + * Returns the first RDN value that matches the given type. + * E.g. for the RDN ou=People,dc=test,dc=de, and type dc it will return 'test'. + * + * @param name the ldap name + * @param type the type of the RDN entry + * @return + */ + public static String findFirstRdnValue(LdapName name, String type) { + for ( Rdn rdn : name.getRdns() ) + { + if ( rdn.getType( ).equals( type ) ) + { + Object val = rdn.getValue( ); + if (val!=null) { + return val.toString( ); + } else { + return ""; + } + } + } + return ""; + } } diff --git a/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/role/DefaultLdapRoleMapper.java b/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/role/DefaultLdapRoleMapper.java index a8348f92..761daee9 100644 --- a/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/role/DefaultLdapRoleMapper.java +++ b/redback-common/redback-common-ldap/src/main/java/org/apache/archiva/redback/common/ldap/role/DefaultLdapRoleMapper.java @@ -48,6 +48,7 @@ import javax.naming.directory.DirContext; import javax.naming.directory.ModificationItem; import javax.naming.directory.SearchControls; import javax.naming.directory.SearchResult; +import javax.naming.ldap.LdapName; import javax.naming.ldap.Rdn; import java.util.ArrayList; import java.util.Collection; @@ -66,7 +67,7 @@ public class DefaultLdapRoleMapper implements LdapRoleMapper { - private Logger log = LoggerFactory.getLogger( getClass() ); + private Logger log = LoggerFactory.getLogger( getClass( ) ); @Inject @Named( value = "ldapConnectionFactory#configurable" ) @@ -96,7 +97,7 @@ public class DefaultLdapRoleMapper private String baseDn; - private String ldapGroupMember = "uniqueMember"; + private String ldapGroupMemberAttribute = "uniqueMember"; private boolean useDefaultRoleName = false; @@ -106,13 +107,28 @@ public class DefaultLdapRoleMapper * possible to user cn=beer or uid=beer or sn=beer etc * so make it configurable */ - private String userIdAttribute = "uid"; + public static String DEFAULT_USER_ID_ATTRIBUTE = "uid"; + private String userIdAttribute = DEFAULT_USER_ID_ATTRIBUTE; + + public static String DEFAULT_GROUP_NAME_ATTRIBUTE = "cn"; + private String groupNameAttribute = DEFAULT_GROUP_NAME_ATTRIBUTE; + + // True, if the member attribute stores the DN, otherwise the userkey is used as entry value + private boolean useDnAsMemberValue = true; + + private static final String POSIX_GROUP = "posixGroup"; @PostConstruct - public void initialize() + public void initialize( ) { this.ldapGroupClass = userConf.getString( UserConfigurationKeys.LDAP_GROUPS_CLASS, this.ldapGroupClass ); + if (StringUtils.equalsIgnoreCase( POSIX_GROUP, this.ldapGroupClass )) { + this.useDnAsMemberValue = false; + } + + this.useDnAsMemberValue = userConf.getBoolean( UserConfigurationKeys.LDAP_GROUPS_USE_DN_AS_MEMBER_VALUE, this.useDnAsMemberValue ); + this.baseDn = userConf.getConcatenatedList( UserConfigurationKeys.LDAP_BASEDN, this.baseDn ); this.groupsDn = userConf.getConcatenatedList( UserConfigurationKeys.LDAP_GROUPS_BASEDN, this.groupsDn ); @@ -127,11 +143,30 @@ public class DefaultLdapRoleMapper this.useDefaultRoleName = userConf.getBoolean( UserConfigurationKeys.LDAP_GROUPS_USE_ROLENAME, this.useDefaultRoleName ); - this.userIdAttribute = userConf.getString( UserConfigurationKeys.LDAP_USER_ID_ATTRIBUTE, this.userIdAttribute ); + this.userIdAttribute = userConf.getString( UserConfigurationKeys.LDAP_USER_ID_ATTRIBUTE, DEFAULT_USER_ID_ATTRIBUTE ); - this.ldapGroupMember = userConf.getString( UserConfigurationKeys.LDAP_GROUPS_MEMBER, this.ldapGroupMember ); + this.ldapGroupMemberAttribute = userConf.getString( UserConfigurationKeys.LDAP_GROUPS_MEMBER, this.ldapGroupMemberAttribute ); this.dnAttr = userConf.getString( UserConfigurationKeys.LDAP_DN_ATTRIBUTE, this.dnAttr ); + + this.groupNameAttribute = userConf.getString( UserConfigurationKeys.LDAP_GROUP_NAME_ATTRIBUTE, DEFAULT_GROUP_NAME_ATTRIBUTE ); + } + + + private String getGroupNameFromResult( SearchResult searchResult ) throws NamingException + { + Attribute gNameAtt = searchResult.getAttributes( ).get( groupNameAttribute ); + if ( gNameAtt != null ) + { + return gNameAtt.get( ).toString( ); + } + else + { + log.error( "Could not get group name from attribute {}. Group DN: {}", groupNameAttribute, searchResult.getNameInNamespace( ) ); + return ""; + } + + } public List getAllGroups( DirContext context ) @@ -142,45 +177,43 @@ public class DefaultLdapRoleMapper try { - SearchControls searchControls = new SearchControls(); + SearchControls searchControls = new SearchControls( ); searchControls.setDerefLinkFlag( true ); searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); + searchControls.setReturningAttributes( new String[]{ this.getLdapDnAttribute(), "objectClass", groupNameAttribute} ); - String filter = "objectClass=" + getLdapGroupClass(); + String filter = "objectClass=" + getLdapGroupClass( ); if ( !StringUtils.isEmpty( this.groupFilter ) ) { filter = "(&(" + filter + ")(" + this.groupFilter + "))"; } - namingEnumeration = context.search( getGroupsDn(), filter, searchControls ); + namingEnumeration = context.search( getGroupsDn( ), filter, searchControls ); - List allGroups = new ArrayList(); + List allGroups = new ArrayList( ); - while ( namingEnumeration.hasMore() ) + while ( namingEnumeration.hasMore( ) ) { - SearchResult searchResult = namingEnumeration.next(); - - String groupName = searchResult.getName(); - // cn=blabla we only want bla bla - groupName = StringUtils.substringAfter( groupName, "=" ); - - log.debug( "found groupName: '{}", groupName ); - - allGroups.add( groupName ); - + SearchResult searchResult = namingEnumeration.next( ); + String groupName = getGroupNameFromResult( searchResult ); + if ( StringUtils.isNotEmpty( groupName ) ) + { + log.debug( "Found groupName: '{}", groupName ); + allGroups.add( groupName ); + } } return allGroups; } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } finally @@ -195,7 +228,7 @@ public class DefaultLdapRoleMapper { try { - namingEnumeration.close(); + namingEnumeration.close( ); } catch ( NamingException e ) { @@ -225,16 +258,16 @@ public class DefaultLdapRoleMapper try { - SearchControls searchControls = new SearchControls(); + SearchControls searchControls = new SearchControls( ); searchControls.setDerefLinkFlag( true ); searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); - String filter = "objectClass=" + getLdapGroupClass(); + String filter = "objectClass=" + getLdapGroupClass( ); - namingEnumeration = context.search( "cn=" + groupName + "," + getGroupsDn(), filter, searchControls ); + namingEnumeration = context.search( groupNameAttribute + "=" + groupName + "," + getGroupsDn( ), filter, searchControls ); - return namingEnumeration.hasMore(); + return namingEnumeration.hasMore( ); } catch ( NameNotFoundException e ) { @@ -243,11 +276,11 @@ public class DefaultLdapRoleMapper } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } finally @@ -261,14 +294,14 @@ public class DefaultLdapRoleMapper { List groups = getAllGroups( context ); - if ( groups.isEmpty() ) + if ( groups.isEmpty( ) ) { - return Collections.emptyList(); + return Collections.emptyList( ); } - Set roles = new HashSet( groups.size() ); + Set roles = new HashSet( groups.size( ) ); - Map> mapping = ldapRoleMapperConfiguration.getLdapGroupMappings(); + Map> mapping = ldapRoleMapperConfiguration.getLdapGroupMappings( ); for ( String group : groups ) { @@ -293,30 +326,29 @@ public class DefaultLdapRoleMapper try { - SearchControls searchControls = new SearchControls(); + SearchControls searchControls = new SearchControls( ); searchControls.setDerefLinkFlag( true ); searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); - String filter = "objectClass=" + getLdapGroupClass(); + String filter = "objectClass=" + getLdapGroupClass( ); - namingEnumeration = context.search( "cn=" + group + "," + getGroupsDn(), filter, searchControls ); + namingEnumeration = context.search( groupNameAttribute + "=" + group + "," + getGroupsDn( ), filter, searchControls ); - List allMembers = new ArrayList(); + List allMembers = new ArrayList( ); - while ( namingEnumeration.hasMore() ) + while ( namingEnumeration.hasMore( ) ) { - SearchResult searchResult = namingEnumeration.next(); + SearchResult searchResult = namingEnumeration.next( ); - Attribute uniqueMemberAttr = searchResult.getAttributes().get( getLdapGroupMember() ); + Attribute uniqueMemberAttr = searchResult.getAttributes( ).get( getLdapGroupMemberAttribute( ) ); if ( uniqueMemberAttr != null ) { - NamingEnumeration allMembersEnum = (NamingEnumeration) uniqueMemberAttr.getAll(); - while ( allMembersEnum.hasMore() ) + NamingEnumeration allMembersEnum = uniqueMemberAttr.getAll( ); + while ( allMembersEnum.hasMore( ) ) { - String userName = allMembersEnum.next(); - // uid=blabla we only want bla bla + String userName = allMembersEnum.next( ).toString( ); userName = StringUtils.substringAfter( userName, "=" ); userName = StringUtils.substringBefore( userName, "," ); log.debug( "found userName for group {}: '{}", group, userName ); @@ -333,11 +365,11 @@ public class DefaultLdapRoleMapper } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } finally @@ -346,32 +378,39 @@ public class DefaultLdapRoleMapper } } + private String getUserDnFromId(String userKey) { + return new StringBuilder().append( this.userIdAttribute ).append( "=" ).append( userKey ).append( "," ).append( + getBaseDn( ) ).toString(); + } + public List getGroups( String username, DirContext context ) throws MappingException { - List userGroups = new ArrayList(); + Set userGroups = new HashSet( ); NamingEnumeration namingEnumeration = null; try { - SearchControls searchControls = new SearchControls(); + SearchControls searchControls = new SearchControls( ); searchControls.setDerefLinkFlag( true ); searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); + + String groupEntry = null; try { //try to look the user up User user = userManager.findUser( username ); - if ( user instanceof LdapUser ) + if ( user != null && user instanceof LdapUser ) { - LdapUser ldapUser = LdapUser.class.cast( user ); - Attribute dnAttribute = ldapUser.getOriginalAttributes().get( getLdapDnAttribute() ); + LdapUser ldapUser = (LdapUser) user ; + Attribute dnAttribute = ldapUser.getOriginalAttributes( ).get( getLdapDnAttribute( ) ); if ( dnAttribute != null ) { - groupEntry = String.class.cast( dnAttribute.get() ); + groupEntry = dnAttribute.get( ).toString(); } } @@ -387,82 +426,47 @@ public class DefaultLdapRoleMapper if ( groupEntry == null ) { //failed to look up the user's groupEntry directly - StringBuilder builder = new StringBuilder(); - String posixGroup = "posixGroup"; - if ( posixGroup.equals( getLdapGroupClass() ) ) + + if ( this.useDnAsMemberValue ) { - builder.append( username ); + groupEntry = getUserDnFromId( username ); } else { - builder.append( this.userIdAttribute ).append( "=" ).append( username ).append( "," ).append( - getBaseDn() ); + groupEntry = username; } - groupEntry = builder.toString(); } String filter = - new StringBuilder().append( "(&" ).append( "(objectClass=" + getLdapGroupClass() + ")" ).append( - "(" ).append( getLdapGroupMember() ).append( "=" ).append( Rdn.escapeValue(groupEntry) ).append( ")" ).append( - ")" ).toString(); + new StringBuilder( ).append( "(&" ).append( "(objectClass=" + getLdapGroupClass( ) + ")" ).append( + "(" ).append( getLdapGroupMemberAttribute( ) ).append( "=" ).append( Rdn.escapeValue( groupEntry ) ).append( ")" ).append( + ")" ).toString( ); log.debug( "filter: {}", filter ); - namingEnumeration = context.search( getGroupsDn(), filter, searchControls ); + namingEnumeration = context.search( getGroupsDn( ), filter, searchControls ); - while ( namingEnumeration.hasMore() ) + while ( namingEnumeration.hasMore( ) ) { - SearchResult searchResult = namingEnumeration.next(); + SearchResult groupSearchResult = namingEnumeration.next( ); + String groupName = getGroupNameFromResult( groupSearchResult ); - List allMembers = new ArrayList(); - - Attribute uniqueMemberAttr = searchResult.getAttributes().get( getLdapGroupMember() ); - - if ( uniqueMemberAttr != null ) - { - NamingEnumeration allMembersEnum = (NamingEnumeration) uniqueMemberAttr.getAll(); - while ( allMembersEnum.hasMore() ) - { - - String userName = allMembersEnum.next(); - //the original dn - allMembers.add( userName ); - // uid=blabla we only want bla bla - userName = StringUtils.substringAfter( userName, "=" ); - userName = StringUtils.substringBefore( userName, "," ); - allMembers.add( userName ); - } - close( allMembersEnum ); - } - - if ( allMembers.contains( username ) ) - { - String groupName = searchResult.getName(); - // cn=blabla we only want bla bla - groupName = StringUtils.substringAfter( groupName, "=" ); - userGroups.add( groupName ); - - } - else if ( allMembers.contains( groupEntry ) ) - { - String groupName = searchResult.getName(); - // cn=blabla we only want bla bla - groupName = StringUtils.substringAfter( groupName, "=" ); + if (StringUtils.isNotEmpty( groupName )) { userGroups.add( groupName ); } } - return userGroups; + return new ArrayList( userGroups ); } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } finally { @@ -475,9 +479,9 @@ public class DefaultLdapRoleMapper { List groups = getGroups( username, context ); - Map> rolesMapping = ldapRoleMapperConfiguration.getLdapGroupMappings(); + Map> rolesMapping = ldapRoleMapperConfiguration.getLdapGroupMappings( ); - Set roles = new HashSet( groups.size() ); + Set roles = new HashSet( groups.size( ) ); for ( String group : groups ) { @@ -504,26 +508,26 @@ public class DefaultLdapRoleMapper { try { - namingEnumeration.close(); + namingEnumeration.close( ); } catch ( NamingException e ) { - log.warn( "fail to close namingEnumeration: {}", e.getMessage() ); + log.warn( "fail to close namingEnumeration: {}", e.getMessage( ) ); } } } - public String getGroupsDn() + public String getGroupsDn( ) { return this.groupsDn; } - public String getLdapGroupClass() + public String getLdapGroupClass( ) { return this.ldapGroupClass; } - public String getLdapDnAttribute() + public String getLdapDnAttribute( ) { return this.dnAttr; } @@ -564,16 +568,16 @@ public class DefaultLdapRoleMapper objectClass.add( "top" ); objectClass.add( "groupOfUniqueNames" ); attributes.put( objectClass ); - attributes.put( "cn", groupName ); + attributes.put( this.groupNameAttribute, groupName ); // attribute mandatory when created a group so add admin as default member - BasicAttribute basicAttribute = new BasicAttribute( getLdapGroupMember() ); - basicAttribute.add( this.userIdAttribute + "=admin," + getBaseDn() ); + BasicAttribute basicAttribute = new BasicAttribute( getLdapGroupMemberAttribute( ) ); + basicAttribute.add( this.userIdAttribute + "=admin," + getBaseDn( ) ); attributes.put( basicAttribute ); try { - String dn = "cn=" + groupName + "," + this.groupsDn; + String dn = this.groupNameAttribute + "=" + groupName + "," + this.groupsDn; context.createSubcontext( dn, attributes ); @@ -588,12 +592,12 @@ public class DefaultLdapRoleMapper } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } } @@ -612,31 +616,31 @@ public class DefaultLdapRoleMapper NamingEnumeration namingEnumeration = null; try { - SearchControls searchControls = new SearchControls(); + SearchControls searchControls = new SearchControls( ); searchControls.setDerefLinkFlag( true ); searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); - String filter = "objectClass=" + getLdapGroupClass(); + String filter = "objectClass=" + getLdapGroupClass( ); - namingEnumeration = context.search( "cn=" + groupName + "," + getGroupsDn(), filter, searchControls ); + namingEnumeration = context.search( this.groupNameAttribute + "=" + groupName + "," + getGroupsDn( ), filter, searchControls ); - while ( namingEnumeration.hasMore() ) + if ( namingEnumeration.hasMore() ) { - SearchResult searchResult = namingEnumeration.next(); - Attribute attribute = searchResult.getAttributes().get( getLdapGroupMember() ); + SearchResult searchResult = namingEnumeration.next( ); + Attribute attribute = searchResult.getAttributes( ).get( getLdapGroupMemberAttribute( ) ); if ( attribute == null ) { - BasicAttribute basicAttribute = new BasicAttribute( getLdapGroupMember() ); - basicAttribute.add( this.userIdAttribute + "=" + username + "," + getBaseDn() ); - context.modifyAttributes( "cn=" + groupName + "," + getGroupsDn(), new ModificationItem[]{ - new ModificationItem( DirContext.ADD_ATTRIBUTE, basicAttribute ) } ); + BasicAttribute basicAttribute = new BasicAttribute( getLdapGroupMemberAttribute( ) ); + basicAttribute.add( this.userIdAttribute + "=" + username + "," + getBaseDn( ) ); + context.modifyAttributes( this.groupNameAttribute + "=" + groupName + "," + getGroupsDn( ), new ModificationItem[]{ + new ModificationItem( DirContext.ADD_ATTRIBUTE, basicAttribute )} ); } else { - attribute.add( this.userIdAttribute + "=" + username + "," + getBaseDn() ); - context.modifyAttributes( "cn=" + groupName + "," + getGroupsDn(), new ModificationItem[]{ - new ModificationItem( DirContext.REPLACE_ATTRIBUTE, attribute ) } ); + attribute.add( this.userIdAttribute + "=" + username + "," + getBaseDn( ) ); + context.modifyAttributes( this.groupNameAttribute + "=" + groupName + "," + getGroupsDn( ), new ModificationItem[]{ + new ModificationItem( DirContext.REPLACE_ATTRIBUTE, attribute )} ); } return true; } @@ -645,11 +649,11 @@ public class DefaultLdapRoleMapper } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } finally @@ -658,7 +662,7 @@ public class DefaultLdapRoleMapper { try { - namingEnumeration.close(); + namingEnumeration.close( ); } catch ( NamingException e ) { @@ -683,25 +687,25 @@ public class DefaultLdapRoleMapper try { - SearchControls searchControls = new SearchControls(); + SearchControls searchControls = new SearchControls( ); searchControls.setDerefLinkFlag( true ); searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); - String filter = "objectClass=" + getLdapGroupClass(); + String filter = "objectClass=" + getLdapGroupClass( ); - namingEnumeration = context.search( "cn=" + groupName + "," + getGroupsDn(), filter, searchControls ); + namingEnumeration = context.search( groupNameAttribute + "=" + groupName + "," + getGroupsDn( ), filter, searchControls ); - while ( namingEnumeration.hasMore() ) + if ( namingEnumeration.hasMore() ) { - SearchResult searchResult = namingEnumeration.next(); - Attribute attribute = searchResult.getAttributes().get( getLdapGroupMember() ); + SearchResult searchResult = namingEnumeration.next( ); + Attribute attribute = searchResult.getAttributes( ).get( getLdapGroupMemberAttribute( ) ); if ( attribute != null ) { - BasicAttribute basicAttribute = new BasicAttribute( getLdapGroupMember() ); - basicAttribute.add( this.userIdAttribute + "=" + username + "," + getGroupsDn() ); - context.modifyAttributes( "cn=" + groupName + "," + getGroupsDn(), new ModificationItem[]{ - new ModificationItem( DirContext.REMOVE_ATTRIBUTE, basicAttribute ) } ); + BasicAttribute basicAttribute = new BasicAttribute( getLdapGroupMemberAttribute( ) ); + basicAttribute.add( this.userIdAttribute + "=" + username + "," + getGroupsDn( ) ); + context.modifyAttributes( groupNameAttribute + "=" + groupName + "," + getGroupsDn( ), new ModificationItem[]{ + new ModificationItem( DirContext.REMOVE_ATTRIBUTE, basicAttribute )} ); } return true; } @@ -710,11 +714,11 @@ public class DefaultLdapRoleMapper } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } finally @@ -723,7 +727,7 @@ public class DefaultLdapRoleMapper { try { - namingEnumeration.close(); + namingEnumeration.close( ); } catch ( NamingException e ) { @@ -733,33 +737,58 @@ public class DefaultLdapRoleMapper } } + + public void removeAllRoles( DirContext context ) throws MappingException { //all mapped roles - Collection groups = ldapRoleMapperConfiguration.getLdapGroupMappings().keySet(); + Collection groups = ldapRoleMapperConfiguration.getLdapGroupMappings( ).keySet( ); try { for ( String groupName : groups ) { - - String dn = "cn=" + groupName + "," + this.groupsDn; - - context.unbind( dn ); - - log.debug( "deleted group with dn:'{}", dn ); + removeGroupByName( context, groupName ); } } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); + } + } + + private void removeGroupByName( DirContext context, String groupName ) throws NamingException + { + NamingEnumeration namingEnumeration = null; + try + { + SearchControls searchControls = new SearchControls( ); + + searchControls.setDerefLinkFlag( true ); + searchControls.setSearchScope( SearchControls.SUBTREE_SCOPE ); + String filter = "(&(objectClass=" + getLdapGroupClass( ) + ")(" + groupNameAttribute + "=" + Rdn.escapeValue( groupName ) + "))"; + // String filter = "(&(objectClass=" + getLdapGroupClass( ) + "))"; + namingEnumeration = context.search( getGroupsDn( ), filter, searchControls ); + + // We delete only the first found group + if ( namingEnumeration != null && namingEnumeration.hasMore( ) ) + { + SearchResult result = namingEnumeration.next( ); + String dn = result.getNameInNamespace( ); + context.unbind( new LdapName( dn ) ); + log.debug( "Deleted group with dn:'{}", dn ); + } + } + finally + { + closeNamingEnumeration( namingEnumeration ); } } @@ -768,25 +797,24 @@ public class DefaultLdapRoleMapper { String groupName = findGroupName( roleName ); - + if (StringUtils.isEmpty( groupName )) { + log.warn( "No group for the given role found: role={}", roleName ); + return; + } try { - String dn = "cn=" + groupName + "," + this.groupsDn; - - context.unbind( dn ); - - log.info( "deleted group with dn:'{}", dn ); + removeGroupByName( context, groupName ); } catch ( LdapException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } catch ( NamingException e ) { - throw new MappingException( e.getMessage(), e ); + throw new MappingException( e.getMessage( ), e ); } } @@ -819,7 +847,7 @@ public class DefaultLdapRoleMapper this.ldapConnectionFactory = ldapConnectionFactory; } - public String getBaseDn() + public String getBaseDn( ) { return baseDn; } @@ -829,14 +857,14 @@ public class DefaultLdapRoleMapper this.baseDn = baseDn; } - public String getLdapGroupMember() + public String getLdapGroupMemberAttribute( ) { - return ldapGroupMember; + return ldapGroupMemberAttribute; } - public void setLdapGroupMember( String ldapGroupMember ) + public void setLdapGroupMemberAttribute( String ldapGroupMemberAttribute ) { - this.ldapGroupMember = ldapGroupMember; + this.ldapGroupMemberAttribute = ldapGroupMemberAttribute; } //------------------- @@ -846,20 +874,20 @@ public class DefaultLdapRoleMapper protected String findGroupName( String role ) throws MappingException { - Map> mapping = ldapRoleMapperConfiguration.getLdapGroupMappings(); + Map> mapping = ldapRoleMapperConfiguration.getLdapGroupMappings( ); - for ( Map.Entry> entry : mapping.entrySet() ) + for ( Map.Entry> entry : mapping.entrySet( ) ) { - if ( entry.getValue().contains( role ) ) + if ( entry.getValue( ).contains( role ) ) { - return entry.getKey(); + return entry.getKey( ); } } return null; } - public String getUserIdAttribute() + public String getUserIdAttribute( ) { return userIdAttribute; } @@ -869,7 +897,7 @@ public class DefaultLdapRoleMapper this.userIdAttribute = userIdAttribute; } - public boolean isUseDefaultRoleName() + public boolean isUseDefaultRoleName( ) { return useDefaultRoleName; } diff --git a/redback-common/redback-common-ldap/src/test/java/org/apache/archiva/redback/common/ldap/role/TestLdapRoleMapper.java b/redback-common/redback-common-ldap/src/test/java/org/apache/archiva/redback/common/ldap/role/TestLdapRoleMapper.java index a0e55ad6..2a101aad 100644 --- a/redback-common/redback-common-ldap/src/test/java/org/apache/archiva/redback/common/ldap/role/TestLdapRoleMapper.java +++ b/redback-common/redback-common-ldap/src/test/java/org/apache/archiva/redback/common/ldap/role/TestLdapRoleMapper.java @@ -88,7 +88,7 @@ public class TestLdapRoleMapper LdapConnectionFactory ldapConnectionFactory; List roleNames = - Arrays.asList( "Archiva System Administrator", "Internal Repo Manager", "Internal Repo Observer" ); + Arrays.asList( "Archiva System Administrator", "Internal Repo Manager", "Internal Repo Observer", "Ldap Group Test Role" ); LdapConnection ldapConnection; @@ -109,6 +109,7 @@ public class TestLdapRoleMapper usersPerGroup.put( "internal-repo-manager", Arrays.asList( "admin", "user.9" ) ); usersPerGroup.put( "internal-repo-observer", Arrays.asList( "admin", "user.7", "user.8" ) ); usersPerGroup.put( "archiva-admin", Arrays.asList( "admin", "user.7" ) ); + usersPerGroup.put( "archiva/group-with-slash", Arrays.asList( "user.8", "user.9" ) ); users = new ArrayList( 4 ); users.add( "admin" ); @@ -161,7 +162,12 @@ public class TestLdapRoleMapper for ( Map.Entry> group : usersPerGroup.entrySet() ) { - context.unbind( createGroupDn( group.getKey() ) ); + try + { + context.unbind( createGroupDn( group.getKey( ) ) ); + } catch (Exception ex) { + // Ignore + } } context.unbind( suffix ); @@ -298,7 +304,7 @@ public class TestLdapRoleMapper log.info( "allGroups: {}", allGroups ); - assertThat( allGroups ).isNotNull().isNotEmpty().contains( "archiva-admin", + assertThat( allGroups ).isNotNull().isNotEmpty().contains( "archiva/group-with-slash", "archiva-admin", "internal-repo-manager" ); } @@ -331,7 +337,7 @@ public class TestLdapRoleMapper groups = ldapRoleMapper.getGroups( "user.8", getDirContext() ); - assertThat( groups ).isNotNull().isNotEmpty().hasSize( 1 ).contains( "internal-repo-observer" ); + assertThat( groups ).isNotNull().isNotEmpty().hasSize( 2 ).contains( "internal-repo-observer", "archiva/group-with-slash" ); groups = ldapRoleMapper.getGroups( "user.7", getDirContext() ); @@ -362,7 +368,7 @@ public class TestLdapRoleMapper log.info( "roles for user.8: {}", roles ); - assertThat( roles ).isNotNull().isNotEmpty().hasSize( 1 ).contains( "Internal Repo Observer" ); + assertThat( roles ).isNotNull().isNotEmpty().hasSize( 2 ).contains( "Internal Repo Observer" ); } @@ -380,5 +386,19 @@ public class TestLdapRoleMapper assertFalse( ldapRoleMapper.hasRole( getDirContext(), "Australian wine is good but not as French! " ) ); } + @Test + public void removeRole() throws Exception + { + assertTrue( ldapRoleMapper.getAllGroups( getDirContext( ) ).contains( "archiva-admin" ) ); + ldapRoleMapper.removeRole( "Archiva System Administrator", getDirContext() ); + assertFalse( ldapRoleMapper.getAllGroups( getDirContext( ) ).contains( "archiva-admin" ) ); + } + @Test + public void removeAllRoles() throws Exception + { + assertEquals( 4, ldapRoleMapper.getAllGroups( getDirContext( ) ).size() ); + ldapRoleMapper.removeAllRoles( getDirContext() ); + assertEquals( 0, ldapRoleMapper.getAllGroups( getDirContext( ) ).size() ); + } } diff --git a/redback-common/redback-common-ldap/src/test/security.properties b/redback-common/redback-common-ldap/src/test/security.properties index 1df88925..eb46b1f9 100644 --- a/redback-common/redback-common-ldap/src/test/security.properties +++ b/redback-common/redback-common-ldap/src/test/security.properties @@ -16,4 +16,5 @@ # under the License. ldap.config.groups.role.archiva-admin=Archiva System Administrator ldap.config.groups.role.internal-repo-manager=Internal Repo Manager -ldap.config.groups.role.internal-repo-observer=Internal Repo Observer \ No newline at end of file +ldap.config.groups.role.internal-repo-observer=Internal Repo Observer +ldap.config.groups.role.archiva/group-with-slash=Ldap Group Test Role \ No newline at end of file diff --git a/redback-configuration/src/main/java/org/apache/archiva/redback/configuration/UserConfigurationKeys.java b/redback-configuration/src/main/java/org/apache/archiva/redback/configuration/UserConfigurationKeys.java index 5bdde914..95141ed5 100644 --- a/redback-configuration/src/main/java/org/apache/archiva/redback/configuration/UserConfigurationKeys.java +++ b/redback-configuration/src/main/java/org/apache/archiva/redback/configuration/UserConfigurationKeys.java @@ -92,10 +92,14 @@ public interface UserConfigurationKeys String LDAP_GROUPS_USE_ROLENAME = "ldap.config.groups.use.rolename"; + String LDAP_GROUPS_USE_DN_AS_MEMBER_VALUE = "ldap.config.groups.useDnAsMemberValue"; + String LDAP_WRITABLE = "ldap.config.writable"; String LDAP_USER_ID_ATTRIBUTE = "ldap.config.user.attribute"; + String LDAP_GROUP_NAME_ATTRIBUTE = "ldap.config.groups.name.attribute"; + String APPLICATION_URL = "application.url"; String EMAIL_URL_PATH = "email.url.path";