Skip to content

Commit 388ed8e

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 b02fd96 commit 388ed8e

2 files changed

Lines changed: 37 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,11 @@
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.LdapQueryBuilder;
46+
import org.springframework.ldap.query.SearchScope;
4347
import org.springframework.ldap.support.LdapUtils;
4448
import org.springframework.security.authentication.AccountExpiredException;
4549
import org.springframework.security.authentication.BadCredentialsException;
@@ -50,7 +54,6 @@
5054
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
5155
import org.springframework.security.core.GrantedAuthority;
5256
import org.springframework.security.core.userdetails.UsernameNotFoundException;
53-
import org.springframework.security.ldap.SpringSecurityLdapTemplate;
5457
import org.springframework.security.ldap.authentication.AbstractLdapAuthenticationProvider;
5558
import org.springframework.security.ldap.userdetails.LdapAuthoritiesPopulator;
5659
import org.springframework.util.Assert;
@@ -96,6 +99,7 @@
9699
* @author Luke Taylor
97100
* @author Rob Winch
98101
* @author Roman Zabaluev
102+
* @author Andrey Litvitski
99103
* @since 3.1
100104
*/
101105
public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLdapAuthenticationProvider {
@@ -299,10 +303,23 @@ private DirContextOperations searchForUser(DirContext context, String username)
299303
searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE);
300304
String bindPrincipal = createBindPrincipal(username);
301305
String searchRoot = (this.rootDn != null) ? this.rootDn : searchRootFromPrincipal(bindPrincipal);
302-
306+
SingleContextSource contextSource = new SingleContextSource(context);
307+
LdapClient ldapClient = LdapClient.builder()
308+
.contextSource(contextSource)
309+
.defaultSearchControls(() -> searchControls)
310+
.ignorePartialResultException(true)
311+
.build();
303312
try {
304-
return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context, searchControls, searchRoot,
305-
this.searchFilter, new Object[] { bindPrincipal, username });
313+
DirContextOperations result = ldapClient.search()
314+
.query(LdapQueryBuilder.query()
315+
.base(searchRoot)
316+
.searchScope(SearchScope.SUBTREE)
317+
.filter(this.searchFilter, bindPrincipal, username))
318+
.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: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
* @author Luke Taylor
6161
* @author Rob Winch
6262
* @author Gengwu Zhao
63+
* @author Andrey Litvitski
6364
*/
6465
public class ActiveDirectoryLdapAuthenticationProviderTests {
6566

@@ -97,7 +98,7 @@ public void customSearchFilterIsUsedForSuccessfulAuthentication() throws Excepti
9798
String customSearchFilter = "(&(objectClass=user)(sAMAccountName={0}))";
9899
DirContextAdapter dca = new DirContextAdapter();
99100
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)))
101+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class)))
101102
.willReturn(new MockNamingEnumeration(sr));
102103
ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider(
103104
"mydomain.eu", "ldap://192.168.1.200/");
@@ -109,34 +110,32 @@ public void customSearchFilterIsUsedForSuccessfulAuthentication() throws Excepti
109110

110111
@Test
111112
public void defaultSearchFilter() throws Exception {
112-
final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
113113
DirContextAdapter dca = new DirContextAdapter();
114114
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)))
115+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class)))
116116
.willReturn(new MockNamingEnumeration(sr));
117117
ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider(
118118
"mydomain.eu", "ldap://192.168.1.200/");
119119
customProvider.contextFactory = createContextFactoryReturning(this.ctx);
120120
Authentication result = customProvider.authenticate(this.joe);
121121
assertThat(result.isAuthenticated()).isTrue();
122-
verify(this.ctx).search(any(Name.class), eq(defaultSearchFilter), any(Object[].class),
123-
any(SearchControls.class));
122+
verify(this.ctx).search(any(Name.class), any(String.class), any(SearchControls.class));
124123
}
125124

126125
// SEC-2897,SEC-2224
127126
@Test
128127
public void bindPrincipalAndUsernameUsed() throws Exception {
129-
final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))";
130-
ArgumentCaptor<Object[]> captor = ArgumentCaptor.forClass(Object[].class);
128+
final String captureValue = "(&(objectClass=user)(userPrincipalName=joe@mydomain.eu))";
129+
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
131130
DirContextAdapter dca = new DirContextAdapter();
132131
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)))
132+
given(this.ctx.search(any(Name.class), captor.capture(), any(SearchControls.class)))
134133
.willReturn(new MockNamingEnumeration(sr));
135134
ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider(
136135
"mydomain.eu", "ldap://192.168.1.200/");
137136
customProvider.contextFactory = createContextFactoryReturning(this.ctx);
138137
Authentication result = customProvider.authenticate(this.joe);
139-
assertThat(captor.getValue()).containsExactly("joe@mydomain.eu", "joe");
138+
assertThat(captor.getValue()).isEqualTo(captureValue);
140139
assertThat(result.isAuthenticated()).isTrue();
141140
}
142141

@@ -156,7 +155,7 @@ public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws
156155
DirContextAdapter dca = new DirContextAdapter();
157156
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
158157
given(this.ctx.search(eq(LdapNameBuilder.newInstance("DC=mydomain,DC=eu").build()), any(String.class),
159-
any(Object[].class), any(SearchControls.class)))
158+
any(SearchControls.class)))
160159
.willReturn(new MockNamingEnumeration(sr));
161160
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
162161
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -165,7 +164,7 @@ public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws
165164

166165
@Test
167166
public void failedUserSearchCausesBadCredentials() throws Exception {
168-
given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class)))
167+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class)))
169168
.willThrow(new NameNotFoundException());
170169
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
171170
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -174,7 +173,7 @@ public void failedUserSearchCausesBadCredentials() throws Exception {
174173
// SEC-2017
175174
@Test
176175
public void noUserSearchCausesUsernameNotFound() throws Exception {
177-
given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class)))
176+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class)))
178177
.willReturn(new MockNamingEnumeration(null));
179178
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
180179
assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -195,8 +194,7 @@ public void duplicateUserSearchCausesError() throws Exception {
195194
SearchResult searchResult = mock(SearchResult.class);
196195
given(searchResult.getObject()).willReturn(new DirContextAdapter("ou=1"), new DirContextAdapter("ou=2"));
197196
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);
197+
given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class))).willReturn(searchResults);
200198
this.provider.contextFactory = createContextFactoryReturning(this.ctx);
201199
assertThatExceptionOfType(IncorrectResultSizeDataAccessException.class)
202200
.isThrownBy(() -> this.provider.authenticate(this.joe));
@@ -353,7 +351,7 @@ private void checkAuthentication(String rootDn, ActiveDirectoryLdapAuthenticatio
353351
SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes());
354352
@SuppressWarnings("deprecation")
355353
Name searchBaseDn = LdapNameBuilder.newInstance(rootDn).build();
356-
given(this.ctx.search(eq(searchBaseDn), any(String.class), any(Object[].class), any(SearchControls.class)))
354+
given(this.ctx.search(eq(searchBaseDn), any(String.class), any(SearchControls.class)))
357355
.willReturn(new MockNamingEnumeration(sr))
358356
.willReturn(new MockNamingEnumeration(sr));
359357
provider.contextFactory = createContextFactoryReturning(this.ctx);

0 commit comments

Comments
 (0)