From 699f6c8e0f33245ac7e302188503f011010bd68b Mon Sep 17 00:00:00 2001 From: Thomas Kuyper Date: Sun, 19 Jun 2016 12:54:27 -0500 Subject: [PATCH] Add support for a custom LdapAuthoritiesPopulator to ActiveDirectoryLdapAuthenticationProvider Added support for passing a custom LdapAuthoritiesPopulator via constructor or injected property. Added a unit test. Fixes SEC-2163 gh-2390 --- ...veDirectoryLdapAuthenticationProvider.java | 51 ++++++++++++++++- ...ectoryLdapAuthenticationProviderTests.java | 57 +++++++++++++++++++ 2 files changed, 106 insertions(+), 2 deletions(-) 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 69d826301ae..d81fe480a46 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 @@ -32,6 +32,7 @@ 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; import org.springframework.util.StringUtils; @@ -58,8 +59,9 @@ * domain name to the username supplied in the authentication request. If no domain name * is configured, it is assumed that the username will always contain the domain name. *

- * The user authorities are obtained from the data contained in the {@code memberOf} - * attribute. + * If a custom {@link #setAuthoritiesPopulator(LdapAuthoritiesPopulator) authoritiesPopulator} + * is not specified, the user authorities are obtained from the data contained in the + * {@code memberOf} attribute. * *

Active Directory Sub-Error Codes

* @@ -107,6 +109,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends private final String url; private boolean convertSubErrorCodesToExceptions; private String searchFilter = "(&(objectClass=user)(userPrincipalName={0}))"; + private LdapAuthoritiesPopulator authoritiesPopulator; // Only used to allow tests to substitute a mock LdapContext ContextFactory contextFactory = new ContextFactory(); @@ -122,6 +125,22 @@ public ActiveDirectoryLdapAuthenticationProvider(String domain, String url, this.domain = StringUtils.hasText(domain) ? domain.toLowerCase() : null; this.url = url; this.rootDn = StringUtils.hasText(rootDn) ? rootDn.toLowerCase() : null; + this.authoritiesPopulator = null; + } + + /** + * @param domain the domain name (may be null or empty) + * @param url an LDAP url (or multiple URLs) + * @param rootDn the root DN (may be null or empty) + * @param authoritiesPopulator (may be null or empty) + */ + public ActiveDirectoryLdapAuthenticationProvider(String domain, String url, + String rootDn, LdapAuthoritiesPopulator authoritiesPopulator) { + Assert.isTrue(StringUtils.hasText(url), "Url cannot be empty"); + this.domain = StringUtils.hasText(domain) ? domain.toLowerCase() : null; + this.url = url; + this.rootDn = StringUtils.hasText(rootDn) ? rootDn.toLowerCase() : null; + this.authoritiesPopulator = authoritiesPopulator; } /** @@ -133,6 +152,31 @@ public ActiveDirectoryLdapAuthenticationProvider(String domain, String url) { this.domain = StringUtils.hasText(domain) ? domain.toLowerCase() : null; this.url = url; rootDn = this.domain == null ? null : rootDnFromDomain(this.domain); + this.authoritiesPopulator = null; + } + + /** + * @param domain the domain name (may be null or empty) + * @param url an LDAP url (or multiple URLs) + * @param authoritiesPopulator (may be null or empty) + */ + public ActiveDirectoryLdapAuthenticationProvider(String domain, String url, + LdapAuthoritiesPopulator authoritiesPopulator) { + Assert.isTrue(StringUtils.hasText(url), "Url cannot be empty"); + this.domain = StringUtils.hasText(domain) ? domain.toLowerCase() : null; + this.url = url; + rootDn = this.domain == null ? null : rootDnFromDomain(this.domain); + this.authoritiesPopulator = authoritiesPopulator; + } + + private void setAuthoritiesPopulator(LdapAuthoritiesPopulator authoritiesPopulator) { + Assert.notNull(authoritiesPopulator, + "An LdapAuthoritiesPopulator must be supplied"); + this.authoritiesPopulator = authoritiesPopulator; + } + + protected LdapAuthoritiesPopulator getAuthoritiesPopulator() { + return this.authoritiesPopulator; } @Override @@ -163,6 +207,9 @@ protected DirContextOperations doAuthentication( @Override protected Collection loadUserAuthorities( DirContextOperations userData, String username, String password) { + if (authoritiesPopulator != null) { + return getAuthoritiesPopulator().getGrantedAuthorities(userData, username); + } String[] groups = userData.getStringAttributes("memberOf"); if (groups == null) { 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 d6463ca99e5..16f688557cf 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 @@ -27,13 +27,18 @@ import org.mockito.ArgumentCaptor; import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.ldap.core.DirContextAdapter; +import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DistinguishedName; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.authentication.AccountExpiredException; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.CredentialsExpiredException; import org.springframework.security.authentication.DisabledException; import org.springframework.security.authentication.LockedException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.ldap.userdetails.LdapAuthoritiesPopulator; import org.springframework.security.core.Authentication; import javax.naming.AuthenticationException; @@ -46,6 +51,8 @@ import javax.naming.directory.SearchControls; import javax.naming.directory.SearchResult; +import java.util.Collection; +import java.util.ArrayList; import java.util.Hashtable; import static org.assertj.core.api.Assertions.assertThat; @@ -393,6 +400,56 @@ public void rootDnProvidedSeparatelyFromDomainAlsoWorks() throws Exception { } + // SEC-2163 + @Test + public void customAuthoritiesPopulatorWorks() throws Exception { + ActiveDirectoryLdapAuthenticationProvider customProvider = new ActiveDirectoryLdapAuthenticationProvider( + "mydomain.eu", "ldap://192.168.1.200/", new LdapAuthoritiesPopulator() { + @Override + public Collection getGrantedAuthorities( + DirContextOperations userData, String username) { + String[] groups = userData.getStringAttributes("customGroups"); + + if (groups == null) { + return AuthorityUtils.NO_AUTHORITIES; + } + ArrayList authorities = new ArrayList( + groups.length); + + for (String group : groups) { + authorities.add(new SimpleGrantedAuthority(new DistinguishedName(group) + .removeLast().getValue())); + } + + return authorities; + } + }); + DirContext ctx = mock(DirContext.class); + when(ctx.getNameInNamespace()).thenReturn(""); + + DirContextAdapter dca = new DirContextAdapter(); + SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", dca, + dca.getAttributes()); + @SuppressWarnings("deprecation") + DistinguishedName searchBaseDn = new DistinguishedName("DC=mydomain,DC=eu"); + when( + ctx.search(eq(searchBaseDn), any(String.class), any(Object[].class), + any(SearchControls.class))).thenReturn( + new MockNamingEnumeration(sr)).thenReturn(new MockNamingEnumeration(sr)); + + customProvider.contextFactory = createContextFactoryReturning(ctx); + + Authentication result = customProvider.authenticate(joe); + + assertThat(result.getAuthorities()).isEmpty(); + + dca.addAttributeValue("customGroups", "CN=Admin,CN=Users,DC=mydomain,DC=eu"); + + result = customProvider.authenticate(joe); + + assertThat(result.getAuthorities()).hasSize(1); + } + ContextFactory createContextFactoryThrowing(final NamingException e) { return new ContextFactory() { @Override