KEYCLOAK-1359 Fix testsuite with OpenLDAP, Other LDAP fixes

federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java

@@ -308,8 +308,7 @@ public class LDAPFederationProvider implements UserFederationProvider {
 
     @Override
     public void preRemove(RealmModel realm, RoleModel role) {
-        // complete I don't think we have to do anything here
-        // TODO: requires implementation... Maybe mappers callback to ensure role deletion propagated to LDAP by RoleLDAPFederationMapper
+        // TODO: Maybe mappers callback to ensure role deletion propagated to LDAP by RoleLDAPFederationMapper?
     }
 
     public boolean validPassword(RealmModel realm, UserModel user, String password) {

federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java

@@ -476,7 +476,7 @@ public class LDAPIdentityStore implements IdentityStore {
                 if (objectClassValue.equals(LDAPConstants.GROUP_OF_NAMES)
                         || objectClassValue.equals(LDAPConstants.GROUP_OF_ENTRIES)
                         || objectClassValue.equals(LDAPConstants.GROUP_OF_UNIQUE_NAMES)) {
-                    entryAttributes.put(LDAPConstants.MEMBER, LDAPConstants.EMPTY_ATTRIBUTE_VALUE);
+                    entryAttributes.put(LDAPConstants.MEMBER, LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE);
                 }
             }
 

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java

@@ -35,7 +35,7 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper {
         String ldapFullNameAttrName = getLdapFullNameAttrName(mapperModel);
         String fullName = ldapUser.getAttributeAsString(ldapFullNameAttrName);
         fullName = fullName.trim();
-        if (fullName != null) {
+        if (fullName != null && !fullName.trim().isEmpty()) {
             int lastSpaceIndex = fullName.lastIndexOf(" ");
             if (lastSpaceIndex == -1) {
                 user.setLastName(fullName);

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java

@@ -233,7 +233,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
 
         // Some membership placeholder needs to be always here as "member" is mandatory attribute on some LDAP servers
         if (memberships.size() == 0) {
-            memberships.add(LDAPConstants.EMPTY_ATTRIBUTE_VALUE);
+            memberships.add(LDAPConstants.EMPTY_MEMBER_ATTRIBUTE_VALUE);
         }
 
         ldapRole.setAttribute(getMembershipLdapAttribute(mapperModel), memberships);

federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java

@@ -55,7 +55,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap
         String ldapAttrName = mapperModel.getConfig().get(LDAP_ATTRIBUTE);
 
         Object ldapAttrValue = ldapUser.getAttribute(ldapAttrName);
-        if (ldapAttrValue != null) {
+        if (ldapAttrValue != null && !ldapAttrValue.toString().trim().isEmpty()) {
             Property<Object> userModelProperty = userModelProperties.get(userModelAttrName);
 
             if (userModelProperty != null) {

model/api/src/main/java/org/keycloak/models/LDAPConstants.java

@@ -70,6 +70,7 @@ public class LDAPConstants {
     public static final String COMMA = ",";
     public static final String EQUAL = "=";
     public static final String EMPTY_ATTRIBUTE_VALUE = " ";
+    public static final String EMPTY_MEMBER_ATTRIBUTE_VALUE = "";
 
     public static final String CUSTOM_ATTRIBUTE_ENABLED = "enabled";
     public static final String CUSTOM_ATTRIBUTE_CREATE_DATE = "createDate";