Re: [mantisbt-dev] Improving LDAP api

[Damien R. <dam...@me...>]

Hi,

A few weeks ago I sent the message below. To date I have received no 
replies from you guys, and I was wondering if this silence was due to a 
lack of interest from the team in what I'm proposing, or if you are just 
too busy to give me some feedback... I hope it's the latter, and that 
you can spare a moment for this at some point :-)

Also, note that I have implemented additional improvements on top of the 
previously attached patches, so please disregard them and get the 
updated version on the bugtracker for issue #12167 - 
http://www.mantisbt.org/bugs/file_download.php?file_id=3162&type=bug

Thanks in advance for your reply !

Damien


Damien Regad/SINT/Merck wrote on 04.11.2010 20:22:46:

> 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[0]
> [$p_field][0]) 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 (
> http://msdn.microsoft.com/en-us/library/cc245685%28v=PROT.13%29.aspx).
>
> 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.
> Damien