devpi / devpi-ldap

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

Fix exception when the attribute_name is dn/distinguishedName and the search fails #36

Open segevfiner opened 7 years ago

segevfiner commented 7 years ago

When the search fails to find the user, the search result doesn't necessarily contain dn/distinguishedName. Trying to get it will cause an exception instead of returning an "unknown" status. This should fix the issue, but note that I'm recalling the fix from memory and I didn't test this.

tomasbedrich commented 6 years ago

@devpi @fschulze any chance to get this merged?

fschulze commented 6 years ago

A test case for this would be nice.

tomasbedrich commented 6 years ago

I was trying to create a test to cover this, but unsuccessfully. At least here are some details.

🛑 Nonexisting user:

$ devpi-ldap /etc/devpi/ldap.yaml nonexisting.user
Password:
2018-05-29 14:11:25,308 DEBUG NOCTX Validating user 'nonexisting.user' against LDAP at ldap://somewhere
Traceback (most recent call last):
  File "/usr/local/bin/devpi-ldap", line 11, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.5/dist-packages/devpi_ldap/main.py", line 297, in main
    result = ldap.validate(username, password)
  File "/usr/local/lib/python3.5/dist-packages/devpi_ldap/main.py", line 245, in validate
    userdn = self._userdn(username)
  File "/usr/local/lib/python3.5/dist-packages/devpi_ldap/main.py", line 222, in _userdn
    result = self._search(None, self['user_search'], username=username)
  File "/usr/local/lib/python3.5/dist-packages/devpi_ldap/main.py", line 198, in _search
    return sum((extract_search(x) for x in conn.response), [])
  File "/usr/local/lib/python3.5/dist-packages/devpi_ldap/main.py", line 198, in <genexpr>
    return sum((extract_search(x) for x in conn.response), [])
  File "/usr/local/lib/python3.5/dist-packages/devpi_ldap/main.py", line 193, in extract_search
    return [s[attribute_name]]
KeyError: 'distinguishedName'

✅ Bad password:

$ devpi-ldap /etc/devpi/ldap.yaml bad.password
Password:
2018-05-29 14:15:40,281 DEBUG NOCTX Validating user 'bad.password' against LDAP at ldap://somewhere
Result: {"status": "unknown"}
No user named 'bad.password' found.

✅ Good password:

$ devpi-ldap /etc/devpi/ldap.yaml good.password
Password:
2018-05-29 14:17:28,037 DEBUG NOCTX Validating user 'good.password' against LDAP at ldap://somewhere
Result: {"groups": ["group1", "group2"], "status": "ok"}
Authentication successful, the user is member of the following groups: group1, group2

I also printed raw contents of conn.response for each case:

# exception is raised
nonexisting_user = [
    {
        'type': 'searchResRef',
        'uri': ['ldap://something']
    }, {
        'type': 'searchResRef',
        'uri': ['ldap://something-else']
    }, {
        'type': 'searchResRef',
        'uri': ['ldap://something-else-2']
    }
]

# works fine
bad_password = good_password = [
    {
        'raw_attributes': {
            'distinguishedName': [
                b'CN=User Name,OU=XXXXX,DC=YYYYY,DC=ZZZZZ']
        },
        'type': 'searchResEntry',
        'attributes': {
            'distinguishedName': 'CN=User Name,OU=XXXXX,DC=YYYYY,DC=ZZZZZ'
        },
        'raw_dn': b'CN=User Name,OU=XXXXX,DC=YYYYY,DC=ZZZZZ',
        'dn': 'CN=User Name,OU=XXXXX,DC=YYYYY,DC=ZZZZZ'
    }, {
        'type': 'searchResRef',
        'uri': ['ldap://something']
    }, {
        'type': 'searchResRef',
        'uri': ['ldap://something-else']
    }, {
        'type': 'searchResRef',
        'uri': ['ldap://something-else-2']
    }
]
fschulze commented 6 years ago

The raw responses should help. I will try to write tests based on them.