From d79d55c8b6c7b1c59b8359f60f7816ea046890ff Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 17 Sep 2007 12:28:07 +0000 Subject: [PATCH] SEC-8: Changes to LDAP authenticator API to take an authentication object rather than username/password. --- core/pom.xml | 1 - .../ldap/LdapAuthenticationProvider.java | 8 +++-- .../providers/ldap/LdapAuthenticator.java | 8 ++--- .../ldap/authenticator/BindAuthenticator.java | 14 +++++--- .../PasswordComparisonAuthenticator.java | 11 ++++-- .../ldap/LdapAuthenticationProviderTests.java | 10 +++--- .../authenticator/BindAuthenticatorTests.java | 16 ++++++--- ...swordComparisonAuthenticatorMockTests.java | 3 +- .../PasswordComparisonAuthenticatorTests.java | 34 +++++++++++-------- pom.xml | 3 +- releasebuild.sh | 9 ++++- 11 files changed, 76 insertions(+), 41 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 9c5d4d65a7..a97b0a4ba5 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -44,7 +44,6 @@ true - org.springframework.ldap spring-ldap 1.2-RC1 diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java b/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java index 4e1d7b61be..7a9320b2e2 100644 --- a/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java +++ b/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticationProvider.java @@ -180,6 +180,10 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio this.userDetailsContextMapper = userDetailsContextMapper; } + protected UserDetailsContextMapper getUserDetailsContextMapper() { + return userDetailsContextMapper; + } + protected void additionalAuthenticationChecks(UserDetails userDetails, UsernamePasswordAuthenticationToken authentication) throws AuthenticationException { @@ -193,7 +197,7 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio } protected UserDetails retrieveUser(String username, UsernamePasswordAuthenticationToken authentication) - throws AuthenticationException { + throws AuthenticationException { if (!StringUtils.hasLength(username)) { throw new BadCredentialsException(messages.getMessage("LdapAuthenticationProvider.emptyUsername", "Empty Username")); @@ -213,7 +217,7 @@ public class LdapAuthenticationProvider extends AbstractUserDetailsAuthenticatio } try { - DirContextOperations user = getAuthenticator().authenticate(username, password); + DirContextOperations user = getAuthenticator().authenticate(authentication); GrantedAuthority[] extraAuthorities = getAuthoritiesPopulator().getGrantedAuthorities(user, username); diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticator.java b/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticator.java index fa7cbc428a..4b6c098dd1 100644 --- a/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticator.java +++ b/core/src/main/java/org/acegisecurity/providers/ldap/LdapAuthenticator.java @@ -15,7 +15,7 @@ package org.acegisecurity.providers.ldap; -import org.acegisecurity.userdetails.ldap.LdapUserDetails; +import org.acegisecurity.Authentication; import org.springframework.ldap.core.DirContextOperations; @@ -36,10 +36,8 @@ public interface LdapAuthenticator { /** * Authenticates as a user and obtains additional user information from the directory. * - * @param username the user's login name (not their DN). - * @param password the user's password supplied at login. - * + * @param authentication * @return the details of the successfully authenticated user. */ - DirContextOperations authenticate(String username, String password); + DirContextOperations authenticate(Authentication authentication); } diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java b/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java index 58bbcca1fe..349cc056c8 100644 --- a/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java +++ b/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticator.java @@ -16,21 +16,20 @@ package org.acegisecurity.providers.ldap.authenticator; import org.acegisecurity.BadCredentialsException; +import org.acegisecurity.Authentication; +import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.ldap.InitialDirContextFactory; import org.acegisecurity.ldap.SpringSecurityLdapTemplate; -import org.acegisecurity.userdetails.ldap.LdapUserDetails; -import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.dao.DataAccessException; import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.core.DirContextOperations; +import org.springframework.util.Assert; import javax.naming.directory.DirContext; -import javax.naming.Name; import java.util.Iterator; @@ -60,8 +59,13 @@ public class BindAuthenticator extends AbstractLdapAuthenticator { //~ Methods ======================================================================================================== - public DirContextOperations authenticate(String username, String password) { + public DirContextOperations authenticate(Authentication authentication) { DirContextOperations user = null; + Assert.isInstanceOf(UsernamePasswordAuthenticationToken.class, authentication, + "Can only process UsernamePasswordAuthenticationToken objects"); + + String username = authentication.getName(); + String password = (String)authentication.getCredentials(); // If DN patterns are configured, try authenticating with them directly Iterator dns = getUserDns(username).iterator(); diff --git a/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java b/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java index 54dc982ee9..86dd00d12b 100644 --- a/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java +++ b/core/src/main/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticator.java @@ -16,12 +16,14 @@ package org.acegisecurity.providers.ldap.authenticator; import org.acegisecurity.BadCredentialsException; +import org.acegisecurity.Authentication; import org.acegisecurity.ldap.InitialDirContextFactory; import org.acegisecurity.ldap.SpringSecurityLdapTemplate; import org.acegisecurity.ldap.LdapUtils; import org.acegisecurity.providers.encoding.PasswordEncoder; +import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.userdetails.UsernameNotFoundException; @@ -69,9 +71,14 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic //~ Methods ======================================================================================================== - public DirContextOperations authenticate(final String username, final String password) { + public DirContextOperations authenticate(final Authentication authentication) { + Assert.isInstanceOf(UsernamePasswordAuthenticationToken.class, authentication, + "Can only process UsernamePasswordAuthenticationToken objects"); // locate the user and check the password + DirContextOperations user = null; + String username = authentication.getName(); + String password = (String)authentication.getCredentials(); Iterator dns = getUserDns(username).iterator(); @@ -105,7 +112,7 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic } if (logger.isDebugEnabled()) { - logger.debug("Password attribute wasn't retrieved for user '" + username + logger.debug("Password attribute wasn't retrieved for user '" + authentication + "'. Performing LDAP compare of password attribute '" + passwordAttributeName + "'"); } diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java index 4825e406ea..173959d1c3 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java +++ b/core/src/test/java/org/acegisecurity/providers/ldap/LdapAuthenticationProviderTests.java @@ -20,11 +20,11 @@ import junit.framework.TestCase; import org.acegisecurity.BadCredentialsException; import org.acegisecurity.GrantedAuthority; import org.acegisecurity.GrantedAuthorityImpl; +import org.acegisecurity.Authentication; import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.userdetails.UserDetails; -import org.acegisecurity.userdetails.ldap.LdapUserDetailsImpl; import org.acegisecurity.userdetails.ldap.LdapUserDetailsMapper; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DirContextAdapter; @@ -32,9 +32,6 @@ import org.springframework.ldap.core.DistinguishedName; import java.util.ArrayList; -import javax.naming.directory.Attributes; -import javax.naming.directory.BasicAttributes; - /** * Tests {@link LdapAuthenticationProvider}. @@ -135,9 +132,12 @@ public class LdapAuthenticationProviderTests extends TestCase { class MockAuthenticator implements LdapAuthenticator { - public DirContextOperations authenticate(String username, String password) { + public DirContextOperations authenticate(Authentication authentication) { DirContextAdapter ctx = new DirContextAdapter(); ctx.setAttributeValue("ou", "FROM_ENTRY"); + String username = authentication.getName(); + String password = (String) authentication.getCredentials(); + if (username.equals("ben") && password.equals("benspassword")) { ctx.setDn(new DistinguishedName("cn=ben,ou=people,dc=acegisecurity,dc=org")); diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticatorTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticatorTests.java index a600456ada..9d229a742f 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticatorTests.java +++ b/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/BindAuthenticatorTests.java @@ -17,6 +17,8 @@ package org.acegisecurity.providers.ldap.authenticator; import org.acegisecurity.AcegiMessageSource; import org.acegisecurity.BadCredentialsException; +import org.acegisecurity.Authentication; +import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.ldap.AbstractLdapIntegrationTests; import org.acegisecurity.ldap.InitialDirContextFactory; @@ -36,18 +38,24 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests { //~ Instance fields ================================================================================================ private BindAuthenticator authenticator; + private Authentication bob; + private Authentication ben; + //~ Methods ======================================================================================================== public void onSetUp() { authenticator = new BindAuthenticator((InitialDirContextFactory) getContextSource()); authenticator.setMessageSource(new AcegiMessageSource()); + bob = new UsernamePasswordAuthenticationToken("bob", "bobspassword"); + ben = new UsernamePasswordAuthenticationToken("ben", "benspassword"); + } public void testAuthenticationWithCorrectPasswordSucceeds() { authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"}); - DirContextOperations user = authenticator.authenticate("bob", "bobspassword"); + DirContextOperations user = authenticator.authenticate(bob); assertEquals("bob", user.getStringAttribute("uid")); } @@ -55,7 +63,7 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests { authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"}); try { - authenticator.authenticate("nonexistentsuser", "bobspassword"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("nonexistentsuser", "password")); fail("Shouldn't be able to bind with invalid username"); } catch (BadCredentialsException expected) {} } @@ -65,14 +73,14 @@ public class BindAuthenticatorTests extends AbstractLdapIntegrationTests { authenticator.setUserSearch(new MockUserSearch(ctx)); authenticator.afterPropertiesSet(); - authenticator.authenticate("bob", "bobspassword"); + authenticator.authenticate(bob); } public void testAuthenticationWithWrongPasswordFails() { authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"}); try { - authenticator.authenticate("bob", "wrongpassword"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("bob", "wrongpassword")); fail("Shouldn't be able to bind with wrong password"); } catch (BadCredentialsException expected) {} } diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java index d7b9c6b217..ff181d83a9 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java +++ b/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorMockTests.java @@ -16,6 +16,7 @@ package org.acegisecurity.providers.ldap.authenticator; import org.acegisecurity.ldap.MockInitialDirContextFactory; +import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.jmock.Mock; import org.jmock.MockObjectTestCase; @@ -57,6 +58,6 @@ public class PasswordComparisonAuthenticatorMockTests extends MockObjectTestCase .with(eq("cn=Bob, ou=people"), eq("(userPassword={0})"), NOT_NULL, NOT_NULL) .will(returnValue(searchResults.getAll())); mockCtx.expects(atLeastOnce()).method("close"); - authenticator.authenticate("Bob", "bobspassword"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("Bob","bobspassword")); } } diff --git a/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java b/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java index a7d44da48c..a1d05e7c30 100644 --- a/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java +++ b/core/src/test/java/org/acegisecurity/providers/ldap/authenticator/PasswordComparisonAuthenticatorTests.java @@ -16,11 +16,13 @@ package org.acegisecurity.providers.ldap.authenticator; import org.acegisecurity.BadCredentialsException; +import org.acegisecurity.Authentication; import org.acegisecurity.ldap.AbstractLdapIntegrationTests; import org.acegisecurity.ldap.InitialDirContextFactory; import org.acegisecurity.providers.encoding.PlaintextPasswordEncoder; +import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.userdetails.UsernameNotFoundException; import org.springframework.ldap.core.DirContextAdapter; @@ -38,6 +40,8 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio //~ Instance fields ================================================================================================ private PasswordComparisonAuthenticator authenticator; + private Authentication bob; + private Authentication ben; //~ Methods ======================================================================================================== @@ -45,6 +49,8 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio super.onSetUp(); authenticator = new PasswordComparisonAuthenticator((InitialDirContextFactory) getContextSource()); authenticator.setUserDnPatterns(new String[] {"uid={0},ou=people"}); + bob = new UsernamePasswordAuthenticationToken("bob", "bobspassword"); + ben = new UsernamePasswordAuthenticationToken("ben", "benspassword"); } public void onTearDown() throws Exception { @@ -53,7 +59,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio } public void testAllAttributesAreRetrievedByDefault() { - DirContextAdapter user = (DirContextAdapter) authenticator.authenticate("bob", "bobspassword"); + DirContextAdapter user = (DirContextAdapter) authenticator.authenticate(bob); //System.out.println(user.getAttributes().toString()); assertEquals("User should have 5 attributes", 5, user.getAttributes().size()); } @@ -65,19 +71,19 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio authenticator.afterPropertiesSet(); try { - authenticator.authenticate("Joe", "password"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("Joe", "pass")); fail("Expected exception on failed user search"); } catch (UsernameNotFoundException expected) {} } public void testLocalComparisonSucceedsWithShaEncodedPassword() { // Ben's password is SHA encoded - authenticator.authenticate("ben", "benspassword"); + authenticator.authenticate(ben); } public void testLocalPasswordComparisonFailsWithWrongPassword() { try { - authenticator.authenticate("Bob", "wrongpassword"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("bob", "wrongpass")); fail("Authentication should fail with wrong password."); } catch (BadCredentialsException expected) {} } @@ -87,14 +93,14 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio // Don't retrieve the password authenticator.setUserAttributes(new String[] {"uid", "cn", "sn"}); try { - authenticator.authenticate("Bob", "wrongpassword"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("bob", "wrongpass")); fail("Authentication should fail with wrong password."); } catch(BadCredentialsException expected) { } } public void testLocalPasswordComparisonSucceedsWithCorrectPassword() { - DirContextOperations user = authenticator.authenticate("bob", "bobspassword"); + DirContextOperations user = authenticator.authenticate(bob); // check username is retrieved. assertEquals("bob", user.getStringAttribute("uid")); String password = new String((byte[])user.getObjectAttribute("userPassword")); @@ -103,14 +109,14 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio public void testMultipleDnPatternsWorkOk() { authenticator.setUserDnPatterns(new String[] {"uid={0},ou=nonexistent", "uid={0},ou=people"}); - authenticator.authenticate("Bob", "bobspassword"); + authenticator.authenticate(bob); } public void testOnlySpecifiedAttributesAreRetrieved() throws Exception { authenticator.setUserAttributes(new String[] {"uid", "userPassword"}); authenticator.setPasswordEncoder(new PlaintextPasswordEncoder()); - DirContextAdapter user = (DirContextAdapter) authenticator.authenticate("Bob", "bobspassword"); + DirContextAdapter user = (DirContextAdapter) authenticator.authenticate(bob); assertEquals("Should have retrieved 2 attribute (uid, userPassword)", 2, user.getAttributes().size()); } @@ -119,13 +125,13 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio authenticator.setUserAttributes(new String[] {"uid"}); // Bob has a plaintext password. authenticator.setPasswordEncoder(new PlaintextPasswordEncoder()); - authenticator.authenticate("bob", "bobspassword"); + authenticator.authenticate(bob); } public void testLdapCompareSucceedsWithShaEncodedPassword() { // Don't retrieve the password authenticator.setUserAttributes(new String[] {"uid"}); - authenticator.authenticate("ben", "benspassword"); + authenticator.authenticate(ben); } public void testPasswordEncoderCantBeNull() { @@ -135,16 +141,16 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio } catch (IllegalArgumentException expected) {} } - public void testUseOfDifferentPasswordAttribute() { + public void testUseOfDifferentPasswordAttributeSucceeds() { authenticator.setPasswordAttributeName("uid"); - authenticator.authenticate("bob", "bob"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("bob", "bob")); } public void testLdapCompareWithDifferentPasswordAttributeSucceeds() { authenticator.setUserAttributes(new String[] {"uid"}); authenticator.setPasswordEncoder(new PlaintextPasswordEncoder()); authenticator.setPasswordAttributeName("cn"); - authenticator.authenticate("bob", "Bob Hamilton"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("ben", "Ben Alex")); } public void testWithUserSearch() { @@ -155,6 +161,6 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio ctx.setAttributeValue("userPassword", "bobspassword"); authenticator.setUserSearch(new MockUserSearch(ctx)); - authenticator.authenticate("ShouldntBeUsed", "bobspassword"); + authenticator.authenticate(new UsernamePasswordAuthenticationToken("shouldntbeused", "bobspassword")); } } diff --git a/pom.xml b/pom.xml index 48f9f10b92..d785ece8f9 100644 --- a/pom.xml +++ b/pom.xml @@ -12,7 +12,8 @@ adapters portlet samples - + ntlm + Acegi Security System for Spring diff --git a/releasebuild.sh b/releasebuild.sh index 1c51026586..8bdca5ae7f 100644 --- a/releasebuild.sh +++ b/releasebuild.sh @@ -178,7 +178,14 @@ ls $RELEASE_DIR | grep -v sha | grep -v md5 | xargs tar -cjf $PROJECT_NAME-$RELE # Create source archive -tar --exclude='*/.svn' -cjf $PROJECT_NAME-$RELEASE_VERSION-src.tar.bz2 notice.txt src-readme.txt license.txt -C core/src/main/java/ org +tar --exclude='*/.svn' -cjf $PROJECT_NAME-$RELEASE_VERSION-src.tar.bz2 notice.txt src-readme.txt license.txt \ + -C core/src/main/java/ org \ + -C ${PROJ_DIR}/core-tiger/main/java org \ + -C ${PROJ_DIR}/adapters/jetty/main/java org \ + -C ${PROJ_DIR}/adapters/jboss/main/java org \ + -C ${PROJ_DIR}/adapters/resin/main/java org \ + -C ${PROJ_DIR}/adapters/cas/main/java org \ + -C ${PROJ_DIR}/adapters/catalina/main/java org