[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#5856) adauth overlay contribution

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.

   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/