<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 29 April 2016 at 10:58, Marek Posolda <span dir="ltr"><<a href="mailto:mposolda@redhat.com" target="_blank">mposolda@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class="">
<div>On 29/04/16 10:22, Stian Thorgersen
wrote:<br>
</div>
<blockquote type="cite">
<p dir="ltr">We have 3 types of providers:</p>
<p dir="ltr">* Server configured - no config or config from
keycloak-server<br>
* Realm configured - config from realm model<br>
* Instance configured - multiple instances per realm</p>
<p dir="ltr">Removing master realm as we plan to do means that
realm configured provider factories can get realm from
KeycloakContext as there's only one realm per-session.</p>
</blockquote></span>
In theory yes. In practice there might be still corner cases when
you need to deal with multiple realms inside same KeycloakSession
(like export/import for example). But hope we can handle most of the
cases by assume that KeycloakContext has correct realm set.</div></blockquote><div><br></div><div>Corner cases like that is easy - we'd use create a KeycloakSession per-realm, making sure KeycloakContext is initialized properly.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><span class=""><br>
<blockquote type="cite">
<p dir="ltr">For instance configured I propose we add
getProvider(Class c, String id, String instanceId) to provider
factory. The it's up to the provider factory itself to extract
the config from the realm model or another source. It also means
that the session can easily keep track of these and only create
one id+instanceId per session.</p>
</blockquote></span>
ah, ok. I somehow missed the proposal. <br>
<br>
It should work fine, I think it's quite similar to what I proposed.
Despite I proposed to send whole state to provider factory (aka.
UserFederationProviderModel) instead of just instanceId and then
assume that state must properly implement "hashCode" to ensure that
session can keep track of these and return provider of already used
state.</div></blockquote><div><br></div><div>Yup, very similar, but I think the devil is in the details. In my proposal the factory itself knows how to extract the state, so it's then up to the factory to decide how state should be stored. A custom provider may need to store config in a separate custom entity, which KeycloakSessionFactory wouldn't know how to retrieve.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><span class="HOEnZb"><font color="#888888"><br>
<br>
Marek</font></span><span class=""><br>
<blockquote type="cite">
<div class="gmail_quote">On 29 Apr 2016 09:43, "Marek Posolda"
<<a href="mailto:mposolda@redhat.com" target="_blank">mposolda@redhat.com</a>>
wrote:<br type="attribution">
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div> Yes, AFAIK we have open JIRA for this for a long time
ago. <br>
<br>
It's the same issue for IdentityProvider (and maybe some
others SPI too) that they bypass "official" way for create
provider via session.getProvider(providerClazz) and hence
they are not registered in KeycloakSession and "close"
method is not called for them.<br>
<br>
The issue is that our SPI is a bit limited IMO and doesn't
support "stateful" providers. The providers are created
through "ProviderFactory.create(KeycloakSession)". So the
only available state of provider ATM is just
ProviderFactory + KeycloakSession, which is sometimes not
sufficient. <br>
<br>
<br>
I can see 2 possibilities to address:<br>
<br>
1) Always make the provider implementation "stateless" and
ensure all the state is passed as argument to provider
methods. This is what we already do for some providers
(for example all methods of UserProvider has RealmModel as
parameter). So if we rewrite UserFederation SPI that
UserFederationProviderModel will be passed as argument to
all methods of UserFederationProvider, then it can use
"official" way too. <br>
<br>
<br>
2) Improve the SPI, so it can properly support "stateful"
providers. This is more flexible then (1) and I would go
this way long term.<br>
<br>
I am thinking about something like this:<br>
<br>
public interface StatefulProvider<S> extends
Provider {<br>
}<br>
<br>
<br>
public class StatefulProviderFactory<T extends
StatefulProvider, S> {<br>
<br>
<span style="color:#20999d">T </span>create(KeycloakSession
session, S state);<br>
<br>
....... <br>
}<br>
<br>
<br>
and on KEycloakSession new method like this:<br>
<br>
<pre style="background-color:#ffffff;color:#000000;font-family:'DejaVu Sans Mono';font-size:9.0pt"><<span style="color:#20999d">S</span>, <span style="color:#20999d">T </span><span style="color:#000080;font-weight:bold">extends </span>StatefulProvider<<span style="color:#20999d">S</span>>> <span style="color:#20999d">T </span>getProvider(Class<<span style="color:#20999d">T</span>> providerClazz, String id, <span style="color:#20999d">S </span>state);</pre>
<br>
The "state" will need to properly implement equals and
hashCode, so the SPI can deal with it and not create
another instance of StatefulProvider if it was called for
this KeycloakSession with same state before.<br>
<br>
Marek
<div><br>
<br>
<br>
On 29/04/16 08:00, Stian Thorgersen wrote:<br>
</div>
</div>
<blockquote type="cite">
<div>
<div dir="ltr">Looking at the code for user federation
it looks like user federation provider instances with
the same configuration can be created multiple times
for a single session. Also they are never closed to
resources aren't released.</div>
<br>
<fieldset></fieldset>
<br>
</div>
<pre>_______________________________________________
keycloak-dev mailing list
<a href="mailto:keycloak-dev@lists.jboss.org" target="_blank">keycloak-dev@lists.jboss.org</a>
<a href="https://lists.jboss.org/mailman/listinfo/keycloak-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/keycloak-dev</a></pre>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</span></div>
</blockquote></div><br></div></div>