[email protected] 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. -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/
