Username case sensitivity

[rmassoth]

When using Active Directory, username lookups are case insensitive but django will create a new user if given a different case username. For example, given a username "steve", if Steve logs in with username "Steve" AD will authenticate the existing user but django will create a new user with. I fixed it with a simple username = username.lower() at the top of the authenticate method to force all lower case usernames in django but I don't know if this is compatible with openLDAP as my company uses AD.

[sjkingo]

Nice catch, I hadn't thought to test that with AD.

Could you please send a pull request for the change, and I will test it on OpenLDAP for you.

[rmassoth]

Yeah, I only caught it because a mobile browser automatically capitalized the first letter when logging in and created a new user. I love this library by the way. It does just what I needed it to out of the box and your examples were very helpful to get started.

[sjkingo]

Thanks for the kind words.. it's great to see it being useful to someone else!

With regards to this change, I have given it some thought and while I agree that there is a definite issue with the case sensitivity of Django's usernames (which is a long standing issue that the devs refuse to do anything about) - I don't think this is the way to go about it.

It brings in an edge case that I don't like: if you pull down (from LDAP) a username such as Bob, but store it in Django as bob by calling .lower() (thus preventing duplicate users being created), you have a new problem where the displayable username is not what the user may be used to.

This may not be a problem for anyone else, but I don't really like it. It's not immediately obvious that bob == Bob in Django's auth table (given Django's case-sensitivity, that is).

Could we do something like store the correct case-sensitive username in Django (such as Bob), but modify the check in django_auth_ldap3 to handle case-insensitivity? This way duplicates won't get created, and a user can authenticate with regards to case-sensitivity.

But you are right - OpenLDAP is the same as AD in that the uid attribute is case-insensitive.

[rmassoth]

Yeah, I was thinking about this after submitting the pull request. This breaks it for current users of this library because if they already have usernames not in all lower case it's going to create new users instead of using the existing. It needs to do a precheck for username existance in django first, authenticate through ldap, then create or log in the user. Usernames still have to be unique but they can be stored in the case specified at creation.

[sjkingo]

Sounds like a solid plan - I agree this is a pretty significant blocker and needs to be resolved.

Do you want to modify the PR to implement this? I can't seeing it take more than a couple of extra lines in the authenticate() method.

[rmassoth]

This is trickier than I thought. In order to support case sensitive names in django you will have to check every permutation of the entered username to see if it exists first in django. This could be n^2 checks against the database. If the username doesn't exist, then authenticate through ldap and create the user. I think the accepted way of handling this is to just force to lower case as I've done but it's up to you. The django devs have certainly created a tricky situation with this one considering how many people might want to use 3rd party apps for authentication.

[sjkingo]

Couldn't you just go:
User.objects.filter(username__icontains='Bob')

[sjkingo]

Sorry, I meant to say username__iexact='Bob'. This would mean the following are true:

• Bob == Bob
• Bob == bob
• BoB == BOB
• etc.

Do you think that will work?

[sjkingo]

I haven't tested this, but I was thinking this may work:

diff --git a/django_auth_ldap3/backends.py b/django_auth_ldap3/backends.py
index c165c32..fbbb5a8 100644
--- a/django_auth_ldap3/backends.py
+++ b/django_auth_ldap3/backends.py
@@ -77,25 +77,27 @@ class LDAPBackend(object):
         # Get or create the User object in Django's auth, populating it with
         # fields from the LDAPUser. Note we set the password to a random hash
         # as authentication should never occur directly off this user.
-        user, created = User.objects.get_or_create(username=username, defaults={
-                'password': hashlib.sha1().hexdigest(),
-                'first_name': ldap_user.givenName,
-                'last_name': ldap_user.sn,
-                'email': ldap_user.mail,
-                'is_superuser': False,
-                'is_staff': admin,
-                'is_active': True
-        })
+        django_user = User.objects.filter(username__iexact=username)
+        if not django_user:
+            # Create new user
+            django_user = User(username=username,
+                    password=hashlib.sha1().hexdigest(),
+                    first_name=ldap_user.givenName,
+                    last_name=ldap_user.sn,
+                    email=ldap_user.mail,
+                    is_superuser=False,
+                    is_staff=admin,
+                    is_active=True
+            )
+        else:
+            # If the user wasn't created, update its fields from the directory.
+            django_user.first_name = ldap_user.givenName
+            django_user.last_name = ldap_user.sn
+            django_user.email = ldap_user.mail
+            django_user.is_staff = admin
+            django_user.save()

-        # If the user wasn't created, update its fields from the directory.
-        if not created:
-            user.first_name = ldap_user.givenName
-            user.last_name = ldap_user.sn
-            user.email = ldap_user.mail
-            user.is_staff = admin
-            user.save()
-
-        return user
+        return django_user

     def get_user(self, user_id):
         """

[rmassoth]

Good call. I'm new to django so I forgot about the awesome db api available. It could suffer from an issue with sqlite here but I think it will work for most cases.

[sjkingo]

Hmm that could cause issues with UTF-8 usernames, but perhaps we should just link to that section in the README with a caveat that if you use sqlite bad things may happen.

Personally I have always exclusively used postgres so I've never come across that issue.

I will test against AD and OpenLDAP later and see if that patch works.

[rmassoth]

I am testing your changes right now too. I'm using sqlite for testing but my live site will use postgres. Doing iexact could still be a performace issue if you have a lot of users but in practice it's probably fine for most.