[keycloak-dev] federation commited need feedback

[Marek Posolda]

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.
>
>>> * Modes for federation will be: READ_ONLY, SYNCED, and UNSYNCED.
>>> SYNCED will update LDAP on demand. UNSYNCED will store changes locally
>>> and require the user to handle synchronization themselves.
>> Is it some difference between READ_ONLY and UNSYNCED? Wonder if 2 modes
>> are sufficient for federation and users are free to choose if they
>> handle synchronization or not? Am I understand correctly that this is
>> just for sync from Keycloak back to LDAP?
>
>
> SYNCED means LDAP is writable and there will be on-demand updates back 
> to LDAP.
>
> UNSYNCED is when LDAP is read-only but you want to be able to manage 
> the user anyways:  update their credentials, address, etc.
>
> READ-ONLY doesn't allow any changes to user.
>
>>> * Going to have an import-attributes on/off switch.  A keycloak->ldap
>>> attribute map will be required to be configured.  If this switch is 
>>> off,
>>> UserModel proxy will load attributes on demand.
>>>
>>>
>>> Questions:
>>> * Is ExternalModelAuthProvider actually a feature requested by users?
>>> I'd like to not have to do this.  At least for 1.0.
>> Probably not needed AFAIK.
>>
>> Sorry to repeat, but I still have some doubts especially about bulk
>> search methods. Example of some corner cases:
>>
>> * If there are 2 federation providers and firstResult==0, maxResults==10
>> and each provider has 10 users, then due to this
>> https://github.com/keycloak/keycloak/blob/master/model/api/src/main/java/org/keycloak/models/FederationManager.java#L193 
>>
>> it may be 20 users in final list (even if maxResults is 10)
>>
>> * If there are 10 users "locally" and 10 completely different users in
>> LDAP, and firstResult ==0, maxResults==10 then it will return 10 "local"
>> users. Now you want next page so next query will be firstResult==10,
>> maxResults==10 . This should return 10 users from LDAP, but actually it
>> won't return any user as LDAP query will have firstResult==10,
>> maxResults==10, but there are just 10 users in LDAP
>>
>
> 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:-)

Marek
>
>> * In some cases it may happen the opposite . Like you have user "john"
>> in both LDAP and local store. Page1 will return 10 local users with
>> "john" between them. Page2 will query LDAP and it may also return "john"
>> between them (the case when "john" is on page1 in "local" store but on
>> page2 on LDAP store)
>>
>>
>> Right now, I am looking at changelog stuff, which is supported by
>> ActiveDirectory but not by some other LDAP servers. Right now I am
>> working with Keycloak on latest Picketlink master, which helped to
>> remove ActiveDirectory workarounds needed in
>> LDAPKeycloakCredentialHandler and KeycloakLDAPIdentityStore . I will
>> also possibly send more PRs to picketlink with missing features
>> regarding changelog and pagination .
>>
>
> FYI, I forked the picketlink code under federation/ldap.  Mainly to 
> keep AuthProvider stuff alive and working until the switch is ready.
>
>