From d6f6a544556dfd940257ef6d2989494176b7421c Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 16 Apr 2010 15:14:01 +0100 Subject: [PATCH] SEC-1444: Backport of changes to 2.0.x --- .../ldap/SpringSecurityLdapTemplate.java | 17 ++-- .../ldap/authenticator/BindAuthenticator.java | 82 ++++++++++++------- .../FilterBasedLdapUserSearchTests.java | 4 +- .../authenticator/BindAuthenticatorTests.java | 28 +++---- core/src/test/resources/test-server.ldif | 10 +++ 5 files changed, 89 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java b/core/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java index a3edf94c44..b9f0e5f48d 100644 --- a/core/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java +++ b/core/src/main/java/org/springframework/security/ldap/SpringSecurityLdapTemplate.java @@ -193,22 +193,27 @@ public class SpringSecurityLdapTemplate extends LdapTemplate { return (DirContextOperations) executeReadOnly(new ContextExecutor() { public Object executeWithContext(DirContext ctx) throws NamingException { DistinguishedName ctxBaseDn = new DistinguishedName(ctx.getNameInNamespace()); + if (logger.isDebugEnabled()) { + logger.debug("Searching for entry in under DN '" + ctxBaseDn + + "', base = '" + base + "', filter = '" + filter + "'"); + } + NamingEnumeration resultsEnum = ctx.search(base, filter, params, searchControls); Set results = new HashSet(); try { while (resultsEnum.hasMore()) { - SearchResult searchResult = (SearchResult) resultsEnum.next(); // Work out the DN of the matched entry - StringBuffer dn = new StringBuffer(searchResult.getName()); + DistinguishedName dn = new DistinguishedName(searchResult.getName()); if (base.length() > 0) { - dn.append(","); - dn.append(base); + dn.prepend(new DistinguishedName(base)); } - results.add(new DirContextAdapter(searchResult.getAttributes(), - new DistinguishedName(dn.toString()), ctxBaseDn)); + if (logger.isDebugEnabled()) { + logger.debug("Found DN: " + dn); + } + results.add(new DirContextAdapter(searchResult.getAttributes(), dn, ctxBaseDn)); } } catch (PartialResultException e) { logger.info("Ignoring PartialResultException"); diff --git a/core/src/main/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticator.java b/core/src/main/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticator.java index 82b4af5cb8..28ed19f155 100644 --- a/core/src/main/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticator.java +++ b/core/src/main/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticator.java @@ -15,22 +15,26 @@ package org.springframework.security.providers.ldap.authenticator; -import org.springframework.security.Authentication; -import org.springframework.security.BadCredentialsException; -import org.springframework.security.ldap.SpringSecurityContextSource; -import org.springframework.security.ldap.SpringSecurityLdapTemplate; -import org.springframework.security.providers.UsernamePasswordAuthenticationToken; -import org.springframework.dao.DataAccessException; -import org.springframework.ldap.core.ContextSource; -import org.springframework.ldap.core.DirContextOperations; -import org.springframework.ldap.core.DistinguishedName; -import org.springframework.util.Assert; +import java.util.Iterator; + +import javax.naming.directory.Attributes; +import javax.naming.directory.DirContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - -import javax.naming.directory.DirContext; -import java.util.Iterator; +import org.springframework.dao.DataAccessException; +import org.springframework.ldap.NamingException; +import org.springframework.ldap.core.ContextSource; +import org.springframework.ldap.core.DirContextAdapter; +import org.springframework.ldap.core.DirContextOperations; +import org.springframework.ldap.core.DistinguishedName; +import org.springframework.ldap.core.support.BaseLdapPathContextSource; +import org.springframework.ldap.support.LdapUtils; +import org.springframework.security.Authentication; +import org.springframework.security.BadCredentialsException; +import org.springframework.security.ldap.SpringSecurityContextSource; +import org.springframework.security.providers.UsernamePasswordAuthenticationToken; +import org.springframework.util.Assert; /** @@ -91,21 +95,42 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { return user; } - private DirContextOperations bindWithDn(String userDn, String username, String password) { - SpringSecurityLdapTemplate template = new SpringSecurityLdapTemplate( - new BindWithSpecificDnContextSource((SpringSecurityContextSource) getContextSource(), userDn, password)); + private DirContextOperations bindWithDn(String userDnStr, String username, String password) { + BaseLdapPathContextSource ctxSource = (BaseLdapPathContextSource) getContextSource(); + DistinguishedName userDn = new DistinguishedName(userDnStr); + DistinguishedName fullDn = new DistinguishedName(userDn); + fullDn.prepend(ctxSource.getBaseLdapPath()); + BindWithSpecificDnContextSource specificDnContextSource = new BindWithSpecificDnContextSource( + (SpringSecurityContextSource) getContextSource(), fullDn, + password); + logger.debug("Attemptimg to bind as " + fullDn); + DirContext ctx = null; + try { + ctx = specificDnContextSource.getReadOnlyContext(); - try { - return template.retrieveEntry(userDn, getUserAttributes()); + Attributes attrs = ctx.getAttributes(userDn, getUserAttributes()); - } catch (BadCredentialsException e) { - // This will be thrown if an invalid user name is used and the method may - // be called multiple times to try different names, so we trap the exception - // unless a subclass wishes to implement more specialized behaviour. - handleBindException(userDn, username, e.getCause()); - } + DirContextAdapter result = new DirContextAdapter(attrs, userDn, ctxSource.getBaseLdapPath()); + + return result; + } catch (NamingException e) { + // This will be thrown if an invalid user name is used and the method may + // be called multiple times to try different names, so we trap the exception + // unless a subclass wishes to implement more specialized behaviour. + if ((e instanceof org.springframework.ldap.AuthenticationException) + || (e instanceof org.springframework.ldap.OperationNotSupportedException)) { + handleBindException(userDnStr, username, e); + } else { + throw e; + } + } catch (javax.naming.NamingException e) { + throw LdapUtils.convertLdapException(e); + } finally { + LdapUtils.closeContext(ctx); + } + + return null; - return null; } /** @@ -120,13 +145,12 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { private class BindWithSpecificDnContextSource implements ContextSource { private SpringSecurityContextSource ctxFactory; - DistinguishedName userDn; + private DistinguishedName userDn; private String password; - public BindWithSpecificDnContextSource(SpringSecurityContextSource ctxFactory, String userDn, String password) { + public BindWithSpecificDnContextSource(SpringSecurityContextSource ctxFactory, DistinguishedName userDn, String password) { this.ctxFactory = ctxFactory; - this.userDn = new DistinguishedName(userDn); - this.userDn.prepend(ctxFactory.getBaseLdapPath()); + this.userDn = userDn; this.password = password; } diff --git a/core/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java b/core/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java index 93b0635920..f3ffbd89db 100644 --- a/core/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java +++ b/core/src/test/java/org/springframework/security/ldap/search/FilterBasedLdapUserSearchTests.java @@ -71,13 +71,11 @@ public class FilterBasedLdapUserSearchTests extends AbstractLdapIntegrationTests @Test public void extraFilterPartToExcludeBob() throws Exception { FilterBasedLdapUserSearch locator = new FilterBasedLdapUserSearch("ou=people", - "(&(cn=*)(!(|(uid={0})(uid=rod)(uid=jerry))))", dirCtxFactory); + "(&(cn=*)(!(|(uid={0})(uid=rod)(uid=jerry)(uid=slashguy))))", dirCtxFactory); // Search for bob, get back ben... DirContextOperations ben = locator.searchForUser("bob"); assertEquals("Ben Alex", ben.getStringAttribute("cn")); - -// assertEquals("uid=ben,ou=people,"+ROOT_DN, ben.getDn()); } @Test(expected=IncorrectResultSizeDataAccessException.class) diff --git a/core/src/test/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticatorTests.java b/core/src/test/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticatorTests.java index 934df0481d..e02a2e5bc4 100644 --- a/core/src/test/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticatorTests.java +++ b/core/src/test/java/org/springframework/security/providers/ldap/authenticator/BindAuthenticatorTests.java @@ -15,18 +15,17 @@ package org.springframework.security.providers.ldap.authenticator; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import org.junit.Test; +import org.springframework.ldap.core.DirContextOperations; import org.springframework.security.Authentication; import org.springframework.security.BadCredentialsException; import org.springframework.security.SpringSecurityMessageSource; import org.springframework.security.ldap.AbstractLdapIntegrationTests; +import org.springframework.security.ldap.search.FilterBasedLdapUserSearch; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; -import org.springframework.ldap.core.DirContextAdapter; -import org.springframework.ldap.core.DirContextOperations; -import org.springframework.ldap.core.DistinguishedName; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; -import org.junit.Test; /** * Tests for {@link BindAuthenticator}. @@ -39,7 +38,6 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests { private BindAuthenticator authenticator; private Authentication bob; -// private Authentication ben; //~ Methods ======================================================================================================== @@ -48,7 +46,6 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests { authenticator = new BindAuthenticator(getContextSource()); authenticator.setMessageSource(new SpringSecurityMessageSource()); bob = new UsernamePasswordAuthenticationToken("bob", "bobspassword"); -// ben = new UsernamePasswordAuthenticationToken("ben", "benspassword"); } @@ -72,11 +69,14 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests { @Test public void testAuthenticationWithUserSearch() throws Exception { - DirContextAdapter ctx = new DirContextAdapter(new DistinguishedName("uid=bob,ou=people")); - - authenticator.setUserSearch(new MockUserSearch(ctx)); - authenticator.afterPropertiesSet(); - authenticator.authenticate(bob); +// DirContextAdapter ctx = new DirContextAdapter(new DistinguishedName("uid=bob,ou=people")); + authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(uid={0})", getContextSource())); + authenticator.afterPropertiesSet(); + authenticator.authenticate(bob); + // SEC-1444 + authenticator.setUserSearch(new FilterBasedLdapUserSearch("ou=people", "(cn={0})", getContextSource())); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("mouse, jerry", "jerryspassword")); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("slash/guy", "slashguyspassword")); } @Test diff --git a/core/src/test/resources/test-server.ldif b/core/src/test/resources/test-server.ldif index 599c615c4f..1124c05077 100644 --- a/core/src/test/resources/test-server.ldif +++ b/core/src/test/resources/test-server.ldif @@ -58,6 +58,16 @@ sn: Mouse uid: jerry userPassword: jerryspassword +dn: cn=slash/guy,ou=people,dc=springframework,dc=org +objectclass: top +objectclass: person +objectclass: organizationalPerson +objectclass: inetOrgPerson +cn: slash/guy +sn: Slash +uid: slashguy +userPassword: slashguyspassword + dn: cn=developers,ou=groups,dc=springframework,dc=org objectclass: top objectclass: groupOfNames