> Hi all,
> I spent some time today working on 2 issues I had logged a few
> months ago about LDAP, 12166 and 12167. I have prepared a couple of
> patches to address these issues (see attached) - please review and
> commit if you think it's good.
> While looking at the code in ldap_api.php as well as the log file
> generated with LOG_LDAP when I was build and testing the patches, I
> realized a few things:
> 1. code is essentially duplicated between
> ldap_get_field_from_username and ldap_authenticate_by_username, as
> both fonctions always perform a complete bind/search/getentries/
> unbind cycle
> 2. this is causing unnecessary multiple ldap cycles to get info when
> authenticating a user (due to the calls to ldap_realname and
> ldap_email_from_username after user is authenticated, which in turn
> call ldap_get_field_from_username), a single cycle of ldap calls
> would be sufficient to achieve the results
> 3. a function seems unused (ldap_has_group) - what is its purpose ?
> If you'd let me, I was thinking about improving this code, maybe by
> building some kind of ldap cache to minimize the number of queries
> against the LDAP server, something along the lines of what is done
> for users in user_api.php.
> Any thoughts, ideas, recommendations ?
> There is also one thing that puzzled me on #1: a difference in the
> logic between the two functions. The former retrieves the required
> user info (field) straight from the ldap entry #0 ($t_info
> [$p_field]) whereas the latter runs a loop on all returned
> entries (and then tries to bind with the dn to validate the
> password). It seems inconsistent to me, but maybe I'm missing
> something, so I was hoping that one of you guys could explain why
> it's done this way.
> Based on my tests, ldap_get_entries calls systematically return a
> single element in the $t_info array, so I would think that the loop
> is not required. But then again, I only have experience with AD,
> where sAMAccountName is guaranteed to be unique for a given domain (
> Is it different with other LDAPs ? And if yes, what would happen
> with the current code in ldap_get_field_from_username, in case
> multiple entries exist ?
> I'm looking forward to your feedback.
> Thanks in advance.