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 b9039b1d74..2ab7f456ca 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -19,6 +19,8 @@ package org.springframework.security.crypto.password; import java.util.HashMap; import java.util.Map; +import org.springframework.util.StringUtils; + /** * A password encoder that delegates to another PasswordEncoder based upon a prefixed * identifier. @@ -129,6 +131,10 @@ public class DelegatingPasswordEncoder implements PasswordEncoder { private static final String DEFAULT_ID_SUFFIX = "}"; + public static final String NO_PASSWORD_ENCODER_MAPPED = "There is no PasswordEncoder mapped for the id \"%s\""; + + public static final String NO_PASSWORD_ENCODER_PREFIX = "You have entered a password with no PasswordEncoder. If that is your intent, it should be prefixed with `{noop}`."; + private final String idPrefix; private final String idSuffix; @@ -286,7 +292,10 @@ public class DelegatingPasswordEncoder implements PasswordEncoder { @Override public boolean matches(CharSequence rawPassword, String prefixEncodedPassword) { String id = extractId(prefixEncodedPassword); - throw new IllegalArgumentException("There is no PasswordEncoder mapped for the id \"" + id + "\""); + if (StringUtils.hasText(id)) { + throw new IllegalArgumentException(String.format(NO_PASSWORD_ENCODER_MAPPED, id)); + } + throw new IllegalArgumentException(NO_PASSWORD_ENCODER_PREFIX); } } 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 ec76f10ba4..a3222fb41e 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-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -43,6 +43,8 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; @ExtendWith(MockitoExtension.class) public class DelegatingPasswordEncoderTests { + public static final String NO_PASSWORD_ENCODER = "You have entered a password with no PasswordEncoder. If that is your intent, it should be prefixed with `{noop}`."; + @Mock private PasswordEncoder bcrypt; @@ -201,7 +203,7 @@ public class DelegatingPasswordEncoderTests { public void matchesWhenNoClosingPrefixStringThenIllegalArgumentException() { assertThatIllegalArgumentException() .isThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "{bcrypt" + this.rawPassword)) - .withMessage("There is no PasswordEncoder mapped for the id \"null\""); + .withMessage(NO_PASSWORD_ENCODER); verifyNoMoreInteractions(this.bcrypt, this.noop); } @@ -209,7 +211,7 @@ public class DelegatingPasswordEncoderTests { public void matchesWhenNoStartingPrefixStringThenFalse() { assertThatIllegalArgumentException() .isThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "bcrypt}" + this.rawPassword)) - .withMessage("There is no PasswordEncoder mapped for the id \"null\""); + .withMessage(NO_PASSWORD_ENCODER); verifyNoMoreInteractions(this.bcrypt, this.noop); } @@ -217,7 +219,7 @@ public class DelegatingPasswordEncoderTests { public void matchesWhenNoIdStringThenFalse() { assertThatIllegalArgumentException() .isThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "{}" + this.rawPassword)) - .withMessage("There is no PasswordEncoder mapped for the id \"\""); + .withMessage(NO_PASSWORD_ENCODER); verifyNoMoreInteractions(this.bcrypt, this.noop); } @@ -226,7 +228,7 @@ public class DelegatingPasswordEncoderTests { assertThatIllegalArgumentException() .isThrownBy(() -> this.passwordEncoder.matches(this.rawPassword, "invalid" + this.bcryptEncodedPassword)) .isInstanceOf(IllegalArgumentException.class) - .withMessage("There is no PasswordEncoder mapped for the id \"null\""); + .withMessage(NO_PASSWORD_ENCODER); verifyNoMoreInteractions(this.bcrypt, this.noop); } @@ -236,7 +238,7 @@ public class DelegatingPasswordEncoderTests { DelegatingPasswordEncoder passwordEncoder = new DelegatingPasswordEncoder(this.bcryptId, this.delegates); assertThatIllegalArgumentException() .isThrownBy(() -> passwordEncoder.matches(this.rawPassword, this.rawPassword)) - .withMessage("There is no PasswordEncoder mapped for the id \"null\""); + .withMessage(NO_PASSWORD_ENCODER); verifyNoMoreInteractions(this.bcrypt, this.noop); } @@ -289,4 +291,14 @@ public class DelegatingPasswordEncoderTests { verifyNoMoreInteractions(this.bcrypt); } + @Test + void matchesShouldThrowIllegalArgumentExceptionWhenNoPasswordEncoderIsMappedForTheId() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.passwordEncoder.matches("rawPassword", "prefixEncodedPassword")) + .isInstanceOf(IllegalArgumentException.class) + .withMessage(NO_PASSWORD_ENCODER); + verifyNoMoreInteractions(this.bcrypt, this.noop); + + } + }