[keycloak-dev] Caching of identity provider links
Stian Thorgersen
sthorger at redhat.com
Tue Mar 22 12:18:23 EDT 2016
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/646fcb96/attachment.html
More information about the keycloak-dev
mailing list