Fix NPE in TokenBasedRememberMeServices with null UserDetails #7252
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
There was a problem hiding this comment.
Thank you for the report and PR @codeconsole . I have left some feedback inline.
| @@ -123,6 +123,11 @@ protected UserDetails processAutoLoginCookie(String[] cookieTokens, | |||
| UserDetails userDetails = getUserDetailsService().loadUserByUsername( | |||
| cookieTokens[0]); | |||
|
|
|||
| if (userDetails == null) { | |||
| throw new InvalidCookieException("Cookie token[0] contained username '" | |||
There was a problem hiding this comment.
Given the contract of the loadUserByUsername method, it should never return null if the user is not found. Instead it should throw a UsernameNotFoundException. Therefore, a more appropriate error message would be that there is an interface contract violation. Here are 2 examples of where we already do this:
and
| throw new InvalidCookieException("Cookie token[0] contained username '" | ||
| + cookieTokens[0] + "' that does not exist."); | ||
| } | ||
|
|
There was a problem hiding this comment.
Please add a test for this scenario in TokenBasedRememberMeServicesTests. The test will be very similar to autoLoginClearsCookieIfUserNotFound.
rwinch
changed the title
|
@codeconsole I've added your commit to master and followed up with a polish commit that adds the changes I mentioned. |
eleftherias
added
in: web
Fixes #7251
Checks to see if the userService found a userDetails or not. If the user has since been deleted, this will end up with an uncaught null pointer exception and result in the user not being able to visit the site.