Enable providing a different default encoding id for the PasswordEncoderFactories#createDelegatingPasswordEncoder

[filiphr]

Summary

It would be good if we can pass a custom encoding id to the PasswordEncoderFactories#createDelegatingPasswordEncoder. This way we can use the defaults from Spring Security, but pick a default encoder.

Actual Behavior

public static PasswordEncoder createDelegatingPasswordEncoder() {
    String encodingId = "bcrypt";
    Map<String, PasswordEncoder> encoders = new HashMap<>();
    encoders.put(encodingId, new BCryptPasswordEncoder());
    encoders.put("ldap", new LdapShaPasswordEncoder());
    encoders.put("MD4", new Md4PasswordEncoder());
    encoders.put("MD5", new MessageDigestPasswordEncoder("MD5"));
    encoders.put("noop", NoOpPasswordEncoder.getInstance());
    encoders.put("pbkdf2", new Pbkdf2PasswordEncoder());
    encoders.put("scrypt", new SCryptPasswordEncoder());
    encoders.put("SHA-1", new MessageDigestPasswordEncoder("SHA-1"));
    encoders.put("SHA-256", new MessageDigestPasswordEncoder("SHA-256"));
    encoders.put("sha256", new StandardPasswordEncoder());

    return new DelegatingPasswordEncoder(encodingId, encoders);
}

Expected Behavior

public static PasswordEncoder createDelegatingPasswordEncoder(String encodingId) {
    // perform validation that the encoding id is within the known ids
    Map<String, PasswordEncoder> encoders = new HashMap<>();
    encoders.put("bcrypt", new BCryptPasswordEncoder());
    encoders.put("ldap", new LdapShaPasswordEncoder());
    encoders.put("MD4", new Md4PasswordEncoder());
    encoders.put("MD5", new MessageDigestPasswordEncoder("MD5"));
    encoders.put("noop", NoOpPasswordEncoder.getInstance());
    encoders.put("pbkdf2", new Pbkdf2PasswordEncoder());
    encoders.put("scrypt", new SCryptPasswordEncoder());
    encoders.put("SHA-1", new MessageDigestPasswordEncoder("SHA-1"));
    encoders.put("SHA-256", new MessageDigestPasswordEncoder("SHA-256"));
    encoders.put("sha256", new StandardPasswordEncoder());

    return new DelegatingPasswordEncoder(encodingId, encoders);
}

public static PasswordEncoder createDelegatingPasswordEncoder() {
    return createDelegatingPasswordEncoder("bcrypt");
}

Version

5.0.7.RELEASE

[rwinch]

Thanks for the report!

We don't really want to provide options for modifications of the defaults as this will add lots of complexities for what is the default behavior. Additionally, you can do this by layering the DelegatingPasswordEncoder. For example:

PasswordEncoder defaults = PasswordEncoderFactories.createDelegatingPasswordEncoder();

String encodingId = "scrypt";
Map<String, PasswordEncoder> encoders = new HashMap<>();
encoders.put(encodingId, new SCryptPasswordEncoder());

DelegatingPasswordEncoder result = new DelegatingPasswordEncoder(encodingId, encoders);
result.setDefaultPasswordEncoderForMatches(defaults);

return result;

Would you be interested in submitting a PR to update the Javadoc to include this information?

[filiphr]

Just curious why would it be complex if the proposed method is added? The default will still be there, there will just be an option to create one wit a different encoding id.

The example you wrote will actually work, but I wanted to avoid doing that (and didn't thought about it). The only caveat is that I would need to have 2 levels of indirection within the encoder.

Would you be interested in submitting a PR to update the Javadoc to include this information?

Yes I can submit such PR. I am also willing to submit a PR with my proposed change as well.

[rwinch]

Because then someone will want to be able to modify something else about it. These all add up to little changes which can already be resolved in a simple way.

[gandrade]

Would be an issue have createDelegatingPasswordEncoder(String encodingId) in order to set an encoder from the existing list? Today, bcrypt it's the default encodingId, it would be just a way to overwrite this behavior, but still relying on the already provided encoders, not overwriting those.

[gandrade]

Let's take the following snippet as sample:

DelegatingPasswordEncoder encoder = (DelegatingPasswordEncoder) PasswordEncoderFactories.createDelegatingPasswordEncoder();
encoder.setDefaultPasswordEncoderForMatches(new MessageDigestPasswordEncoder("SHA-1"));

For whom that needs to overcome the default encoder implementation, which it's the case, I need to create a new instance of MessageDigestPasswordEncoder("SHA-1") and to invoke setDefaultPasswordEncoderForMatches passing a MessageDigestPasswordEncoder instance. However, there's already an SHA-1 implementation inside of PasswordEncoderFactories class. So, in this sense, it would be handy to have setDefaultPasswordEncoderForMatches accepting an encodingId as a parameter. What do you think @rwinch?

DelegatingPasswordEncoder encoder = (DelegatingPasswordEncoder) PasswordEncoderFactories.createDelegatingPasswordEncoder();
encoder.setDefaultPasswordEncoderForMatches("SHA-1");

[hbourada]

I add same request: #7761