Skip to content
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

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

Closed
sjkingo opened this issue Jul 7, 2015 · 4 comments
Labels
Milestone

Comments

@sjkingo
Copy link
Owner

sjkingo commented Jul 7, 2015

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 sjkingo added the bug label Jul 7, 2015
@sjkingo sjkingo added this to the v1.0 milestone Jul 7, 2015
@sjkingo sjkingo changed the title Whitespace normalisation is unhandled in library, possibly causing duplicate users Bypassing authentication flow by manually creating a local Django user may cause duplicate users to be created Jul 7, 2015
@sjkingo
Copy link
Owner Author

sjkingo commented Jul 7, 2015

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

sjkingo commented May 5, 2016

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

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

sjkingo commented May 12, 2016

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

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

No branches or pull requests

2 participants