diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java index 2158d7636a..265dde632e 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -54,7 +54,17 @@ public class DefaultSpringSecurityContextSourceTests { @Test public void supportsSpacesInUrl() { - new DefaultSpringSecurityContextSource("ldap://myhost:10389/dc=spring%20framework,dc=org"); + DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource( + "ldap://myhost:10389/dc=spring%20framework,dc=org"); + assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org"); + } + + // gh-9742 + @Test + public void constructorWhenUrlEncodedSpacesWithPlusCharacterThenBaseDnIsProperlyDecoded() { + DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource( + "ldap://blah:123/dc=spring+framework,dc=org ldap://blah:456/dc=spring+framework,dc=org"); + assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org"); } @Test @@ -94,6 +104,7 @@ public class DefaultSpringSecurityContextSourceTests { public void serverUrlWithSpacesIsSupported() { DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource( this.contextSource.getUrls()[0] + "ou=space%20cadets,dc=springframework,dc=org"); + assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("ou=space cadets,dc=springframework,dc=org"); contextSource.afterPropertiesSet(); contextSource.getContext("uid=space cadet,ou=space cadets,dc=springframework,dc=org", "spacecadetspassword"); } @@ -135,6 +146,18 @@ public class DefaultSpringSecurityContextSourceTests { assertThat(ctxSrc.isPooled()).isTrue(); } + // gh-9742 + @Test + public void constructorWhenServerListWithSpacesInBaseDnThenSuccess() { + List serverUrls = new ArrayList<>(); + serverUrls.add("ldap://ad1.example.org:789"); + serverUrls.add("ldap://ad2.example.org:389"); + serverUrls.add("ldaps://ad3.example.org:636"); + DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(serverUrls, + "dc=spring framework,dc=org"); + assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org"); + } + @Test public void instantiationFailsWithIncorrectServerUrl() { List serverUrls = new ArrayList<>(); diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchWithSpacesTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchWithSpacesTests.java new file mode 100644 index 0000000000..7a0589f8e7 --- /dev/null +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchWithSpacesTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2002-2021 the original author or authors. + * + * 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 + * + * https://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.springframework.security.ldap.search; + +import javax.naming.ldap.LdapName; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.ldap.core.ContextSource; +import org.springframework.ldap.core.DirContextOperations; +import org.springframework.security.ldap.DefaultSpringSecurityContextSource; +import org.springframework.security.ldap.server.ApacheDSContainer; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Additional tests for {@link FilterBasedLdapUserSearch} with spaces in the base dn. + * + * @author Steve Riesenberg + */ +@RunWith(SpringRunner.class) +@ContextConfiguration(classes = FilterBasedLdapUserSearchWithSpacesTests.ApacheDsContainerWithSpacesConfig.class) +public class FilterBasedLdapUserSearchWithSpacesTests { + + @Autowired + private DefaultSpringSecurityContextSource contextSource; + + // gh-9742 + @Test + public void searchForUserWhenSpacesInBaseDnThenSuccess() throws Exception { + FilterBasedLdapUserSearch locator = new FilterBasedLdapUserSearch("ou=space cadets", "(uid={0})", + this.contextSource); + locator.setSearchSubtree(false); + locator.setSearchTimeLimit(0); + locator.setDerefLinkFlag(false); + + DirContextOperations bob = locator.searchForUser("space cadet"); + assertThat(bob.getStringAttribute("uid")).isEqualTo("space cadet"); + assertThat(bob.getDn()).isEqualTo(new LdapName("uid=space cadet,ou=space cadets")); + } + + @Configuration + static class ApacheDsContainerWithSpacesConfig implements DisposableBean { + + private ApacheDSContainer container; + + @Bean + ApacheDSContainer ldapContainer() throws Exception { + this.container = new ApacheDSContainer("dc=spring framework,dc=org", + "classpath:test-server-with-spaces.ldif"); + this.container.setPort(0); + return this.container; + } + + @Bean + ContextSource contextSource(ApacheDSContainer ldapContainer) { + return new DefaultSpringSecurityContextSource( + "ldap://127.0.0.1:" + ldapContainer.getLocalPort() + "/dc=spring%20framework,dc=org"); + } + + @Override + public void destroy() { + this.container.stop(); + } + + } + +} diff --git a/ldap/src/integration-test/resources/test-server-with-spaces.ldif b/ldap/src/integration-test/resources/test-server-with-spaces.ldif new file mode 100644 index 0000000000..495ee3f4c0 --- /dev/null +++ b/ldap/src/integration-test/resources/test-server-with-spaces.ldif @@ -0,0 +1,14 @@ +dn: ou=space cadets,dc=spring framework,dc=org +objectclass: top +objectclass: organizationalUnit +ou: space cadets + +dn: uid=space cadet,ou=space cadets,dc=spring framework,dc=org +objectclass: top +objectclass: person +objectclass: organizationalPerson +objectclass: inetOrgPerson +cn: Space Cadet +sn: Cadet +uid: space cadet +userPassword: spacecadetspassword diff --git a/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java b/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java index f9c6293712..0d8e8403b7 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java +++ b/ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,10 @@ package org.springframework.security.ldap; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Hashtable; import java.util.List; @@ -74,7 +78,7 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { rootDn = (rootDn != null) ? rootDn : urlRootDn; } setUrls(urls.toArray(new String[0])); - setBase(rootDn); + setBase((rootDn != null) ? decodeUrl(rootDn) : null); setPooled(true); setAuthenticationStrategy(new SimpleDirContextAuthenticationStrategy() { @@ -136,7 +140,7 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { private static String buildProviderUrl(List urls, String baseDn) { Assert.notNull(baseDn, "The Base DN for the LDAP server must not be null."); Assert.notEmpty(urls, "At least one LDAP server URL must be provided."); - String trimmedBaseDn = baseDn.trim(); + String encodedBaseDn = encodeUrl(baseDn.trim()); StringBuilder providerUrl = new StringBuilder(); for (String serverUrl : urls) { String trimmedUrl = serverUrl.trim(); @@ -147,11 +151,29 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { if (!trimmedUrl.endsWith("/")) { providerUrl.append("/"); } - providerUrl.append(trimmedBaseDn); + providerUrl.append(encodedBaseDn); providerUrl.append(" "); } return providerUrl.toString(); } + private static String encodeUrl(String url) { + try { + return URLEncoder.encode(url, StandardCharsets.UTF_8.toString()); + } + catch (UnsupportedEncodingException ex) { + throw new IllegalStateException(ex); + } + } + + private String decodeUrl(String url) { + try { + return URLDecoder.decode(url, StandardCharsets.UTF_8.toString()); + } + catch (UnsupportedEncodingException ex) { + throw new IllegalStateException(ex); + } + } + }