[keycloak-dev] Progress on LDAP enhancements

Wed May 27 02:42:55 EDT 2015

----- Original Message -----
> From: "Marek Posolda" <mposolda at redhat.com>
> To: "Stian Thorgersen" <stian at redhat.com>
> Cc: keycloak-dev at lists.jboss.org
> Sent: Tuesday, 26 May, 2015 7:03:18 PM
> Subject: Re: [keycloak-dev] Progress on LDAP enhancements
> 
> On 26.5.2015 08:46, Stian Thorgersen wrote:
> >
> > ----- Original Message -----
> >> From: "Marek Posolda" <mposolda at redhat.com>
> >> To: "Stian Thorgersen" <stian at redhat.com>
> >> Cc: keycloak-dev at lists.jboss.org
> >> Sent: Friday, 22 May, 2015 9:43:51 PM
> >> Subject: Re: [keycloak-dev] Progress on LDAP enhancements
> >>
> >> On 22.5.2015 13:25, Stian Thorgersen wrote:
> >>> ----- Original Message -----
> >>>> From: "Marek Posolda" <mposolda at redhat.com>
> >>>> To: keycloak-dev at lists.jboss.org
> >>>> Sent: Wednesday, 20 May, 2015 11:33:16 PM
> >>>> Subject: [keycloak-dev] Progress on LDAP enhancements
> >>>>
> >>>> I think I am finished with step 1 prototype on LDAP enhancements. Right
> >>>> now it's just in my local fork
> >>>> https://github.com/mposolda/keycloak/tree/ldap , I can't send PR as
> >>>> model and admin console are kind of broken in my fork (ATM I am testing
> >>>> with some hardcoded configs).
> >>>>
> >>>> What I did so far:
> >>>>
> >>>> - Refactoring and simplifying of forked picketlink LDAP code and removed
> >>>> some abstractions to make it easier to use
> >>>>
> >>>> - Added UserFederationMapper SPI. The plan is to be in line with already
> >>>> existing ProtocolMappers and IdentityProviderMappers.
> >>>>
> >>>> - Added LDAPFederationMapper. This mapper is invoked from
> >>>> LDAPFederationProvider and provides some callbacks and extension points.
> >>>> For more details, see methods and javadoc:
> >>>> https://github.com/mposolda/keycloak/blob/ldap/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPFederationMapper.java
> >>>> . I have 3 implementations right now:
> >>>> -- UserAttributeLDAPFederationMapper - This is used to map LDAP
> >>>> attributes to UserModel attributes/properties .
> >>>> -- FullNameLDAPFederationMapper - Many LDAP deployments store fullName
> >>>> of user in single LDAP attribute (usually "cn" attribute). So this
> >>>> allows to map fullName from LDAP to firstName and lastName on UserModel
> >>>> -- RoleLDAPFederationMapper - allows to map LDAP roles and user role
> >>>> mappings from some DN in LDAP tree to either realm roles or client roles
> >>>> of some client. Of course it's not a problem to plug more mappers, so
> >>>> it's not an issue to map for example LDAP roles from tree
> >>>> "ou=finance,dc=myorg,dc=org" to client roles of client "finance" and
> >>>> roles from tree "ou=development,dc=myorg,dc=org" to client roles of
> >>>> client "development" etc.
> >>>>
> >>>> - Minor refactoring of some methods on UserFederationProvider interface.
> >>>> I needed to add RealmModel as parameter to some methods (it was already
> >>>> in most of them) because RealmModel will be used to retrieve list of
> >>>> configured UserFederationMapperModels .
> >>>>
> >>>>
> >>>> Next planned steps:
> >>>> - Model - I plan to add CRUD for UserFederationMapperModel to RealmModel
> >>>> (similarly like IdentityProviderMapperModel)
> >>>>
> >>>> - Admin console - I've been thinking about new tab "Mappers" under "User
> >>>> Federation". The tab will be active when you have some Federation
> >>>> provider chosen and it will allow to add/update/remove mappers for
> >>>> particular provider. Again something very similar to already existing
> >>>> broker mappers. WDYT?
> >>> +1
> >>>
> >>>> - Address remaining subtasks of
> >>>> https://issues.jboss.org/browse/KEYCLOAK-886 . Not everything will be
> >>>> addressed by mappers. For example there is performance issue
> >>>> https://issues.jboss.org/browse/KEYCLOAK-838 . I am thinking about
> >>>> adding some caching at LDAPIdentityStore level, but also I am thinking
> >>>> about some simple per-request caching at UserFederationManager level,
> >>>> which might help with federation performance in general (not just
> >>>> specific to LDAP). Maybe combination of both? But I am not sure atm...
> >>>>
> >>>> - DB migration
> >>>>
> >>>> Question: I wonder if we can put RealmModel accessible from
> >>>> UserFederationProviderModel? It shouldn't be a problem from
> >>>> implementation perspective as UserFederationProviderModel CRUD methods
> >>>> are on RealmModel, so realm can just inject itself before return
> >>>> UserFederationProviderModel. Since UserFederationProvider instances are
> >>>> created by method "getInstance" on UserFederationProviderFactory, which
> >>>> has UserFederationProviderModel as one of parameters, it will make
> >>>> RealmModel accessible in UserFederationProvider and hence it won't be
> >>>> needed to have RealmModel in signature of methods on
> >>>> UserFederationProvider.
> >>>>
> >>>> WDYT?
> >>> Realm is already available from KeycloakContext /
> >>> session.getContext().getRealm()
> >> Yeah, however this is not available in all cases (for example during
> >> tests or admin requests). And you can't be sure that RealmModel from the
> >> context is the RealmModel corresponding to your
> >> UserFederationProviderModel.
> > For tests it can be fixed.
> How about admin requests? Currently both "realm" and "client" are null
> in session.getContext() . Should we set realm to context in
> RealmsAdminResource.getRealmAdmin ? And similarly client before
> ClientResource is invoked?
> 
> It's a bit tricky as admin can be authenticated to realm "master" but he
> is administrating realm "foo", but it seems to me that it's fine to set
> the realm, which he is administrating (sending admin requests too)
> rather than the realm where he is authenticated.
> > Why can't you be sure it's the same realm?
> I mean for example if you have code like this:
> 
> RealmModel masterRealm = session.realms().getRealmByName("master");
> UserModel admin = session.users().getUserByUsername("admin", masterRealm);
> 
> RealmModel fooRealm = session.realms().getRealmByName("foo");
> UserModel foo = session.users().getUserByUsername("foo", fooRealm);
> 
> and both realms are configured to use federation and realm "foo" is set
> in the context. Then when calling the second line:
> 
> session.users().getUserByUsername("admin", masterRealm)
> 
> the UserFederationProvider will use incorrect realm "foo" from the
> context when it should use realm "master" .
> 
> It seems the proper behaviour is, that UserFederationManager will just
> use the realm send to the "getUserByUsername" (or any other method) as
> argument and pass same realm instance to the UserFederationProvider as
> argument. Is it clearer what I mean?

Yep, I think passing the realm is what works for now, but I wonder if a session should always be linked to a specific realm, rather than having to pass the realm around all the time.

> 
> Marek
> >> I think I will keep it as it is for now, also due to some other possible
> >> issues. After thinking more it seems that adding RealmModel as variable
> >> to UserFederationProviderModel is not very good idea...
> >>
> >> Marek
> >>>> Marek
> >>>> _______________________________________________
> >>>> keycloak-dev mailing list
> >>>> keycloak-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
> >>>>
> >>
> 
> 