1:59 p.m.
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