[keycloak-dev] Caching of identity provider links

[Stian Thorgersen]

Ok, if Bill reviews and is happy to include in 1.9.x let's do it.
On 22 Mar 2016 16:29, "Marek Posolda" <mposolda at redhat.com> wrote:

> Yeah, there might be some risk of regression. On the other hand, there are
> DB queries during each social login or every time when you click on
> "federated identities" in account management.
>
> If you think missing caching is acceptable from performance perspective
> etc, I can merge just to master and avoid 1.9.x.
>
> Marek
>
> On 22/03/16 17:18, Stian Thorgersen wrote:
>
> Is this really critical for 1.9.x? I'm worried about us intruding more
> complexity so late and I'd rather postpone to 2.x.
> On 22 Mar 2016 15:36, "Marek Posolda" <mposolda at redhat.com> wrote:
>
>> On 22/03/16 15:18, Bill Burke wrote:
>> > Some comments on code:
>> >
>> > * Why have a special flag in invalidateUser?  Just invalidate the links
>> > if they exist.  Aggressive eviction isn't a bad thing, is it?
>> The only reason was performance. The flag "false" is used when you're
>> adding new user. But this new user is not yet in cache, so calling
>> "getFederatedIdentities" will always need to send DB query to get all
>> links of user. But when you're adding new user, you know that this user
>> doesn't yet have any social links. So in the end, without the flag there
>> will be one more DB query, which will always return empty set.
>>
>> Currently we don't support store/load of social links from federation
>> provider. But maybe in the future, if we support it and we chain cache
>> on top of federation storage, it could theoretically happen that new
>> user have social links at the moment when he is registered. So I can do
>> what you proposed as it seems to be a bit safer for the future.
>>
>> Marek
>> >
>> > On a side note, cache needs more refactoring, we're reusing the same
>> > pattern over and over.  Wonder if functions can help.
>> >
>> >
>> > On 3/22/2016 5:42 AM, Marek Posolda wrote:
>> >> Until now, we don't have support for caching of identityProvider
>> >> (social) links. So every social login or every going to "federated
>> >> identities" tab in account management needs to send DB queries. I was
>> >> looking at fixing it and I've send PR
>> >> https://github.com/keycloak/keycloak/pull/2404 . It turned to be a bit
>> >> tricky because of:
>> >>
>> >> 1) Caching needs to be done on both directions. For social login, you
>> >> need to lookup user by social link. But on the other hand, you also
>> need
>> >> to look all social/identityProvider links of particular user when you
>> go
>> >> to account management etc.
>> >>
>> >> 2) Because of "store token" option, the link may need to be updated in
>> >> DB quite often (in theory even during each social login).
>> >>
>> >> I was thinking that storing links directly on CachedUser doesn't work
>> >> very well, because during each update of social link (which may be
>> often
>> >> because of "store token") the user would need to be fully invalidated
>> >> from cache. Having separate cache entry for each social link also has
>> >> some downsides (many items in cache, need to have separate entry for
>> >> store the all links of user anyway). So I ended up with having the
>> cache
>> >> entry, which contains list of all links of particular user. It needs to
>> >> be updated when any social link is added,removed or updated. This
>> seemed
>> >> to me like good compromise. WDYT?
>> >>
>> >> There are also entries for lookup user by federated identity, so you
>> >> don't need to query DB during social login.
>> >>
>> >> So ATM there are not DB queries during social login or during go to
>> >> "federated identities" . I hope I handled all corner cases and
>> >> invalidations correctly, but if someone want to take look it will be
>> >> good. I don't want to add new regressions atm :-)
>> >>
>> >> Marek
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> _______________________________________________
>> >> 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/20160322/2742ba9e/attachment.html