Skip to content

Commit 44836b2

Browse files
therepanictherepanic
committed
Change ActiveDirectoryLdapAuthenticationProvider to use LdapClient
Replaces SpringSecurityLdapTemplate with LdapClient for user search operations. Closes: gh-17291 Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
1 parent 544f635 commit 44836b2

2 files changed

Lines changed: 43 additions & 19 deletions

File tree

‎ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@
3939
import org.springframework.dao.IncorrectResultSizeDataAccessException;
4040
import org.springframework.ldap.CommunicationException;
4141
import org.springframework.ldap.core.DirContextOperations;
42+
import org.springframework.ldap.core.LdapClient;
4243
import org.springframework.ldap.core.support.DefaultDirObjectFactory;
44+
import org.springframework.ldap.core.support.SingleContextSource;
45+
import org.springframework.ldap.query.LdapQuery;
46+
import org.springframework.ldap.query.LdapQueryBuilder;
47+
import org.springframework.ldap.query.SearchScope;
4348
import org.springframework.ldap.support.LdapUtils;
4449
import org.springframework.security.authentication.AccountExpiredException;
4550
import org.springframework.security.authentication.BadCredentialsException;
@@ -50,7 +55,6 @@
5055
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
5156
import org.springframework.security.core.GrantedAuthority;
5257
import org.springframework.security.core.userdetails.UsernameNotFoundException;
53-
import org.springframework.security.ldap.SpringSecurityLdapTemplate;
5458
import org.springframework.security.ldap.authentication.AbstractLdapAuthenticationProvider;
5559
import org.springframework.security.ldap.userdetails.LdapAuthoritiesPopulator;
5660
import org.springframework.util.Assert;
@@ -96,6 +100,7 @@
96100
* @author Luke Taylor
97101
* @author Rob Winch
98102
* @author Roman Zabaluev
103+
* @author Andrey Litvitski
99104
* @since 3.1
100105
*/
101106
public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLdapAuthenticationProvider {
@@ -299,10 +304,22 @@ private DirContextOperations searchForUser(DirContext context, String username)
299304
searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
300305
String bindPrincipal = createBindPrincipal(username);
301306
String searchRoot = (this.rootDn != null) ? this.rootDn : searchRootFromPrincipal(bindPrincipal);
302-
307+
SingleContextSource contextSource = new SingleContextSource(context);
308+
LdapClient ldapClient = LdapClient.builder()
309+
.contextSource(contextSource)
310+
.defaultSearchControls(() -> searchControls)
311+
.ignorePartialResultException(true)
312+
.build();
303313
try {
304-
return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context, searchControls, searchRoot,
305-
this.searchFilter, new Object[] { bindPrincipal, username });
314+
LdapQuery query = LdapQueryBuilder.query()
315+
.base(searchRoot)
316+
.searchScope(SearchScope.SUBTREE)
317+
.filter(this.searchFilter, bindPrincipal, username);
318+
DirContextOperations result = ldapClient.search().query(query).toEntry();
319+
if (result == null) {
320+
throw new IncorrectResultSizeDataAccessException(1, 0);
321+
}
322+
return result;
306323
}
307324
catch (CommunicationException ex) {
308325
throw badLdapConnection(ex);
@@ -316,6 +333,9 @@ private DirContextOperations searchForUser(DirContext context, String username)
316333
UsernameNotFoundException userNameNotFoundException = UsernameNotFoundException.fromUsername(username, ex);
317334
throw badCredentials(userNameNotFoundException);
318335
}
336+
catch (org.springframework.ldap.NamingException ex) {
337+
throw badCredentials(ex);
338+
}
319339
}
320340

321341
private String searchRootFromPrincipal(String bindPrincipal) {

‎ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.security.ldap.authentication.ad;
1818

19+
import java.text.MessageFormat;
1920
import java.util.Collections;
2021
import java.util.Hashtable;
2122

@@ -60,6 +61,7 @@
6061
* @author Luke Taylor
6162
* @author Rob Winch
6263
* @author Gengwu Zhao
64+
* @author Andrey Litvitski
6365
*/
6466
public class ActiveDirectoryLdapAuthenticationProviderTests {
6567

@@ -95,9 +97,11 @@ public void successfulAuthenticationProducesExpectedAuthorities() throws Excepti
9597
@Test
9698
public void customSearchFilterIsUsedForSuccessfulAuthentication() throws Exception {
9799
String customSearchFilter = "(&(objectClass=user)(sAMAccountName={0}))";
100+
String domain = "mydomain.eu";
101+
String encoded = MessageFormat.format(customSearchFilter, this.joe.getPrincipal() + "@" + domain);
98102
DirContextAdapter dca = new DirContextAdapter();
99103
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
100-
given(this.ctx.search(any(Name.class), eq(customSearchFilter), any(Object[].class), any(SearchControls.class)))
104+
given(this.ctx.search(any(Name.class), eq(encoded), any(SearchControls.class)))
101105
.willReturn(new MockNamingEnumeration(sr));
102106
ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider(
103107
"mydomain.eu", "ldap://192.168.1.200/");
@@ -109,34 +113,35 @@ public void customSearchFilterIsUsedForSuccessfulAuthentication() throws Excepti
109113

110114
@Test
111115
public void defaultSearchFilter() throws Exception {
112-
final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
116+
String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
117+
String domain = "mydomain.eu";
118+
String encoded = MessageFormat.format(defaultSearchFilter, this.joe.getPrincipal() + "@" + domain);
113119
DirContextAdapter dca = new DirContextAdapter();
114120
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
115-
given(this.ctx.search(any(Name.class), eq(defaultSearchFilter), any(Object[].class), any(SearchControls.class)))
121+
given(this.ctx.search(any(Name.class), eq(encoded), any(SearchControls.class)))
116122
.willReturn(new MockNamingEnumeration(sr));
117123
ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider(
118124
"mydomain.eu", "ldap://192.168.1.200/");
119125
customProvider.contextFactory = createContextFactoryReturning(this.ctx);
120126
Authentication result = customProvider.authenticate(this.joe);
121127
assertThat(result.isAuthenticated()).isTrue();
122-
verify(this.ctx).search(any(Name.class), eq(defaultSearchFilter), any(Object[].class),
123-
any(SearchControls.class));
128+
verify(this.ctx).search(any(Name.class), any(String.class), any(SearchControls.class));
124129
}
125130

126131
// SEC-2897,SEC-2224
127132
@Test
128133
public void bindPrincipalAndUsernameUsed() throws Exception {
129-
final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
130-
ArgumentCaptor<Object[]> captor = ArgumentCaptor.forClass(Object[].class);
134+
final String captureValue = "(&(objectClass=user)(userPrincipalName=joe@mydomain.eu))";
135+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
131136
DirContextAdapter dca = new DirContextAdapter();
132137
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
133-
given(this.ctx.search(any(Name.class), eq(defaultSearchFilter), captor.capture(), any(SearchControls.class)))
138+
given(this.ctx.search(any(Name.class), captor.capture(), any(SearchControls.class)))
134139
.willReturn(new MockNamingEnumeration(sr));
135140
ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider(
136141
"mydomain.eu", "ldap://192.168.1.200/");
137142
customProvider.contextFactory = createContextFactoryReturning(this.ctx);
138143
Authentication result = customProvider.authenticate(this.joe);
139-
assertThat(captor.getValue()).containsExactly("joe@mydomain.eu", "joe");
144+
assertThat(captor.getValue()).isEqualTo(captureValue);
140145
assertThat(result.isAuthenticated()).isTrue();
141146
}
142147

@@ -156,7 +161,7 @@ public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws
156161
DirContextAdapter dca = new DirContextAdapter();
157162
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
158163
given(this.ctx.search(eq(LdapNameBuilder.newInstance("DC=mydomain,DC=eu").build()), any(String.class),
159-
any(Object[].class), any(SearchControls.class)))
164+
any(SearchControls.class)))
160165
.willReturn(new MockNamingEnumeration(sr));
161166
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
162167
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -165,7 +170,7 @@ public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws
165170

166171
@Test
167172
public void failedUserSearchCausesBadCredentials() throws Exception {
168-
given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class)))
173+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class)))
169174
.willThrow(new NameNotFoundException());
170175
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
171176
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -174,7 +179,7 @@ public void failedUserSearchCausesBadCredentials() throws Exception {
174179
// SEC-2017
175180
@Test
176181
public void noUserSearchCausesUsernameNotFound() throws Exception {
177-
given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class)))
182+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class)))
178183
.willReturn(new MockNamingEnumeration(null));
179184
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
180185
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -195,8 +200,7 @@ public void duplicateUserSearchCausesError() throws Exception {
195200
SearchResult searchResult = mock(SearchResult.class);
196201
given(searchResult.getObject()).willReturn(new DirContextAdapter("ou=1"), new DirContextAdapter("ou=2"));
197202
given(searchResults.next()).willReturn(searchResult);
198-
given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class)))
199-
.willReturn(searchResults);
203+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class))).willReturn(searchResults);
200204
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
201205
assertThatExceptionOfType(IncorrectResultSizeDataAccessException.class)
202206
.isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -353,7 +357,7 @@ private void checkAuthentication(String rootDn, ActiveDirectoryLdapAuthenticatio
353357
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
354358
@SuppressWarnings("deprecation")
355359
Name searchBaseDn = LdapNameBuilder.newInstance(rootDn).build();
356-
given(this.ctx.search(eq(searchBaseDn), any(String.class), any(Object[].class), any(SearchControls.class)))
360+
given(this.ctx.search(eq(searchBaseDn), any(String.class), any(SearchControls.class)))
357361
.willReturn(new MockNamingEnumeration(sr))
358362
.willReturn(new MockNamingEnumeration(sr));
359363
provider.contextFactory = createContextFactoryReturning(this.ctx);

0 commit comments

Comments
 (0)