From da606627b6199b237248f45bb23bb88b1be14725 Mon Sep 17 00:00:00 2001 From: Jihoon Cha Date: Tue, 29 Mar 2022 19:28:41 +0900 Subject: [PATCH] Prevent instantiation of DelegatingPasswordEncoder if idPrefix contains idSuffix Closes gh-10933 --- .../crypto/password/DelegatingPasswordEncoder.java | 4 ++++ .../password/DelegatingPasswordEncoderTests.java | 11 +++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) 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 811c558155..5aaeb5f452 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 @@ -119,6 +119,7 @@ import java.util.Map; * @author Rob Winch * @author Michael Simons * @author heowc + * @author Jihoon Cha * @since 5.0 * @see org.springframework.security.crypto.factory.PasswordEncoderFactories */ @@ -173,6 +174,9 @@ public class DelegatingPasswordEncoder implements PasswordEncoder { if (idSuffix == null || idSuffix.isEmpty()) { throw new IllegalArgumentException("suffix cannot be empty"); } + if (idPrefix.contains(idSuffix)) { + throw new IllegalArgumentException("idPrefix " + idPrefix + " cannot contain idSuffix " + idSuffix); + } if (!idToPasswordEncoder.containsKey(idForEncode)) { throw new IllegalArgumentException( 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 dca1bc8b06..988529cba8 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 @@ -37,6 +37,7 @@ import static org.mockito.Mockito.verifyZeroInteractions; * @author Rob Winch * @author Michael Simons * @author heowc + * @author Jihoon Cha * @since 5.0 */ @ExtendWith(MockitoExtension.class) @@ -119,9 +120,9 @@ public class DelegatingPasswordEncoderTests { @Test public void constructorWhenIdContainsPrefixThenIllegalArgumentException() { - this.delegates.put('$' + this.bcryptId, this.bcrypt); + this.delegates.put('{' + this.bcryptId, this.bcrypt); assertThatIllegalArgumentException() - .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", "$")); + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates)); } @Test @@ -131,6 +132,12 @@ public class DelegatingPasswordEncoderTests { .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "", "$")); } + @Test + public void constructorWhenPrefixContainsSuffixThenIllegalArgumentException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new DelegatingPasswordEncoder(this.bcryptId, this.delegates, "$", "$")); + } + @Test public void setDefaultPasswordEncoderForMatchesWhenNullThenIllegalArgumentException() { assertThatIllegalArgumentException()