From 6a3e981c8020d28d561f6147d2c16346c576cd4f Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Sun, 22 Oct 2017 14:11:09 -0500 Subject: [PATCH] Remove BaseDigestPasswordEncoder from core Issue: gh-4674 --- .../authentication/PasswordEncoderParser.java | 11 -- .../security/config/spring-security-5.0.rnc | 2 +- .../security/config/spring-security-5.0.xsd | 6 - .../encoding/BaseDigestPasswordEncoder.java | 49 ------ .../encoding/BasePasswordEncoder.java | 105 ------------ .../encoding/BasePasswordEncoderTests.java | 155 ------------------ docs/manual/src/docs/asciidoc/index.adoc | 5 - 7 files changed, 1 insertion(+), 332 deletions(-) delete mode 100644 core/src/main/java/org/springframework/security/authentication/encoding/BaseDigestPasswordEncoder.java delete mode 100644 core/src/main/java/org/springframework/security/authentication/encoding/BasePasswordEncoder.java delete mode 100644 core/src/test/java/org/springframework/security/authentication/encoding/BasePasswordEncoderTests.java diff --git a/config/src/main/java/org/springframework/security/config/authentication/PasswordEncoderParser.java b/config/src/main/java/org/springframework/security/config/authentication/PasswordEncoderParser.java index 3ed56095ed..154121b711 100644 --- a/config/src/main/java/org/springframework/security/config/authentication/PasswordEncoderParser.java +++ b/config/src/main/java/org/springframework/security/config/authentication/PasswordEncoderParser.java @@ -26,7 +26,6 @@ import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.factory.xml.ParserContext; -import org.springframework.security.authentication.encoding.BaseDigestPasswordEncoder; import org.springframework.security.authentication.encoding.LdapShaPasswordEncoder; import org.springframework.security.config.Elements; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; @@ -107,16 +106,6 @@ public class PasswordEncoderParser { Class beanClass = ENCODER_CLASSES.get(hash); BeanDefinitionBuilder beanBldr = BeanDefinitionBuilder .rootBeanDefinition(beanClass); - - if (useBase64) { - if (BaseDigestPasswordEncoder.class.isAssignableFrom(beanClass)) { - beanBldr.addPropertyValue("encodeHashAsBase64", "true"); - } - else { - logger.warn(ATT_BASE_64 + " isn't compatible with " + hash - + " and will be ignored"); - } - } return beanBldr.getBeanDefinition(); } diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.0.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-5.0.rnc index 4c6a3b3725..beda9c117c 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.0.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.0.rnc @@ -56,7 +56,7 @@ password-encoder = ## element which defines a password encoding strategy. Used by an authentication provider to convert submitted passwords to hashed versions, for example. element password-encoder {password-encoder.attlist, salt-source?} password-encoder.attlist &= - ref | (hash? & base64?) + ref | (hash) salt-source = ## Password salting strategy. A system-wide constant or a property from the UserDetails object can be used. diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-5.0.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-5.0.xsd index 0f5907c9e6..bf71adc097 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-5.0.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-5.0.xsd @@ -147,12 +147,6 @@ - - - Whether a string should be base64 encoded - - - diff --git a/core/src/main/java/org/springframework/security/authentication/encoding/BaseDigestPasswordEncoder.java b/core/src/main/java/org/springframework/security/authentication/encoding/BaseDigestPasswordEncoder.java deleted file mode 100644 index d8195d0d9e..0000000000 --- a/core/src/main/java/org/springframework/security/authentication/encoding/BaseDigestPasswordEncoder.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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.authentication.encoding; - -/** - *

- * Convenience base for digest password encoders. - *

- * - * @author colin sampaleanu - */ -public abstract class BaseDigestPasswordEncoder extends BasePasswordEncoder { - // ~ Instance fields - // ================================================================================================ - - private boolean encodeHashAsBase64 = false; - - // ~ Methods - // ======================================================================================================== - - public boolean getEncodeHashAsBase64() { - return encodeHashAsBase64; - } - - /** - * The encoded password is normally returned as Hex (32 char) version of the hash - * bytes. Setting this property to true will cause the encoded pass to be returned as - * Base64 text, which will consume 24 characters. - * - * @param encodeHashAsBase64 set to true for Base64 output - */ - public void setEncodeHashAsBase64(boolean encodeHashAsBase64) { - this.encodeHashAsBase64 = encodeHashAsBase64; - } -} diff --git a/core/src/main/java/org/springframework/security/authentication/encoding/BasePasswordEncoder.java b/core/src/main/java/org/springframework/security/authentication/encoding/BasePasswordEncoder.java deleted file mode 100644 index ee8d2d3f94..0000000000 --- a/core/src/main/java/org/springframework/security/authentication/encoding/BasePasswordEncoder.java +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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.authentication.encoding; - -/** - *

- * Convenience base for all password encoders. - *

- * - * @author Ben Alex - */ -public abstract class BasePasswordEncoder implements PasswordEncoder { - // ~ Methods - // ======================================================================================================== - - /** - * Used by subclasses to extract the password and salt from a merged - * String created using - * {@link #mergePasswordAndSalt(String,Object,boolean)}. - *

- * The first element in the returned array is the password. The second element is the - * salt. The salt array element will always be present, even if no salt was found in - * the mergedPasswordSalt argument. - *

- * - * @param mergedPasswordSalt as generated by mergePasswordAndSalt - * - * @return an array, in which the first element is the password and the second the - * salt - * - * @throws IllegalArgumentException if mergedPasswordSalt is null or empty. - */ - protected String[] demergePasswordAndSalt(String mergedPasswordSalt) { - if ((mergedPasswordSalt == null) || "".equals(mergedPasswordSalt)) { - throw new IllegalArgumentException("Cannot pass a null or empty String"); - } - - String password = mergedPasswordSalt; - String salt = ""; - - int saltBegins = mergedPasswordSalt.lastIndexOf("{"); - - if ((saltBegins != -1) && ((saltBegins + 1) < mergedPasswordSalt.length())) { - salt = mergedPasswordSalt.substring(saltBegins + 1, - mergedPasswordSalt.length() - 1); - password = mergedPasswordSalt.substring(0, saltBegins); - } - - return new String[] { password, salt }; - } - - /** - * Used by subclasses to generate a merged password and salt String. - *

- * The generated password will be in the form of password{salt}. - *

- *

- * A null can be passed to either method, and will be handled correctly. - * If the salt is null or empty, the resulting generated - * password will simply be the passed password. The toString - * method of the salt will be used to represent the salt. - *

- * - * @param password the password to be used (can be null) - * @param salt the salt to be used (can be null) - * @param strict ensures salt doesn't contain the delimiters - * - * @return a merged password and salt String - * - * @throws IllegalArgumentException if the salt contains '{' or '}' characters. - */ - protected String mergePasswordAndSalt(String password, Object salt, boolean strict) { - if (password == null) { - password = ""; - } - - if (strict && (salt != null)) { - if ((salt.toString().lastIndexOf("{") != -1) - || (salt.toString().lastIndexOf("}") != -1)) { - throw new IllegalArgumentException("Cannot use { or } in salt.toString()"); - } - } - - if ((salt == null) || "".equals(salt)) { - return password; - } - else { - return password + "{" + salt.toString() + "}"; - } - } -} diff --git a/core/src/test/java/org/springframework/security/authentication/encoding/BasePasswordEncoderTests.java b/core/src/test/java/org/springframework/security/authentication/encoding/BasePasswordEncoderTests.java deleted file mode 100644 index a762785328..0000000000 --- a/core/src/test/java/org/springframework/security/authentication/encoding/BasePasswordEncoderTests.java +++ /dev/null @@ -1,155 +0,0 @@ -/* - * Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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.authentication.encoding; - -import static org.assertj.core.api.Assertions.*; - -import org.junit.Test; - - -/** - *

- * TestCase for BasePasswordEncoder. - *

- * - * @author Ben Alex - */ -public class BasePasswordEncoderTests { - // ~ Methods - // ======================================================================================================== - - @Test - public void testDemergeHandlesEmptyAndNullSalts() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - String merged = pwd.nowMergePasswordAndSalt("password", null, true); - - String[] demerged = pwd.nowDemergePasswordAndSalt(merged); - assertThat(demerged[0]).isEqualTo("password"); - assertThat(demerged[1]).isEqualTo(""); - - merged = pwd.nowMergePasswordAndSalt("password", "", true); - - demerged = pwd.nowDemergePasswordAndSalt(merged); - assertThat(demerged[0]).isEqualTo("password"); - assertThat(demerged[1]).isEqualTo(""); - } - @Test - public void testDemergeWithEmptyStringIsRejected() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - try { - pwd.nowDemergePasswordAndSalt(""); - fail("Should have thrown IllegalArgumentException"); - } - catch (IllegalArgumentException expected) { - assertThat(expected.getMessage()).isEqualTo("Cannot pass a null or empty String"); - } - } - @Test - public void testDemergeWithNullIsRejected() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - try { - pwd.nowDemergePasswordAndSalt(null); - fail("Should have thrown IllegalArgumentException"); - } - catch (IllegalArgumentException expected) { - assertThat(expected.getMessage()).isEqualTo("Cannot pass a null or empty String"); - } - } - @Test - public void testMergeDemerge() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - String merged = pwd.nowMergePasswordAndSalt("password", "foo", true); - assertThat(merged).isEqualTo("password{foo}"); - - String[] demerged = pwd.nowDemergePasswordAndSalt(merged); - assertThat(demerged[0]).isEqualTo("password"); - assertThat(demerged[1]).isEqualTo("foo"); - } - @Test - public void testMergeDemergeWithDelimitersInPassword() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - String merged = pwd.nowMergePasswordAndSalt("p{ass{w{o}rd", "foo", true); - assertThat(merged).isEqualTo("p{ass{w{o}rd{foo}"); - - String[] demerged = pwd.nowDemergePasswordAndSalt(merged); - - assertThat(demerged[0]).isEqualTo("p{ass{w{o}rd"); - assertThat(demerged[1]).isEqualTo("foo"); - } - @Test - public void testMergeDemergeWithNullAsPassword() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - String merged = pwd.nowMergePasswordAndSalt(null, "foo", true); - assertThat(merged).isEqualTo("{foo}"); - - String[] demerged = pwd.nowDemergePasswordAndSalt(merged); - assertThat(demerged[0]).isEqualTo(""); - assertThat(demerged[1]).isEqualTo("foo"); - } - @Test - public void testStrictMergeRejectsDelimitersInSalt1() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - try { - pwd.nowMergePasswordAndSalt("password", "f{oo", true); - fail("Should have thrown IllegalArgumentException"); - } - catch (IllegalArgumentException expected) { - assertThat(expected.getMessage()).isEqualTo("Cannot use { or } in salt.toString()"); - } - } - @Test - public void testStrictMergeRejectsDelimitersInSalt2() { - MockPasswordEncoder pwd = new MockPasswordEncoder(); - - try { - pwd.nowMergePasswordAndSalt("password", "f}oo", true); - fail("Should have thrown IllegalArgumentException"); - } - catch (IllegalArgumentException expected) { - assertThat(expected.getMessage()).isEqualTo("Cannot use { or } in salt.toString()"); - } - } - - // ~ Inner Classes - // ================================================================================================== - - private class MockPasswordEncoder extends BasePasswordEncoder { - public String encodePassword(String rawPass, Object salt) { - throw new UnsupportedOperationException("mock method not implemented"); - } - - public boolean isPasswordValid(String encPass, String rawPass, Object salt) { - throw new UnsupportedOperationException("mock method not implemented"); - } - - public String[] nowDemergePasswordAndSalt(String password) { - return demergePasswordAndSalt(password); - } - - public String nowMergePasswordAndSalt(String password, Object salt, boolean strict) { - return mergePasswordAndSalt(password, salt, strict); - } - } -} - diff --git a/docs/manual/src/docs/asciidoc/index.adoc b/docs/manual/src/docs/asciidoc/index.adoc index 21cba98a0a..df8d3d9416 100644 --- a/docs/manual/src/docs/asciidoc/index.adoc +++ b/docs/manual/src/docs/asciidoc/index.adoc @@ -9185,11 +9185,6 @@ Authentication providers can optionally be configured to use a password encoder ===== Attributes -[[nsa-password-encoder-base64]] -* **base64** -Whether a string should be base64 encoded - - [[nsa-password-encoder-hash]] * **hash** Defines the hashing algorithm used on user passwords. We recommend strongly against using MD4, as it is a very weak hashing algorithm.