Merge lp:~michael.nelson/django-openid-auth/801499-check-name-length into lp:~django-openid-auth/django-openid-auth/trunk

[Michael Nelson]

Overview
========
This branch fixes the following bugs:
 * bug 801499 - long first/last names are too long for the django.contrib.auth.models.User fields.
 * bug 632857 - leading/trailing spaces in fullname don't cause errors.

Together these two bugs were causing approx. 4 oopses each day for the Ubuntu software center server.

Details
=======
I also took the chance to refactor the tests for readability, adding a factory for creating successful (AX) responses.

To test: `make check`

[Anthony Lenton (elachuni)]

Looks fine!

review: Approve

=== modified file 'django_openid_auth/auth.py'
--- django_openid_auth/auth.py	2011-05-12 09:46:37 +0000
+++ django_openid_auth/auth.py	2011-06-24 13:05:30 +0000
@@ -44,7 +44,7 @@
 
 class StrictUsernameViolation(Exception):
     pass
-    
+
 class OpenIDBackend:
     """A django.contrib.auth backend that authenticates the user based on
     an OpenID response."""
@@ -132,8 +132,10 @@
         if fullname and not (first_name or last_name):
             # Django wants to store first and last names separately,
             # so we do our best to split the full name.
-            if ' ' in fullname:
-                first_name, last_name = fullname.rsplit(None, 1)
+            fullname = fullname.strip()
+            split_names = fullname.rsplit(None, 1)
+            if len(split_names) == 2:
+                first_name, last_name = split_names
             else:
                 first_name = u''
                 last_name = fullname
@@ -147,7 +149,7 @@
         if getattr(settings, 'OPENID_STRICT_USERNAMES', False):
             if nickname is None or nickname == '':
                 raise StrictUsernameViolation("No username")
-                
+
         # If we don't have a nickname, and we're not being strict, use a default
         nickname = nickname or 'openiduser'
 
@@ -157,7 +159,7 @@
         except User.DoesNotExist:
             # No conflict, we can use this nickname
             return nickname
-            
+
         # Check if we already have nickname+i for this identity_url
         try:
             user_openid = UserOpenID.objects.get(
@@ -178,7 +180,7 @@
         except UserOpenID.DoesNotExist:
             # No user associated with this identity_url
             pass
-            
+
 
         if getattr(settings, 'OPENID_STRICT_USERNAMES', False):
             if User.objects.filter(username__exact=nickname).count() > 0:
@@ -197,7 +199,7 @@
                 break
             i += 1
         return username
-    
+
     def create_user_from_openid(self, openid_response):
         details = self._extract_user_details(openid_response)
         nickname = details['nickname'] or 'openiduser'
@@ -234,10 +236,10 @@
     def update_user_details(self, user, details, openid_response):
         updated = False
         if details['first_name']:
-            user.first_name = details['first_name']
+            user.first_name = details['first_name'][:30]
             updated = True
         if details['last_name']:
-            user.last_name = details['last_name']
+            user.last_name = details['last_name'][:30]
             updated = True
         if details['email']:
             user.email = details['email']

=== modified file 'django_openid_auth/tests/test_auth.py'
--- django_openid_auth/tests/test_auth.py	2010-08-10 08:58:07 +0000
+++ django_openid_auth/tests/test_auth.py	2011-06-24 13:05:30 +0000
@@ -28,6 +28,7 @@
 
 import unittest
 
+from django.contrib.auth.models import User
 from django.test import TestCase
 
 from django_openid_auth.auth import OpenIDBackend
@@ -60,74 +61,95 @@
                                 "last_name": "User",
                                 "email": "[e-mail]"})
 
+    def make_response_ax(self, schema="http://axschema.org/",
+        fullname="Some User", nickname="someuser", email="[e-mail]",
+        first=None, last=None):
+        endpoint = OpenIDServiceEndpoint()
+        message = Message(OPENID2_NS)
+        attributes = [
+            ("nickname", schema + "namePerson/friendly", nickname),
+            ("fullname", schema + "namePerson", fullname),
+            ("email", schema + "contact/email", email),
+            ]
+        if first:
+            attributes.append(
+                ("first", "http://axschema.org/namePerson/first", first))
+        if last:
+            attributes.append(
+                ("last", "http://axschema.org/namePerson/last", last))
+
+        message.setArg(AX_NS, "mode", "fetch_response")
+        for (alias, uri, value) in attributes:
+            message.setArg(AX_NS, "type.%s" % alias, uri)
+            message.setArg(AX_NS, "value.%s" % alias, value)
+        return SuccessResponse(
+            endpoint, message, signed_fields=message.toPostArgs().keys())
+
     def test_extract_user_details_ax(self):
-        endpoint = OpenIDServiceEndpoint()
-        message = Message(OPENID2_NS)
-        attributes = [
-            ("nickname", "http://axschema.org/namePerson/friendly", "someuser"),
-            ("fullname", "http://axschema.org/namePerson", "Some User"),
-            ("email", "http://axschema.org/contact/email", "[e-mail]"),
-            ]
-        message.setArg(AX_NS, "mode", "fetch_response")
-        for (alias, uri, value) in attributes:
-            message.setArg(AX_NS, "type.%s" % alias, uri)
-            message.setArg(AX_NS, "value.%s" % alias, value)
-        response = SuccessResponse(
-            endpoint, message, signed_fields=message.toPostArgs().keys())
+        response = self.make_response_ax(fullname="Some User",
+            nickname="someuser", email="[e-mail]")
 
         data = self.backend._extract_user_details(response)
+
         self.assertEqual(data, {"nickname": "someuser",
                                 "first_name": "Some",
                                 "last_name": "User",
                                 "email": "[e-mail]"})
 
     def test_extract_user_details_ax_split_name(self):
-        endpoint = OpenIDServiceEndpoint()
-        message = Message(OPENID2_NS)
-        attributes = [
-            ("nickname", "http://axschema.org/namePerson/friendly", "someuser"),
-            # Include this key too to show that the split data takes
-            # precedence.
-            ("fullname", "http://axschema.org/namePerson", "Bad Data"),
-            ("first", "http://axschema.org/namePerson/first", "Some"),
-            ("last", "http://axschema.org/namePerson/last", "User"),
-            ("email", "http://axschema.org/contact/email", "[e-mail]"),
-            ]
-        message.setArg(AX_NS, "mode", "fetch_response")
-        for (alias, uri, value) in attributes:
-            message.setArg(AX_NS, "type.%s" % alias, uri)
-            message.setArg(AX_NS, "value.%s" % alias, value)
-        response = SuccessResponse(
-            endpoint, message, signed_fields=message.toPostArgs().keys())
+        # Include fullname too to show that the split data takes
+        # precedence.
+        response = self.make_response_ax(
+            fullname="Bad Data", first="Some", last="User")
 
         data = self.backend._extract_user_details(response)
+
         self.assertEqual(data, {"nickname": "someuser",
                                 "first_name": "Some",
                                 "last_name": "User",
                                 "email": "[e-mail]"})
 
     def test_extract_user_details_ax_broken_myopenid(self):
-        endpoint = OpenIDServiceEndpoint()
-        message = Message(OPENID2_NS)
-        attributes = [
-            ("nickname", "http://schema.openid.net/namePerson/friendly",
-             "someuser"),
-            ("fullname", "http://schema.openid.net/namePerson", "Some User"),
-            ("email", "http://schema.openid.net/contact/email",
-             "[e-mail]"),
-            ]
-        message.setArg(AX_NS, "mode", "fetch_response")
-        for (alias, uri, value) in attributes:
-            message.setArg(AX_NS, "type.%s" % alias, uri)
-            message.setArg(AX_NS, "value.%s" % alias, value)
-        response = SuccessResponse(
-            endpoint, message, signed_fields=message.toPostArgs().keys())
+        response = self.make_response_ax(
+            schema="http://schema.openid.net/", fullname="Some User",
+            nickname="someuser", email="[e-mail]")
 
         data = self.backend._extract_user_details(response)
+
         self.assertEqual(data, {"nickname": "someuser",
                                 "first_name": "Some",
                                 "last_name": "User",
                                 "email": "[e-mail]"})
 
+    def test_update_user_details_long_names(self):
+        response = self.make_response_ax()
+        user = User.objects.create_user('someuser', '[e-mail]',
+            password=None)
+        data = dict(first_name=u"Some56789012345678901234567890123",
+            last_name=u"User56789012345678901234567890123",
+            email=u"[e-mail]")
+
+        self.backend.update_user_details(user, data, response)
+
+        self.assertEqual("Some56789012345678901234567890",  user.first_name)
+        self.assertEqual("User56789012345678901234567890",  user.last_name)
+
+    def test_extract_user_details_name_with_trailing_space(self):
+        response = self.make_response_ax(fullname="SomeUser ")
+
+        data = self.backend._extract_user_details(response)
+
+        self.assertEqual("", data['first_name'])
+        self.assertEqual("SomeUser", data['last_name'])
+
+    def test_extract_user_details_name_with_thin_space(self):
+        response = self.make_response_ax(fullname=u"Some\u2009User")
+
+        data = self.backend._extract_user_details(response)
+
+        self.assertEqual("Some", data['first_name'])
+        self.assertEqual("User", data['last_name'])
+
+
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)