[keycloak-user] Why doesn't UserCacheSession try to cache the new user model after invalidation?

[Jared Blashka]

I've been looking into the UserCacheSession behavior after we updated to
2.5.5.Final and I'm seeing some strange behavior related to cached
UserModel invalidation.

After a UserModel in the user cache is flagged for invalidation, either
because there was some update to the user model (e.g. Adding a required
action when setting up OTP credentials) or the cache's invalidation time
limit was reached, every single subsequent call within that thread to
getUserById or getUserByUsername will delegate that call to the Storage
Provider responsible for that user without caching the result. And it isn't
until the *next* thread that tries to fetch that user from the infinispan
cache that the result is actually cached again.

Is this intended behavior?

In my testing I have an OIDC client and a SAML client for my 2.5.5 server.
If the OIDC client makes a token refresh request after the user has passed
its lifespan time it triggers 7 (each UserPropertyMapper makes a separate
userSession.getUser call) separate invocations of my Storage Provider's
getUserById method, each one triggering a query against our data store. If
I then try to access a second client (my SAML client) Keycloak still
doesn't have the user model cached and delegates the call to my storage
provider yet again, but caches the result this time.

Are our custom Storage Providers expected to cache the results of our
getUserById/Username calls for situations like these? I would think that
UserCacheSession would more gracefully handle cache invalidation so that
every customer with their own Storage Provider wouldn't have to manage
their own additional cache layer on top of what Keycloak is already
providing.

My knowledge of infinispan is pretty lacking, so is there a reason that the
UserCacheSesssion.getUserById call short-circuits to the delegate with:

if (isRegisteredForInvalidation(realm, id)) {
    logger.trace("registered for invalidation return delegate");
    return getDelegate().getUserById(id, realm);
}


rather than caching the result with something like:

if (invalidations.contains(id)) {
    Long loaded = cache.getCurrentRevision(id);
    UserModel delegate = getDelegate().getUserById(id, realm);
    adapter = cacheUser(realm, delegate, loaded);
    invalidations.remove(id)
    managedUsers.put(id, adapter);
    return adapter;
}



With this behavior we're actually seeing more activity against our backend
server than if we were just using the NO_CACHE policy.

Thanks!
Jared