sjkingo / django_auth_ldap3

A library for connecting Django's authentication system to an LDAP directory
BSD 2-Clause "Simplified" License
23 stars 13 forks source link

Bypassing authentication flow by manually creating a local Django user may cause duplicate users to be created #9

Closed sjkingo closed 8 years ago

sjkingo commented 9 years ago

LDAP specifies in rfc4518 that whitespace is normalised in search and binds such that requesting cn=@@@sam may return cn=sam as the user (replace @ with a space).

This behaviour is squashed by the patch in #7 by using the ldap_user.username attribute instead of the user's input, however an edge case still exists if a local Django user is created without applying the correct normalisation (assuming an LDAP user sam exists):

  1. Create local Django user @@@sam (replace @ with a space - that is, sam prefixed with 3 spaces)
  2. Log in as @@@sam
  3. A new user sam is created as this is the user returned by LDAP

There are now duplicates in the local Django database as @@@sam and sam, even though they refer to the same user.

7 fixes the case-normalisation issue by always using the LDAP's username to create the local user, however if an admin bypasses the usual authentication flow and manually creates a new user in Django, this edge case may be hit.

sjkingo commented 9 years ago

This is more general than I first thought - it can be summarised to:

If you bypass the usual authentication flow (as specified by settings.AUTHENTICATION_BACKENDS) and create a local Django user, then a corner case exists where the normal LDAP normalisation rules will not apply. This may cause duplicate users to be created (e.g. Sam != sam, @@@sam != sam).

The question is: is this a bug that should be handled, or a corner case where we can simply say "don't do this" and ignore?

sjkingo commented 8 years ago

@alandmoore: with your experience with this library and LDAP - do you think this issue is a problem that should be resolved in the library, or should we just make a note in the README and close this?

alandmoore commented 8 years ago

I'm not sure I completely understand the issue; I'm kind of surprised Django allows usernames with leading/trailing spaces or is case-sensitive WRT usernames, since that doesn't seem like an expected behavior of any auth system.

I guess the compelling issue is whether the duplicate account situation creates either a security issue or a denial-of-service issue (can I still login as 'sam' via LDAP if local user 'Sam' exists?).

sjkingo commented 8 years ago

Yep: the case-sensitive usernames is a long standing issue that they refuse to "fix" (coredevs don't accept it as a problem).

Leading/trailing spaces are more subjective I guess.. I think we should probably just put a note in the README stating the limitations in Django's username handling and leave it at that.

No sense committing in cruft to handle an archaic edge case.

Thanks