spring-projects / spring-projects/spring-security
CachingUserDetailsService doesn't properly support expiraton of cache
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 6.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 52
Description
Describe the bug
Using Spring Security 6.3.1 we're using LdapUserDetailsService to fetch user data and authorities. To decrease number of requests to LDAP infrastructure, we're using CachingUserDetailsService configured with LdapUserDetailsService as delegate, and SpringCacheBasedUserCache backed by Caffeine.
Caffeine is set up with "expireAfterWrite=1m" spec, so authorities of user are cached 1 minute after LDAP call.
The issue: delegate method LdapUserDetailsService::loadUserByUsername should be called every minute if user is sending requests in small intervals (<1 minute), but it's called only once per request batch - each access of user details in CachingUserDetailsService refreshes the cache with cached value:
@Override
public UserDetails loadUserByUsername(String username) {
UserDetails user = this.userCache.getUserFromCache(username);
if (user == null) {
user = this.delegate.loadUserByUsername(username);
}
Assert.notNull(user, () -> "UserDetailsService " + this.delegate + " returned null for username " + username
+ ". " + "This is an interface contract violation");
// FIXME it's putting value read from cache back into the cache, thus resetting expiryAfterWrite timeout
this.userCache.putUserInCache(user);
return user;
}
moving putUserInCache just after user is loaded from delegate inside the if block should fix the issue.
To Reproduce
Configure UserDetailsService as:
@Bean
UserDetailsService userDetailsService(LdapUserSearch search, LdapAuthoritiesPopulator populator, LdapDetailsMapper mapper) {
// ldap search
var delegate = new LdapUserDetailsService(search, populator);
delegate.setUserDetailsMapper(mapper);
// cache definition
var caffeine = new CaffeineCacheManager();
caffeine.setCacheSpecification("expireAfterWrite=1m");
//final bean
var userDetailsService = new CachingUserDetailsService(delegate);
userDetailsService.setUserCache(new SpringCacheBasedUserCache(caffeine.getCache("userCache")));
return userDetailsService;
}
then require user detais more often that expiryAfterWrite cache timeout.
Expected behavior
Cache expires a minute after first requests, refreshes authorities from LDAP.
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 in core/src/main/java/org/springframework/security/authentication/CachingUserDetailsService.java, especially loadUserByUsername, and reproduce the behavior with the Caffeine expireAfterWrite=1m configuration described in the issue. Done means repeated requests no longer reset the original cache expiry, so the delegate refreshes user details after the timeout.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- authentication
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100