devpi / devpi-ldap

Plugin for devpi-server which provides LDAP authentication.
36 stars 20 forks source link

validate will not work without userdn in config file #13

Open patter001 opened 9 years ago

patter001 commented 9 years ago

There's an issue where the validate function in main.py is taking the username and password to be validated. However it flows through _userdn to verify the username which will end up defaulting to a connection without a username and password. I think this means that the only way for the plugin to work is via specifying the userdn and password in the config file. All our users should have access to the ldap server, so I would like to use the requesting users username and password instead of putting that information in a config file.

I'm not sure what the solution is. Does validate need 3 parameters? user, userdn, and password? That way userdn can be specified differently than the user to be validated?

Thanks,

patter001 commented 9 years ago

So what "works for me" but is probably breaking the intent of some of the code is in the validate function I comment out the username escape and userdn lookup (which is causing the bad connection).

    # BEFORE
    #username = escape(username)
    #userdn = self._userdn(username)
    # WORKS
    userdn = username

Now at this point, the connection uses the username and password passed in, and goes on to do the correct group matching.

jaraco commented 8 years ago

We're experiencing this issue as well.

This behavior contradicts the documentation, which indicates, "If you don't have anonymous user search or if the users can't search their own groups, then you need to set [userdn] to a user which has the necessary rights," which implies that if users can search their own groups, one does not need to set the userdn in the user_search section.

Interestingly, I would have thought given this information that @patter001's patch would have worked for us too, but it does not because it seems that some users are able to _open_and_bind, passing only the username as the userdn, but for other users, the distinguished name must be passed as the DN.

jaraco commented 8 years ago

We've worked around the issue by creating a dedicated user for performing our authentication lookups, which is what we've done with other apps for authenticating against LDAP.

fschulze commented 8 years ago

Any suggestions to improve the documentation for this?

jaraco commented 8 years ago

I'm afraid I'm stuck on this one. As I understand it, there are several different ways in which the initial user DN can be resolved:

>>> import ldap3
>>> srv = ldap3.Server('ldap://our_ad_server')
>>> import getpass
>>> pass1 = getpass.getpass()
Password: 
>>> pass2 = getpass.getpass()
Password: 
>>> conn = ldap3.Connection(srv, read_only=True, user='dedicated_account', password=pass1)
>>> conn.open()
>>> conn.bind()
True
>>> conn = ldap3.Connection(srv, read_only=True, user='jaraco', password=pass2)
>>> conn.open()
>>> conn.bind()
False

Even more curious, if I force NTLM auth on the connection and supply the domain, then I can bind with only the username:

>>> conn = ldap3.Connection(srv, read_only=True, user='ORG\\jaraco', password=pass2, authentication=ldap3.NTLM)
>>> conn.open()
>>> conn.bind()
True

So it's not clear to me under what circumstances the third option is viable.

I could imagine an option bypass user distinguished name resolution which could be enabled by the OP to have his desired effect, or perhaps user dn resolve technique which could be one of 'anonymous', 'same user', 'NTLM' (requiring a 'domain parameter'), or 'static' (requiring the userdn and password).

I don't like either of those options so for now I'm going to punt and suggest a change to the docs to address the status quo.

jaraco commented 8 years ago

As I started editing the docs, I realized they already subtly covered the existing status quo, but I made them more explicit, so it's clear that a userdn is required for user_search when anonymous is not allowed and userdn is required for group_search when a user cannot search his own groups.