Spring Security BCryptPasswordEncoder apparently uses an incorrect pattern

It sounds implausible, but it seems the pattern that BCryptPasswordEncoder uses is incorrect. It has an unescaped slash. If I escape it manually, the match occurs 🤷‍♂️

Comment From: jzheaux

Hi, @NadChel, thanks for the report. Can you do me a favor by adding to your report something a bit more copy-pastable? The images, while helpful for context, are difficult to use in the way of generating a unit test that can confirm the behavior you are seeing. A minimal sample is ideal.

Note that the following unit test passes, which makes me think we are missing something here:

@Test
public void matches() {
    BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
    String password = "mickey";
    String encoded = encoder.encode(password);
    assertThat(encoder.matches(password, encoded)).isTrue();
}

Furthermore, something roughly like your hash also passes:

@Test
public void slashPasses() {
    Pattern pattern = Pattern.compile("\\A\\$2(a|y|b)?\\$(\\d\\d)\\$[./0-9A-Za-z]{53}");
    assertThat(pattern.matcher("$2a$10$zJijCisT3IVbh5v2AitaJOwkfEy3VPvBmaYzdf/K.0/rCS7S3glQe").matches()).isTrue();
}

If I were to guess, the issue is the {bcrypt} value on the front which is, indeed, not a valid prefix for a BCrypt hash. That is added/removed when using PasswordEncoderFactories.createDelegatingPasswordEncoder(). So, if you encoded with that encoder, but then decoded directly with BCryptPasswordEncoder, then that would cause this behavior. For example:

@Test
public void failsToMatch() {
    PasswordEncoder delegating = PasswordEncoderFactories.createDelegatingPasswordEncoder();
    BCryptPasswordEncoder bcrypt = new BCryptPasswordEncoder();
    String password = "mickey";
    String encoded = delegating.encode(password);
    assertThat(bcrypt.matches(password, encoded)).isFalse();
}

Is that enough context to address the issue?

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: NadChel

Hi, @NadChel, thanks for the report. Can you do me a favor by adding to your report something a bit more copy-pastable? The images, while helpful for context, are difficult to use in the way of generating a unit test that can confirm the behavior you are seeing. A minimal sample is ideal.

Note that the following unit test passes, which makes me think we are missing something here:

java @Test public void matches() { BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(); String password = "mickey"; String encoded = encoder.encode(password); assertThat(encoder.matches(password, encoded)).isTrue(); }

Furthermore, something roughly like your hash also passes:

java @Test public void slashPasses() { Pattern pattern = Pattern.compile("\\A\\$2(a|y|b)?\\$(\\d\\d)\\$[./0-9A-Za-z]{53}"); assertThat(pattern.matcher("$2a$10$zJijCisT3IVbh5v2AitaJOwkfEy3VPvBmaYzdf/K.0/rCS7S3glQe").matches()).isTrue(); }

If I were to guess, the issue is the {bcrypt} value on the front which is, indeed, not a valid prefix for a BCrypt hash. That is added/removed when using PasswordEncoderFactories.createDelegatingPasswordEncoder(). So, if you encoded with that encoder, but then decoded directly with BCryptPasswordEncoder, then that would cause this behavior. For example:

java @Test public void failsToMatch() { PasswordEncoder delegating = PasswordEncoderFactories.createDelegatingPasswordEncoder(); BCryptPasswordEncoder bcrypt = new BCryptPasswordEncoder(); String password = "mickey"; String encoded = delegating.encode(password); assertThat(bcrypt.matches(password, encoded)).isFalse(); }

Is that enough context to address the issue?

You were right, removing {bcrypt} indeed resolved the situation (a while ago, actually). I'm sorry I didn't inform you earlier. I can't say I understood your explanation (I was told in one course video to include those {bcrypt} prefixes in my table), but I appreciate your effort 🙏

Comment From: jzheaux

Glad you got it working, @NadChel.

The {bcrypt} prefix is needed if you are using PasswordEncoderFactories, for example:

@Bean 
PasswordEncoder passwordEncoder() {
    return PasswordEncoderFactories.createDelegatingPasswordEncoder(); // should use {bcrypt} prefix
}

If you are using BCryptPasswordEncoder directly (or any other password encoder directly), then the prefix is not needed. For example:

@Bean 
PasswordEncoder passwordEncoder() {
    return new BCryptPasswordEncoder(); // should not use {bcrypt} prefix
}

The reason is because PasswordEncoderFactories supports many formats simultaneously. So, it requires a prefix to tell which algorithm is used for each hash.