[keycloak-dev] federation commited need feedback

Bill Burke bburke at redhat.com
Thu Jul 24 14:59:38 EDT 2014



On 7/24/2014 1:51 PM, Marek Posolda wrote:
> On 24.7.2014 15:21, Bill Burke wrote:
>>
>>
>> On 7/24/2014 5:31 AM, Marek Posolda wrote:
>>> Great stuff, some feedback inline
>>>
>>> On 23.7.2014 23:33, Bill Burke wrote:
>>>> First iteration is commited.  I still have a lot to do.
>>>>
>>>> * AuthenticationProvider currently co-exists with Federation. I will
>>>> delete it after the review of FederationProvider.
>>>> * UserModel is proxied.  Some updates delegated to LDAP.  Need to
>>>> expand.
>>>> * Still need to do admin console UI for federation
>>>> * Still need to implement search and other queries for LDAP
>>>> * Still need to test disjoint credential type storage.
>>>>
>>>> Feedback on unimplemented features for LDAP:
>>>> * registration supported switch.
>>>> * Importing username and email will be required.  Everything else will
>>>> be optional.  That cool?
>>> yeah. Not directly related but I wonder if federationLink on UserModel
>>> is sufficient? For example if user "john" is deleted in LDAP and then
>>> user with same username "john" is added again to LDAP, it's defacto
>>> different user but the Keycloak user "john" will be linked to the "new"
>>> LDAP john. Hence AuthLinkModel had link to provider and also uuid of
>>> user from LDAP.
>>
>> This is an implementation detail and shouldn't really be exposed
>> through the Model API.
> Ok, but how you would handle the case when user "john" is deleted in
> LDAP and then different user with same username "john" created again?
> Current LDAPFederationProvider will treat them in Keycloak as same user
> even if they are not.

So, you're saying we're supposed to check and verify with the LDAP 
server every time the UserModel is referenced to make sure the uid 
hasn't changed since the last time we accessed it?  In your current 
implementation you only do this when validating password or updating the 
credential.

FYI, if this happens with the AuthenticationProvider, that particular 
user is hosed and can never log in again.  The user needs to be deleted 
if this occurs (probably in a separate transaction).



>> I haven't actually tested or even executed searches yet.  Thanks for
>> pointing outthe bugs.
> I don't know if bug is correct word. IMO Federation+pagination can't
> never properly work (without syncing all LDAP users into local storage
> first). I would be happy if I am wrong:-)
>

You're right.  The API would have to change to note the provider that 
was last used and how many were consumed for that provider.

class Result {

    List<UserModel> results;

    String lastProvider;
    int lastIndex;

}

then UserProvider search would need these methods:

Result search(criteria..., int maxResults);  // start from beginning
Result search(criteria..., String lastProvider, int lastIndex, int 
maxResults);

As a side effect of this, I don't think FederationProvider extending 
UserProvider is going to work anymore. There were already some add 
methods that didn't make sense and would never be called.  Now 
pagination needs a different API between UserProvider and 
FederationProvider.

-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com


More information about the keycloak-dev mailing list