From d0332eb71ad9fab5b5ffffb13dd94564dc0981eb Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 19 Oct 2017 21:09:19 -0500 Subject: [PATCH] Add DelegatingPasswordEncoder Fixes gh-4666 --- .../password/DelegatingPasswordEncoder.java | 235 ++++++++++++++++++ .../DelegatingPasswordEncoderTests.java | 186 ++++++++++++++ 2 files changed, 421 insertions(+) create mode 100644 crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java create mode 100644 crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java diff --git a/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java new file mode 100644 index 0000000000..c42f020467 --- /dev/null +++ b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java @@ -0,0 +1,235 @@ +/* + * Copyright 2002-2017 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.crypto.password; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import java.util.Map; + +/** + * A password encoder that delegates to another PasswordEncoder based upon a prefixed + * identifier. + * + *

Constructing an instance

+ * + * + * 
+ * String idForEncode = "bcrypt";
+ * Map encoders = new HashMap<>();
+ * encoders.put(idForEncode, new BCryptPasswordEncoder());
+ * encoders.put("noop", NoOpPasswordEncoder.getInstance());
+ * encoders.put("pbkdf2", new Pbkdf2PasswordEncoder());
+ * encoders.put("scrypt", new SCryptPasswordEncoder());
+ * encoders.put("sha256", new StandardPasswordEncoder());
+ *
+ * PasswordEncoder passwordEncoder = new DelegatingPasswordEncoder(idForEncode, encoders);
+ *
+ * + * + *

Password Storage Format

+ * + * The general format for a password is: + * + * 
+ * {id}encodedPassword
+ *
+ * + * Such that "id" is an identifier used to look up which {@link PasswordEncoder} should + * be used and "encodedPassword" is the original encoded password for the selected + * {@link PasswordEncoder}. The "id" must be at the beginning of the password, start with + * "{" and end with "}". If the "id" cannot be found, the "id" will be null. + * + * For example, the following might be a list of passwords encoded using different "id". + * All of the original passwords are "password". + * + * 
+ * {bcrypt}$2a$10$dXJ3SW6G7P50lGmMkkmwe.20cQQubK3.HZWzG3YB1tlRy.fqvM/BG
+ * {noop}password
+ * {pbkdf2}5d923b44a6d129f3ddf3e3c8d29412723dcbde72445e8ef6bf3b508fbf17fa4ed4d6b99ca763d8dc
+ * {scrypt}$e0801$8bWJaSu2IKSn9Z9kM+TPXfOc/9bdYSrN1oD9qfVThWEwdRTnO7re7Ei+fUZRJ68k9lTyuTeUp4of4g24hHnazw==$OAOec05+bXxvuu/1qZ6NUR+xQYvYv7BeL1QxwRpY5Pc=
+ * {sha256}97cde38028ad898ebc02e690819fa220e88c62e0699403e94fff291cfffaf8410849f27605abcbc0
+ *
+ * + * For the DelegatingPasswordEncoder that we constructed above: + * + *
    + *
  1. The first password would have a {@code PasswordEncoder} id of "bcrypt" and + * encodedPassword of "$2a$10$dXJ3SW6G7P50lGmMkkmwe.20cQQubK3.HZWzG3YB1tlRy.fqvM/BG". + * When matching it would delegate to + * {@link org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder}
    2. + *
  2. The second password would have a {@code PasswordEncoder} id of "noop" and + * encodedPassword of "thisisextremelyunsafe". When matching it would delegate to + * {@link NoOpPasswordEncoder}
    3. + *
  3. The third password would have a {@code PasswordEncoder} id of "pbkdf2" and + * encodedPassword of + * "5d923b44a6d129f3ddf3e3c8d29412723dcbde72445e8ef6bf3b508fbf17fa4ed4d6b99ca763d8dc". + * When matching it would delegate to {@link Pbkdf2PasswordEncoder}
    4. + *
  4. The fourth password would have a {@code PasswordEncoder} id of "scrypt" and + * encodedPassword of + * "$e0801$8bWJaSu2IKSn9Z9kM+TPXfOc/9bdYSrN1oD9qfVThWEwdRTnO7re7Ei+fUZRJ68k9lTyuTeUp4of4g24hHnazw==$OAOec05+bXxvuu/1qZ6NUR+xQYvYv7BeL1QxwRpY5Pc=" + * When matching it would delegate to + * {@link org.springframework.security.crypto.scrypt.SCryptPasswordEncoder}
    5. + *
  5. The final password would have a {@code PasswordEncoder} id of "sha256" and + * encodedPassword of + * "97cde38028ad898ebc02e690819fa220e88c62e0699403e94fff291cfffaf8410849f27605abcbc0". + * When matching it would delegate to {@link StandardPasswordEncoder}
    6. + *
+ * + *

Password Encoding

+ * + * The @{code idForEncode} passed into the constructor determines which + * {@link PasswordEncoder} will be used for encoding passwords. In the + * {@code DelegatingPasswordEncoder} we constructed above, that means that the result of + * encoding "password" would be delegated to {@code BCryptPasswordEncoder} and be prefixed + * with "{bcrypt}". The end result would look like: + * + * 
+ * {bcrypt}$2a$10$dXJ3SW6G7P50lGmMkkmwe.20cQQubK3.HZWzG3YB1tlRy.fqvM/BG
+ *
+ * + *

Password Matching

+ * + * Matching is done based upon the "id" and the mapping of the "id" to the + * {@link PasswordEncoder} provided in the constructor. Our example in "Password Storage + * Format" provides a working example of how this is done. + * + * By default the result of invoking {@link #matches(CharSequence, String)} with a + * password with an "id" that is not mapped (including a null id) will result in an + * {@link IllegalArgumentException}. This behavior can be customized using + * {@link #setDefaultPasswordEncoderForMatches(PasswordEncoder)}. + * + * @author Rob Winch + * @since 5.0 + */ +public class DelegatingPasswordEncoder implements PasswordEncoder { + private static final String PREFIX = "{"; + private static final String SUFFIX = "}"; + private final Log logger = LogFactory.getLog(getClass()); + private final String idForEncode; + private final PasswordEncoder passwordEncoderForEncode; + private final Map idToPasswordEncoder; + private PasswordEncoder defaultPasswordEncoderForMatches = new UnmappedIdPasswordEncoder(); + + /** + * Creates a new instance + * @param idForEncode the id used to lookup which {@link PasswordEncoder} should be + * used for {@link #encode(CharSequence)} + * @param idToPasswordEncoder a Map of id to {@link PasswordEncoder} used to determine + * which {@link PasswordEncoder} should be used for {@link #matches(CharSequence, String)} + */ + public DelegatingPasswordEncoder(String idForEncode, + Map idToPasswordEncoder) { + if(idForEncode == null) { + throw new IllegalArgumentException("idForEncode cannot be null"); + } + if(!idToPasswordEncoder.containsKey(idForEncode)) { + throw new IllegalArgumentException("idForEncode " + idForEncode + "is not found in idToPasswordEncoder " + idToPasswordEncoder); + } + for(String id : idToPasswordEncoder.keySet()) { + if(id == null) { + continue; + } + if(id.contains(PREFIX)) { + throw new IllegalArgumentException("id " + id + " cannot contain " + PREFIX); + } + if(id.contains(SUFFIX)) { + throw new IllegalArgumentException("id " + id + " cannot contain " + SUFFIX); + } + } + this.idForEncode = idForEncode; + this.passwordEncoderForEncode = idToPasswordEncoder.get(idForEncode); + this.idToPasswordEncoder = idToPasswordEncoder; + } + + /** + * Sets the {@link PasswordEncoder} to delegate to for + * {@link #matches(CharSequence, String)} if the id is not mapped to a + * {@link PasswordEncoder}. + * + *

+ The encodedPassword provided will be the full password + * passed in including the {"id"} portion.* For example, if the password of + * "{notmapped}foobar" was used, the "id" would be "notmapped" and the encodedPassword + * passed into the {@link PasswordEncoder} would be "{notmapped}foobar". + *

+ * @param defaultPasswordEncoderForMatches the encoder to use. The default is to + * throw an {@link IllegalArgumentException} + */ + public void setDefaultPasswordEncoderForMatches( + PasswordEncoder defaultPasswordEncoderForMatches) { + if(defaultPasswordEncoderForMatches == null) { + throw new IllegalArgumentException("defaultPasswordEncoderForMatches cannot be null"); + } + this.defaultPasswordEncoderForMatches = defaultPasswordEncoderForMatches; + } + + @Override + public String encode(CharSequence rawPassword) { + return PREFIX + this.idForEncode + SUFFIX + this.passwordEncoderForEncode.encode(rawPassword); + } + + @Override + public boolean matches(CharSequence rawPassword, String prefixEncodedPassword) { + if(rawPassword == null && prefixEncodedPassword == null) { + return true; + } + String id = extractId(prefixEncodedPassword); + PasswordEncoder delegate = this.idToPasswordEncoder.get(id); + if(delegate == null) { + return this.defaultPasswordEncoderForMatches + .matches(rawPassword, prefixEncodedPassword); + } + String encodedPassword = extractEncodedPassword(prefixEncodedPassword); + return delegate.matches(rawPassword, encodedPassword); + } + + private String extractId(String prefixEncodedPassword) { + int start = prefixEncodedPassword.indexOf(PREFIX); + if(start != 0) { + return null; + } + int end = prefixEncodedPassword.indexOf(SUFFIX, start); + if(end < 0) { + return null; + } + return prefixEncodedPassword.substring(start + 1, end); + } + + private String extractEncodedPassword(String prefixEncodedPassword) { + int start = prefixEncodedPassword.indexOf(SUFFIX); + return prefixEncodedPassword.substring(start + 1); + } + + /** + * Default {@link PasswordEncoder} that throws an exception that a id could + */ + private class UnmappedIdPasswordEncoder implements PasswordEncoder { + + @Override + public String encode(CharSequence rawPassword) { + throw new UnsupportedOperationException("encode is not supported"); + } + + @Override + public boolean matches(CharSequence rawPassword, + String prefixEncodedPassword) { + String id = extractId(prefixEncodedPassword); + throw new IllegalArgumentException("There is no PasswordEncoder mapped for the id \"" + id + "\""); + } + } +} diff --git a/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java b/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java new file mode 100644 index 0000000000..f819515104 --- /dev/null +++ b/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java @@ -0,0 +1,186 @@ +/* + * Copyright 2002-2017 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.crypto.password; + +import com.sun.org.apache.xpath.internal.SourceTree; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import org.springframework.security.crypto.scrypt.SCryptPasswordEncoder; + +import java.util.HashMap; +import java.util.Map; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +/** + * @author Rob Winch + * @since 5.0 + */ +@RunWith(MockitoJUnitRunner.class) +public class DelegatingPasswordEncoderTests { + @Mock + private PasswordEncoder bcrypt; + + @Mock + private PasswordEncoder noop; + + @Mock + private PasswordEncoder invalidId; + + private String bcryptId = "bcrypt"; + + private String noopId = "noop"; + + private String rawPassword = "password"; + + private String encodedPassword = "ENCODED-PASSWORD"; + + private String bcryptEncodedPassword = "{bcrypt}" + this.encodedPassword; + + private String noopEncodedPassword = "{noop}" + this.encodedPassword; + + private Map delegates; + + private DelegatingPasswordEncoder passwordEncoder; + + @Before + public void setup() { + this.delegates = new HashMap<>(); + this.delegates.put(this.bcryptId, this.bcrypt); + this.delegates.put(this.noopId, this.noop); + + this.passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenIdForEncodeNullThenIllegalArgumentException() { + String id = null; + new DelegatingPasswordEncoder(id, this.delegates); + } + + @Test(expected = IllegalArgumentException.class) + public void constructorWhenIdForEncodeDoesNotExistThenIllegalArgumentException() { + new DelegatingPasswordEncoder(this.bcryptId + "INVALID", this.delegates); + } + + @Test(expected = IllegalArgumentException.class) + public void setDefaultPasswordEncoderForMatchesWhenNullThenIllegalArgumentException() { + this.passwordEncoder.setDefaultPasswordEncoderForMatches(null); + } + + @Test + public void matchesWhenCustomDefaultPasswordEncoderForMatchesThenDelegates() { + String encodedPassword = "{unmapped}" + this.rawPassword; + this.passwordEncoder.setDefaultPasswordEncoderForMatches(this.invalidId); + + assertThat(this.passwordEncoder.matches(this.rawPassword, encodedPassword)).isFalse(); + + verify(this.invalidId).matches(this.rawPassword, encodedPassword); + verifyZeroInteractions(this.bcrypt, this.noop); + } + + @Test + public void encodeWhenValidThenUsesIdForEncode() { + when(this.bcrypt.encode(this.rawPassword)).thenReturn(this.encodedPassword); + + assertThat(this.passwordEncoder.encode(this.rawPassword)).isEqualTo(this.bcryptEncodedPassword); + } + + @Test + public void matchesWhenBCryptThenDelegatesToBCrypt() { + when(this.bcrypt.matches(this.rawPassword, this.encodedPassword)).thenReturn(true); + + assertThat(this.passwordEncoder.matches(this.rawPassword, this.bcryptEncodedPassword)).isTrue(); + + verify(this.bcrypt).matches(this.rawPassword, this.encodedPassword); + verifyZeroInteractions(this.noop); + } + + @Test + public void matchesWhenNoopThenDelegatesToNoop() { + when(this.noop.matches(this.rawPassword, this.encodedPassword)).thenReturn(true); + + assertThat(this.passwordEncoder.matches(this.rawPassword, this.noopEncodedPassword)).isTrue(); + + verify(this.noop).matches(this.rawPassword, this.encodedPassword); + verifyZeroInteractions(this.bcrypt); + } + + @Test + public void matchesWhenUnMappedThenIllegalArgumentException() { + assertThatThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "{unmapped}" + this.rawPassword)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There is no PasswordEncoder mapped for the id \"unmapped\""); + + verifyZeroInteractions(this.bcrypt, this.noop); + } + + @Test + public void matchesWhenNoClosingPrefixStringThenIllegalArgumentExcetion() { + assertThatThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "{bcrypt" + this.rawPassword)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There is no PasswordEncoder mapped for the id \"null\""); + + verifyZeroInteractions(this.bcrypt, this.noop); + } + + @Test + public void matchesWhenNoStartingPrefixStringThenFalse() { + assertThatThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "bcrypt}" + this.rawPassword)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There is no PasswordEncoder mapped for the id \"null\""); + + verifyZeroInteractions(this.bcrypt, this.noop); + } + + @Test + public void matchesWhenNoIdStringThenFalse() { + assertThatThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "{}" + this.rawPassword)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There is no PasswordEncoder mapped for the id \"\""); + + verifyZeroInteractions(this.bcrypt, this.noop); + } + + @Test + public void matchesWhenPrefixInMiddleThenFalse() { + assertThatThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "invalid" + this.bcryptEncodedPassword)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("There is no PasswordEncoder mapped for the id \"null\""); + + verifyZeroInteractions(this.bcrypt, this.noop); + } + + @Test + public void matchesWhenNullIdThenDelegatesToInvalidId() { + this.delegates.put(null, this.invalidId); + this.passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates); + when(this.invalidId.matches(this.rawPassword, this.encodedPassword)).thenReturn(true); + + assertThat(this.passwordEncoder.matches(this.rawPassword, this.encodedPassword)).isTrue(); + + verify(this.invalidId).matches(this.rawPassword, this.encodedPassword); + verifyZeroInteractions(this.bcrypt, this.noop); + } +}