From a34cb92fc10ac879878901a76b2679df0ba2a298 Mon Sep 17 00:00:00 2001 From: mposolda Date: Wed, 3 Jun 2015 18:44:23 +0200 Subject: [PATCH] KEYCLOAK-1359 Fix testsuite with OpenLDAP, Other LDAP fixes --- .../org/keycloak/federation/ldap/LDAPFederationProvider.java | 3 +-- .../federation/ldap/idm/store/ldap/LDAPIdentityStore.java | 2 +- .../federation/ldap/mappers/FullNameLDAPFederationMapper.java | 2 +- .../federation/ldap/mappers/RoleLDAPFederationMapper.java | 2 +- .../ldap/mappers/UserAttributeLDAPFederationMapper.java | 2 +- model/api/src/main/java/org/keycloak/models/LDAPConstants.java | 1 + 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java index 4524d485d6..7ad05d34d7 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java +++ b/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) { diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java index 7ba169204e..338271b1d0 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java +++ b/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); } } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java index 109b0b0775..be1a7b4d25 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java +++ b/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); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java index 83e8d66721..0116db7877 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java +++ b/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); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java index c4c30291dd..26f666f6b8 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java +++ b/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 userModelProperty = userModelProperties.get(userModelAttrName); if (userModelProperty != null) { diff --git a/model/api/src/main/java/org/keycloak/models/LDAPConstants.java b/model/api/src/main/java/org/keycloak/models/LDAPConstants.java index 3b9de4a68d..6ab0d08e9d 100644 --- a/model/api/src/main/java/org/keycloak/models/LDAPConstants.java +++ b/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";