Lazily initialize userNotFoundEncodedPassword

Update `DaoAuthenticationProvider` so that `userNotFoundEncodedPassword`
is lazily initialized on the first call to `retrieveUser`, rather than
in `doAfterPropertiesSet`.

Since some `PasswordEncoder` implementations can be slow, this change
can help to improve application startup times and the expense of some
delay with the first login.

Note that `userNotFoundEncodedPassword` creation occurs on the first
user retrieval, regardless of whether the user is ultimately found. This
ensures consistent processing times, regardless of the outcome.

First Call:
	Found      = encode(userNotFound) + decode(supplied)
	Not-Found  = encode(userNotFound) + decode(userNotFound)

Subsequent Call:
	Found      = decode(supplied)
	Not-Found  = decode(userNotFound)

Fixes gh-4915
This commit is contained in:
Phillip Webb 2017-12-21 21:38:43 -08:00 committed by Rob Winch
parent 0eef5b4b42
commit fd78d055aa
1 changed files with 27 additions and 18 deletions

View File

@ -58,7 +58,7 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication
* {@link PasswordEncoder} implementations will short circuit if the password is not * {@link PasswordEncoder} implementations will short circuit if the password is not
* in a valid format. * in a valid format.
*/ */
private String userNotFoundEncodedPassword; private volatile String userNotFoundEncodedPassword;
private UserDetailsService userDetailsService; private UserDetailsService userDetailsService;
@ -94,34 +94,43 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication
protected void doAfterPropertiesSet() throws Exception { protected void doAfterPropertiesSet() throws Exception {
Assert.notNull(this.userDetailsService, "A UserDetailsService must be set"); Assert.notNull(this.userDetailsService, "A UserDetailsService must be set");
this.userNotFoundEncodedPassword = this.passwordEncoder.encode(USER_NOT_FOUND_PASSWORD);
} }
protected final UserDetails retrieveUser(String username, protected final UserDetails retrieveUser(String username,
UsernamePasswordAuthenticationToken authentication) UsernamePasswordAuthenticationToken authentication)
throws AuthenticationException { throws AuthenticationException {
UserDetails loadedUser; prepareTimingAttackProtection();
try { try {
loadedUser = this.getUserDetailsService().loadUserByUsername(username); UserDetails loadedUser = this.getUserDetailsService().loadUserByUsername(username);
} if (loadedUser == null) {
catch (UsernameNotFoundException notFound) { throw new InternalAuthenticationServiceException(
if (authentication.getCredentials() != null) { "UserDetailsService returned null, which is an interface contract violation");
String presentedPassword = authentication.getCredentials().toString();
passwordEncoder.matches(presentedPassword, userNotFoundEncodedPassword);
} }
throw notFound; return loadedUser;
} }
catch (Exception repositoryProblem) { catch (UsernameNotFoundException ex) {
throw new InternalAuthenticationServiceException( mitigateAgainstTimingAttack(authentication);
repositoryProblem.getMessage(), repositoryProblem); throw ex;
} }
catch (InternalAuthenticationServiceException ex) {
throw ex;
}
catch (Exception ex) {
throw new InternalAuthenticationServiceException(ex.getMessage(), ex);
}
}
if (loadedUser == null) { private void prepareTimingAttackProtection() {
throw new InternalAuthenticationServiceException( if (this.userNotFoundEncodedPassword == null) {
"UserDetailsService returned null, which is an interface contract violation"); this.userNotFoundEncodedPassword = this.passwordEncoder.encode(USER_NOT_FOUND_PASSWORD);
}
}
private void mitigateAgainstTimingAttack(UsernamePasswordAuthenticationToken authentication) {
if (authentication.getCredentials() != null) {
String presentedPassword = authentication.getCredentials().toString();
this.passwordEncoder.matches(presentedPassword, this.userNotFoundEncodedPassword);
} }
return loadedUser;
} }
/** /**