Re: [keycloak-dev] federation commited need feedback
On 7/24/2014 9:25 AM, Stian Thorgersen wrote:
Looks good. Only two comments from me:
1. FederationManager.getFederationProvider uses factories directly
Why is this? This will cause the provider instance not to be registered with the session,
so won't be closed automatically when the session is closed, nor will it be able to
attach to the transaction.
FederationProviders are not singletons within KeycloakSession like
RealmProvider, UserProvider, and UserSessionProvider are. You can have
multiple FederationProvider instances per realm.
public interface FederationProviderFactory extends
ProviderFactory<FederationProvider> {
FederationProvider getInstance(KeycloakSession session,
FederationProviderModel model);
}
FederationProviders implementations have the option to enlist themselves
with the KeycloakSession transaction manager. LDAP (really Picketlink)
doesn't have the concept of a session or transaction, so it doesn't
enlist itself.
I guess we do need a enlistForClose() method on KeycloakSession though.
2. TOTP SPI (just related)
Once I've finished access code work I was going to start on TOTP SPI. I think a
UserProvider should only be able to verify password credentials, and TOTP providers should
be used to verify TOTP. I'll send a separate email about this tomorrow so we can
discuss it in more detail, just a heads up.
UserProvider.validCredentials() should still continue to exist as the
primary high-level call. With federation, credential validation may be
executed in different places (and even within non-Keycloak code)
depending on the capabilities of the FederationProvider.
In fact, IMO, there should be no changes needed to any of our APIs/SPIs.
The CredentialValidation class is used by all UserProviders now and
you can plug in any credential validation SPI you want there along with
adding additional credential types to UserCredentialModel i.e.
TOTP_GOOGLE, TOTP_FREEOTP
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com