Merge branch 'spring-projects:main' into main

spring-projects · Apr 27, 2024 · 6a5b217 · 6a5b217
2 parents 1bd9b74 + 1825dcb
commit 6a5b217
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 16 deletions.
diff --git a/...c/main/java/org/springframework/security/config/authentication/PasswordEncoderParser.java b/...c/main/java/org/springframework/security/config/authentication/PasswordEncoderParser.java
@@ -41,8 +41,6 @@ public class PasswordEncoderParser {
 
 	public static final String ATT_HASH = "hash";
 
-	static final String ATT_BASE_64 = "base64";
-
 	static final String OPT_HASH_BCRYPT = "bcrypt";
 
 	private static final Map<String, Class<?>> ENCODER_CLASSES = Collections.singletonMap(OPT_HASH_BCRYPT,
@@ -62,19 +60,17 @@ private void parse(Element element, ParserContext parserContext) {
 			return;
 		}
 		String hash = element.getAttribute(ATT_HASH);
-		boolean useBase64 = StringUtils.hasText(element.getAttribute(ATT_BASE_64))
-				&& Boolean.parseBoolean(element.getAttribute(ATT_BASE_64));
 		String ref = element.getAttribute(ATT_REF);
 		if (StringUtils.hasText(ref)) {
 			this.passwordEncoder = new RuntimeBeanReference(ref);
 		}
 		else {
-			this.passwordEncoder = createPasswordEncoderBeanDefinition(hash, useBase64);
+			this.passwordEncoder = createPasswordEncoderBeanDefinition(hash);
 			((RootBeanDefinition) this.passwordEncoder).setSource(parserContext.extractSource(element));
 		}
 	}
 
-	public static BeanDefinition createPasswordEncoderBeanDefinition(String hash, boolean useBase64) {
+	public static BeanDefinition createPasswordEncoderBeanDefinition(String hash) {
 		Class<?> beanClass = ENCODER_CLASSES.get(hash);
 		BeanDefinitionBuilder beanBldr = BeanDefinitionBuilder.rootBeanDefinition(beanClass);
 		return beanBldr.getBeanDefinition();

diff --git a/.../main/java/org/springframework/security/config/ldap/LdapProviderBeanDefinitionParser.java b/.../main/java/org/springframework/security/config/ldap/LdapProviderBeanDefinitionParser.java
@@ -98,7 +98,7 @@ else if (searchBean == null) {
 			}
 			else if (StringUtils.hasText(hash)) {
 				authenticatorBuilder.addPropertyValue("passwordEncoder",
-						PasswordEncoderParser.createPasswordEncoderBeanDefinition(hash, false));
+						PasswordEncoderParser.createPasswordEncoderBeanDefinition(hash));
 			}
 		}
 		authenticatorBuilder.addConstructorArgValue(contextSource);

diff --git a/...t/java/org/springframework/security/config/authentication/PasswordEncoderParserTests.java b/...t/java/org/springframework/security/config/authentication/PasswordEncoderParserTests.java
@@ -20,10 +20,13 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.security.config.test.SpringTestContext;
 import org.springframework.security.config.test.SpringTestContextExtension;
+import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
 import org.springframework.test.web.servlet.MockMvc;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -65,4 +68,15 @@ public void passwordEncoderDefaultsToPasswordEncoderBean() throws Exception {
 		// @formatter:on
 	}
 
+	@Test
+	void testCreatePasswordEncoderBeanDefinition() throws Exception {
+		String hash = "bcrypt";
+		Class<?> expectedBeanClass = BCryptPasswordEncoder.class;
+
+		BeanDefinition beanDefinition = PasswordEncoderParser.createPasswordEncoderBeanDefinition(hash);
+
+		Class<?> actualBeanClass = Class.forName(beanDefinition.getBeanClassName());
+		assertThat(actualBeanClass).isEqualTo(expectedBeanClass);
+	}
+
 }
diff --git a/...src/main/java/org/springframework/security/crypto/password/DelegatingPasswordEncoder.java b/...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.
@@ -129,6 +129,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 +290,10 @@ public String encode(CharSequence rawPassword) {
 		@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 (id != null && !id.isEmpty()) {
+				throw new IllegalArgumentException(String.format(NO_PASSWORD_ENCODER_MAPPED, id));
+			}
+			throw new IllegalArgumentException(NO_PASSWORD_ENCODER_PREFIX);
 		}
 
 	}

diff --git a/...est/java/org/springframework/security/crypto/password/DelegatingPasswordEncoderTests.java b/...est/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 @@
 @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,23 +203,23 @@ public void matchesWhenUnMappedThenIllegalArgumentException() {
 	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);
 	}
 
 	@Test
 	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);
 	}
 
 	@Test
 	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 void matchesWhenPrefixInMiddleThenFalse() {
 		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 void matchesWhenIdIsNullThenFalse() {
 		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 void upgradeEncodingWhenDifferentIdThenTrue() {
 		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);
+
+	}
+
 }
diff --git a/docs/modules/ROOT/pages/servlet/getting-started.adoc b/docs/modules/ROOT/pages/servlet/getting-started.adoc
@@ -131,7 +131,7 @@ public class DefaultSecurityConfig {
     InMemoryUserDetailsManager inMemoryUserDetailsManager() { <2>
         String generatedPassword = // ...;
         return new InMemoryUserDetailsManager(User.withUsername("user")
-                .password(generatedPassword).roles("ROLE_USER").build());
+                .password(generatedPassword).roles("USER").build());
     }
 
     @Bean