libre.sh/keycloak-scim
4
0
Fork 0
Code Issues 15 Pull requests Releases 1 Activity Actions

KEYCLOAK-1487 Fix LDAP case-sensitivity. Show warning in case of duplicate username

Browse source
This commit is contained in:
mposolda 2015-06-25 19:27:22 +02:00
parent 493dddde02
commit 773bb43b41
9 changed files with 95 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java
View file

@ -1,6 +1,5 @@
package org.keycloak.federation.ldap;
import java.util.List;
import java.util.Set;
import org.keycloak.federation.ldap.idm.model.LDAPDn;
@ -61,7 +60,7 @@ public class LDAPUtils {
// ldapUser has filled attributes, but doesn't have filled dn.
private static void computeAndSetDn(LDAPConfig config, LDAPObject ldapUser) {
String rdnLdapAttrName = config.getRdnLdapAttribute();
String rdnLdapAttrValue = ldapUser.getAttributeAsString(rdnLdapAttrName);
String rdnLdapAttrValue = ldapUser.getAttributeAsStringCaseInsensitive(rdnLdapAttrName);
if (rdnLdapAttrValue == null) {
throw new ModelException("RDN Attribute [" + rdnLdapAttrName + "] is not filled. Filled attributes: " + ldapUser.getAttributes());
}
@ -73,6 +72,6 @@ public class LDAPUtils {
public static String getUsername(LDAPObject ldapUser, LDAPConfig config) {
String usernameAttr = config.getUsernameLdapAttribute();
return ldapUser.getAttributeAsString(usernameAttr);
return ldapUser.getAttributeAsStringCaseInsensitive(usernameAttr);
}
}

41
federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java
View file

@ -6,18 +6,29 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import org.jboss.logging.Logger;
/**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class LDAPObject {
private static final Logger logger = Logger.getLogger(LDAPObject.class);
private String uuid;
private LDAPDn dn;
private String rdnAttributeName;
private final List<String> objectClasses = new LinkedList<String>();
private final List<String> readOnlyAttributeNames = new LinkedList<String>();
private final Map<String, Object> attributes = new HashMap<String, Object>();
private final List<String> objectClasses = new LinkedList<>();
// NOTE: names of read-only attributes are lower-cased to avoid case sensitivity issues
private final List<String> readOnlyAttributeNames = new LinkedList<>();
private final Map<String, Object> attributes = new HashMap<>();
// Copy of "attributes" containing lower-cased keys
private final Map<String, Object> lowerCasedAttributes = new HashMap<>();
public String getUuid() {
return uuid;
@ -49,7 +60,7 @@ public class LDAPObject {
}
public void addReadOnlyAttributeName(String readOnlyAttribute) {
readOnlyAttributeNames.add(readOnlyAttribute);
readOnlyAttributeNames.add(readOnlyAttribute.toLowerCase());
}
public String getRdnAttributeName() {
@ -62,21 +73,23 @@ public class LDAPObject {
public void setAttribute(String attributeName, Object attributeValue) {
attributes.put(attributeName, attributeValue);
lowerCasedAttributes.put(attributeName.toLowerCase(), attributeValue);
}
public void removeAttribute(String name) {
attributes.remove(name);
public Object getAttributeCaseInsensitive(String name) {
return lowerCasedAttributes.get(name.toLowerCase());
}
public Object getAttribute(String name) {
return attributes.get(name);
}
public String getAttributeAsString(String name) {
Object attrValue = attributes.get(name);
public String getAttributeAsStringCaseInsensitive(String name) {
Object attrValue = lowerCasedAttributes.get(name.toLowerCase());
if (attrValue != null && !(attrValue instanceof String)) {
throw new IllegalStateException("Expected String but attribute was " + attrValue + " of type " + attrValue.getClass().getName());
logger.warnf("Expected String but attribute '%s' has value '%s' of type '%s' ", name, attrValue, attrValue.getClass().getName());
if (attrValue instanceof Collection) {
Collection<String> attrValues = (Collection<String>) attrValue;
attrValue = attrValues.iterator().next();
logger.warnf("Returning just first founded value '%s' from the collection", attrValue);
}
}
return (String) attrValue;

3
federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPIdentityQuery.java
View file

@ -40,6 +40,7 @@ public class LDAPIdentityQuery {
private final Set<String> returningLdapAttributes = new LinkedHashSet<String>();
// Contains just those returningLdapAttributes, which are read-only. They will be marked as read-only in returned LDAPObject instances as well
// NOTE: names of attributes are lower-cased to avoid case sensitivity issues (LDAP searching is usually case-insensitive, so we want to be as well)
private final Set<String> returningReadOnlyLdapAttributes = new LinkedHashSet<String>();
private final Set<String> objectClasses = new LinkedHashSet<String>();
@ -77,7 +78,7 @@ public class LDAPIdentityQuery {
}
public LDAPIdentityQuery addReturningReadOnlyLdapAttribute(String ldapAttributeName) {
this.returningReadOnlyLdapAttributes.add(ldapAttributeName);
this.returningReadOnlyLdapAttributes.add(ldapAttributeName.toLowerCase());
return this;
}

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

@ -4,11 +4,11 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.TreeSet;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
@ -382,12 +382,6 @@ public class LDAPIdentityStore implements IdentityStore {
NamingEnumeration<? extends Attribute> ldapAttributes = attributes.getAll();
// Exact name of attributes might be different
List<String> uppercasedReadOnlyAttrNames = new ArrayList<>();
for (String readonlyAttr : readOnlyAttrNames) {
uppercasedReadOnlyAttrNames.add(readonlyAttr.toUpperCase());
}
while (ldapAttributes.hasMore()) {
Attribute ldapAttribute = ldapAttributes.next();
@ -403,7 +397,7 @@ public class LDAPIdentityStore implements IdentityStore {
Object uuidValue = ldapAttribute.get();
ldapObject.setUuid(this.operationManager.decodeEntryUUID(uuidValue));
} else {
Set<String> attrValues = new TreeSet<>();
Set<String> attrValues = new LinkedHashSet<>();
NamingEnumeration<?> enumm = ldapAttribute.getAll();
while (enumm.hasMoreElements()) {
String attrVal = enumm.next().toString();
@ -419,7 +413,8 @@ public class LDAPIdentityStore implements IdentityStore {
ldapObject.setAttribute(ldapAttributeName, attrValues);
}
if (uppercasedReadOnlyAttrNames.contains(ldapAttributeName.toUpperCase())) {
// readOnlyAttrNames are lower-cased
if (readOnlyAttrNames.contains(ldapAttributeName.toLowerCase())) {
ldapObject.addReadOnlyAttributeName(ldapAttributeName);
}
}
@ -443,7 +438,9 @@ public class LDAPIdentityStore implements IdentityStore {
for (Map.Entry<String, Object> attrEntry : ldapObject.getAttributes().entrySet()) {
String attrName = attrEntry.getKey();
Object attrValue = attrEntry.getValue();
if (!ldapObject.getReadOnlyAttributeNames().contains(attrName) && (isCreate || !ldapObject.getRdnAttributeName().equalsIgnoreCase(attrName))) {
// ldapObject.getReadOnlyAttributeNames() are lower-cased
if (!ldapObject.getReadOnlyAttributeNames().contains(attrName.toLowerCase()) && (isCreate || !ldapObject.getRdnAttributeName().equalsIgnoreCase(attrName))) {
if (String.class.isInstance(attrValue)) {
if (attrValue.toString().trim().length() == 0) {

2
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java
View file

@ -28,7 +28,7 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper {
@Override
public void onImportUserFromLDAP(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, UserModel user, RealmModel realm, boolean isCreate) {
String ldapFullNameAttrName = getLdapFullNameAttrName(mapperModel);
String fullName = ldapUser.getAttributeAsString(ldapFullNameAttrName);
String fullName = ldapUser.getAttributeAsStringCaseInsensitive(ldapFullNameAttrName);
fullName = fullName.trim();
if (fullName != null && !fullName.trim().isEmpty()) {
int lastSpaceIndex = fullName.lastIndexOf(" ");

8
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java
View file

@ -74,7 +74,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
// Import role mappings from LDAP into Keycloak DB
String roleNameAttr = getRoleNameLdapAttribute(mapperModel);
for (LDAPObject ldapRole : ldapRoles) {
String roleName = ldapRole.getAttributeAsString(roleNameAttr);
String roleName = ldapRole.getAttributeAsStringCaseInsensitive(roleNameAttr);
RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm);
RoleModel role = roleContainer.getRole(roleName);
@ -103,7 +103,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm);
String rolesRdnAttr = getRoleNameLdapAttribute(mapperModel);
for (LDAPObject ldapRole : ldapRoles) {
String roleName = ldapRole.getAttributeAsString(rolesRdnAttr);
String roleName = ldapRole.getAttributeAsStringCaseInsensitive(rolesRdnAttr);
if (roleContainer.getRole(roleName) == null) {
logger.infof("Syncing role [%s] from LDAP to keycloak DB", roleName);
@ -249,7 +249,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
protected Set<String> getExistingMemberships(UserFederationMapperModel mapperModel, LDAPObject ldapRole) {
String memberAttrName = getMembershipLdapAttribute(mapperModel);
Set<String> memberships = new TreeSet<String>();
Object existingMemberships = ldapRole.getAttribute(memberAttrName);
Object existingMemberships = ldapRole.getAttributeCaseInsensitive(memberAttrName);
if (existingMemberships != null) {
if (existingMemberships instanceof String) {
@ -411,7 +411,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper {
Set<RoleModel> roles = new HashSet<RoleModel>();
String roleNameLdapAttr = getRoleNameLdapAttribute(mapperModel);
for (LDAPObject role : ldapRoles) {
String roleName = role.getAttributeAsString(roleNameLdapAttr);
String roleName = role.getAttributeAsStringCaseInsensitive(roleNameLdapAttr);
RoleModel modelRole = roleContainer.getRole(roleName);
if (modelRole == null) {
// Add role to local DB

2
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java
View file

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

28
testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java
View file

@ -262,6 +262,34 @@ public class FederationProvidersIntegrationTest {
}
}
@Test
public void testCaseSensitiveAttributeName() {
KeycloakSession session = keycloakRule.startSession();
UserFederationMapperModel zipCodeMapper = null;
try {
RealmModel appRealm = new RealmManager(session).getRealmByName("test");
LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
LDAPObject johnZip = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "johnzip", "John", "Zip", "johnzip@email.org", "12398");
// Remove default zipcode mapper
UserFederationMapperModel currentZipMapper = appRealm.getUserFederationMapperByName(ldapModel.getId(), "zipCodeMapper");
appRealm.removeUserFederationMapper(currentZipMapper);
// Add zipcode mapper for "POstalCode"
FederationTestUtils.addUserAttributeMapper(appRealm, ldapModel, "zipCodeMapper-cs", "postal_code", "POstalCode");
// Fetch user from LDAP and check that postalCode is filled
UserModel user = session.users().getUserByUsername("johnzip", appRealm);
String postalCode = user.getAttribute("postal_code");
Assert.assertEquals("12398", postalCode);
} finally {
keycloakRule.stopSession(session, false);
}
}
@Test
public void testFullNameMapper() {
KeycloakSession session = keycloakRule.startSession();

23
testsuite/integration/src/test/resources/ldap/users.ldif
View file

@ -19,4 +19,27 @@ objectclass: top
objectclass: organizationalUnit
ou: FinanceRoles
dn: uid=jbrown,ou=People,dc=keycloak,dc=org
objectclass: top
objectclass: person
objectclass: organizationalPerson
objectclass: inetOrgPerson
uid: jbrown
cn: James
sn: Brown
mail: jbrown@keycloak.org
postalCode: 88441
dn: uid=bwilson,ou=People,dc=keycloak,dc=org
objectclass: top
objectclass: person
objectclass: organizationalPerson
objectclass: inetOrgPerson
uid: bwilson
cn: Bruce
sn: Wilson
mail: bwilson@keycloak.org
postalCode: 88441
postalCode: 77332