[keycloak-dev] User Federation Provider Cache

[Stian Thorgersen]

I've never been a fan of how creating user feds outside of the session was
done. It's a completely broken concept and has several flaws:

a) KeycloakSession doesn't manage instances - we have issues with both
multiple instances being created as well as instances not being closed.
b) The code that requires an instance needs to know how to create one
c) No way to create a custom way to configure/setup - the model approach
may work for some, but what if a custom provider wants to store config
differently

With that in mind this needs to be fix and not monkey patched.

When requesting an instance of a user federation it should be:

session.getProvider(UserFederationProvider.class, String instanceId)

That's it. It would then be up to the factory of figuring out how to
instantiate it, not the calling code.

On 13 June 2016 at 09:39, Marek Posolda <mposolda at redhat.com> 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>:
>
>> 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>
>> 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 listkeycloak-dev at lists.jboss.orghttps://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/20160613/079aae9d/attachment.html