diff --git a/ldap/spring-security-ldap.gradle b/ldap/spring-security-ldap.gradle index 41b3fa1ea1..7988e2e772 100644 --- a/ldap/spring-security-ldap.gradle +++ b/ldap/spring-security-ldap.gradle @@ -20,12 +20,15 @@ dependencies { exclude(group: 'org.springframework.data', module: 'spring-data-commons') } + testCompile project(':spring-security-test') testCompile 'org.slf4j:jcl-over-slf4j' testCompile 'org.slf4j:slf4j-api' } integrationTest { - include('**/ApacheDSServerIntegrationTests.class', '**/ApacheDSEmbeddedLdifTests.class') + include('**/ApacheDSServerIntegrationTests.class', + '**/ApacheDSEmbeddedLdifTests.class', + '**/LdapUserDetailsManagerModifyPasswordTests.class') // exclude('**/OpenLDAPIntegrationTestSuite.class') maxParallelForks = 1 } diff --git a/ldap/src/integration-test/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManagerModifyPasswordTests.java b/ldap/src/integration-test/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManagerModifyPasswordTests.java new file mode 100644 index 0000000000..ede18aa5be --- /dev/null +++ b/ldap/src/integration-test/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManagerModifyPasswordTests.java @@ -0,0 +1,120 @@ +/* + * Copyright 2002-2018 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.ldap.userdetails; + +import javax.annotation.PreDestroy; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.ldap.core.ContextSource; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.ldap.DefaultLdapUsernameToDnMapper; +import org.springframework.security.ldap.DefaultSpringSecurityContextSource; +import org.springframework.security.ldap.SpringSecurityLdapTemplate; +import org.springframework.security.ldap.server.UnboundIdContainer; +import org.springframework.security.test.context.annotation.SecurityTestExecutionListeners; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +/** + * Tests for {@link LdapUserDetailsManager#changePassword}, specifically relating to the + * use of the Modify Password Extended Operation. + * + * @author Josh Cummings + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SecurityTestExecutionListeners +public class LdapUserDetailsManagerModifyPasswordTests { + + ConfigurableApplicationContext context; + + LdapUserDetailsManager userDetailsManager; + ContextSource contextSource; + + @Before + public void setup() { + this.context = new AnnotationConfigApplicationContext(ContainerConfiguration.class, LdapConfiguration.class); + this.contextSource = this.context.getBean(ContextSource.class); + + this.userDetailsManager = new LdapUserDetailsManager(this.contextSource); + this.userDetailsManager.setUsePasswordModifyExtensionOperation(true); + this.userDetailsManager.setUsernameMapper(new DefaultLdapUsernameToDnMapper("ou=people", "uid")); + } + + @After + public void teardown() { + this.context.close(); + } + + @Test + @WithMockUser(username="bob", password="bobspassword", authorities="ROLE_USER") + public void changePasswordWhenOldPasswordIsIncorrectThenThrowsException() { + assertThatCode(() -> + this.userDetailsManager.changePassword("wrongoldpassword", "bobsnewpassword")) + .isInstanceOf(BadCredentialsException.class); + } + + @Test + @WithMockUser(username="bob", password="bobspassword", authorities="ROLE_USER") + public void changePasswordWhenOldPasswordIsCorrectThenPasses() { + SpringSecurityLdapTemplate template = new SpringSecurityLdapTemplate(this.contextSource); + + this.userDetailsManager.changePassword("bobspassword", + "bobsshinynewandformidablylongandnearlyimpossibletorememberthoughdemonstrablyhardtocrackduetoitshighlevelofentropypasswordofjustice"); + + assertThat(template.compare("uid=bob,ou=people", "userPassword", + "bobsshinynewandformidablylongandnearlyimpossibletorememberthoughdemonstrablyhardtocrackduetoitshighlevelofentropypasswordofjustice")).isTrue(); + } + + @Configuration + static class LdapConfiguration { + @Autowired UnboundIdContainer container; + + @Bean + ContextSource contextSource() throws Exception { + return new DefaultSpringSecurityContextSource("ldap://127.0.0.1:" + + this.container.getPort() + "/dc=springframework,dc=org"); + } + } + + @Configuration + static class ContainerConfiguration { + UnboundIdContainer container = new UnboundIdContainer("dc=springframework,dc=org", + "classpath:test-server.ldif"); + + @Bean + UnboundIdContainer ldapContainer() { + this.container.setPort(0); + return this.container; + } + + @PreDestroy + void shutdown() { + this.container.stop(); + } + } +} diff --git a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java index d0b6758e0a..bf694d5d4c 100644 --- a/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java +++ b/ldap/src/main/java/org/springframework/security/ldap/userdetails/LdapUserDetailsManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited + * Copyright 2002-2018 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. @@ -15,12 +15,13 @@ */ package org.springframework.security.ldap.userdetails; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.util.Arrays; import java.util.Collection; import java.util.LinkedList; import java.util.List; import java.util.ListIterator; - import javax.naming.Context; import javax.naming.NameNotFoundException; import javax.naming.NamingEnumeration; @@ -32,10 +33,13 @@ import javax.naming.directory.DirContext; import javax.naming.directory.ModificationItem; import javax.naming.directory.SearchControls; import javax.naming.directory.SearchResult; +import javax.naming.ldap.ExtendedRequest; +import javax.naming.ldap.ExtendedResponse; import javax.naming.ldap.LdapContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.ldap.core.AttributesMapper; import org.springframework.ldap.core.AttributesMapperCallbackHandler; import org.springframework.ldap.core.ContextExecutor; @@ -70,6 +74,7 @@ import org.springframework.util.Assert; * setup. * * @author Luke Taylor + * @author Josh Cummings * @since 2.0 */ public class LdapUserDetailsManager implements UserDetailsManager { @@ -123,6 +128,8 @@ public class LdapUserDetailsManager implements UserDetailsManager { private String[] attributesToRetrieve; + private boolean usePasswordModifyExtensionOperation = false; + public LdapUserDetailsManager(ContextSource contextSource) { template = new LdapTemplate(contextSource); } @@ -157,8 +164,21 @@ public class LdapUserDetailsManager implements UserDetailsManager { /** * Changes the password for the current user. The username is obtained from the * security context. + * + * There are two supported strategies for modifying the user's password depending on + * the capabilities of the corresponding LDAP server. + * *

- * If the old password is supplied, the update will be made by rebinding as the user, + * Configured one way, this method will modify the user's password via the + * + * LDAP Password Modify Extended Operation + * . + * + * See {@link LdapUserDetailsManager#setUsePasswordModifyExtensionOperation(boolean)} for details. + *

+ * + *

+ * By default, though, if the old password is supplied, the update will be made by rebinding as the user, * thus modifying the password using the user's permissions. If * oldPassword is null, the update will be attempted using a standard * read/write context supplied by the context source. @@ -178,38 +198,13 @@ public class LdapUserDetailsManager implements UserDetailsManager { logger.debug("Changing password for user '" + username); - final DistinguishedName dn = usernameMapper.buildDn(username); - final ModificationItem[] passwordChange = new ModificationItem[] { new ModificationItem( - DirContext.REPLACE_ATTRIBUTE, new BasicAttribute(passwordAttributeName, - newPassword)) }; + DistinguishedName userDn = usernameMapper.buildDn(username); - if (oldPassword == null) { - template.modifyAttributes(dn, passwordChange); - return; + if (usePasswordModifyExtensionOperation) { + changePasswordUsingExtensionOperation(userDn, oldPassword, newPassword); + } else { + changePasswordUsingAttributeModification(userDn, oldPassword, newPassword); } - - template.executeReadWrite(new ContextExecutor() { - - public Object executeWithContext(DirContext dirCtx) throws NamingException { - LdapContext ctx = (LdapContext) dirCtx; - ctx.removeFromEnvironment("com.sun.jndi.ldap.connect.pool"); - ctx.addToEnvironment(Context.SECURITY_PRINCIPAL, - LdapUtils.getFullDn(dn, ctx).toString()); - ctx.addToEnvironment(Context.SECURITY_CREDENTIALS, oldPassword); - // TODO: reconnect doesn't appear to actually change the credentials - try { - ctx.reconnect(null); - } - catch (javax.naming.AuthenticationException e) { - throw new BadCredentialsException( - "Authentication for password change failed."); - } - - ctx.modifyAttributes(dn, passwordChange); - - return null; - } - }); } /** @@ -414,4 +409,174 @@ public class LdapUserDetailsManager implements UserDetailsManager { public void setRoleMapper(AttributesMapper roleMapper) { this.roleMapper = roleMapper; } + + /** + * Sets the method by which a user's password gets modified. + * + * If set to {@code true}, then {@link LdapUserDetailsManager#changePassword} will modify + * the user's password by way of the + * Password Modify Extension Operation. + * + * If set to {@code false}, then {@link LdapUserDetailsManager#changePassword} will modify + * the user's password by directly modifying attributes on the corresponding entry. + * + * Before using this setting, ensure that the corresponding LDAP server supports this extended operation. + * + * By default, {@code usePasswordModifyExtensionOperation} is false. + * + * @param usePasswordModifyExtensionOperation + * @since 4.2.9 + */ + public void setUsePasswordModifyExtensionOperation(boolean usePasswordModifyExtensionOperation) { + this.usePasswordModifyExtensionOperation = usePasswordModifyExtensionOperation; + } + + private void changePasswordUsingAttributeModification + (DistinguishedName userDn, String oldPassword, String newPassword) { + + final ModificationItem[] passwordChange = new ModificationItem[] { new ModificationItem( + DirContext.REPLACE_ATTRIBUTE, new BasicAttribute(passwordAttributeName, + newPassword)) }; + + if (oldPassword == null) { + template.modifyAttributes(userDn, passwordChange); + return; + } + + template.executeReadWrite(dirCtx -> { + LdapContext ctx = (LdapContext) dirCtx; + ctx.removeFromEnvironment("com.sun.jndi.ldap.connect.pool"); + ctx.addToEnvironment(Context.SECURITY_PRINCIPAL, + LdapUtils.getFullDn(userDn, ctx).toString()); + ctx.addToEnvironment(Context.SECURITY_CREDENTIALS, oldPassword); + // TODO: reconnect doesn't appear to actually change the credentials + try { + ctx.reconnect(null); + } catch (javax.naming.AuthenticationException e) { + throw new BadCredentialsException( + "Authentication for password change failed."); + } + + ctx.modifyAttributes(userDn, passwordChange); + + return null; + }); + + } + + private void changePasswordUsingExtensionOperation + (DistinguishedName userDn, String oldPassword, String newPassword) { + + template.executeReadWrite(dirCtx -> { + LdapContext ctx = (LdapContext) dirCtx; + + String userIdentity = LdapUtils.getFullDn(userDn, ctx).encode(); + PasswordModifyRequest request = + new PasswordModifyRequest(userIdentity, oldPassword, newPassword); + + try { + return ctx.extendedOperation(request); + } catch (javax.naming.AuthenticationException e) { + throw new BadCredentialsException( + "Authentication for password change failed."); + } + }); + } + + /** + * An implementation of the + * + * LDAP Password Modify Extended Operation + * + * client request. + * + * Can be directed at any LDAP server that supports the Password Modify Extended Operation. + * + * @author Josh Cummings + * @since 4.2.9 + */ + private static class PasswordModifyRequest implements ExtendedRequest { + private static final byte SEQUENCE_TYPE = 48; + + private static final String PASSWORD_MODIFY_OID = "1.3.6.1.4.1.4203.1.11.1"; + private static final byte USER_IDENTITY_OCTET_TYPE = -128; + private static final byte OLD_PASSWORD_OCTET_TYPE = -127; + private static final byte NEW_PASSWORD_OCTET_TYPE = -126; + + private final ByteArrayOutputStream value = new ByteArrayOutputStream(); + + public PasswordModifyRequest(String userIdentity, String oldPassword, String newPassword) { + ByteArrayOutputStream elements = new ByteArrayOutputStream(); + + if (userIdentity != null) { + berEncode(USER_IDENTITY_OCTET_TYPE, userIdentity.getBytes(), elements); + } + + if (oldPassword != null) { + berEncode(OLD_PASSWORD_OCTET_TYPE, oldPassword.getBytes(), elements); + } + + if (newPassword != null) { + berEncode(NEW_PASSWORD_OCTET_TYPE, newPassword.getBytes(), elements); + } + + berEncode(SEQUENCE_TYPE, elements.toByteArray(), this.value); + } + + @Override + public String getID() { + return PASSWORD_MODIFY_OID; + } + + @Override + public byte[] getEncodedValue() { + return this.value.toByteArray(); + } + + @Override + public ExtendedResponse createExtendedResponse(String id, byte[] berValue, int offset, int length) { + return null; + } + + /** + * Only minimal support for + * + * BER encoding + * ; just what is necessary for the Password Modify request. + * + */ + private void berEncode(byte type, byte[] src, ByteArrayOutputStream dest) { + int length = src.length; + + dest.write(type); + + if (length < 128) { + dest.write(length); + } else if ((length & 0x0000_00FF) == length) { + dest.write((byte) 0x81); + dest.write((byte) (length & 0xFF)); + } else if ((length & 0x0000_FFFF) == length) { + dest.write((byte) 0x82); + dest.write((byte) ((length >> 8) & 0xFF)); + dest.write((byte) (length & 0xFF)); + } else if ((length & 0x00FF_FFFF) == length) { + dest.write((byte) 0x83); + dest.write((byte) ((length >> 16) & 0xFF)); + dest.write((byte) ((length >> 8) & 0xFF)); + dest.write((byte) (length & 0xFF)); + } else { + dest.write((byte) 0x84); + dest.write((byte) ((length >> 24) & 0xFF)); + dest.write((byte) ((length >> 16) & 0xFF)); + dest.write((byte) ((length >> 8) & 0xFF)); + dest.write((byte) (length & 0xFF)); + } + + try { + dest.write(src); + } catch (IOException e) { + throw new IllegalArgumentException("Failed to BER encode provided value of type: " + type); + } + } + } }