diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java index 4212ed7635..3054c62418 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProvider.java @@ -39,7 +39,12 @@ import org.springframework.core.log.LogMessage; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.ldap.CommunicationException; import org.springframework.ldap.core.DirContextOperations; +import org.springframework.ldap.core.LdapClient; import org.springframework.ldap.core.support.DefaultDirObjectFactory; +import org.springframework.ldap.core.support.SingleContextSource; +import org.springframework.ldap.query.LdapQuery; +import org.springframework.ldap.query.LdapQueryBuilder; +import org.springframework.ldap.query.SearchScope; import org.springframework.ldap.support.LdapUtils; import org.springframework.security.authentication.AccountExpiredException; import org.springframework.security.authentication.BadCredentialsException; @@ -50,7 +55,6 @@ import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.userdetails.UsernameNotFoundException; -import org.springframework.security.ldap.SpringSecurityLdapTemplate; import org.springframework.security.ldap.authentication.AbstractLdapAuthenticationProvider; import org.springframework.security.ldap.userdetails.LdapAuthoritiesPopulator; import org.springframework.util.Assert; @@ -96,6 +100,7 @@ import org.springframework.util.StringUtils; * @author Luke Taylor * @author Rob Winch * @author Roman Zabaluev + * @author Andrey Litvitski * @since 3.1 */ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLdapAuthenticationProvider { @@ -299,10 +304,22 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE); String bindPrincipal = createBindPrincipal(username); String searchRoot = (this.rootDn != null) ? this.rootDn : searchRootFromPrincipal(bindPrincipal); - + SingleContextSource contextSource = new SingleContextSource(context); + LdapClient ldapClient = LdapClient.builder() + .contextSource(contextSource) + .defaultSearchControls(() -> searchControls) + .ignorePartialResultException(true) + .build(); try { - return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context, searchControls, searchRoot, - this.searchFilter, new Object[] { bindPrincipal, username }); + LdapQuery query = LdapQueryBuilder.query() + .base(searchRoot) + .searchScope(SearchScope.SUBTREE) + .filter(this.searchFilter, bindPrincipal, username); + DirContextOperations result = ldapClient.search().query(query).toEntry(); + if (result == null) { + throw new IncorrectResultSizeDataAccessException(1, 0); + } + return result; } catch (CommunicationException ex) { throw badLdapConnection(ex); @@ -316,6 +333,12 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda UsernameNotFoundException userNameNotFoundException = UsernameNotFoundException.fromUsername(username, ex); throw badCredentials(userNameNotFoundException); } + catch (org.springframework.ldap.NamingException ex) { + if (ex.getCause() instanceof NamingException original) { + throw original; + } + throw badCredentials(ex); + } } private String searchRootFromPrincipal(String bindPrincipal) { diff --git a/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java b/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java index 45f76f2aea..514cf23759 100644 --- a/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java +++ b/ldap/src/test/java/org/springframework/security/ldap/authentication/ad/ActiveDirectoryLdapAuthenticationProviderTests.java @@ -16,6 +16,7 @@ package org.springframework.security.ldap.authentication.ad; +import java.text.MessageFormat; import java.util.Collections; import java.util.Hashtable; @@ -60,6 +61,7 @@ import static org.mockito.Mockito.verify; * @author Luke Taylor * @author Rob Winch * @author Gengwu Zhao + * @author Andrey Litvitski */ public class ActiveDirectoryLdapAuthenticationProviderTests { @@ -95,9 +97,11 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { @Test public void customSearchFilterIsUsedForSuccessfulAuthentication() throws Exception { String customSearchFilter = "(&(objectClass=user)(sAMAccountName={0}))"; + String domain = "mydomain.eu"; + String encoded = MessageFormat.format(customSearchFilter, this.joe.getPrincipal() + "@" + domain); DirContextAdapter dca = new DirContextAdapter(); SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes()); - given(this.ctx.search(any(Name.class), eq(customSearchFilter), any(Object[].class), any(SearchControls.class))) + given(this.ctx.search(any(Name.class), eq(encoded), any(SearchControls.class))) .willReturn(new MockNamingEnumeration(sr)); ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider( "mydomain.eu", "ldap://192.168.1.200/"); @@ -109,34 +113,35 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { @Test public void defaultSearchFilter() throws Exception { - final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))"; + String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))"; + String domain = "mydomain.eu"; + String encoded = MessageFormat.format(defaultSearchFilter, this.joe.getPrincipal() + "@" + domain); DirContextAdapter dca = new DirContextAdapter(); SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes()); - given(this.ctx.search(any(Name.class), eq(defaultSearchFilter), any(Object[].class), any(SearchControls.class))) + given(this.ctx.search(any(Name.class), eq(encoded), any(SearchControls.class))) .willReturn(new MockNamingEnumeration(sr)); ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider( "mydomain.eu", "ldap://192.168.1.200/"); customProvider.contextFactory = createContextFactoryReturning(this.ctx); Authentication result = customProvider.authenticate(this.joe); assertThat(result.isAuthenticated()).isTrue(); - verify(this.ctx).search(any(Name.class), eq(defaultSearchFilter), any(Object[].class), - any(SearchControls.class)); + verify(this.ctx).search(any(Name.class), any(String.class), any(SearchControls.class)); } // SEC-2897,SEC-2224 @Test public void bindPrincipalAndUsernameUsed() throws Exception { - final String defaultSearchFilter = "(&(objectClass=user)(userPrincipalName={0}))"; - ArgumentCaptor captor = ArgumentCaptor.forClass(Object[].class); + final String captureValue = "(&(objectClass=user)(userPrincipalName=joe@mydomain.eu))"; + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); DirContextAdapter dca = new DirContextAdapter(); SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes()); - given(this.ctx.search(any(Name.class), eq(defaultSearchFilter), captor.capture(), any(SearchControls.class))) + given(this.ctx.search(any(Name.class), captor.capture(), any(SearchControls.class))) .willReturn(new MockNamingEnumeration(sr)); ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider( "mydomain.eu", "ldap://192.168.1.200/"); customProvider.contextFactory = createContextFactoryReturning(this.ctx); Authentication result = customProvider.authenticate(this.joe); - assertThat(captor.getValue()).containsExactly("joe@mydomain.eu", "joe"); + assertThat(captor.getValue()).isEqualTo(captureValue); assertThat(result.isAuthenticated()).isTrue(); } @@ -156,7 +161,7 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { DirContextAdapter dca = new DirContextAdapter(); SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes()); given(this.ctx.search(eq(LdapNameBuilder.newInstance("DC=mydomain,DC=eu").build()), any(String.class), - any(Object[].class), any(SearchControls.class))) + any(SearchControls.class))) .willReturn(new MockNamingEnumeration(sr)); this.provider.contextFactory = createContextFactoryReturning(this.ctx); assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe)); @@ -165,7 +170,7 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { @Test public void failedUserSearchCausesBadCredentials() throws Exception { - given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class))) + given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class))) .willThrow(new NameNotFoundException()); this.provider.contextFactory = createContextFactoryReturning(this.ctx); assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe)); @@ -174,7 +179,7 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { // SEC-2017 @Test public void noUserSearchCausesUsernameNotFound() throws Exception { - given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class))) + given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class))) .willReturn(new MockNamingEnumeration(null)); this.provider.contextFactory = createContextFactoryReturning(this.ctx); assertThatExceptionOfType(BadCredentialsException.class).isThrownBy(() -> this.provider.authenticate(this.joe)); @@ -195,8 +200,7 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { SearchResult searchResult = mock(SearchResult.class); given(searchResult.getObject()).willReturn(new DirContextAdapter("ou=1"), new DirContextAdapter("ou=2")); given(searchResults.next()).willReturn(searchResult); - given(this.ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class))) - .willReturn(searchResults); + given(this.ctx.search(any(Name.class), any(String.class), any(SearchControls.class))).willReturn(searchResults); this.provider.contextFactory = createContextFactoryReturning(this.ctx); assertThatExceptionOfType(IncorrectResultSizeDataAccessException.class) .isThrownBy(() -> this.provider.authenticate(this.joe)); @@ -353,7 +357,7 @@ public class ActiveDirectoryLdapAuthenticationProviderTests { SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, dca.getAttributes()); @SuppressWarnings("deprecation") Name searchBaseDn = LdapNameBuilder.newInstance(rootDn).build(); - given(this.ctx.search(eq(searchBaseDn), any(String.class), any(Object[].class), any(SearchControls.class))) + given(this.ctx.search(eq(searchBaseDn), any(String.class), any(SearchControls.class))) .willReturn(new MockNamingEnumeration(sr)) .willReturn(new MockNamingEnumeration(sr)); provider.contextFactory = createContextFactoryReturning(this.ctx);