From c79cb8eff653f9a01de6b13a92c9ea8120bf6ed0 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Tue, 18 May 2021 11:00:15 -0500 Subject: [PATCH] Handle encoded spaces in the root dn Fixes an issue where provider URLs passed to the constructor of the DefaultSpringSecurityContextSource can be URL encoded, resulting in an invalid base dn. Additionally adds support for list constructor to support spaces in base dn. Closes gh-9742 # Conflicts: # ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java # ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java --- ...faultSpringSecurityContextSourceTests.java | 26 +++++- ...terBasedLdapUserSearchWithSpacesTests.java | 89 +++++++++++++++++++ .../resources/test-server-with-spaces.ldif | 14 +++ .../DefaultSpringSecurityContextSource.java | 30 ++++++- 4 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 ldap/src/integration-test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchWithSpacesTests.java create mode 100644 ldap/src/integration-test/resources/test-server-with-spaces.ldif 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 e2da339fcf..f88554e16d 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. @@ -53,8 +53,17 @@ public class DefaultSpringSecurityContextSourceTests { @Test public void supportsSpacesInUrl() { - new DefaultSpringSecurityContextSource( + 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 @@ -105,6 +114,7 @@ public class DefaultSpringSecurityContextSourceTests { 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", @@ -147,6 +157,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(expected = IllegalArgumentException.class) 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..3d2643f4fa --- /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(53390); + return this.container; + } + + @Bean + ContextSource contextSource(ApacheDSContainer ldapContainer) { + return new DefaultSpringSecurityContextSource( + "ldap://127.0.0.1:" + ldapContainer.getPort() + "/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 f9790d606d..7e95ac66b8 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. @@ -15,6 +15,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; @@ -85,7 +89,7 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { } setUrls(urls.toArray(new String[0])); - setBase(this.rootDn); + setBase((this.rootDn != null) ? decodeUrl(this.rootDn) : null); setPooled(true); setAuthenticationStrategy(new SimpleDirContextAuthenticationStrategy() { @Override @@ -150,7 +154,7 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { 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) { @@ -163,7 +167,7 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { if (!trimmedUrl.endsWith("/")) { providerUrl.append("/"); } - providerUrl.append(trimmedBaseDn); + providerUrl.append(encodedBaseDn); providerUrl.append(" "); } @@ -171,4 +175,22 @@ public class DefaultSpringSecurityContextSource extends LdapContextSource { } + 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); + } + } + }