Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

segevfiner
Copy link

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.

… search fails

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
Copy link

@devpi @fschulze any chance to get this merged?

@fschulze
Copy link
Contributor

A test case for this would be nice.

@tomasbedrich
Copy link

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants