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 43bb8305279..7423376e7fc 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at @@ -47,11 +47,12 @@ * Specialized LDAP authentication provider which uses Active Directory configuration conventions. *

* It will authenticate using the Active Directory - * {@code userPrincipalName} - * (in the form {@code username@domain}). If the username does not already end with the domain name, the - * {@code userPrincipalName} will be built by appending the configured 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. + * {@code userPrincipalName} or + * {@code sAMAccountName} (or a custom + * {@link #setSearchFilter(String) searchFilter}) in the form {@code username@domain}. If the username does not + * already end with the domain name, the {@code userPrincipalName} will be built by appending the configured 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. * @@ -96,17 +97,11 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda private final String rootDn; private final String url; private boolean convertSubErrorCodesToExceptions; + private String searchFilter = "(&(objectClass=user)(|(sAMAccountName={0})(userPrincipalName={0})))"; // Only used to allow tests to substitute a mock LdapContext ContextFactory contextFactory = new ContextFactory(); - /** - * @param domain the domain for which authentication should take place - */ -// public ActiveDirectoryLdapAuthenticationProvider(String domain) { -// this (domain, null); -// } - /** * @param domain the domain name (may be null or empty) * @param url an LDAP url (or multiple URLs) @@ -114,7 +109,6 @@ public final class ActiveDirectoryLdapAuthenticationProvider extends AbstractLda public ActiveDirectoryLdapAuthenticationProvider(String domain, String url) { Assert.isTrue(StringUtils.hasText(url), "Url cannot be empty"); this.domain = StringUtils.hasText(domain) ? domain.toLowerCase() : null; - //this.url = StringUtils.hasText(url) ? url : null; this.url = url; rootDn = this.domain == null ? null : rootDnFromDomain(this.domain); } @@ -122,13 +116,12 @@ public ActiveDirectoryLdapAuthenticationProvider(String domain, String url) { @Override protected DirContextOperations doAuthentication(UsernamePasswordAuthenticationToken auth) { String username = auth.getName(); - String password = (String)auth.getCredentials(); + String password = (String) auth.getCredentials(); DirContext ctx = bindAsUser(username, password); try { return searchForUser(ctx, username); - } catch (NamingException e) { logger.error("Failed to locate directory entry for authenticated user: " + username, e); throw badCredentials(e); @@ -168,7 +161,7 @@ private DirContext bindAsUser(String username, String password) { // TODO. add DNS lookup based on domain final String bindUrl = url; - Hashtable env = new Hashtable(); + Hashtable env = new Hashtable(); env.put(Context.SECURITY_AUTHENTICATION, "simple"); String bindPrincipal = createBindPrincipal(username); env.put(Context.SECURITY_PRINCIPAL, bindPrincipal); @@ -189,25 +182,26 @@ private DirContext bindAsUser(String username, String password) { } } - void handleBindException(String bindPrincipal, NamingException exception) { + private void handleBindException(String bindPrincipal, NamingException exception) { if (logger.isDebugEnabled()) { logger.debug("Authentication for " + bindPrincipal + " failed:" + exception); } int subErrorCode = parseSubErrorCode(exception.getMessage()); - if (subErrorCode > 0) { - logger.info("Active Directory authentication failed: " + subCodeToLogMessage(subErrorCode)); - - if (convertSubErrorCodesToExceptions) { - raiseExceptionForErrorCode(subErrorCode, exception); - } - } else { + if (subErrorCode <= 0) { logger.debug("Failed to locate AD-specific sub-error code in message"); + return; + } + + logger.info("Active Directory authentication failed: " + subCodeToLogMessage(subErrorCode)); + + if (convertSubErrorCodesToExceptions) { + raiseExceptionForErrorCode(subErrorCode, exception); } } - int parseSubErrorCode(String message) { + private int parseSubErrorCode(String message) { Matcher m = SUB_ERROR_CODE.matcher(message); if (m.matches()) { @@ -217,7 +211,7 @@ int parseSubErrorCode(String message) { return -1; } - void raiseExceptionForErrorCode(int code, NamingException exception) { + private void raiseExceptionForErrorCode(int code, NamingException exception) { String hexString = Integer.toHexString(code); Throwable cause = new ActiveDirectoryAuthenticationException(hexString, exception.getMessage(), exception); switch (code) { @@ -238,7 +232,7 @@ void raiseExceptionForErrorCode(int code, NamingException exception) { } } - String subCodeToLogMessage(int code) { + private String subCodeToLogMessage(int code) { switch (code) { case USERNAME_NOT_FOUND: return "User was not found in directory"; @@ -270,28 +264,24 @@ private BadCredentialsException badCredentials(Throwable cause) { return (BadCredentialsException) badCredentials().initCause(cause); } - @SuppressWarnings("deprecation") - private DirContextOperations searchForUser(DirContext ctx, String username) throws NamingException { - SearchControls searchCtls = new SearchControls(); - searchCtls.setSearchScope(SearchControls.SUBTREE_SCOPE); - - String searchFilter = "(&(objectClass=user)(userPrincipalName={0}))"; - - final String bindPrincipal = createBindPrincipal(username); + private DirContextOperations searchForUser(DirContext context, String username) throws NamingException { + SearchControls searchControls = new SearchControls(); + searchControls.setSearchScope(SearchControls.SUBTREE_SCOPE); + String bindPrincipal = createBindPrincipal(username); String searchRoot = rootDn != null ? rootDn : searchRootFromPrincipal(bindPrincipal); try { - return SpringSecurityLdapTemplate.searchForSingleEntryInternal(ctx, searchCtls, searchRoot, searchFilter, - new Object[]{bindPrincipal}); + return SpringSecurityLdapTemplate.searchForSingleEntryInternal(context, searchControls, + searchRoot, searchFilter, new Object[]{username}); } catch (IncorrectResultSizeDataAccessException incorrectResults) { - if (incorrectResults.getActualSize() == 0) { - UsernameNotFoundException userNameNotFoundException = new UsernameNotFoundException("User " + username + " not found in directory.", username); - userNameNotFoundException.initCause(incorrectResults); - throw badCredentials(userNameNotFoundException); + // Search should never return multiple results if properly configured - just rethrow + if (incorrectResults.getActualSize() != 0) { + throw incorrectResults; } - // Search should never return multiple results if properly configured, so just rethrow - throw incorrectResults; + UsernameNotFoundException userNameNotFoundException = new UsernameNotFoundException("User " + username + + " not found in directory.", incorrectResults); + throw badCredentials(userNameNotFoundException); } } @@ -303,7 +293,7 @@ private String searchRootFromPrincipal(String bindPrincipal) { throw badCredentials(); } - return rootDnFromDomain(bindPrincipal.substring(atChar+ 1, bindPrincipal.length())); + return rootDnFromDomain(bindPrincipal.substring(atChar + 1, bindPrincipal.length())); } private String rootDnFromDomain(String domain) { @@ -342,6 +332,21 @@ public void setConvertSubErrorCodesToExceptions(boolean convertSubErrorCodesToEx this.convertSubErrorCodesToExceptions = convertSubErrorCodesToExceptions; } + /** + * The LDAP filter string to search for the user being authenticated. + * Occurrences of {0} are replaced with the {@code username@domain}. + *

+ * Defaults to: {@code (&(objectClass=user)(|(sAMAccountName={0})(userPrincipalName={0})))} + *

+ * + * @param searchFilter the filter string + * + * @since 3.2 + */ + public void setSearchFilter(String searchFilter) { + this.searchFilter = searchFilter; + } + 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 d90d1139c8c..9c1e5dab818 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,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at @@ -44,6 +44,7 @@ import java.util.Hashtable; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; @@ -97,6 +98,32 @@ public void successfulAuthenticationProducesExpectedAuthorities() throws Excepti assertEquals(1, result.getAuthorities().size()); } + // SEC-1915 + @Test + public void customSearchFilterIsUsedForSuccessfulAuthentication() throws Exception { + //given + String customSearchFilter = "(&(objectClass=user)(sAMAccountName={0}))"; + + 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()); + when(ctx.search(any(Name.class), eq(customSearchFilter), any(Object[].class), any(SearchControls.class))) + .thenReturn(new MockNamingEnumeration(sr)); + + ActiveDirectoryLdapAuthenticationProvider customProvider + = new ActiveDirectoryLdapAuthenticationProvider("mydomain.eu", "ldap://192.168.1.200/"); + customProvider.contextFactory = createContextFactoryReturning(ctx); + + //when + customProvider.setSearchFilter(customSearchFilter); + Authentication result = customProvider.authenticate(joe); + + //then + assertTrue(result.isAuthenticated()); + } + @Test public void nullDomainIsSupportedIfAuthenticatingWithFullUserPrincipal() throws Exception { provider = new ActiveDirectoryLdapAuthenticationProvider(null, "ldap://192.168.1.200/"); @@ -319,17 +346,4 @@ public SearchResult nextElement() { return next(); } } - -// @Test -// public void realAuthenticationIsSucessful() throws Exception { -// ActiveDirectoryLdapAuthenticationProvider provider = -// new ActiveDirectoryLdapAuthenticationProvider(null, "ldap://192.168.1.200/"); -// -// provider.setConvertSubErrorCodesToExceptions(true); -// -// Authentication result = provider.authenticate(new UsernamePasswordAuthenticationToken("luke@fenetres.monkeymachine.eu","p!ssw0rd")); -// -// assertEquals(1, result.getAuthorities().size()); -// assertTrue(result.getAuthorities().contains(new SimpleGrantedAuthority("blah"))); -// } }