[keycloak-dev] Federation & LDAP caching

[Marek Posolda]

We have still open issue https://issues.jboss.org/browse/KEYCLOAK-838 . 
In shortcut, during authentication of user into LDAP there are 9 same 
queries to LDAP to load this single user. Similar situation might be for 
other federation providers. Few ideas to improve:

1) Simple per-request caching at UserFederationManager level - currently 
each call to "session.users().getUserByXXX()" always call "isValid" on 
federation provider causing that validation/loading of same user might 
be done more time per request. With very simple enhancement at 
UserFederationManager level, we ensure that loading of user will be done 
just once per request (per KeycloakSession), which is not an issue IMO 
as KeycloakSession is really short-living object. Note that this is not 
specific to LDAP, but should improve performance of UserFederation in 
general. I have very simple patch here 
https://github.com/mposolda/keycloak/commit/f8302dac0e927756ca0d49a67500f6d5149ac8ae 
. With this, number of loads to LDAP per authentication is reduced from 
9 calls to 3. Any objections against merging it?


2) Global caching - this is obviously much more tricky, but I think that 
earlier or later we need to address it as (1) might not be sufficient. 
Few ideas:

2.a) Allow to switch the user cache on top of delegation. Currently we 
have UserProvider delegation chain like:

User Federation => User Cache Provider => JPA/Mongo UserProvider

The alternative I mean is, that people will have possibility to 
configure the chain like:

User Cache Provider => User Federation => JPA/Mongo UserProvider

By default, the order will be still the same like it's now, however I 
can imagine that for many LDAP (and other federation) deployments, the 
admins use them just as read-only store and don't do any direct changes 
here. Hence there is no reason to always validate/proxy users and ask 
federation storage if user still exists etc but instead suffer from our 
already existing cache impl and enjoy improved performance.

I propose we will just add on/off switch to admin console, which will 
allow to specify when cache should be put on top of federation or after 
(like it's now). We may also need to add the button to clear the cache. 
WDYT?

2.b) Another alternative is to put cache layer on top of 
LDAPIdentityStore. TBH I am not the fan of it as it's specific to LDAP 
and it requires another implementation of caching with all the pitfalls 
(Figure which items should be invalidated during writes, abstraction to 
support both "mem" and cluster/infinispan deployments etc...)

my proposal is to go with (1) for this  release and then look at (2.a) 
if I have time as it may be another few days of work (so maybe postpone 
to next release)

WDYT, other better ideas to improve?
Marek