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

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<String> 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<String> serverUrls = new ArrayList<>();

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();
+		}
+
+	}
+
+}

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

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);
+		}
+	}
+
 }