Re: [keycloak-dev] Progress on LDAP enhancements

From: "Marek Posolda" <mposolda(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt; Cc: keycloak-dev(a)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(a)redhat.com&gt; >> To: keycloak-dev(a)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/j... >> . 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.

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(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/keycloak-dev >>