Skip to content

Commit 4f31e42

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 # Conflicts: # ldap/src/integration-test/java/org/springframework/security/ldap/DefaultSpringSecurityContextSourceTests.java # ldap/src/main/java/org/springframework/security/ldap/DefaultSpringSecurityContextSource.java
1 parent 2227232 commit 4f31e42

File tree

4 files changed

+153
-6
lines changed

4 files changed

+153
-6
lines changed

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

Lines changed: 24 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.
@@ -53,8 +53,17 @@ public void instantiationSucceedsWithExpectedProperties() {
5353

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

6069
@Test
@@ -105,6 +114,7 @@ public void serverUrlWithSpacesIsSupported() {
105114
DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(
106115
this.contextSource.getUrls()[0]
107116
+ "ou=space%20cadets,dc=springframework,dc=org");
117+
assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("ou=space cadets,dc=springframework,dc=org");
108118
contextSource.afterPropertiesSet();
109119
contextSource.getContext(
110120
"uid=space cadet,ou=space cadets,dc=springframework,dc=org",
@@ -147,6 +157,18 @@ public void instantiationSuceedsWithEmtpyBaseDn() {
147157
assertThat(ctxSrc.isPooled()).isTrue();
148158
}
149159

160+
// gh-9742
161+
@Test
162+
public void constructorWhenServerListWithSpacesInBaseDnThenSuccess() {
163+
List<String> serverUrls = new ArrayList<>();
164+
serverUrls.add("ldap://ad1.example.org:789");
165+
serverUrls.add("ldap://ad2.example.org:389");
166+
serverUrls.add("ldaps://ad3.example.org:636");
167+
DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(serverUrls,
168+
"dc=spring framework,dc=org");
169+
assertThat(contextSource.getBaseLdapPathAsString()).isEqualTo("dc=spring framework,dc=org");
170+
}
171+
150172
@Test(expected = IllegalArgumentException.class)
151173
public void instantiationFailsWithIncorrectServerUrl() {
152174
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(53390);
73+
return this.container;
74+
}
75+
76+
@Bean
77+
ContextSource contextSource(ApacheDSContainer ldapContainer) {
78+
return new DefaultSpringSecurityContextSource(
79+
"ldap://127.0.0.1:" + ldapContainer.getPort() + "/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.
@@ -15,6 +15,10 @@
1515
*/
1616
package org.springframework.security.ldap;
1717

18+
import java.io.UnsupportedEncodingException;
19+
import java.net.URLDecoder;
20+
import java.net.URLEncoder;
21+
import java.nio.charset.StandardCharsets;
1822
import java.util.ArrayList;
1923
import java.util.Hashtable;
2024
import java.util.List;
@@ -85,7 +89,7 @@ else if (!this.rootDn.equals(urlRootDn)) {
8589
}
8690

8791
setUrls(urls.toArray(new String[0]));
88-
setBase(this.rootDn);
92+
setBase((this.rootDn != null) ? decodeUrl(this.rootDn) : null);
8993
setPooled(true);
9094
setAuthenticationStrategy(new SimpleDirContextAuthenticationStrategy() {
9195
@Override
@@ -150,7 +154,7 @@ private static String buildProviderUrl(List<String> urls, String baseDn) {
150154
Assert.notNull(baseDn, "The Base DN for the LDAP server must not be null.");
151155
Assert.notEmpty(urls, "At least one LDAP server URL must be provided.");
152156

153-
String trimmedBaseDn = baseDn.trim();
157+
String encodedBaseDn = encodeUrl(baseDn.trim());
154158
StringBuilder providerUrl = new StringBuilder();
155159

156160
for (String serverUrl : urls) {
@@ -163,12 +167,30 @@ private static String buildProviderUrl(List<String> urls, String baseDn) {
163167
if (!trimmedUrl.endsWith("/")) {
164168
providerUrl.append("/");
165169
}
166-
providerUrl.append(trimmedBaseDn);
170+
providerUrl.append(encodedBaseDn);
167171
providerUrl.append(" ");
168172
}
169173

170174
return providerUrl.toString();
171175

172176
}
173177

178+
private static String encodeUrl(String url) {
179+
try {
180+
return URLEncoder.encode(url, StandardCharsets.UTF_8.toString());
181+
}
182+
catch (UnsupportedEncodingException ex) {
183+
throw new IllegalStateException(ex);
184+
}
185+
}
186+
187+
private String decodeUrl(String url) {
188+
try {
189+
return URLDecoder.decode(url, StandardCharsets.UTF_8.toString());
190+
}
191+
catch (UnsupportedEncodingException ex) {
192+
throw new IllegalStateException(ex);
193+
}
194+
}
195+
174196
}

0 commit comments

Comments
 (0)