[keycloak-dev] Progress on LDAP enhancements

[Stian Thorgersen]

----- 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. Why can't you be sure it's the same realm?

> 
> 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
> >>
> 
>