Skip to content

Commit cff6f18

Browse files
committed
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
1 parent 803cdcf commit cff6f18

File tree

1 file changed

+27
-18
lines changed

1 file changed

+27
-18
lines changed

Diff for: core/src/main/java/org/springframework/security/authentication/dao/DaoAuthenticationProvider.java

+27-18
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class DaoAuthenticationProvider extends AbstractUserDetailsAuthentication
5858
* {@link PasswordEncoder} implementations will short circuit if the password is not
5959
* in a valid format.
6060
*/
61-
private String userNotFoundEncodedPassword;
61+
private volatile String userNotFoundEncodedPassword;
6262

6363
private UserDetailsService userDetailsService;
6464

@@ -94,34 +94,43 @@ protected void additionalAuthenticationChecks(UserDetails userDetails,
9494

9595
protected void doAfterPropertiesSet() throws Exception {
9696
Assert.notNull(this.userDetailsService, "A UserDetailsService must be set");
97-
this.userNotFoundEncodedPassword = this.passwordEncoder.encode(USER_NOT_FOUND_PASSWORD);
9897
}
9998

10099
protected final UserDetails retrieveUser(String username,
101100
UsernamePasswordAuthenticationToken authentication)
102101
throws AuthenticationException {
103-
UserDetails loadedUser;
104-
102+
prepareTimingAttackProtection();
105103
try {
106-
loadedUser = this.getUserDetailsService().loadUserByUsername(username);
107-
}
108-
catch (UsernameNotFoundException notFound) {
109-
if (authentication.getCredentials() != null) {
110-
String presentedPassword = authentication.getCredentials().toString();
111-
passwordEncoder.matches(presentedPassword, userNotFoundEncodedPassword);
104+
UserDetails loadedUser = this.getUserDetailsService().loadUserByUsername(username);
105+
if (loadedUser == null) {
106+
throw new InternalAuthenticationServiceException(
107+
"UserDetailsService returned null, which is an interface contract violation");
112108
}
113-
throw notFound;
109+
return loadedUser;
110+
}
111+
catch (UsernameNotFoundException ex) {
112+
mitigateAgainstTimingAttack(authentication);
113+
throw ex;
114+
}
115+
catch (InternalAuthenticationServiceException ex) {
116+
throw ex;
114117
}
115-
catch (Exception repositoryProblem) {
116-
throw new InternalAuthenticationServiceException(
117-
repositoryProblem.getMessage(), repositoryProblem);
118+
catch (Exception ex) {
119+
throw new InternalAuthenticationServiceException(ex.getMessage(), ex);
118120
}
121+
}
122+
123+
private void prepareTimingAttackProtection() {
124+
if (this.userNotFoundEncodedPassword == null) {
125+
this.userNotFoundEncodedPassword = this.passwordEncoder.encode(USER_NOT_FOUND_PASSWORD);
126+
}
127+
}
119128

120-
if (loadedUser == null) {
121-
throw new InternalAuthenticationServiceException(
122-
"UserDetailsService returned null, which is an interface contract violation");
129+
private void mitigateAgainstTimingAttack(UsernamePasswordAuthenticationToken authentication) {
130+
if (authentication.getCredentials() != null) {
131+
String presentedPassword = authentication.getCredentials().toString();
132+
this.passwordEncoder.matches(presentedPassword, this.userNotFoundEncodedPassword);
123133
}
124-
return loadedUser;
125134
}
126135

127136
/**

0 commit comments

Comments
 (0)