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 index d83dec021d7..fae98775128 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java +++ b/crypto/src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java @@ -116,14 +116,19 @@ * * @author Rob Winch * @author Michael Simons + * @author heowc * @since 5.0 * @see org.springframework.security.crypto.factory.PasswordEncoderFactories */ public class DelegatingPasswordEncoder implements PasswordEncoder { - private static final String PREFIX = "{"; + private static final String DEFAULT_PREFIX = "{"; - private static final String SUFFIX = "}"; + private static final String DEFAULT_SUFFIX = "}"; + + private final String prefix; + + private final String suffix; private final String idForEncode; @@ -142,9 +147,31 @@ public class DelegatingPasswordEncoder implements PasswordEncoder { * {@link #matches(CharSequence, String)} */ public DelegatingPasswordEncoder(String idForEncode, Map idToPasswordEncoder) { + this(idForEncode, idToPasswordEncoder, DEFAULT_PREFIX, DEFAULT_SUFFIX); + } + + /** + * 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 + * @param prefix the prefix that denotes the start of an {@code idForEncode} + * @param suffix the suffix that denotes the end of an {@code idForEncode} + * {@link #matches(CharSequence, String)} + */ + public DelegatingPasswordEncoder(String idForEncode, Map idToPasswordEncoder, + String prefix, String suffix) { if (idForEncode == null) { throw new IllegalArgumentException("idForEncode cannot be null"); } + if (prefix == null) { + throw new IllegalArgumentException("prefix cannot be null"); + } + if (suffix == null || suffix.isEmpty()) { + throw new IllegalArgumentException("suffix cannot be empty"); + } + if (!idToPasswordEncoder.containsKey(idForEncode)) { throw new IllegalArgumentException( "idForEncode " + idForEncode + "is not found in idToPasswordEncoder " + idToPasswordEncoder); @@ -153,16 +180,18 @@ public DelegatingPasswordEncoder(String idForEncode, Map(idToPasswordEncoder); + this.prefix = prefix; + this.suffix = suffix; } /** @@ -188,7 +217,7 @@ public void setDefaultPasswordEncoderForMatches(PasswordEncoder defaultPasswordE @Override public String encode(CharSequence rawPassword) { - return PREFIX + this.idForEncode + SUFFIX + this.passwordEncoderForEncode.encode(rawPassword); + return this.prefix + this.idForEncode + this.suffix + this.passwordEncoderForEncode.encode(rawPassword); } @Override @@ -209,15 +238,15 @@ private String extractId(String prefixEncodedPassword) { if (prefixEncodedPassword == null) { return null; } - int start = prefixEncodedPassword.indexOf(PREFIX); + int start = prefixEncodedPassword.indexOf(this.prefix); if (start != 0) { return null; } - int end = prefixEncodedPassword.indexOf(SUFFIX, start); + int end = prefixEncodedPassword.indexOf(this.suffix, start); if (end < 0) { return null; } - return prefixEncodedPassword.substring(start + 1, end); + return prefixEncodedPassword.substring(start + this.prefix.length(), end); } @Override @@ -233,8 +262,8 @@ public boolean upgradeEncoding(String prefixEncodedPassword) { } private String extractEncodedPassword(String prefixEncodedPassword) { - int start = prefixEncodedPassword.indexOf(SUFFIX); - return prefixEncodedPassword.substring(start + 1); + int start = prefixEncodedPassword.indexOf(this.suffix); + return prefixEncodedPassword.substring(start + this.suffix.length()); } /** 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 index a67ae334de8..a130aa1fc5b 100644 --- a/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java +++ b/crypto/src/test/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2021 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. @@ -36,6 +36,7 @@ /** * @author Rob Winch * @author Michael Simons + * @author heowc * @since 5.0 */ @ExtendWith(MockitoExtension.class) @@ -64,12 +65,16 @@ public class DelegatingPasswordEncoderTests { private DelegatingPasswordEncoder passwordEncoder; + private DelegatingPasswordEncoder onlySuffixPasswordEncoder; + @BeforeEach public void setup() { this.delegates = new HashMap<>(); this.delegates.put(this.bcryptId, this.bcrypt); this.delegates.put("noop", this.noop); this.passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates); + + this.onlySuffixPasswordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$"); } @Test @@ -83,6 +88,49 @@ public void constructorWhenIdForEncodeDoesNotExistThenIllegalArgumentException() .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId + "INVALID", this.delegates)); } + @Test + public void constructorWhenPrefixIsNull() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, null, "$")); + } + + @Test + public void constructorWhenSuffixIsNull() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", null)); + } + + @Test + public void constructorWhenPrefixIsEmpty() { + assertThat(new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$")).isNotNull(); + } + + @Test + public void constructorWhenSuffixIsEmpty() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", "")); + } + + @Test + public void constructorWhenPrefixAndSuffixAreEmpty() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "")); + } + + @Test + public void constructorWhenIdContainsPrefixThenIllegalArgumentException() { + this.delegates.put('$' + this.bcryptId, this.bcrypt); + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", "$")); + } + + @Test + public void constructorWhenIdContainsSuffixThenIllegalArgumentException() { + this.delegates.put(this.bcryptId + '$', this.bcrypt); + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$")); + } + @Test public void setDefaultPasswordEncoderForMatchesWhenNullThenIllegalArgumentException() { assertThatIllegalArgumentException() @@ -104,6 +152,12 @@ public void encodeWhenValidThenUsesIdForEncode() { assertThat(this.passwordEncoder.encode(this.rawPassword)).isEqualTo(this.bcryptEncodedPassword); } + @Test + public void encodeWhenValidBySpecifyDelegatingPasswordEncoderThenUsesIdForEncode() { + given(this.bcrypt.encode(this.rawPassword)).willReturn(this.encodedPassword); + assertThat(this.onlySuffixPasswordEncoder.encode(this.rawPassword)).isEqualTo("bcrypt$" + this.encodedPassword); + } + @Test public void matchesWhenBCryptThenDelegatesToBCrypt() { given(this.bcrypt.matches(this.rawPassword, this.encodedPassword)).willReturn(true); @@ -112,6 +166,14 @@ public void matchesWhenBCryptThenDelegatesToBCrypt() { verifyZeroInteractions(this.noop); } + @Test + public void matchesWhenBCryptBySpecifyDelegatingPasswordEncoderThenDelegatesToBCrypt() { + given(this.bcrypt.matches(this.rawPassword, this.encodedPassword)).willReturn(true); + assertThat(this.onlySuffixPasswordEncoder.matches(this.rawPassword, "bcrypt$" + this.encodedPassword)).isTrue(); + verify(this.bcrypt).matches(this.rawPassword, this.encodedPassword); + verifyZeroInteractions(this.noop); + } + @Test public void matchesWhenNoopThenDelegatesToNoop() { given(this.noop.matches(this.rawPassword, this.encodedPassword)).willReturn(true);