[keycloak-dev] Progress on LDAP enhancements

[Marek Posolda]

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