spring-projects / spring-projects/spring-security
TokenBasedRememberMeServices::onLoginSuccess can throw a NPE when UserDetailsService::loadUserByUsername returns null.
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 6.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 52
Description
When the Authentication does not contain a password, TokenBasedRememberMeServices::onLoginSuccess performs a call to UserDetailsService::loadUserByUsername (~line 227), to obtain a UserDetails object.
It then proceeds to read UserDetails::getPassword without performing a null check.
In our workflow only a subset of usernames support "remember me", meaning the three possible responses from UserDetailsService::loadUserByUsername are:
- When the username exists and supports remember me --> an instance of
UserDetails - When the username exists and does NOT support remember me --> a null response
- When the username does NOT exist --> a
UsernameNotFoundException
Because TokenBasedRememberMeServices::onLoginSuccess is designed to simply return without generating a cookie when the password cannot be obtained, this issue can be fixed by simply changing line 228
from:
password = user.getPassword();
to:
password = (user == null) ? password : user.getPassword();
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at TokenBasedRememberMeServices::onLoginSuccess around line 227 and inspect the path where Authentication has no password and loadUserByUsername returns null. Done means this case returns without generating a cookie or throwing a NullPointerException, while existing UserDetails and UsernameNotFoundException behavior remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring
- Domain
- authentication, backend, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100