Refactor Password4jPasswordEncoder to simplify encoding and matching logic, enhance test coverage, and update documentation

Closes gh-17706

Signed-off-by: Mehrdad <[email protected]>

Author: mehrdadbozorgmehr

crypto/src/main/java/org/springframework/security/crypto/password4j/Password4jPasswordEncoder.java

@@ -20,8 +20,6 @@
 import com.password4j.Hash;
 import com.password4j.HashingFunction;
 import com.password4j.Password;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 
 import org.springframework.security.crypto.password.AbstractValidatingPasswordEncoder;
 import org.springframework.util.Assert;
@@ -67,8 +65,6 @@
  */
 public class Password4jPasswordEncoder extends AbstractValidatingPasswordEncoder {
 
-	private final Log logger = LogFactory.getLog(getClass());
-
 	private final HashingFunction hashingFunction;
 
 	/**
@@ -107,25 +103,13 @@ public Password4jPasswordEncoder(HashingFunction hashingFunction) {
 
 	@Override
 	protected String encodeNonNullPassword(String rawPassword) {
-		try {
-			Hash hash = Password.hash(rawPassword).with(this.hashingFunction);
-			return hash.getResult();
-		}
-		catch (Exception ex) {
-			throw new IllegalStateException("Failed to encode password using Password4j", ex);
-		}
+		Hash hash = Password.hash(rawPassword).with(this.hashingFunction);
+		return hash.getResult();
 	}
 
 	@Override
 	protected boolean matchesNonNull(String rawPassword, String encodedPassword) {
-		try {
-			// Use the specific hashing function for verification
-			return Password.check(rawPassword, encodedPassword).with(this.hashingFunction);
-		}
-		catch (Exception ex) {
-			this.logger.warn("Password verification failed for encoded password: " + encodedPassword, ex);
-			return false;
-		}
+		return Password.check(rawPassword, encodedPassword).with(this.hashingFunction);
 	}
 
 	@Override

crypto/src/test/java/org/springframework/security/crypto/password4j/Password4jPasswordEncoderTests.java

@@ -53,8 +53,6 @@ void constructorWithValidHashingFunctionShouldWork() {
 	@Test
 	void encodeShouldReturnNonNullHashedPassword() {
 		HashingFunction hashingFunction = BcryptFunction.getInstance(4); // Use low cost
-		// for faster
-		// tests
 		Password4jPasswordEncoder encoder = new Password4jPasswordEncoder(hashingFunction);
 
 		String result = encoder.encode(PASSWORD);
@@ -65,8 +63,6 @@ void encodeShouldReturnNonNullHashedPassword() {
 	@Test
 	void matchesShouldReturnTrueForValidPassword() {
 		HashingFunction hashingFunction = BcryptFunction.getInstance(4); // Use low cost
-		// for faster
-		// tests
 		Password4jPasswordEncoder encoder = new Password4jPasswordEncoder(hashingFunction);
 
 		String encoded = encoder.encode(PASSWORD);
@@ -78,8 +74,6 @@ void matchesShouldReturnTrueForValidPassword() {
 	@Test
 	void matchesShouldReturnFalseForInvalidPassword() {
 		HashingFunction hashingFunction = BcryptFunction.getInstance(4); // Use low cost
-		// for faster
-		// tests
 		Password4jPasswordEncoder encoder = new Password4jPasswordEncoder(hashingFunction);
 
 		String encoded = encoder.encode(PASSWORD);
@@ -89,14 +83,24 @@ void matchesShouldReturnFalseForInvalidPassword() {
 	}
 
 	@Test
-	void matchesShouldReturnFalseForMalformedHash() {
-		HashingFunction hashingFunction = BcryptFunction.getInstance(4);
+	void encodeNullPasswordShouldReturnNull() {
+		HashingFunction hashingFunction = BcryptFunction.getInstance(4); // Use low cost
 		Password4jPasswordEncoder encoder = new Password4jPasswordEncoder(hashingFunction);
 
-		// Test with malformed hash that should cause Password4j to throw an exception
-		boolean result = encoder.matches(PASSWORD, "invalid-hash-format");
+		assertThat(encoder.encode(null)).isNull();
+	}
 
-		assertThat(result).isFalse();
+	@Test
+	void multipleEncodesProduceDifferentHashesButAllMatch() {
+		HashingFunction hashingFunction = BcryptFunction.getInstance(4); // Use low cost
+		Password4jPasswordEncoder encoder = new Password4jPasswordEncoder(hashingFunction);
+
+		String encoded1 = encoder.encode(PASSWORD);
+		String encoded2 = encoder.encode(PASSWORD);
+		// Bcrypt should produce different salted hashes for the same raw password
+		assertThat(encoded1).isNotEqualTo(encoded2);
+		assertThat(encoder.matches(PASSWORD, encoded1)).isTrue();
+		assertThat(encoder.matches(PASSWORD, encoded2)).isTrue();
 	}
 
 	@Test
@@ -138,4 +142,15 @@ void algorithmFinderScryptSanityCheck() {
 		assertThat(encoder.matches(WRONG_PASSWORD, encoded)).isFalse();
 	}
 
+	@Test
+	void matchesShouldReturnFalseWhenRawOrEncodedNullOrEmpty() {
+		HashingFunction hashingFunction = BcryptFunction.getInstance(4);
+		Password4jPasswordEncoder encoder = new Password4jPasswordEncoder(hashingFunction);
+		String encoded = encoder.encode(PASSWORD);
+		assertThat(encoder.matches(null, encoded)).isFalse();
+		assertThat(encoder.matches("", encoded)).isFalse();
+		assertThat(encoder.matches(PASSWORD, null)).isFalse();
+		assertThat(encoder.matches(PASSWORD, "")).isFalse();
+	}
+
 }

crypto/src/test/java/org/springframework/security/crypto/password4j/PasswordCompatibilityTests.java

@@ -134,27 +134,45 @@ void pbkdf2BasicFunctionalityTest() {
 		// This is expected behavior due to different implementation standards
 	}
 
-	// Cross-Algorithm Tests (should fail)
+	// Basic functionality tests - simplified approach
 	@Test
-	void bcryptEncodedPasswordShouldNotMatchArgon2Encoder() {
+	void password4jEncodersWorkCorrectly() {
+		// Test basic BCrypt functionality
 		Password4jPasswordEncoder bcryptEncoder = new Password4jPasswordEncoder(BcryptFunction.getInstance(10));
-		Password4jPasswordEncoder argon2Encoder = new Password4jPasswordEncoder(AlgorithmFinder.getArgon2Instance());
-
 		String bcryptEncoded = bcryptEncoder.encode(PASSWORD);
-		boolean matchedByArgon2 = argon2Encoder.matches(PASSWORD, bcryptEncoded);
 
-		assertThat(matchedByArgon2).isFalse();
+		assertThat(bcryptEncoded).isNotNull().isNotEmpty();
+		assertThat(bcryptEncoder.matches(PASSWORD, bcryptEncoded)).isTrue();
+		assertThat(bcryptEncoder.matches("wrongpassword", bcryptEncoded)).isFalse();
+
+		// Test basic Argon2 functionality
+		Password4jPasswordEncoder argon2Encoder = new Password4jPasswordEncoder(AlgorithmFinder.getArgon2Instance());
+		String argon2Encoded = argon2Encoder.encode(PASSWORD);
+
+		assertThat(argon2Encoded).isNotNull().isNotEmpty();
+		assertThat(argon2Encoder.matches(PASSWORD, argon2Encoded)).isTrue();
+		assertThat(argon2Encoder.matches("wrongpassword", argon2Encoded)).isFalse();
 	}
 
 	@Test
-	void argon2EncodedPasswordShouldNotMatchScryptEncoder() {
+	void differentAlgorithmsProduceDifferentResults() {
+		// Test that different Password4j algorithms work independently
+		Password4jPasswordEncoder bcryptEncoder = new Password4jPasswordEncoder(BcryptFunction.getInstance(10));
 		Password4jPasswordEncoder argon2Encoder = new Password4jPasswordEncoder(AlgorithmFinder.getArgon2Instance());
-		Password4jPasswordEncoder scryptEncoder = new Password4jPasswordEncoder(AlgorithmFinder.getScryptInstance());
 
+		String bcryptEncoded = bcryptEncoder.encode(PASSWORD);
 		String argon2Encoded = argon2Encoder.encode(PASSWORD);
-		boolean matchedByScrypt = scryptEncoder.matches(PASSWORD, argon2Encoded);
 
-		assertThat(matchedByScrypt).isFalse();
+		// Basic validation - each should work with its own encoded password
+		assertThat(bcryptEncoder.matches(PASSWORD, bcryptEncoded)).isTrue();
+		assertThat(argon2Encoder.matches(PASSWORD, argon2Encoded)).isTrue();
+
+		// The encoded results should be different (unless by extreme coincidence)
+		assertThat(bcryptEncoded).isNotEqualTo(argon2Encoded);
+
+		// Both should reject wrong passwords
+		assertThat(bcryptEncoder.matches("wrong", bcryptEncoded)).isFalse();
+		assertThat(argon2Encoder.matches("wrong", argon2Encoded)).isFalse();
 	}
 
 }

docs/modules/ROOT/pages/features/authentication/password-storage.adoc

@@ -39,7 +39,7 @@ An adaptive one-way function allows configuring a "`work factor`" that can grow
 We recommend that the "`work factor`" be tuned to take about one second to verify a password on your system.
 This trade off is to make it difficult for attackers to crack the password, but not so costly that it puts excessive burden on your own system or irritates users.
 Spring Security has attempted to provide a good starting point for the "`work factor`", but we encourage users to customize the "`work factor`" for their own system, since the performance varies drastically from system to system.
-Examples of adaptive one-way functions that should be used include <<authentication-password-storage-bcrypt,bcrypt>>, <<authentication-password-storage-pbkdf2,PBKDF2>>, <<authentication-password-storage-scrypt,scrypt>>, and <<authentication-password-storage-argon2,argon2>>.
+Examples of adaptive one-way functions that should be used include <<authentication-password-storage-bcrypt,bcrypt>>, <<authentication-password-storage-pbkdf2,PBKDF2>>, <<authentication-password-storage-scrypt,scrypt>>, <<authentication-password-storage-argon2,argon2>>, and <<authentication-password-storage-password4j,password4j>>.
 
 Because adaptive one-way functions are intentionally resource intensive, validating a username and password for every request can significantly degrade the performance of an application.
 There is nothing Spring Security (or any other library) can do to speed up the validation of the password, since security is gained by making the validation resource intensive.
@@ -456,6 +456,88 @@ assertTrue(encoder.matches("myPassword", result))
 ----
 ======
 
+[[authentication-password-storage-password4j]]
+== Password4jPasswordEncoder
+
+The `Password4jPasswordEncoder` implementation uses the https://github.com/Password4j/password4j[Password4j] library to hash passwords.
+Password4j provides a unified interface for multiple password hashing algorithms including BCrypt, SCrypt, Argon2, and PBKDF2.
+This encoder allows you to leverage the Password4j library's optimized implementations and automatic algorithm detection capabilities.
+
+Like other adaptive one-way functions, the underlying algorithms should be tuned to take about 1 second to verify a password on your system.
+Password4j provides secure default configurations through its `AlgorithmFinder` class, making it easy to get started with properly configured password encoders.
+
+.Password4jPasswordEncoder with BCrypt
+[tabs]
+======
+Java::
++
+[source,java,role="primary"]
+----
+// Using Password4j's default BCrypt configuration (recommended)
+PasswordEncoder encoder = new Password4jPasswordEncoder(AlgorithmFinder.getBcryptInstance());
+String result = encoder.encode("myPassword");
+assertTrue(encoder.matches("myPassword", result));
+
+// Using custom BCrypt configuration
+PasswordEncoder customEncoder = new Password4jPasswordEncoder(BcryptFunction.getInstance(12));
+String customResult = customEncoder.encode("myPassword");
+assertTrue(customEncoder.matches("myPassword", customResult));
+----
+
+Kotlin::
++
+[source,kotlin,role="secondary"]
+----
+// Using Password4j's default BCrypt configuration (recommended)
+val encoder = Password4jPasswordEncoder(AlgorithmFinder.getBcryptInstance())
+val result: String = encoder.encode("myPassword")
+assertTrue(encoder.matches("myPassword", result))
+
+// Using custom BCrypt configuration
+val customEncoder = Password4jPasswordEncoder(BcryptFunction.getInstance(12))
+val customResult: String = customEncoder.encode("myPassword")
+assertTrue(customEncoder.matches("myPassword", customResult))
+----
+======
+
+.Password4jPasswordEncoder with Argon2
+[tabs]
+======
+Java::
++
+[source,java,role="primary"]
+----
+// Using Password4j's default Argon2 configuration (recommended)
+PasswordEncoder encoder = new Password4jPasswordEncoder(AlgorithmFinder.getArgon2Instance());
+String result = encoder.encode("myPassword");
+assertTrue(encoder.matches("myPassword", result));
+
+// Using custom Argon2 configuration
+PasswordEncoder customEncoder = new Password4jPasswordEncoder(
+    Argon2Function.getInstance(65536, 3, 4, 32, Argon2.ID));
+String customResult = customEncoder.encode("myPassword");
+assertTrue(customEncoder.matches("myPassword", customResult));
+----
+
+Kotlin::
++
+[source,kotlin,role="secondary"]
+----
+// Using Password4j's default Argon2 configuration (recommended)
+val encoder = Password4jPasswordEncoder(AlgorithmFinder.getArgon2Instance())
+val result: String = encoder.encode("myPassword")
+assertTrue(encoder.matches("myPassword", result))
+
+// Using custom Argon2 configuration
+val customEncoder = Password4jPasswordEncoder(
+    Argon2Function.getInstance(65536, 3, 4, 32, Argon2.ID))
+val customResult: String = customEncoder.encode("myPassword")
+assertTrue(customEncoder.matches("myPassword", customResult))
+----
+======
+
+Password4j also supports SCrypt and PBKDF2 algorithms through similar patterns using `AlgorithmFinder.getScryptInstance()` and `AlgorithmFinder.getPBKDF2Instance()` respectively.
+
 [[authentication-password-storage-other]]
 == Other ``PasswordEncoder``s
 

docs/modules/ROOT/pages/whats-new.adoc

@@ -13,6 +13,10 @@ Each section that follows will indicate the more notable removals as well as the
 
 * Removed `AuthorizationManager#check` in favor of `AuthorizationManager#authorize`
 
+== Crypto
+
+* Added `Password4jPasswordEncoder` that integrates with the https://github.com/Password4j/password4j[Password4j] library, providing support for multiple password hashing algorithms including BCrypt, SCrypt, Argon2, and PBKDF2 through a unified interface
+
 == Config
 
 * Support modular configuration in xref::servlet/configuration/java.adoc#modular-httpsecurity-configuration[Servlets] and xref::reactive/configuration/webflux.adoc#modular-serverhttpsecurity-configuration[WebFlux]