From 79c1650c15c638358c446ef2410c311d20074dd3 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 9 Dec 2016 15:41:55 +0100 Subject: [PATCH 1/2] KEYCLOAK-2545 KEYCLOAK-3668 KEYCLOAK-3247 LDAP escaping --- .../storage/ldap/LDAPStorageProvider.java | 37 ++-- .../org/keycloak/storage/ldap/LDAPUtils.java | 3 +- .../storage/ldap/idm/model/LDAPDn.java | 10 +- .../ldap/idm/query/EscapeStrategy.java | 102 +++++++++ .../idm/query/internal/EqualCondition.java | 14 +- .../internal/LDAPQueryConditionsBuilder.java | 7 +- .../idm/store/ldap/LDAPIdentityStore.java | 6 +- .../mappers/FullNameLDAPStorageMapper.java | 6 +- .../mappers/membership/MembershipType.java | 3 +- .../group/GroupLDAPStorageMapper.java | 4 +- .../role/RoleLDAPStorageMapper.java | 2 +- .../storage/ldap/idm/model/EscapeTest.java | 47 ++++ .../storage/ldap/idm/model/LDAPDnTest.java | 8 +- .../storage/ldap/LDAPGroupMapperSyncTest.java | 2 +- .../storage/ldap/LDAPGroupMapperTest.java | 33 ++- .../storage/ldap/LDAPSpecialCharsTest.java | 205 ++++++++++++++++++ 16 files changed, 455 insertions(+), 34 deletions(-) create mode 100644 federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/EscapeStrategy.java create mode 100644 federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/EscapeTest.java create mode 100644 testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPSpecialCharsTest.java diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index 77131ff309..d1fc068f00 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -43,6 +43,7 @@ import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.idm.query.Condition; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery; import org.keycloak.storage.ldap.idm.query.internal.LDAPQueryConditionsBuilder; import org.keycloak.storage.ldap.idm.store.ldap.LDAPIdentityStore; @@ -312,17 +313,27 @@ public class LDAPStorageProvider implements UserStorageProvider, List results = new ArrayList(); if (attributes.containsKey(UserModel.USERNAME)) { - LDAPObject user = loadLDAPUserByUsername(realm, attributes.get(UserModel.USERNAME)); - if (user != null) { - results.add(user); - } + LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + + // Mapper should replace "username" in parameter name with correct LDAP mapped attribute + Condition usernameCondition = conditionsBuilder.equal(UserModel.USERNAME, attributes.get(UserModel.USERNAME), EscapeStrategy.NON_ASCII_CHARS_ONLY); + ldapQuery.addWhereCondition(usernameCondition); + + List ldapObjects = ldapQuery.getResultList(); + results.addAll(ldapObjects); } if (attributes.containsKey(UserModel.EMAIL)) { - LDAPObject user = queryByEmail(realm, attributes.get(UserModel.EMAIL)); - if (user != null) { - results.add(user); - } + LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); + LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); + + // Mapper should replace "email" in parameter name with correct LDAP mapped attribute + Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, attributes.get(UserModel.EMAIL), EscapeStrategy.NON_ASCII_CHARS_ONLY); + ldapQuery.addWhereCondition(emailCondition); + + List ldapObjects = ldapQuery.getResultList(); + results.addAll(ldapObjects); } if (attributes.containsKey(UserModel.FIRST_NAME) || attributes.containsKey(UserModel.LAST_NAME)) { @@ -331,10 +342,10 @@ public class LDAPStorageProvider implements UserStorageProvider, // Mapper should replace parameter with correct LDAP mapped attributes if (attributes.containsKey(UserModel.FIRST_NAME)) { - ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.FIRST_NAME, attributes.get(UserModel.FIRST_NAME))); + ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.FIRST_NAME, attributes.get(UserModel.FIRST_NAME), EscapeStrategy.NON_ASCII_CHARS_ONLY)); } if (attributes.containsKey(UserModel.LAST_NAME)) { - ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.LAST_NAME, attributes.get(UserModel.LAST_NAME))); + ldapQuery.addWhereCondition(conditionsBuilder.equal(UserModel.LAST_NAME, attributes.get(UserModel.LAST_NAME), EscapeStrategy.NON_ASCII_CHARS_ONLY)); } List ldapObjects = ldapQuery.getResultList(); @@ -405,7 +416,7 @@ public class LDAPStorageProvider implements UserStorageProvider, LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); // Mapper should replace "email" in parameter name with correct LDAP mapped attribute - Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, email); + Condition emailCondition = conditionsBuilder.equal(UserModel.EMAIL, email, EscapeStrategy.DEFAULT); ldapQuery.addWhereCondition(emailCondition); return ldapQuery.getFirstResult(); @@ -624,7 +635,7 @@ public class LDAPStorageProvider implements UserStorageProvider, LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); String usernameMappedAttribute = this.ldapIdentityStore.getConfig().getUsernameLdapAttribute(); - Condition usernameCondition = conditionsBuilder.equal(usernameMappedAttribute, username); + Condition usernameCondition = conditionsBuilder.equal(usernameMappedAttribute, username, EscapeStrategy.DEFAULT); ldapQuery.addWhereCondition(usernameCondition); LDAPObject ldapUser = ldapQuery.getFirstResult(); @@ -636,7 +647,7 @@ public class LDAPStorageProvider implements UserStorageProvider, } public LDAPStorageMapper getMapper(ComponentModel mapperModel) { - LDAPStorageMapper ldapMapper = (LDAPStorageMapper) getSession().getProvider(LDAPStorageMapper.class, mapperModel); + LDAPStorageMapper ldapMapper = getSession().getProvider(LDAPStorageMapper.class, mapperModel); if (ldapMapper == null) { throw new ModelException("Can't find mapper type with ID: " + mapperModel.getProviderId()); } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java index a5e48a7439..3a391d3aa2 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPUtils.java @@ -189,9 +189,8 @@ public class LDAPUtils { * @param memberAttrName usually 'member' * @param ldapParent role or group * @param ldapChild usually user (or child group or child role) - * @param sendLDAPUpdateRequest if true, the method will send LDAP update request too. Otherwise it will skip it */ - public static void deleteMember(LDAPStorageProvider ldapProvider, MembershipType membershipType, String memberAttrName, LDAPObject ldapParent, LDAPObject ldapChild, boolean sendLDAPUpdateRequest) { + public static void deleteMember(LDAPStorageProvider ldapProvider, MembershipType membershipType, String memberAttrName, LDAPObject ldapParent, LDAPObject ldapChild) { Set memberships = getExistingMemberships(memberAttrName, ldapParent); String userMembership = getMemberValueOfChildObject(ldapChild, membershipType); diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java index e95e8adafd..8bfbf6d3c0 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/model/LDAPDn.java @@ -88,7 +88,7 @@ public class LDAPDn { */ public String getFirstRdn() { Entry firstEntry = entries.getFirst(); - return firstEntry.attrName + "=" + firstEntry.attrValue; + return firstEntry.attrName + "=" + unescapeValue(firstEntry.attrValue); } /** @@ -104,7 +104,13 @@ public class LDAPDn { */ public String getFirstRdnAttrValue() { Entry firstEntry = entries.getFirst(); - return firstEntry.attrValue; + String dnEscaped = firstEntry.attrValue; + return unescapeValue(dnEscaped); + } + + private String unescapeValue(String escaped) { + // Something needed to handle non-String types? + return Rdn.unescapeValue(escaped).toString(); } /** diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/EscapeStrategy.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/EscapeStrategy.java new file mode 100644 index 0000000000..a17116957d --- /dev/null +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/EscapeStrategy.java @@ -0,0 +1,102 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.storage.ldap.idm.query; + +import java.io.UnsupportedEncodingException; + +/** + * @author Marek Posolda + */ +public enum EscapeStrategy { + + + // LDAP special characters like * ( ) \ are not escaped. Only non-ASCII characters like é are escaped + NON_ASCII_CHARS_ONLY { + + @Override + public String escape(String input) { + try { + StringBuilder output = new StringBuilder(); + + for (byte b : input.getBytes("UTF-8")) { + appendByte(b, output); + } + + return output.toString(); + } catch (UnsupportedEncodingException uee) { + throw new RuntimeException(uee); + } + } + + }, + + + // Escaping of LDAP special characters including non-ASCII characters like é + DEFAULT { + + + @Override + public String escape(String input) { + try { + StringBuilder output = new StringBuilder(); + + for (byte b : input.getBytes("UTF-8")) { + switch (b) { + case 0x5c: + output.append("\\5c"); // \ + break; + case 0x2a: + output.append("\\2a"); // * + break; + case 0x28: + output.append("\\28"); // ( + break; + case 0x29: + output.append("\\29"); // ) + break; + case 0x00: + output.append("\\00"); // \u0000 + break; + default: { + appendByte(b, output); + } + } + } + + return output.toString(); + } catch (UnsupportedEncodingException uee) { + throw new RuntimeException(uee); + } + } + + }; + + + public abstract String escape(String input); + + + protected void appendByte(byte b, StringBuilder output) { + if (b >= 0) { + output.append((char) b); + } else { + int i = -256 ^ b; + output.append("\\").append(Integer.toHexString(i)); + } + } + +} diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/EqualCondition.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/EqualCondition.java index e82fe376d2..430fddfbdc 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/EqualCondition.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/EqualCondition.java @@ -18,6 +18,7 @@ package org.keycloak.storage.ldap.idm.query.internal; import org.keycloak.models.LDAPConstants; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.store.ldap.LDAPUtil; import java.util.Date; @@ -28,16 +29,22 @@ import java.util.Date; public class EqualCondition extends NamedParameterCondition { private final Object value; + private final EscapeStrategy escapeStrategy; - public EqualCondition(String name, Object value) { + public EqualCondition(String name, Object value, EscapeStrategy escapeStrategy) { super(name); this.value = value; + this.escapeStrategy = escapeStrategy; } public Object getValue() { return this.value; } + public EscapeStrategy getEscapeStrategy() { + return escapeStrategy; + } + @Override public void applyCondition(StringBuilder filter) { Object parameterValue = value; @@ -45,7 +52,9 @@ public class EqualCondition extends NamedParameterCondition { parameterValue = LDAPUtil.formatDate((Date) parameterValue); } - filter.append("(").append(getParameterName()).append(LDAPConstants.EQUAL).append(parameterValue).append(")"); + String escaped = escapeStrategy.escape(parameterValue.toString()); + + filter.append("(").append(getParameterName()).append(LDAPConstants.EQUAL).append(escaped).append(")"); } @Override @@ -53,6 +62,7 @@ public class EqualCondition extends NamedParameterCondition { return "EqualCondition{" + "paramName=" + getParameterName() + ", value=" + value + + ", escapeStrategy=" + escapeStrategy + '}'; } } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/LDAPQueryConditionsBuilder.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/LDAPQueryConditionsBuilder.java index 715ec3da36..4ced23b19d 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/LDAPQueryConditionsBuilder.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/query/internal/LDAPQueryConditionsBuilder.java @@ -19,6 +19,7 @@ package org.keycloak.storage.ldap.idm.query.internal; import org.keycloak.models.ModelException; import org.keycloak.storage.ldap.idm.query.Condition; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.query.Sort; /** @@ -27,7 +28,11 @@ import org.keycloak.storage.ldap.idm.query.Sort; public class LDAPQueryConditionsBuilder { public Condition equal(String parameter, Object value) { - return new EqualCondition(parameter, value); + return new EqualCondition(parameter, value, EscapeStrategy.DEFAULT); + } + + public Condition equal(String parameter, Object value, EscapeStrategy escapeStrategy) { + return new EqualCondition(parameter, value, escapeStrategy); } public Condition greaterThan(String paramName, Object x) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java index 6d0e2cc0b3..b36c17c60a 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPIdentityStore.java @@ -24,6 +24,7 @@ import org.keycloak.storage.ldap.LDAPConfig; import org.keycloak.storage.ldap.idm.model.LDAPDn; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.idm.query.Condition; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.query.internal.EqualCondition; import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery; import org.keycloak.storage.ldap.idm.store.IdentityStore; @@ -408,7 +409,10 @@ public class LDAPIdentityStore implements IdentityStore { try { // we need this to retrieve the entry's identifier from the ldap server String uuidAttrName = getConfig().getUuidLDAPAttributeName(); - List search = this.operationManager.search(ldapObject.getDn().toString(), "(" + ldapObject.getDn().getFirstRdn() + ")", Arrays.asList(uuidAttrName), SearchControls.OBJECT_SCOPE); + + String rdn = ldapObject.getDn().getFirstRdn(); + String filter = "(" + EscapeStrategy.DEFAULT.escape(rdn) + ")"; + List search = this.operationManager.search(ldapObject.getDn().toString(), filter, Arrays.asList(uuidAttrName), SearchControls.OBJECT_SCOPE); Attribute id = search.get(0).getAttributes().get(getConfig().getUuidLDAPAttributeName()); if (id == null) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java index 45ee17a3b5..43d631ea72 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java @@ -25,6 +25,7 @@ import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.ldap.LDAPStorageProvider; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.idm.query.Condition; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.query.internal.EqualCondition; import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery; @@ -164,7 +165,10 @@ public class FullNameLDAPStorageMapper extends AbstractLDAPStorageMapper { } else { return; } - EqualCondition fullNameCondition = new EqualCondition(ldapFullNameAttrName, fullName); + + EscapeStrategy escapeStrategy = firstNameCondition!=null ? firstNameCondition.getEscapeStrategy() : lastNameCondition.getEscapeStrategy(); + + EqualCondition fullNameCondition = new EqualCondition(ldapFullNameAttrName, fullName, escapeStrategy); query.addWhereCondition(fullNameCondition); } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java index 7c171ead56..f2df5cd49f 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/MembershipType.java @@ -25,6 +25,7 @@ import org.keycloak.storage.ldap.LDAPUtils; import org.keycloak.storage.ldap.idm.model.LDAPDn; import org.keycloak.storage.ldap.idm.model.LDAPObject; import org.keycloak.storage.ldap.idm.query.Condition; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; import org.keycloak.storage.ldap.idm.query.internal.LDAPQuery; import org.keycloak.storage.ldap.idm.query.internal.LDAPQueryConditionsBuilder; import org.keycloak.storage.ldap.mappers.membership.group.GroupLDAPStorageMapper; @@ -101,7 +102,7 @@ public enum MembershipType { Condition[] orSubconditions = new Condition[dns.size()]; int index = 0; for (LDAPDn userDn : dns) { - Condition condition = conditionsBuilder.equal(userDn.getFirstRdnAttrName(), userDn.getFirstRdnAttrValue()); + Condition condition = conditionsBuilder.equal(userDn.getFirstRdnAttrName(), userDn.getFirstRdnAttrValue(), EscapeStrategy.DEFAULT); orSubconditions[index] = condition; index++; } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java index 16c5b62846..fe4930caf8 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java @@ -462,7 +462,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements for (LDAPDn toRemoveDN : toRemoveSubgroupsDNs) { LDAPObject fakeGroup = new LDAPObject(); fakeGroup.setDn(toRemoveDN); - LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, config.getMembershipLdapAttribute(), ldapGroup, fakeGroup, false); + LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, config.getMembershipLdapAttribute(), ldapGroup, fakeGroup); } // Update group to LDAP @@ -501,7 +501,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements } public void deleteGroupMappingInLDAP(LDAPObject ldapUser, LDAPObject ldapGroup) { - LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapGroup, ldapUser, true); + LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapGroup, ldapUser); } protected List getLDAPGroupMappings(LDAPObject ldapUser) { diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java index e3a524fec9..fd78877222 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/role/RoleLDAPStorageMapper.java @@ -256,7 +256,7 @@ public class RoleLDAPStorageMapper extends AbstractLDAPStorageMapper implements } public void deleteRoleMappingInLDAP(LDAPObject ldapUser, LDAPObject ldapRole) { - LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapRole, ldapUser, true); + LDAPUtils.deleteMember(ldapProvider, config.getMembershipTypeLdapAttribute(), config.getMembershipLdapAttribute(), ldapRole, ldapUser); } public LDAPObject loadLDAPRoleByName(String roleName) { diff --git a/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/EscapeTest.java b/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/EscapeTest.java new file mode 100644 index 0000000000..64cb6413ae --- /dev/null +++ b/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/EscapeTest.java @@ -0,0 +1,47 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.storage.ldap.idm.model; + +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.storage.ldap.idm.query.EscapeStrategy; + +/** + * @author Marek Posolda + */ +public class EscapeTest { + + @Test + public void testNoAsciiOnlyEscaping() throws Exception { + String text = "Véronique* Martin(john)second\\fff//eee\u0000"; + Assert.assertEquals(EscapeStrategy.NON_ASCII_CHARS_ONLY.escape(text), "V\\c3\\a9ronique* Martin(john)second\\fff//eee\u0000"); + + text = "Hi This is a test #çà"; + Assert.assertEquals(EscapeStrategy.DEFAULT.escape(text), "Hi This is a test #\\c3\\a7\\c3\\a0"); + } + + @Test + public void testEscaping() throws Exception { + String text = "Véronique* Martin(john)second\\fff//eee\u0000"; + Assert.assertEquals(EscapeStrategy.DEFAULT.escape(text), "V\\c3\\a9ronique\\2a Martin\\28john\\29second\\5cfff//eee\\00"); + + text = "Hi This is a test #çà"; + Assert.assertEquals(EscapeStrategy.DEFAULT.escape(text), "Hi This is a test #\\c3\\a7\\c3\\a0"); + + } +} diff --git a/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/LDAPDnTest.java b/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/LDAPDnTest.java index 566d6c9efb..a668cd7281 100644 --- a/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/LDAPDnTest.java +++ b/federation/ldap/src/test/java/org/keycloak/storage/ldap/idm/model/LDAPDnTest.java @@ -31,9 +31,9 @@ public class LDAPDnTest { dn.addFirst("ou", "People"); Assert.assertEquals("ou=People,dc=keycloak,dc=org", dn.toString()); - dn.addFirst("uid", "Johny,Depp+Pepp"); - Assert.assertEquals("uid=Johny\\,Depp\\+Pepp,ou=People,dc=keycloak,dc=org", dn.toString()); - Assert.assertEquals(LDAPDn.fromString("uid=Johny\\,Depp\\+Pepp,ou=People,dc=keycloak,dc=org"), dn); + dn.addFirst("uid", "Johny,Depp+Pepp\\Foo"); + Assert.assertEquals("uid=Johny\\,Depp\\+Pepp\\\\Foo,ou=People,dc=keycloak,dc=org", dn.toString()); + Assert.assertEquals(LDAPDn.fromString("uid=Johny\\,Depp\\+Pepp\\\\Foo,ou=People,dc=keycloak,dc=org"), dn); Assert.assertEquals("ou=People,dc=keycloak,dc=org", dn.getParentDn()); @@ -44,6 +44,6 @@ public class LDAPDnTest { Assert.assertFalse(dn.isDescendantOf(dn)); Assert.assertEquals("uid", dn.getFirstRdnAttrName()); - Assert.assertEquals("Johny\\,Depp\\+Pepp", dn.getFirstRdnAttrValue()); + Assert.assertEquals("Johny,Depp+Pepp\\Foo", dn.getFirstRdnAttrValue()); } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java index cc29081a6b..29e79c45fd 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperSyncTest.java @@ -171,7 +171,7 @@ public class LDAPGroupMapperSyncTest { Assert.assertEquals("group12 - description", kcGroup12.getFirstAttribute(descriptionAttrName)); // Cleanup - remove recursive mapping in LDAP - LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group12, group1, true); + LDAPUtils.deleteMember(ldapProvider, MembershipType.DN, LDAPConstants.MEMBER, group12, group1); } finally { keycloakRule.stopSession(session, false); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java index 1371d7bba8..022a0760f2 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPGroupMapperTest.java @@ -64,7 +64,19 @@ public class LDAPGroupMapperTest { private static ComponentModel ldapModel = null; private static String descriptionAttrName = null; - private static KeycloakRule keycloakRule = new KeycloakRule(new KeycloakRule.KeycloakSetup() { + + static class GroupTestKeycloakSetup extends KeycloakRule.KeycloakSetup { + + private final LDAPRule ldapRule; + + ComponentModel ldapModel = null; + String descriptionAttrName = null; + + + public GroupTestKeycloakSetup(LDAPRule ldapRule) { + this.ldapRule = ldapRule; + } + @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { @@ -97,12 +109,13 @@ public class LDAPGroupMapperTest { LDAPObject group1 = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group1", descriptionAttrName, "group1 - description"); LDAPObject group11 = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group11"); LDAPObject group12 = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group12", descriptionAttrName, "group12 - description"); + LDAPObject groupSpecialCharacters = LDAPTestUtils.createLDAPGroup(manager.getSession(), appRealm, ldapModel, "group-spec,ia*l_characžter)s", descriptionAttrName, "group-special-characters"); LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, group1, group11, false); LDAPUtils.addMember(ldapFedProvider, MembershipType.DN, LDAPConstants.MEMBER, group1, group12, true); // Sync LDAP groups to Keycloak DB - ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(appRealm,ldapModel, "groupsMapper"); + ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(appRealm, ldapModel, "groupsMapper"); new GroupLDAPStorageMapperFactory().create(session, mapperModel).syncDataFromFederationProviderToKeycloak(appRealm); // Delete all LDAP users @@ -121,8 +134,22 @@ public class LDAPGroupMapperTest { LDAPObject james = LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "jameskeycloak", "James", "Brown", "james@email.org", null, "8910"); LDAPTestUtils.updateLDAPPassword(ldapFedProvider, james, "Password1"); + LDAPObject james2 = LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "jamees,key*cložak)ppp", "James2", "Brown2", "james2@email.org", null, "89102"); + LDAPTestUtils.updateLDAPPassword(ldapFedProvider, james2, "Password1"); + + postSetup(); } - }); + + + void postSetup() { + LDAPGroupMapperTest.ldapModel = this.ldapModel; + LDAPGroupMapperTest.descriptionAttrName = this.descriptionAttrName; + } + + } + + + private static KeycloakRule keycloakRule = new KeycloakRule(new GroupTestKeycloakSetup(ldapRule)); @ClassRule public static TestRule chain = RuleChain diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPSpecialCharsTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPSpecialCharsTest.java new file mode 100644 index 0000000000..e9581b5a2b --- /dev/null +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/storage/ldap/LDAPSpecialCharsTest.java @@ -0,0 +1,205 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.federation.storage.ldap; + +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.FixMethodOrder; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; +import org.junit.runners.MethodSorters; +import org.keycloak.OAuth2Constants; +import org.keycloak.admin.client.Keycloak; +import org.keycloak.component.ComponentModel; +import org.keycloak.models.Constants; +import org.keycloak.models.GroupModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.storage.ldap.mappers.membership.LDAPGroupMapperMode; +import org.keycloak.storage.ldap.mappers.membership.group.GroupMapperConfig; +import org.keycloak.testsuite.OAuthClient; +import org.keycloak.testsuite.pages.AppPage; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.RegisterPage; +import org.keycloak.testsuite.rule.KeycloakRule; +import org.keycloak.testsuite.rule.LDAPRule; +import org.keycloak.testsuite.rule.WebResource; +import org.keycloak.testsuite.rule.WebRule; +import org.openqa.selenium.WebDriver; + +import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.MASTER; +import static org.keycloak.models.AdminRoles.ADMIN; +import static org.keycloak.testsuite.Constants.AUTH_SERVER_ROOT; + +/** + * @author Marek Posolda + */ +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +public class LDAPSpecialCharsTest { + + private static LDAPRule ldapRule = new LDAPRule(); + + static ComponentModel ldapModel = null; + static String descriptionAttrName = null; + + + private static KeycloakRule keycloakRule = new KeycloakRule(new LDAPGroupMapperTest.GroupTestKeycloakSetup(ldapRule) { + + @Override + protected void postSetup() { + LDAPSpecialCharsTest.ldapModel = this.ldapModel; + LDAPSpecialCharsTest.descriptionAttrName = this.descriptionAttrName; + } + + }); + + + @ClassRule + public static TestRule chain = RuleChain + .outerRule(ldapRule) + .around(keycloakRule); + + + protected Keycloak adminClient; + + @Rule + public WebRule webRule = new WebRule(this); + + @WebResource + protected OAuthClient oauth; + + @WebResource + protected WebDriver driver; + + @WebResource + protected AppPage appPage; + + @WebResource + protected RegisterPage registerPage; + + @WebResource + protected LoginPage loginPage; + + + @Before + public void before() { + adminClient = Keycloak.getInstance(AUTH_SERVER_ROOT, MASTER, ADMIN, ADMIN, Constants.ADMIN_CLI_CLIENT_ID); + } + + @After + public void after() { + adminClient.close(); + } + + + @Test + public void test01_userSearch() { + List users = adminClient.realm("test").users().search("j*", 0, 10); + Assert.assertEquals(3, users.size()); + + List usernames = users.stream().map((UserRepresentation user) -> { + + return user.getUsername(); + + }).collect(Collectors.toList()); + Collections.sort(usernames); + + Assert.assertEquals("jamees,key*cložak)ppp", usernames.get(0)); + Assert.assertEquals("jameskeycloak", usernames.get(1)); + Assert.assertEquals("johnkeycloak", usernames.get(2)); + } + + + @Test + public void test02_loginWithSpecialCharacter() { + // Fail login with wildcard + loginPage.open(); + loginPage.login("john*", "Password1"); + Assert.assertEquals("Invalid username or password.", loginPage.getError()); + + // Fail login with wildcard + loginPage.login("j*", "Password1"); + Assert.assertEquals("Invalid username or password.", loginPage.getError()); + + // Success login as username exactly match + loginPage.login("jamees,key*cložak)ppp", "Password1"); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + } + + + @Test + public void test03_specialCharUserJoiningSpecialCharGroup() { + KeycloakSession session = keycloakRule.startSession(); + try { + RealmModel appRealm = session.realms().getRealmByName("test"); + + ComponentModel mapperModel = LDAPTestUtils.getSubcomponentByName(appRealm,ldapModel, "groupsMapper"); + LDAPTestUtils.updateGroupMapperConfigOptions(mapperModel, GroupMapperConfig.MODE, LDAPGroupMapperMode.LDAP_ONLY.toString()); + appRealm.updateComponent(mapperModel); + + UserModel specialUser = session.users().getUserByUsername("jamees,key*cložak)ppp", appRealm); + Assert.assertNotNull(specialUser); + + // 1 - Grant some groups in LDAP + + // This group should already exists as it was imported from LDAP + GroupModel specialGroup = KeycloakModelUtils.findGroupByPath(appRealm, "/group-spec,ia*l_characžter)s"); + Assert.assertNotNull(specialGroup); + + specialUser.joinGroup(specialGroup); + + // 3 - Check that group mappings are in LDAP and hence available through federation + + Set userGroups = specialUser.getGroups(); + Assert.assertEquals(1, userGroups.size()); + Assert.assertTrue(userGroups.contains(specialGroup)); + + // 4 - Check through userProvider + List groupMembers = session.users().getGroupMembers(appRealm, specialGroup, 0, 10); + + Assert.assertEquals(1, groupMembers.size()); + Assert.assertEquals("jamees,key*cložak)ppp", groupMembers.get(0).getUsername()); + + // 4 - Delete some group mappings and check they are deleted + + specialUser.leaveGroup(specialGroup); + + userGroups = specialUser.getGroups(); + Assert.assertEquals(0, userGroups.size()); + + } finally { + keycloakRule.stopSession(session, false); + } + } + +} From 8c99a13387dacfbdf5ad370ad7b54e4f736e2d07 Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 12 Dec 2016 13:08:39 +0100 Subject: [PATCH 2/2] Minor synchronize update --- .../org/keycloak/adapters/rotation/JWKPublicKeyLocator.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/rotation/JWKPublicKeyLocator.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/rotation/JWKPublicKeyLocator.java index b4187e2f0d..5fec6c8f70 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/rotation/JWKPublicKeyLocator.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/rotation/JWKPublicKeyLocator.java @@ -73,8 +73,10 @@ public class JWKPublicKeyLocator implements PublicKeyLocator { @Override public void reset(KeycloakDeployment deployment) { - sendRequest(deployment); - lastRequestTime = Time.currentTime(); + synchronized (this) { + sendRequest(deployment); + lastRequestTime = Time.currentTime(); + } }