[keycloak-dev] User Federation Provider Cache

[Marek Posolda]

On 15/06/16 17:33, Bill Burke wrote:
> This redesign is quite complex as you have to take a lot of variables 
> into account.  I also am trying to make things backward compatible and 
> have the new SPI co-exist with the old. Just remember though, the 
> biggest reason for this rewrite is to AVOID IMPORTING!
>
> Here's what I'm currently doing:
>
> Federated Data
> --------------
> There will be a built-in federated user data service.  It exists to 
> store additional data about a user, for example:
>
> public interface UserAttributeFederatedStorage {
>     void setSingleAttribute(RealmModel realm, UserModel user, String 
> name, String value);
>     void setAttribute(RealmModel realm, UserModel user, String name, 
> List<String> values);
>     void removeAttribute(RealmModel realm, UserModel user, String name);
>     MultivaluedHashMap<String, String> getAttributes(RealmModel realm, 
> UserModel user);
> }
>
> There will be federated storage options for all data.
>
> User Storage Provider SPI
> -----------------
> Current UserProvider is going to be split up into smaller interface. 
> SPI will remain the same though.  There is going to be a 
> UserStorageProvider SPI.  Instances can implement one or more of the 
> smaller UserProvider interfaces.
>
> User Storage Provider instances will be returning UserModel instances. 
> This means providers will be responsible for implementing some sort of 
> UserAdapter implementation.  These custom UserAdapters will be 
> responsible for interacting with the Federated Data Store and their 
> custom storage:  Merging data between i.e. LDAP and the Federated 
> Storage, managing which store stores what data, managing what data 
> lives where.  Going to provide a helper abstract UserAdapter class 
> that helps managing this data.
Ok. Am I understand correctly that "smaller" providers are not limited 
to be either fully-LDAP based or fully-localDB based? For example I want 
"attribute-a" of user "john" to be stored in LDAP, however "attribute-b" 
of same user will be stored in local DB. From what you wrote, I assume 
it will work as UserAdapter can delegate to other store if it doesn't 
find "attribute-b" in LDAP?


Adding again the scenario with requiredActions, which we currently 
support and which we briefly discussed on F2F. I am adding it just to 
ensure that we won't loose the support for this :)
- Password in MSAD is expired
- Keycloak reads some corresponding attribute from MSAD and it sets 
UPDATE_PASSWORD requiredAction on the user based on it
- Keycloak asks user to update password and this is then propagated to MSAD

In other words, just the UPDATE_PASSWORD requiredAction is handled by 
MSAD, but all the other required actions need to be handled by Keycloak 
local DB.


>
> There will be a UserStorageManager class that implements all 
> UserProvider instances.  Our caching layer will delegate to that 
> instead of the actual storage provider.  The UserStorageManager will 
> work similarly to UserFederationManager.  So, it would work like this:
>
> 1. session.users().getUserByUsername(realm, username)
> 2. User cache looks in cache, doesn't find it,
> 3. UserStorageManager.getUserByUsername()
> 4. UserStorageManager iterates all UserStorageProviders looking to see 
> which one offers session.users().getUserByUsername() and invoke it.
> 5. LDAP.getUserByUsername()
> 6. LDAP finds the user, creates a UserAdapter, returns it.
> 7. User cache gets the UserModel returned by LDAP, it caches exactly 
> like it is implemented now.  The LDAP UserAdapter, is responsible for 
> pulling data and merging it from LDAP store and Federation Store.
>
> User ids are an URN
>
> "f:" + providerId + ":" + provider-user-metadata
>
> UserStorageManager.getUserById() will parse the ID and call directly 
> on the UserStorageProvider that initially loaded the user.
>
> Updates are invoked directly on UserModel class.  LDAP UserAdapter 
> will be responsible for figuring out what data is stored/updated in 
> LDAP and what should be stored in Federated Storage.
>
> More comments follow:
>
>
> On 6/15/16 4:47 AM, Marek Posolda wrote:
>> Hi Bill,
>>
>> few notes from me. I am mostly adding the scenarios, which will be good
>> to support/improve or which we should still keep supporting after
>> refactoring IMO. Sorry for bit longer email :)
>>
>> Note: When I talk "LDAP", I usually mean "LDAP or any other 3rd party
>> federation storage"
>>
>>
>> Persistent vs. Transient modes
>> ----------------------------------------
>>
>> I think we should still keep some support for "persistent" mode (the
>> case when user is imported to Keycloak local DB).
>>
>
> I want to get rid of import and only have it as an option for 
> Brokering or for transient users (users that can't be looked up from a 
> store out of band).  The LDAP adapter will be responsible for knowing 
> what data is stored in LDAP and what data is stored in the Federated 
> Store, so there is no need to "import"
Ok, so LDAP data are not imported, however the non-LDAP data (or 
overwritten data) are read from local store right? So we still support 
the scenario with overwriting data (unsynced mode) like this?

- User from LDAP authenticates and his pasword is verified against LDAP
- User changes his password, but LDAP is read-only, so new password 
needs to be written to local DB
- User authenticates again, but now his password needs to be verified 
against local DB

>
>
>> Assuming the scenario:
>> - john is imported from LDAP with "unsynced" mode
>> - john changes his password in account management. LDAP is unsynced
>> (read-only), so the password needs to be changed in Keycloak storage
>> - After server restart, john should have set the new password, not the
>> old one from LDAP. Hence the only possibility seems to keep support for
>> "import" this user into Keycloak DB and have user persistent.
>>
>> The similar situation applies for any update of user, which can't be
>> saved back to LDAP (either because LDAP is read-only or because it
>> doesn't support store the keycloak metadata like social links, consents,
>> required actions etc)
>>
>> Regarding this, I am thinking about possible import modes like:
>> 1) Persistent : User attributes from LDAP are imported into Keycloak DB
>> (same behaviour like now)
>>
>
> No more importing.
>
>> 2) Transient : User attributes from LDAP are not imported into Keycloak
>> DB, but just cached locally. Any updates of the user are either dropped
>> after server restart or disallowed. For example: Maybe consents can be
>> just dropped, but some other things like requiredRoles should be rather
>> disallowed to change? For example if admin adds some requiredAction to
>> user "john", the requiredAction shouldn't disappear after server
>> restart. This would be a security hole IMO. So in that case, if admin
>> tries to manually add requiredAction to such user, the exception will be
>> thrown so admin is aware that this is not supported. Anyway, for support
>> any writing to cache, the cache will need to be replicated though (for
>> example: user consent saved into cache on node1 should be visible on
>> node2 as well).
>>
>
>
>> 3) Hybrid : LDAP user is not imported into Keycloak DB immediatelly.
>> They are imported "on-demand" after user is updated and the update can't
>> be saved to LDAP
>>
>
> No more on-demand loading of data.  IIRC, we discussed this in Brno. 
> LDAP users will be cached in the same way JPA Users currently are.
>
>
>
>>
>> Caching
>> -----------
>>
>> I hope new SPI will allow us more flexibility to support various
>> scenarios. It will be good to support at least those IMO:
>>
>
> See above.  LDAP users will be cached in same way JPA Users currently 
> are.
So if I understand correctly,we will loose the option (1), which we 
currently support like:

LDAP user data are always read from "online" LDAP in every request, but 
all other user data are cached, so no need to read them from local 
Keycloak DB (even for many hours).

In other words, we will have option to use different expiration times 
(ie. LDAP user is expired from cache every 5 minutes, when fully-local 
user is expired every 2 hours), however you will need to read each 5 
minutes all the data of user "john" from both LDAP and local-db, right?

However loosing this one is probably not so big issue....
>
>> 3) I agree that will be cool to support different expiration time based
>> if it's LDAP user or just Keycloak-local user. Infinispan allows it
>> though with something like : cache.put("ldapuser", ldapUser, 60,
>> TimeUnit.SECONDS);
>>
>
> I think this will be an option that we will want to support.
>
>>
>>
>> Transactions and 3rd party updates
>> -----------------------------------------------
>>
>> - Will be good to improve registration of user to LDAP. Ideally during
>> registration new user to LDAP, we should allow to send all data at once.
>> (currently UserFederationProvider.register supports sending just
>> username). Also we should allow to specify if register to 3rd party
>> provider should be done *before* or *after* the registration to Keycloak
>> local DB. For details, see https://issues.jboss.org/browse/KEYCLOAK-1075
>> and all the comments from users...
>>
>> - Also updating user should be ideally at once. For example if you call:
>> user.setFirstName("john");
>> user.setLastName("Doe");
>> user.setEmail("john at email.cz");
>>
>> we shouldn't have 3 update calls to LDAP, but just one. Maybe we can
>> address this with transaction abstraction? I've already did something
>> for LDAP provider (see TxAwareLDAPUserModelDelegate ), however will be
>> good to provide something more generic for user storage SPI. Then when
>> KeycloakTransactionManager.commit is called, the data are send to
>> federation storage
>>
>
> This would be an implementation detail.  Similar to JPA Users, the 
> LDAP UserAdapter will be remembered per session.  The LDAP adapter 
> could queue up updates then at commit time flush them all.
>
> Maybe you should consider writing an LDAP version of JPA?  ;-) 
> Probably a lot of code could be borrowed from PL IDM API.
I've wrote already something similar for Mongo ;) Also I already 
addressed the multiple-updates issue for LDAP (see above). However this 
one is mainly issue for 3rd party providers rather than for LDAP 
provider. Especially since 3rd party provider may have some validations 
for some attributes.


For example consider this scenario, which can happen in current 
implementation:

UserModel john = session.users().addUser(realm, "john");
john.setEmail("john at email.org");

All of this happens during "addUser" call:

- User "john" is saved to local-db
- SomeUserFederationProvider.register calls the registration request to 
3rd party federation storage. User "john" is saved in 3rd party storage now.

Then john.setEmail is invoked
- There is request to 3rd party federation storage to update email.
- 3rd party storage refuse to save the email because there is already an 
existing user with email "john at email.org" . So it sends back the 
validation error. Now keycloak can rollback the transaction, however 
user "john" was already registered in 3rd party federation storage from 
previous "register" call -> FAIL

So now you need to manually remove the user from 3rd party storage, 
which is dirty hack. There are other possible hacks to address this. For 
example someone "postpone" the registration in "register" method by 
adding some helper attributes and sends the registration request later 
when all the attributes are set. I suggest to read the comments in 
https://issues.jboss.org/browse/KEYCLOAK-1075 for details.

IMO the proper solution is to ensure that "register" request to 3rd 
party is postponed until the transaction.commit when user is filled with 
all the attributes. Similarly update should be done just once. IMO we 
should provide something at SPI level to simplify this scenario.

I understand that main motivation for you is to avoid "imports", however 
since we are refactoring anyway, we should try to address other SPI 
limitations too. And IMO this one is pretty bad and worth improve ;)
>
>
>
>>
>> Sync users
>> --------------
>> We should still keep the option to sync users into Keycloak DB as we
>> have now. Note some persistent storages like LDAP are limited with
>> pagination. So the easiest possibility for some admins is just to sync
>> users, so they can easily search them in admin console.
>>
> Doing a full import just to support pagination is overkill.  I'm 
> guessing that a lot of deployments will not manage users through the 
> Keycload admin console.  We can offer a manual a Sync SPI that 
> providers can implement.
Maybe it's overkill for 90% of deployments, but remaining 10% want to 
see all LDAP users in admin console immediatelly and hence want to sync 
them? IMO it's always good to have SPI flexible so it can easily support 
all the possible requirements. However I understand that it's not always 
possible...

Marek
>
>
>
>
>>
>> Proxy yes/no?
>> ------------------
>>
>
> Proxy yes, but it won't work the same.  See above.
>
> Bill