Description
Summary
When using DelegatingPasswordEncoder to which is provided a custom PasswordEncoder implementation in which upgradeEncoding() is overridden, neither DaoAuthenticationProvider.createSuccessAuthentication() nor the called DelegatingPasswordEncoder.upgradeEncoding() consults the overridden PasswordEncoder.upgradeEncoding()
Actual Behavior
DaoAuthenticationProvider.createSuccessAuthentication() reencodes the password if it has a different encoding than what DelegatingPasswordEncoder was created with (idForEncode).
It consults DelegatingPasswordEncoder.upgradeEncoding() but ignores PasswordEncoder.upgradeEncoding()
Expected Behavior
DelegatingPasswordEncoder.upgradeEncoding() should check with the delegate (for the {id})
if it should upgrade the encoding.
Similar to how DelegatingPasswordEncoder.matches(CharSequence,String) does it.
Version
spring-security-core:5.1.1.RELEASE
Sample
I create an instance of DelegatingPasswordEncoder to which I specify encoders
that should handle the different types of encoded passwords it might come across.
@Bean
public PasswordEncoder delegatingPasswordEncoder() {
PasswordEncoder defaultEncoder = new StandardPasswordEncoder();
Map<String, PasswordEncoder> encoders = new HashMap<>();
encoders.put("bcrypt", new BCryptPasswordEncoder());
encoders.put("scrypt", new SCryptPasswordEncoder());
encoders.put("sha1", new MySHA1PasswordEncoder());
DelegatingPasswordEncoder passworEncoder = new DelegatingPasswordEncoder(
"bcrypt", encoders);
passworEncoder.setDefaultPasswordEncoderForMatches(defaultEncoder);
return passworEncoder;
}
For whatever reason, I might want to re-encode passwords, but not when they're 'sha1'. Thus, return false:
class MySHA1PasswordEncoder implements PasswordEncoder
@Override String encode() {..}
@Override boolean matches() {..}
@Override boolean upgradeEncoding() {
return false;
}
But this has no effect. After successful auth:
DaoAuthenticationProvider.createSuccessAuthentication() doesn't consult MySHA1PasswordEncoder.upgradeEncoding():
protected Authentication createSuccessAuthentication(Object principal,
Authentication authentication, UserDetails user) {
boolean upgradeEncoding = this.userDetailsPasswordService != null
&& this.passwordEncoder.upgradeEncoding(user.getPassword());
'this.passwordEncoder' is the DelegatingPasswordEncoder created earlier, and its impl:
@Override
public boolean upgradeEncoding(String encodedPassword) {
String id = extractId(encodedPassword);
return !this.idForEncode.equalsIgnoreCase(id);
}
I would expect either of these to check with the delegate, eg MySHA1PasswordEncoder.upgradeEncoding()
if the re-encoding should happen.
DelegatingPasswordEncoder.upgradeEncoding() seems to assume I definitely want it to happen, even though I had to option to override PasswordEncoder.upgradeEncoding()
--
There are examples where people use AuthenticationSuccessEvent, but seems unncessary given the ability to override upgradeEncoding()
- https://www.baeldung.com/spring-security-5-password-storage
- https://github.com/michael-simons/passwordmigration/blob/86cd982d13f009e162d744cf6a266f207871afa0/src/main/java/ac/simons/spring/passwordmigration/PasswordmigrationApplication.java
--
Any insight would be valued.