[keycloak-dev] User Federation Provider Cache
Marek Posolda
mposolda at redhat.com
Wed Jun 15 04:47:46 EDT 2016
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).
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)
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
Caching
-----------
I hope new SPI will allow us more flexibility to support various
scenarios. It will be good to support at least those IMO:
1) Keep the option we have now (LDAP user data are always read from
"online" LDAP, but other user data are cached, so no need to read them
from Keycloak DB)
2) Option to allow support for caching of LDAP data even if mode is
"persistent" . I want LDAP users imported into Keycloak DB, but still I
don't want to always "validate" user in LDAP during each request like
it's now. This will be good for people, who prefer performance rather
then always seeing latest LDAP data.
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);
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
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.
Proxy yes/no?
------------------
As we discussed in F2F, the proxy is maybe a bit complicated pattern,
but it's very flexible. It allows to proxy/override exactly just the
methods you want (for example: add UPDATE_PASSWORD requiredAction to
user if ActiveDirectory password is expired. Add group mappings or role
mappings dynamically etc)
Marek
On 13/06/16 14:56, Bill Burke wrote:
>
> I'm working on a new SPI right now. Here is my notebad to capture how
> things work, issues weed to consider, and problems we have to solve:
>
> https://github.com/keycloak/keycloak/wiki/2.0-User-Federation-Storage-SPI
>
>
> On 6/13/16 3:39 AM, Marek Posolda wrote:
>> We discussed some time ago how to ensure that UserFederationProvider
>> lifecycle is properly tight to KeycloakSession
>> http://lists.jboss.org/pipermail/keycloak-dev/2016-April/007123.html
>> . The last we discussed was to add new method on KeycloakSession like:
>>
>> <T extends Provider> T getProvider(Class<T> clazz, String id, String
>> instanceId);
>>
>> where instanceId is the state associated with the provider (in case
>> of UserFederationProvider it will be DB ID of
>> UserFederationProviderModelId). That way, the
>> UserFederationProviderFactory.create can load the
>> UserFederationProviderModel (assumption is that RealmModel is
>> available in KeycloakContext, so UserFederationProviderFactory.create
>> has access to RealmModel + providerDatabaseId to load it from DB).
>>
>> In the thread, you can see that I've initially proposed something
>> similar to your proposal, but it's a bit more complex though.
>> Hopefully going "simple" way and adding just the method with
>> "instanceId" String argument can solve the issue.
>>
>> Marek
>>
>> On 10/06/16 01:36, Ariel Carrera wrote:
>>> There is not problem! :)
>>> One more thing, I solved the problem of multiple "federation
>>> provider" instances, adding this code to the DefaultKeycloakSession
>>> (and the method definition in KeycloakSession interface):
>>>
>>> public <T extends Provider> void registerProvider(Class<T>
>>> clazz, Provider provider, String id) {
>>> Integer hash = clazz.hashCode() + id.hashCode();
>>> providers.put(hash, provider);
>>> }
>>>
>>>
>>> And into MyUserFederationProviderFactory.getInstance(session, model)
>>> something like this:
>>>
>>> public UserFederationProvider getInstance(KeycloakSession
>>> session, UserFederationProviderModel model){
>>> UserFederationProvider provider = (UserFederationProvider)
>>> session.getProvider(UserFederationProvider.class, model.getId());
>>> if (provider == null){
>>> lazyInit(session);
>>> provider = new MyUserFederationProvider(session, model,
>>> config, ......);
>>> ((KeycloakSession)session).registerProvider(UserFederationProvider.class,
>>> provider, model.getId());
>>> };
>>> return provider;
>>> }
>>>
>>>
>>> After a few tests and debug it seems to work... creating, catching,
>>> and closing provider instances as expected.
>>>
>>>
>>> In future versions as you said, maybe would be better include a way
>>> to instantiate a complex object/provider instead of doing
>>>
>>> ProviderFactory.create(KeycloakSession session)
>>> some kind of method like
>>> ProviderFactory.create(KeycloakSession session, Object... obj);
>>>
>>> and the appropriate method into the KeycloakSession
>>>
>>> <T extends Provider> T getProvider(Class<T> clazz, Object... obj);
>>> <T extends Provider> T getProvider(Class<T> clazz, String id,
>>> Object... obj);
>>>
>>>
>>> And why not a map into the keycloakSession to store some additional
>>> context data to share between providers during same request? It's
>>> only a vague idea
>>>
>>> Regards!
>>>
>>> 2016-06-09 17:14 GMT-03:00 Bill Burke <bburke at redhat.com
>>> <mailto:bburke at redhat.com>>:
>>>
>>> Its gonna be awhile. Its going to be difficult to make
>>> everything both backward compatible and cover all the current
>>> and future use cases we need to cover. Listen on the dev list.
>>> I should post some info soon on what the new impl will look like.
>>>
>>>
>>> On 6/9/16 3:57 PM, Ariel Carrera wrote:
>>>> Yes Bill, exactly! I will waiting to test it Thanks!
>>>>
>>>> 2016-06-09 16:29 GMT-03:00 Bill Burke <bburke at redhat.com>:
>>>>
>>>>
>>>>
>>>> On 6/9/16 2:52 PM, Ariel Carrera wrote:
>>>>
>>>> Hi Bill, is a little expensive for me because I am
>>>> creating a new entity manager to connect with a legacy
>>>> database, and creating/enlisting a transaction per
>>>> instance.
>>>> For example in a simple flow case where a user needs to
>>>> click "I forgot my password" link to recover the
>>>> password, there is more than nine or ten instances
>>>> created to do this. It's really not a big problem but I
>>>> think that is not necessary and can be implemented like
>>>> others spi providers catched into the keycloak session.
>>>>
>>>> This is good feedback. We need a way to associate a
>>>> provider, by name, to the KeycloakSession. Maybe we just
>>>> need a way to associate anything with the KeycloakSession
>>>> period.
>>>>
>>>> In my case, another difficulty is synchronization
>>>> between an old authentication system and keycloak
>>>> implemented on demand (there is no full/partial
>>>> syncrhonization because the legacy system is still
>>>> working and need to work together for a while). Also I
>>>> implemented synchronization support but at this moment
>>>> it not used.
>>>> Every time that keycloak needs to validate a user
>>>> (isValid) recovered from the user storage or cache, a
>>>> query to the legacy system is made. Added to this... I
>>>> need to recover some attributes and roles changes
>>>> produced on the legacy system.... so I decided to
>>>> implement a "user federation cache" with a short term
>>>> expiration to improve the performance with certain
>>>> synchronization delay tolerance.
>>>>
>>>> In a few words I have: a custom User Federation
>>>> Provider + on deman synchronization + a user Federation
>>>> Provider Cache (my own cache SPI).
>>>>
>>>> Maybe an optional spi to obtain a custom container from
>>>> infinispan could be a good choice to add to the new
>>>> implementation and provide another one tool to do
>>>> things with better performance.
>>>>
>>>> I think the new model might solve your caching needs.
>>>> There will be no importing by default. This means no
>>>> synching, etc. Keycloak will only store metadata that your
>>>> user store can't provide. User Federation Providers will
>>>> work just as the default Keycloak user store and user cache.
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Tatú
>>>
>>>
>>>
>>>
>>> --
>>> Tatú
>>>
>>>
>>> _______________________________________________
>>> keycloak-dev mailing list
>>> keycloak-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>>
>>
>> _______________________________________________
>> keycloak-dev mailing list
>> keycloak-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20160615/ad17ab2f/attachment-0001.html
More information about the keycloak-dev
mailing list