diff --git a/config/src/integration-test/groovy/org/springframework/security/config/ldap/LdapProviderBeanDefinitionParserTests.groovy b/config/src/integration-test/groovy/org/springframework/security/config/ldap/LdapProviderBeanDefinitionParserTests.groovy index f01ce9c0a6..25ef3580a3 100644 --- a/config/src/integration-test/groovy/org/springframework/security/config/ldap/LdapProviderBeanDefinitionParserTests.groovy +++ b/config/src/integration-test/groovy/org/springframework/security/config/ldap/LdapProviderBeanDefinitionParserTests.groovy @@ -1,7 +1,13 @@ package org.springframework.security.config.ldap + +import static org.mockito.Mockito.* + import java.text.MessageFormat + import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer +import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.security.config.AbstractXmlConfigTests import org.springframework.security.config.BeanIds import org.springframework.security.util.FieldUtils @@ -122,6 +128,28 @@ class LdapProviderBeanDefinitionParserTests extends AbstractXmlConfigTests { notThrown(AuthenticationException) } + def 'SEC-2472: Supports Crypto PasswordEncoder'() { + setup: + xml.'ldap-server'(ldif:'test-server.ldif') + xml.'authentication-manager'{ + 'ldap-authentication-provider'('user-dn-pattern': 'uid={0},ou=people') { + 'password-compare'() { + 'password-encoder'(ref: 'pe') + } + } + } + xml.'b:bean'(id:'pe','class':BCryptPasswordEncoder.class.name) + + createAppContext('') + def am = appContext.getBean(BeanIds.AUTHENTICATION_MANAGER) + + when: + def auth = am.authenticate(new UsernamePasswordAuthenticationToken("bcrypt", 'password')) + + then: + auth != null + } + def inetOrgContextMapperIsSupported() { xml.'ldap-server'(url: 'ldap://127.0.0.1:343/dc=springframework,dc=org') xml.'authentication-manager'{ diff --git a/config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/LdapAuthenticationProviderBuilderSecurityBuilderTests.groovy b/config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/LdapAuthenticationProviderBuilderSecurityBuilderTests.groovy index 16cb7117c4..d03bda4ae7 100644 --- a/config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/LdapAuthenticationProviderBuilderSecurityBuilderTests.groovy +++ b/config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/LdapAuthenticationProviderBuilderSecurityBuilderTests.groovy @@ -28,6 +28,7 @@ import org.springframework.security.config.annotation.BaseSpringSpec import org.springframework.security.config.annotation.SecurityBuilder; import org.springframework.security.config.annotation.authentication.AuthenticationManagerBuilder import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; +import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.security.ldap.DefaultSpringSecurityContextSource; import org.springframework.security.ldap.authentication.LdapAuthenticationProvider; import org.springframework.security.ldap.server.ApacheDSContainer; @@ -134,6 +135,30 @@ class LdapAuthenticationProviderBuilderSecurityBuilderTests extends BaseSpringSp } } + def "SEC-2472: Can use crypto PasswordEncoder"() { + setup: + PasswordEncoderConfig.PE = Mock(PasswordEncoder) + loadConfig(PasswordEncoderConfig) + when: + AuthenticationManager auth = context.getBean(AuthenticationManager) + then: + auth.authenticate(new UsernamePasswordAuthenticationToken("admin","password")).authorities.collect { it.authority }.sort() == ["ROLE_ADMIN","ROLE_USER"] + PasswordEncoderConfig.PE.matches(_, _) << true + } + + @Configuration + static class PasswordEncoderConfig extends BaseLdapServerConfig { + static PasswordEncoder PE + protected void configure(AuthenticationManagerBuilder auth) throws Exception { + auth + .ldapAuthentication() + .contextSource(contextSource()) + .passwordEncoder(PE) + .groupSearchBase("ou=groups") + .userDnPatterns("uid={0},ou=people"); + } + } + def ldapProvider() { context.getBean(AuthenticationManager).providers[0] } diff --git a/config/src/integration-test/resources/test-server.ldif b/config/src/integration-test/resources/test-server.ldif index 599c615c4f..39104a4829 100644 --- a/config/src/integration-test/resources/test-server.ldif +++ b/config/src/integration-test/resources/test-server.ldif @@ -28,6 +28,16 @@ sn: Alex uid: ben userPassword: {SHA}nFCebWjxfaLbHHG1Qk5UU4trbvQ= +dn: uid=bcrypt,ou=people,dc=springframework,dc=org +objectclass: top +objectclass: person +objectclass: organizationalPerson +objectclass: inetOrgPerson +cn: BCrypt user +sn: BCrypt +uid: bcrypt +userPassword: $2a$10$FBAKClV1zBIOOC9XMXf3AO8RoGXYVYsfvUdoLxGkd/BnXEn4tqT3u + dn: uid=bob,ou=people,dc=springframework,dc=org objectclass: top objectclass: person diff --git a/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/ldap/LdapAuthenticationProviderConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/ldap/LdapAuthenticationProviderConfigurer.java index 7e5f761371..4ad740b9fd 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/ldap/LdapAuthenticationProviderConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/authentication/configurers/ldap/LdapAuthenticationProviderConfigurer.java @@ -40,6 +40,7 @@ import org.springframework.security.ldap.userdetails.LdapAuthoritiesPopulator; import org.springframework.security.ldap.userdetails.LdapUserDetailsMapper; import org.springframework.security.ldap.userdetails.PersonContextMapper; import org.springframework.security.ldap.userdetails.UserDetailsContextMapper; +import org.springframework.util.Assert; /** * Configures LDAP {@link AuthenticationProvider} in the {@link ProviderManagerBuilder}. @@ -204,12 +205,40 @@ public class LdapAuthenticationProviderConfigurer passwordEncoder(PasswordEncoder passwordEncoder) { this.passwordEncoder = passwordEncoder; return this; } + /** + * Specifies the {@link org.springframework.security.crypto.password.PasswordEncoder} to be used when authenticating with + * password comparison. + * + * @param passwordEncoder the {@link org.springframework.security.crypto.password.PasswordEncoder} to use + * @return the {@link LdapAuthenticationProviderConfigurer} for further customization + */ + public LdapAuthenticationProviderConfigurer passwordEncoder(final org.springframework.security.crypto.password.PasswordEncoder passwordEncoder) { + Assert.notNull(passwordEncoder, "passwordEncoder must not be null."); + passwordEncoder(new PasswordEncoder() { + public String encodePassword(String rawPass, Object salt) { + checkSalt(salt); + return passwordEncoder.encode(rawPass); + } + + public boolean isPasswordValid(String encPass, String rawPass, Object salt) { + checkSalt(salt); + return passwordEncoder.matches(rawPass, encPass); + } + + private void checkSalt(Object salt) { + Assert.isNull(salt, "Salt value must be null when used with crypto module PasswordEncoder"); + } + }); + return this; + } + /** * If your users are at a fixed location in the directory (i.e. you can work * out the DN directly from the username without doing a directory search), diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticatorTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticatorTests.java index 654fb69697..da98e01626 100644 --- a/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticatorTests.java +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticatorTests.java @@ -20,6 +20,7 @@ import org.junit.*; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.authentication.encoding.LdapShaPasswordEncoder; +import org.springframework.security.authentication.encoding.PasswordEncoder; import org.springframework.security.authentication.encoding.PlaintextPasswordEncoder; import org.springframework.security.core.Authentication; import org.springframework.security.core.userdetails.UsernameNotFoundException; @@ -112,7 +113,7 @@ public class PasswordComparisonAuthenticatorTests extends AbstractLdapIntegratio @Test(expected = IllegalArgumentException.class) public void testPasswordEncoderCantBeNull() { - authenticator.setPasswordEncoder(null); + authenticator.setPasswordEncoder((PasswordEncoder)null); } @Test diff --git a/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java b/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java index 9ce5da4695..77cd7b62be 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java +++ b/ldap/src/main/java/org/springframework/security/ldap/authentication/PasswordComparisonAuthenticator.java @@ -18,6 +18,7 @@ package org.springframework.security.ldap.authentication; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.ldap.NameNotFoundException; +import org.springframework.ldap.NamingException; import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.support.BaseLdapPathContextSource; import org.springframework.security.authentication.BadCredentialsException; @@ -52,6 +53,7 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic private PasswordEncoder passwordEncoder = new LdapShaPasswordEncoder(); private String passwordAttributeName = "userPassword"; + private boolean usePasswordAttrCompare = false; //~ Constructors =================================================================================================== @@ -95,15 +97,25 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic user.getDn() +"'"); } + if (usePasswordAttrCompare && isPasswordAttrCompare(user, password)) { + return user; + } else if(isLdapPasswordCompare(user, ldapTemplate, password)) { + return user; + } + throw new BadCredentialsException(messages.getMessage("PasswordComparisonAuthenticator.badCredentials", + "Bad credentials")); + } + + private boolean isPasswordAttrCompare(DirContextOperations user, String password) { + Object passwordAttrValue = user.getObjectAttribute(passwordAttributeName); + return passwordEncoder.isPasswordValid(new String((byte[])passwordAttrValue), password, null); + } + + private boolean isLdapPasswordCompare(DirContextOperations user, + SpringSecurityLdapTemplate ldapTemplate, String password) { String encodedPassword = passwordEncoder.encodePassword(password, null); byte[] passwordBytes = Utf8.encode(encodedPassword); - - if (!ldapTemplate.compare(user.getDn().toString(), passwordAttributeName, passwordBytes)) { - throw new BadCredentialsException(messages.getMessage("PasswordComparisonAuthenticator.badCredentials", - "Bad credentials")); - } - - return user; + return ldapTemplate.compare(user.getDn().toString(), passwordAttributeName, passwordBytes); } public void setPasswordAttributeName(String passwordAttribute) { @@ -111,8 +123,40 @@ public final class PasswordComparisonAuthenticator extends AbstractLdapAuthentic this.passwordAttributeName = passwordAttribute; } - public void setPasswordEncoder(PasswordEncoder passwordEncoder) { + private void setPasswordEncoder(PasswordEncoder passwordEncoder) { Assert.notNull(passwordEncoder, "passwordEncoder must not be null."); this.passwordEncoder = passwordEncoder; } + + public void setPasswordEncoder(Object passwordEncoder) { + if (passwordEncoder instanceof PasswordEncoder) { + this.usePasswordAttrCompare = false; + setPasswordEncoder((PasswordEncoder) passwordEncoder); + return; + } + + if (passwordEncoder instanceof org.springframework.security.crypto.password.PasswordEncoder) { + final org.springframework.security.crypto.password.PasswordEncoder delegate = + (org.springframework.security.crypto.password.PasswordEncoder)passwordEncoder; + setPasswordEncoder(new PasswordEncoder() { + public String encodePassword(String rawPass, Object salt) { + checkSalt(salt); + return delegate.encode(rawPass); + } + + public boolean isPasswordValid(String encPass, String rawPass, Object salt) { + checkSalt(salt); + return delegate.matches(rawPass, encPass); + } + + private void checkSalt(Object salt) { + Assert.isNull(salt, "Salt value must be null when used with crypto module PasswordEncoder"); + } + }); + this.usePasswordAttrCompare = true; + return; + } + + throw new IllegalArgumentException("passwordEncoder must be a PasswordEncoder instance"); + } }