neil.dunbar@hp.com wrote:
On Tue, 2008-12-16 at 14:52 +0000, Neil Dunbar wrote:
Andrew:
One suggestion following a very quick scan of the code: I think it would be worth bringing the warning about turning off TLS checks into the manual page.
Agreed. Done.
In particular, there is no reason for this to be AD-specific and it should be easy to adapt it to authenticate against any [collection of] remote LDAP servers.
Actually, it may not be AD specific as is.
Are we any further forward to getting this rolled into contrib? Is there more we need to do to make it fit better? The tarball has been sitting there a while, it would seem.
I've reviewed the package and have some comments.
I'm puzzled by the design of this overlay; in ActiveDirectory a user can exist in only one domain and there is a one-to-one mapping between their domain name and the suffix of their LDAP DN. As such, the lookup of the adauth_domain_attribute seems clumsy and unnecessary.
The adauth_default_realm and adauth_default_domain terms are confusing. Your adauth_default_realm item is really just the hostname of a particular server. Your use of the word "realm" is basically inconsistent with common usage of this term.
The usage for adauth_mapping is clumsy; you could simply have made it a list of LDAP URLs and not bothered with an external file. Is there a commonly used external file that other software uses, that prompted this approach?
The description of adauth_dn_attribute is misleading. Is it a DN, or is it a userPrincipalName? In what way is this a "map" at all? Examining the code, it is simply "the DN of the user to Bind against on the remote LDAP server" - it is not a map, nor is it a userPrincipalName.
I wonder in what context this overlay is easily used. If I have 100,000 users in my LDAP directory, I don't want to maintain 100,000 AD_DN attributes for them; I want a simple rule that actually performs an algorithmic mapping of their local DN to their remote DN. I suppose using an AD_DN attribute for the users whose DNs don't easily map might be a good fallback, but it shouldn't be the primary mechanism for linking a local user to a remote user. I would re-use the authz-regexp machinery here.
It seems there's a lot of AD-specific overhead in this code, for what is really a pretty simple and generic operation - passthru Bind to a remote LDAP server. I would have used the back-ldap/chain.c code instead, which would also provide connection pooling and the other benefits of back-ldap's already comprehensive support for communicating with remote LDAP servers.