Skip to content

Commit ab59adb

Browse files
author
Steve Riesenberg
committed
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
1 parent de4b3a4 commit ab59adb

File tree

4 files changed

+154
-6
lines changed

4 files changed

+154
-6
lines changed

‎ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -54,7 +54,17 @@ public void instantiationSucceedsWithExpectedProperties() {
5454

5555
@Test
5656
public void supportsSpacesInUrl() {
57-
new DefaultSpringSecurityContextSource("ldap://myhost:10389/dc=spring%20framework,dc=org");
57+
DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(
58+
"ldap://myhost:10389/dc=spring%20framework,dc=org");
59+
assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org");
60+
}
61+
62+
// gh-9742
63+
@Test
64+
public void constructorWhenUrlEncodedSpacesWithPlusCharacterThenBaseDnIsProperlyDecoded() {
65+
DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(
66+
"ldap://blah:123/dc=spring+framework,dc=org ldap://blah:456/dc=spring+framework,dc=org");
67+
assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org");
5868
}
5969

6070
@Test
@@ -94,6 +104,7 @@ public void cantBindWithWrongPasswordImmediatelyAfterSuccessfulBind() throws Exc
94104
public void serverUrlWithSpacesIsSupported() {
95105
DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(
96106
this.contextSource.getUrls()[0] + "ou=space%20cadets,dc=springframework,dc=org");
107+
assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("ou=space cadets,dc=springframework,dc=org");
97108
contextSource.afterPropertiesSet();
98109
contextSource.getContext("uid=space cadet,ou=space cadets,dc=springframework,dc=org", "spacecadetspassword");
99110
}
@@ -135,6 +146,18 @@ public void instantiationSuceedsWithEmtpyBaseDn() {
135146
assertThat(ctxSrc.isPooled()).isTrue();
136147
}
137148

149+
// gh-9742
150+
@Test
151+
public void constructorWhenServerListWithSpacesInBaseDnThenSuccess() {
152+
List<String> serverUrls = new ArrayList<>();
153+
serverUrls.add("ldap://ad1.example.org:789");
154+
serverUrls.add("ldap://ad2.example.org:389");
155+
serverUrls.add("ldaps://ad3.example.org:636");
156+
DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(serverUrls,
157+
"dc=spring framework,dc=org");
158+
assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org");
159+
}
160+
138161
@Test
139162
public void instantiationFailsWithIncorrectServerUrl() {
140163
List<String> serverUrls = new ArrayList<>();

‎ldap/src/integration-test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchWithSpacesTests.java

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright 2002-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.ldap.search;
18+
19+
import javax.naming.ldap.LdapName;
20+
21+
import org.junit.Test;
22+
import org.junit.runner.RunWith;
23+
24+
import org.springframework.beans.factory.DisposableBean;
25+
import org.springframework.beans.factory.annotation.Autowired;
26+
import org.springframework.context.annotation.Bean;
27+
import org.springframework.context.annotation.Configuration;
28+
import org.springframework.ldap.core.ContextSource;
29+
import org.springframework.ldap.core.DirContextOperations;
30+
import org.springframework.security.ldap.DefaultSpringSecurityContextSource;
31+
import org.springframework.security.ldap.server.ApacheDSContainer;
32+
import org.springframework.test.context.ContextConfiguration;
33+
import org.springframework.test.context.junit4.SpringRunner;
34+
35+
import static org.assertj.core.api.Assertions.assertThat;
36+
37+
/**
38+
* Additional tests for {@link FilterBasedLdapUserSearch} with spaces in the base dn.
39+
*
40+
* @author Steve Riesenberg
41+
*/
42+
@RunWith(SpringRunner.class)
43+
@ContextConfiguration(classes = FilterBasedLdapUserSearchWithSpacesTests.ApacheDsContainerWithSpacesConfig.class)
44+
public class FilterBasedLdapUserSearchWithSpacesTests {
45+
46+
@Autowired
47+
private DefaultSpringSecurityContextSource contextSource;
48+
49+
// gh-9742
50+
@Test
51+
public void searchForUserWhenSpacesInBaseDnThenSuccess() throws Exception {
52+
FilterBasedLdapUserSearch locator = new FilterBasedLdapUserSearch("ou=space cadets", "(uid={0})",
53+
this.contextSource);
54+
locator.setSearchSubtree(false);
55+
locator.setSearchTimeLimit(0);
56+
locator.setDerefLinkFlag(false);
57+
58+
DirContextOperations bob = locator.searchForUser("space cadet");
59+
assertThat(bob.getStringAttribute("uid")).isEqualTo("space cadet");
60+
assertThat(bob.getDn()).isEqualTo(new LdapName("uid=space cadet,ou=space cadets"));
61+
}
62+
63+
@Configuration
64+
static class ApacheDsContainerWithSpacesConfig implements DisposableBean {
65+
66+
private ApacheDSContainer container;
67+
68+
@Bean
69+
ApacheDSContainer ldapContainer() throws Exception {
70+
this.container = new ApacheDSContainer("dc=spring framework,dc=org",
71+
"classpath:test-server-with-spaces.ldif");
72+
this.container.setPort(0);
73+
return this.container;
74+
}
75+
76+
@Bean
77+
ContextSource contextSource(ApacheDSContainer ldapContainer) {
78+
return new DefaultSpringSecurityContextSource(
79+
"ldap://127.0.0.1:" + ldapContainer.getLocalPort() + "/dc=spring%20framework,dc=org");
80+
}
81+
82+
@Override
83+
public void destroy() {
84+
this.container.stop();
85+
}
86+
87+
}
88+
89+
}

‎ldap/src/integration-test/resources/test-server-with-spaces.ldif

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
dn: ou=space cadets,dc=spring framework,dc=org
2+
objectclass: top
3+
objectclass: organizationalUnit
4+
ou: space cadets
5+
6+
dn: uid=space cadet,ou=space cadets,dc=spring framework,dc=org
7+
objectclass: top
8+
objectclass: person
9+
objectclass: organizationalPerson
10+
objectclass: inetOrgPerson
11+
cn: Space Cadet
12+
sn: Cadet
13+
uid: space cadet
14+
userPassword: spacecadetspassword

‎ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,10 @@
1616

1717
package org.springframework.security.ldap;
1818

19+
import java.io.UnsupportedEncodingException;
20+
import java.net.URLDecoder;
21+
import java.net.URLEncoder;
22+
import java.nio.charset.StandardCharsets;
1923
import java.util.ArrayList;
2024
import java.util.Hashtable;
2125
import java.util.List;
@@ -74,7 +78,7 @@ public DefaultSpringSecurityContextSource(String providerUrl) {
7478
rootDn = (rootDn != null) ? rootDn : urlRootDn;
7579
}
7680
setUrls(urls.toArray(new String[0]));
77-
setBase(rootDn);
81+
setBase((rootDn != null) ? decodeUrl(rootDn) : null);
7882
setPooled(true);
7983
setAuthenticationStrategy(new SimpleDirContextAuthenticationStrategy() {
8084

@@ -136,7 +140,7 @@ public DefaultSpringSecurityContextSource(List<String> urls, String baseDn) {
136140
private static String buildProviderUrl(List<String> urls, String baseDn) {
137141
Assert.notNull(baseDn, "The Base DN for the LDAP server must not be null.");
138142
Assert.notEmpty(urls, "At least one LDAP server URL must be provided.");
139-
String trimmedBaseDn = baseDn.trim();
143+
String encodedBaseDn = encodeUrl(baseDn.trim());
140144
StringBuilder providerUrl = new StringBuilder();
141145
for (String serverUrl : urls) {
142146
String trimmedUrl = serverUrl.trim();
@@ -147,11 +151,29 @@ private static String buildProviderUrl(List<String> urls, String baseDn) {
147151
if (!trimmedUrl.endsWith("/")) {
148152
providerUrl.append("/");
149153
}
150-
providerUrl.append(trimmedBaseDn);
154+
providerUrl.append(encodedBaseDn);
151155
providerUrl.append(" ");
152156
}
153157
return providerUrl.toString();
154158

155159
}
156160

161+
private static String encodeUrl(String url) {
162+
try {
163+
return URLEncoder.encode(url, StandardCharsets.UTF_8.toString());
164+
}
165+
catch (UnsupportedEncodingException ex) {
166+
throw new IllegalStateException(ex);
167+
}
168+
}
169+
170+
private String decodeUrl(String url) {
171+
try {
172+
return URLDecoder.decode(url, StandardCharsets.UTF_8.toString());
173+
}
174+
catch (UnsupportedEncodingException ex) {
175+
throw new IllegalStateException(ex);
176+
}
177+
}
178+
157179
}

0 commit comments

Comments
 (0)