From 373d07ce46ba55da433fdfe3d95d70096179aa2a Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 15 Apr 2011 20:10:40 +0100 Subject: [PATCH] SEC-1181: Added mock testing, to avoid need for AD server --- ...veDirectoryLdapAuthenticationProvider.java | 32 ++- ...ectoryLdapAuthenticationProviderTests.java | 225 +++++++++++++++++- 2 files changed, 239 insertions(+), 18 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 a3d795844b..3094d4ad98 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 @@ -21,9 +21,9 @@ import javax.naming.AuthenticationException; import javax.naming.Context; import javax.naming.NamingException; import javax.naming.OperationNotSupportedException; +import javax.naming.directory.DirContext; import javax.naming.directory.SearchControls; import javax.naming.ldap.InitialLdapContext; -import javax.naming.ldap.LdapContext; import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -64,7 +64,7 @@ import java.util.regex.Pattern; * @since 3.1 */ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLdapAuthenticationProvider { - private static final Pattern SUB_ERROR_CODE = Pattern.compile(".*\\s([0-9a-f]{3,4}).*"); + private static final Pattern SUB_ERROR_CODE = Pattern.compile(".*data\\s([0-9a-f]{3,4}).*"); // Error codes private static final int USERNAME_NOT_FOUND = 0x525; @@ -81,6 +81,9 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda private final String url; private boolean convertSubErrorCodesToExceptions; + // Only used to allow tests to substitute a mock LdapContext + ContextFactory contextFactory = new ContextFactory(); + /** * @param domain the domain for which authentication should take place */ @@ -89,7 +92,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda // } /** - * @param domain the domain name + * @param domain the domain name (may be null or empty) * @param url an LDAP url (or multiple URLs) */ public ActiveDirectoryLdapAuthenticationProvider(String domain, String url) { @@ -104,7 +107,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda String username = auth.getName(); String password = (String)auth.getCredentials(); - LdapContext ctx = bindAsUser(username, password); + DirContext ctx = bindAsUser(username, password); try { return searchForUser(ctx, username); @@ -144,7 +147,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda return authorities; } - private LdapContext bindAsUser(String username, String password) { + private DirContext bindAsUser(String username, String password) { // TODO. add DNS lookup based on domain final String bindUrl = url; @@ -157,7 +160,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); try { - return new InitialLdapContext(env, null); + return contextFactory.createContext(env); } catch (NamingException e) { if ((e instanceof AuthenticationException) || (e instanceof OperationNotSupportedException)) { handleBindException(bindPrincipal, e); @@ -241,7 +244,7 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda "LdapAuthenticationProvider.badCredentials", "Bad credentials")); } - private DirContextOperations searchForUser(LdapContext ctx, String username) throws NamingException { + private DirContextOperations searchForUser(DirContext ctx, String username) throws NamingException { SearchControls searchCtls = new SearchControls(); searchCtls.setSearchScope(SearchControls.SUBTREE_SCOPE); @@ -256,7 +259,14 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda } private String searchRootFromPrincipal(String bindPrincipal) { - return rootDnFromDomain(bindPrincipal.substring(bindPrincipal.lastIndexOf('@') + 1, bindPrincipal.length())); + int atChar = bindPrincipal.lastIndexOf('@'); + + if (atChar < 0) { + logger.debug("User principal '" + bindPrincipal + "' does not contain the domain, and no domain has been configured"); + throw badCredentials(); + } + + return rootDnFromDomain(bindPrincipal.substring(atChar+ 1, bindPrincipal.length())); } private String rootDnFromDomain(String domain) { @@ -295,5 +305,9 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda this.convertSubErrorCodesToExceptions = convertSubErrorCodesToExceptions; } - + static class ContextFactory { + DirContext createContext(Hashtable env) throws NamingException { + return new InitialLdapContext(env, 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 dd61e14d62..22a77eacf2 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 @@ -1,35 +1,242 @@ package org.springframework.security.ldap.authentication.ad; import static org.junit.Assert.*; -import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.*; +import static org.springframework.security.ldap.authentication.ad.ActiveDirectoryLdapAuthenticationProvider.ContextFactory; import org.junit.*; +import org.springframework.ldap.core.DirContextAdapter; +import org.springframework.ldap.core.DistinguishedName; +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.core.Authentication; import org.springframework.security.core.authority.SimpleGrantedAuthority; -import javax.naming.Context; -import javax.naming.InitialContext; +import javax.naming.AuthenticationException; +import javax.naming.CommunicationException; +import javax.naming.Name; +import javax.naming.NameNotFoundException; +import javax.naming.NamingEnumeration; import javax.naming.NamingException; -import javax.naming.ldap.LdapContext; -import javax.naming.spi.InitialContextFactory; -import javax.naming.spi.InitialContextFactoryBuilder; -import javax.naming.spi.NamingManager; +import javax.naming.directory.DirContext; +import javax.naming.directory.SearchControls; +import javax.naming.directory.SearchResult; import java.util.*; /** * @author Luke Taylor */ public class ActiveDirectoryLdapAuthenticationProviderTests { + ActiveDirectoryLdapAuthenticationProvider provider; + UsernamePasswordAuthenticationToken joe = new UsernamePasswordAuthenticationToken("joe", "password"); + + @Before + public void setUp() throws Exception { + provider = new ActiveDirectoryLdapAuthenticationProvider("mydomain.eu", "ldap://192.168.1.200/"); + } @Test public void bindPrincipalIsCreatedCorrectly() throws Exception { - ActiveDirectoryLdapAuthenticationProvider provider = - new ActiveDirectoryLdapAuthenticationProvider("mydomain.eu", "ldap://192.168.1.200/"); assertEquals("joe@mydomain.eu", provider.createBindPrincipal("joe")); assertEquals("joe@mydomain.eu", provider.createBindPrincipal("joe@mydomain.eu")); } + @Test + public void successfulAuthenticationProducesExpectedAuthorities() throws Exception { + DirContext ctx = mock(DirContext.class); + when(ctx.getNameInNamespace()).thenReturn(""); + + DirContextAdapter dca = new DirContextAdapter(); + SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", null, dca.getAttributes()); + when(ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class))) + .thenReturn(new MockNamingEnumeration(sr)) + .thenReturn(new MockNamingEnumeration(sr)); + + provider.contextFactory = createContextFactoryReturning(ctx); + + Authentication result = provider.authenticate(joe); + + assertEquals(0, result.getAuthorities().size()); + + dca.addAttributeValue("memberOf","CN=Admin,CN=Users,DC=mydomain,DC=eu"); + + sr.setAttributes(dca.getAttributes()); + + result = provider.authenticate(joe); + + assertEquals(1, result.getAuthorities().size()); + } + + @Test + public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws Exception { + provider = new ActiveDirectoryLdapAuthenticationProvider(null, "ldap://192.168.1.200/"); + DirContext ctx = mock(DirContext.class); + when(ctx.getNameInNamespace()).thenReturn(""); + + DirContextAdapter dca = new DirContextAdapter(); + SearchResult sr = new SearchResult("CN=Joe Jannsen,CN=Users", null, dca.getAttributes()); + when(ctx.search(eq(new DistinguishedName("DC=mydomain,DC=eu")), any(String.class), any(Object[].class), any(SearchControls.class))) + .thenReturn(new MockNamingEnumeration(sr)); + provider.contextFactory = createContextFactoryReturning(ctx); + + try { + provider.authenticate(joe); + fail("Expected BadCredentialsException for user with no domain information"); + } catch (BadCredentialsException expected) { + } + + provider.authenticate(new UsernamePasswordAuthenticationToken("joe@mydomain.eu", "password")); + } + + @Test(expected = BadCredentialsException.class) + public void failedUserSearchCausesBadCredentials() throws Exception { + DirContext ctx = mock(DirContext.class); + when(ctx.getNameInNamespace()).thenReturn(""); + when(ctx.search(any(Name.class), any(String.class), any(Object[].class), any(SearchControls.class))) + .thenThrow(new NameNotFoundException()); + + provider.contextFactory = createContextFactoryReturning(ctx); + + provider.authenticate(joe); + } + + static final String msg = "[LDAP: error code 49 - 80858585: LdapErr: DSID-DECAFF0, comment: AcceptSecurityContext error, data "; + + @Test(expected = BadCredentialsException.class) + public void userNotFoundIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "525, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = BadCredentialsException.class) + public void incorrectPasswordIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "52e, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = BadCredentialsException.class) + public void notPermittedIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "530, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = BadCredentialsException.class) + public void passwordNeedsResetIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "773, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = CredentialsExpiredException.class) + public void expiredPasswordIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "532, xxxx]")); + + try { + provider.authenticate(joe); + fail(); + } catch (BadCredentialsException expected) { + } + + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = DisabledException.class) + public void accountDisabledIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "533, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = AccountExpiredException.class) + public void accountExpiredIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "701, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = LockedException.class) + public void accountLockedIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "775, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = BadCredentialsException.class) + public void unknownErrorCodeIsCorrectlyMapped() { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg + "999, xxxx]")); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = BadCredentialsException.class) + public void errorWithNoSubcodeIsHandledCleanly() throws Exception { + provider.contextFactory = createContextFactoryThrowing(new AuthenticationException(msg)); + provider.setConvertSubErrorCodesToExceptions(true); + provider.authenticate(joe); + } + + @Test(expected = org.springframework.ldap.CommunicationException.class) + public void nonAuthenticationExceptionIsConvertedToSpringLdapException() throws Exception { + provider.contextFactory = createContextFactoryThrowing(new CommunicationException(msg)); + provider.authenticate(joe); + } + + ContextFactory createContextFactoryThrowing(final NamingException e) { + return new ContextFactory() { + @Override + DirContext createContext(Hashtable env) throws NamingException { + throw e; + } + }; + } + + + ContextFactory createContextFactoryReturning(final DirContext ctx) { + return new ContextFactory() { + @Override + DirContext createContext(Hashtable env) throws NamingException { + return ctx; + } + }; + } + + static class MockNamingEnumeration implements NamingEnumeration { + private SearchResult sr; + + public MockNamingEnumeration(SearchResult sr) { + this.sr = sr; + } + + public SearchResult next() { + SearchResult result = sr; + sr = null; + return result; + } + + public boolean hasMore() { + return sr != null; + } + + public void close() { + } + + public boolean hasMoreElements() { + return hasMore(); + } + + public SearchResult nextElement() { + return next(); + } + } + // @Test // public void realAuthenticationIsSucessful() throws Exception { // ActiveDirectoryLdapAuthenticationProvider provider =