From: "Marek Posolda" <mposolda(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
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(a)redhat.com>
>> To: "Stian Thorgersen" <stian(a)redhat.com>
>> 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>
>>>> 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
, 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
>>>> some abstractions to make it easier to use
>>>> - Added UserFederationMapper SPI. The plan is to be in line with
>>>> existing ProtocolMappers and IdentityProviderMappers.
>>>> - Added LDAPFederationMapper. This mapper is invoked from
>>>> LDAPFederationProvider and provides some callbacks and extension
>>>> For more details, see methods and javadoc:
>>>> . 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
>>>> 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
>>>> of some client. Of course it's not a problem to plug more mappers,
>>>> it's not an issue to map for example LDAP roles from tree
>>>> "ou=finance,dc=myorg,dc=org" to client roles of client
>>>> roles from tree "ou=development,dc=myorg,dc=org" to client
>>>> client "development" etc.
>>>> - Minor refactoring of some methods on UserFederationProvider
>>>> 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
>>>> (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?
>>>> - Address remaining subtasks of
. Not everything will be
>>>> addressed by mappers. For example there is performance issue
. 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
>>>> has UserFederationProviderModel as one of parameters, it will make
>>>> RealmModel accessible in UserFederationProvider and hence it won't
>>>> needed to have RealmModel in signature of methods on
>>> Realm is already available from KeycloakContext /
>> 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
> For tests it can be fixed.
How about admin requests? Currently both "realm" and "client" are
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:
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
>> 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...
>>>> keycloak-dev mailing list