[keycloak-dev] Progress on LDAP enhancements

[Marek Posolda]

I've commited first version to the master. "Mappers" tab is not yet 
added to admin console, but model works and all the tests are passing. 
Still quite a lot of work remaining though...

Marek

On 22.5.2015 21:43, Marek Posolda wrote:
> 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.
>
> 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
>>>
>