Skip to content

Commit 743960d

Browse files
committed
SEC-2122: Fix broken integration tests.
Modified BCryptPasswordEncoder to no longer throw an IllegalArgumentException when the encoded password is empty or the incorrect format for bcrypt. Instead it now logs a warning that non bcrypt data was found. The Dms integration tests were failing after being changed to use bcrypt and this fixes the issue.
1 parent d872763 commit 743960d

File tree

2 files changed

+13
-15
lines changed

2 files changed

+13
-15
lines changed

crypto/src/main/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoder.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
import java.security.SecureRandom;
1919
import java.util.regex.Pattern;
20+
import org.apache.commons.logging.Log;
21+
import org.apache.commons.logging.LogFactory;
2022

2123
import org.springframework.security.crypto.password.PasswordEncoder;
2224

@@ -30,6 +32,7 @@
3032
*/
3133
public class BCryptPasswordEncoder implements PasswordEncoder {
3234
private Pattern BCRYPT_PATTERN = Pattern.compile("\\A\\$2a?\\$\\d\\d\\$[./0-9A-Za-z]{53}");
35+
private final Log logger = LogFactory.getLog(getClass());
3336

3437
private final int strength;
3538

@@ -74,11 +77,13 @@ public String encode(CharSequence rawPassword) {
7477

7578
public boolean matches(CharSequence rawPassword, String encodedPassword) {
7679
if (encodedPassword == null || encodedPassword.length() == 0) {
77-
throw new IllegalArgumentException("Encoded password cannot be null or empty");
80+
logger.warn("Empty encoded password");
81+
return false;
7882
}
7983

8084
if (!BCRYPT_PATTERN.matcher(encodedPassword).matches()) {
81-
throw new IllegalArgumentException("Encoded password does not look like BCrypt");
85+
logger.warn("Encoded password does not look like BCrypt");
86+
return false;
8287
}
8388

8489
return BCrypt.checkpw(rawPassword.toString(), encodedPassword);

crypto/src/test/java/org/springframework/security/crypto/bcrypt/BCryptPasswordEncoderTests.java

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,27 +66,20 @@ public void customRandom() {
6666
assertTrue(encoder.matches("password", result));
6767
}
6868

69-
@Test(expected = IllegalArgumentException.class)
70-
public void barfsOnNullEncodedValue() {
69+
@Test
70+
public void doesntMatchNullEncodedValue() {
7171
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
7272
assertFalse(encoder.matches("password", null));
7373
}
7474

75-
@Test(expected = IllegalArgumentException.class)
76-
public void barfsOnEmptyEncodedValue() {
75+
@Test
76+
public void doesntMatchEmptyEncodedValue() {
7777
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
7878
assertFalse(encoder.matches("password", ""));
7979
}
8080

81-
@Test(expected = IllegalArgumentException.class)
82-
public void barfsOnShortEncodedValue() {
83-
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
84-
String result = encoder.encode("password");
85-
assertFalse(encoder.matches("password", result.substring(0, 4)));
86-
}
87-
88-
@Test(expected = IllegalArgumentException.class)
89-
public void barfsOnBogusEncodedValue() {
81+
@Test
82+
public void doesntMatchBogusEncodedValue() {
9083
BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
9184
assertFalse(encoder.matches("password", "012345678901234567890123456789"));
9285
}

0 commit comments

Comments
 (0)