Repository: james-project Updated Branches: refs/heads/master de561e452 -> 7189bd054
JAMES-2243 Encode special characters in LDAP search filter Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/7189bd05 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/7189bd05 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/7189bd05 Branch: refs/heads/master Commit: 7189bd054dcd9afb200c3d05c16380307d349341 Parents: de561e4 Author: Thibaut SAUTEREAU <[email protected]> Authored: Wed Nov 29 17:17:36 2017 +0700 Committer: Antoine Duprat <[email protected]> Committed: Thu Nov 30 17:27:33 2017 +0100 ---------------------------------------------------------------------- pom.xml | 5 ++ .../ldap/ReadOnlyUsersLDAPRepositoryTest.java | 8 +++ server/data/data-ldap/pom.xml | 4 ++ .../user/ldap/ReadOnlyUsersLDAPRepository.java | 54 ++++++++++---------- 4 files changed, 43 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/7189bd05/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 899bfb4..608b4be 100644 --- a/pom.xml +++ b/pom.xml @@ -1816,6 +1816,11 @@ <version>${derby.version}</version> </dependency> <dependency> + <groupId>org.apache.directory.api</groupId> + <artifactId>api-ldap-model</artifactId> + <version>1.0.0</version> + </dependency> + <dependency> <groupId>org.apache.felix</groupId> <artifactId>org.apache.felix.framework</artifactId> <version>${felix.version}</version> http://git-wip-us.apache.org/repos/asf/james-project/blob/7189bd05/server/data/data-ldap-integration-testing/src/test/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepositoryTest.java ---------------------------------------------------------------------- diff --git a/server/data/data-ldap-integration-testing/src/test/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepositoryTest.java b/server/data/data-ldap-integration-testing/src/test/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepositoryTest.java index 619f2af..bd8ed52 100644 --- a/server/data/data-ldap-integration-testing/src/test/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepositoryTest.java +++ b/server/data/data-ldap-integration-testing/src/test/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepositoryTest.java @@ -177,4 +177,12 @@ public class ReadOnlyUsersLDAPRepositoryTest { startUsersRepository(ldapRepositoryConfigurationWithVirtualHosting()); assertThat(ldapRepository.contains(ldapRepository.getUser(new MailAddress(JAMES_USER_MAIL)))).isTrue(); } + + @Test + public void specialCharacterInUserInputShouldBeSanitized() throws Exception { + String patternMatchingMultipleUsers = "j*"; + + startUsersRepository(ldapRepositoryConfigurationWithVirtualHosting()); + assertThat(ldapRepository.test(patternMatchingMultipleUsers, PASSWORD)).isFalse(); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/7189bd05/server/data/data-ldap/pom.xml ---------------------------------------------------------------------- diff --git a/server/data/data-ldap/pom.xml b/server/data/data-ldap/pom.xml index 0a144df..cc3c26c 100644 --- a/server/data/data-ldap/pom.xml +++ b/server/data/data-ldap/pom.xml @@ -59,6 +59,10 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.apache.directory.api</groupId> + <artifactId>api-ldap-model</artifactId> + </dependency> + <dependency> <groupId>org.apache.geronimo.specs</groupId> <artifactId>geronimo-annotation_1.1_spec</artifactId> </dependency> http://git-wip-us.apache.org/repos/asf/james-project/blob/7189bd05/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java ---------------------------------------------------------------------- diff --git a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java index 27092ac..5b3328e 100644 --- a/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java +++ b/server/data/data-ldap/src/main/java/org/apache/james/user/ldap/ReadOnlyUsersLDAPRepository.java @@ -50,6 +50,7 @@ import org.apache.james.util.retry.naming.ldap.RetryingLdapContext; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.HierarchicalConfiguration; import org.apache.commons.lang.StringUtils; +import org.apache.directory.api.ldap.model.filter.FilterEncoder; import org.apache.james.core.MailAddress; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -562,7 +563,6 @@ public class ReadOnlyUsersLDAPRepository implements UsersRepository, Configurabl return results; } - /** * For a given name, this method makes ldap search in userBase with filter {@link #userIdAttribute}=name and objectClass={@link #userObjectClass} * and builds {@link User} based on search result. @@ -576,36 +576,34 @@ public class ReadOnlyUsersLDAPRepository implements UsersRepository, Configurabl * Propagated by the underlying LDAP communication layer. */ private ReadOnlyLDAPUser searchAndBuildUser(String name) throws NamingException { - SearchControls sc = new SearchControls(); - sc.setSearchScope(SearchControls.SUBTREE_SCOPE); - sc.setReturningAttributes(new String[] { userIdAttribute }); - sc.setCountLimit(1); - - StringBuilder builderFilter = new StringBuilder("(&("); - builderFilter.append(userIdAttribute).append("=").append(name).append(")") - .append("(objectClass=").append(userObjectClass).append(")"); - - if(StringUtils.isNotEmpty(filter)){ - builderFilter.append(filter).append(")"); - } - else{ - builderFilter.append(")"); - } - - NamingEnumeration<SearchResult> sr = ldapContext.search(userBase, builderFilter.toString(), - sc); - - if (!sr.hasMore()) - return null; + SearchControls sc = new SearchControls(); + sc.setSearchScope(SearchControls.SUBTREE_SCOPE); + sc.setReturningAttributes(new String[] { userIdAttribute }); + sc.setCountLimit(1); + + String filterTemplate = "(&({0}={1})(objectClass={2})" + + StringUtils.defaultString(filter, "") + + ")"; - SearchResult r = sr.next(); - Attribute userName = r.getAttributes().get(userIdAttribute); + String sanitizedFilter = FilterEncoder.format( + filterTemplate, + userIdAttribute, + name, + userObjectClass); - if (!restriction.isActivated() - || userInGroupsMembershipList(r.getNameInNamespace(), restriction.getGroupMembershipLists(ldapContext))) - return new ReadOnlyLDAPUser(userName.get().toString(), r.getNameInNamespace(), ldapContext); + NamingEnumeration<SearchResult> sr = ldapContext.search(userBase, sanitizedFilter, sc); - return null; + if (!sr.hasMore()) + return null; + + SearchResult r = sr.next(); + Attribute userName = r.getAttributes().get(userIdAttribute); + + if (!restriction.isActivated() + || userInGroupsMembershipList(r.getNameInNamespace(), restriction.getGroupMembershipLists(ldapContext))) + return new ReadOnlyLDAPUser(userName.get().toString(), r.getNameInNamespace(), ldapContext); + + return null; } /** --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
