Merged in lp:~mhall119/django-openid-auth/fixes-642132

README.txt

@@ -126,6 +126,16 @@ It is worth noting that a user needs to be be marked as a "staff user" to be abl
 The easiest way to resolve this is to use traditional authentication (OPENID_USE_AS_ADMIN_LOGIN = False) to sign in as your first user with a password and authorise your 
 openid user to be staff.
 
+== Change Django usernames if the nickname changes on the provider ==
+
+If you want your Django username to change when a user updates the nickname on their provider, add the following setting:
+
+        OPENID_FOLLOW_RENAMES = True
+
+If the new nickname is available as a Django username, the user is renamed.
+Otherwise the user will be renamed to nickname+i for an incrememnting value of i until no conflict occurs.
+If the user has already been renamed to nickname+1 due to a conflict, and the nickname is still not available, the user will keep their existing username.
+
 == Require a valid nickname ==
 
 If you must have a valid, unique nickname in order to create a user accont, add the following setting:

django_openid_auth/auth.py

@@ -86,7 +86,7 @@ class OpenIDBackend:
 
         if getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False):
             details = self._extract_user_details(openid_response)
-            self.update_user_details(user, details)
+            self.update_user_details(user, details, openid_response)
 
         teams_response = teams.TeamsResponse.fromSuccessResponse(
             openid_response)
@@ -103,7 +103,6 @@ class OpenIDBackend:
             email = sreg_response.get('email')
             fullname = sreg_response.get('fullname')
             nickname = sreg_response.get('nickname')
-
         # If any attributes are provided via Attribute Exchange, use
         # them in preference.
         fetch_response = ax.FetchResponse.fromSuccessResponse(openid_response)
@@ -142,17 +141,49 @@ class OpenIDBackend:
         return dict(email=email, nickname=nickname,
                     first_name=first_name, last_name=last_name)
 
-    def create_user_from_openid(self, openid_response):
-        details = self._extract_user_details(openid_response)
-        nickname = details['nickname'] or 'openiduser'
-        email = details['email'] or ''
-
+    def _get_available_username(self, nickname, identity_url):
+        # If we're being strict about usernames, throw an error if we didn't
+        # get one back from the provider
         if getattr(settings, 'OPENID_STRICT_USERNAMES', False):
-            if details['nickname'] is None or details['nickname'] == '':
+            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'
+
+        # See if we already have this nickname assigned to a username
+        try:
+            user = User.objects.get(username__exact=nickname)
+        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(
+                claimed_id__exact=identity_url,
+                user__username__startswith=nickname)
+            # No exception means we have an existing user for this identity
+            # that starts with this nickname, so it's possible we've had to
+            # assign them to nickname+i already.
+            oid_username = user_openid.user.username
+            if len(oid_username) > len(nickname):
+                try:
+                    # check that it ends with a number
+                    int(oid_username[len(nickname):])
+                    return oid_username
+                except ValueError:
+                    # username starts with nickname, but isn't nickname+#
+                    pass
+        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:
                 raise StrictUsernameViolation("Duplicate username: %s" % nickname)
-                
+
         # Pick a username for the user based on their nickname,
         # checking for conflicts.
         i = 1
@@ -161,15 +192,23 @@ class OpenIDBackend:
             if i > 1:
                 username += str(i)
             try:
-                User.objects.get(username__exact=username)
+                user = User.objects.get(username__exact=username)
             except User.DoesNotExist:
                 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'
+        email = details['email'] or ''
 
-        user = User.objects.create_user(username, email, password=None)
-        self.update_user_details(user, details)
+        username = self._get_available_username(details['nickname'], openid_response.identity_url)
 
+        user = User.objects.create_user(username, email, password=None)
         self.associate_openid(user, openid_response)
+        self.update_user_details(user, details, openid_response)
+
         return user
 
     def associate_openid(self, user, openid_response):
@@ -192,7 +231,7 @@ class OpenIDBackend:
 
         return user_openid
 
-    def update_user_details(self, user, details):
+    def update_user_details(self, user, details, openid_response):
         updated = False
         if details['first_name']:
             user.first_name = details['first_name']
@@ -203,6 +242,9 @@ class OpenIDBackend:
         if details['email']:
             user.email = details['email']
             updated = True
+        if getattr(settings, 'OPENID_FOLLOW_RENAMES', False):
+            user.username = self._get_available_username(details['nickname'], openid_response.identity_url)
+            updated = True
 
         if updated:
             user.save()

django_openid_auth/tests/test_views.py

@@ -136,6 +136,7 @@ class RelyingPartyTests(TestCase):
         self.old_sso_server_url = getattr(settings, 'OPENID_SSO_SERVER_URL', None)
         self.old_teams_map = getattr(settings, 'OPENID_LAUNCHPAD_TEAMS_MAPPING', {})
         self.old_use_as_admin_login = getattr(settings, 'OPENID_USE_AS_ADMIN_LOGIN', False)
+        self.old_follow_renames = getattr(settings, 'OPENID_FOLLOW_RENAMES', False)
 
         settings.OPENID_CREATE_USERS = False
         settings.OPENID_STRICT_USERNAMES = False
@@ -143,6 +144,7 @@ class RelyingPartyTests(TestCase):
         settings.OPENID_SSO_SERVER_URL = None
         settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = {}
         settings.OPENID_USE_AS_ADMIN_LOGIN = False
+        settings.OPENID_FOLLOW_RENAMES = False
 
     def tearDown(self):
         settings.LOGIN_REDIRECT_URL = self.old_login_redirect_url
@@ -152,6 +154,7 @@ class RelyingPartyTests(TestCase):
         settings.OPENID_SSO_SERVER_URL = self.old_sso_server_url
         settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = self.old_teams_map
         settings.OPENID_USE_AS_ADMIN_LOGIN = self.old_use_as_admin_login
+        settings.OPENID_FOLLOW_RENAMES = self.old_follow_renames
 
         setDefaultFetcher(None)
         super(RelyingPartyTests, self).tearDown()
@@ -289,6 +292,213 @@ class RelyingPartyTests(TestCase):
         self.assertEquals(user.last_name, 'User')
         self.assertEquals(user.email, 'foo@example.com')
 
+    def _do_user_login(self, openid_req, openid_resp):
+        # Posting in an identity URL begins the authentication request:
+        response = self.client.post('/openid/login/', openid_req)
+        self.assertContains(response, 'OpenID transaction in progress')
+
+        # Complete the request, passing back some simple registration
+        # data.  The user is redirected to the next URL.
+        openid_request = self.provider.parseFormPost(response.content)
+        sreg_request = sreg.SRegRequest.fromOpenIDRequest(openid_request)
+        openid_response = openid_request.answer(True)
+        sreg_response = sreg.SRegResponse.extractResponse(
+            sreg_request, openid_resp)
+        openid_response.addExtension(sreg_response)
+        response = self.complete(openid_response)
+        self.assertRedirects(response, 'http://testserver/getuser/')
+
+    def test_login_without_nickname(self):
+        settings.OPENID_CREATE_USERS = True
+
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        openid_resp =  {'nickname': '', 'fullname': 'Openid User',
+                 'email': 'foo@example.com'}
+        self._do_user_login(openid_req, openid_resp)
+        response = self.client.get('/getuser/')
+
+        # username defaults to 'openiduser'
+        self.assertEquals(response.content, 'openiduser')
+
+        # The user's full name and email have been updated.
+        user = User.objects.get(username=response.content)
+        self.assertEquals(user.first_name, 'Openid')
+        self.assertEquals(user.last_name, 'User')
+        self.assertEquals(user.email, 'foo@example.com')
+        
+    def test_login_follow_rename(self):
+        settings.OPENID_FOLLOW_RENAMES = True
+        settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
+        user = User.objects.create_user('testuser', 'someone@example.com')
+        useropenid = UserOpenID(
+            user=user,
+            claimed_id='http://example.com/identity',
+            display_id='http://example.com/identity')
+        useropenid.save()
+
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        openid_resp =  {'nickname': 'someuser', 'fullname': 'Some User',
+                 'email': 'foo@example.com'}
+        self._do_user_login(openid_req, openid_resp)
+        response = self.client.get('/getuser/')
+
+        # If OPENID_FOLLOW_RENAMES, they are logged in as 
+        # someuser (the passed in nickname has changed the username)
+        self.assertEquals(response.content, 'someuser')
+
+        # The user's full name and email have been updated.
+        user = User.objects.get(username=response.content)
+        self.assertEquals(user.first_name, 'Some')
+        self.assertEquals(user.last_name, 'User')
+        self.assertEquals(user.email, 'foo@example.com')
+        
+    def test_login_follow_rename_conflict(self):
+        settings.OPENID_FOLLOW_RENAMES = True
+        settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
+        # Setup existing user who's name we're going to switch to
+        user = User.objects.create_user('testuser', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=user,
+            claimed_id='http://example.com/existing_identity',
+            display_id='http://example.com/existing_identity')
+
+        # Setup user who is going to try to change username to 'testuser'
+        renamed_user = User.objects.create_user('renameuser', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=renamed_user,
+            claimed_id='http://example.com/identity',
+            display_id='http://example.com/identity')
+
+        # identity url is for 'renameuser'
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        # but returned username is for 'testuser', which already exists for another identity
+        openid_resp =  {'nickname': 'testuser', 'fullname': 'Rename User',
+                 'email': 'rename@example.com'}
+        self._do_user_login(openid_req, openid_resp)
+        response = self.client.get('/getuser/')
+
+        # If OPENID_FOLLOW_RENAMES, attempt to change username to 'testuser'
+        # but since that username is already taken by someone else, we go through
+        # the process of adding +i to it, and get testuser2.
+        self.assertEquals(response.content, 'testuser2')
+
+        # The user's full name and email have been updated.
+        user = User.objects.get(username=response.content)
+        self.assertEquals(user.first_name, 'Rename')
+        self.assertEquals(user.last_name, 'User')
+        self.assertEquals(user.email, 'rename@example.com')
+        
+    def test_login_follow_rename_false_onlyonce(self):
+        settings.OPENID_FOLLOW_RENAMES = True
+        settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
+        # Setup existing user who's name we're going to switch to
+        user = User.objects.create_user('testuser', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=user,
+            claimed_id='http://example.com/existing_identity',
+            display_id='http://example.com/existing_identity')
+
+        # Setup user who is going to try to change username to 'testuser'
+        renamed_user = User.objects.create_user('testuser2000eight', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=renamed_user,
+            claimed_id='http://example.com/identity',
+            display_id='http://example.com/identity')
+
+        # identity url is for 'testuser2000eight'
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        # but returned username is for 'testuser', which already exists for another identity
+        openid_resp =  {'nickname': 'testuser2', 'fullname': 'Rename User',
+                 'email': 'rename@example.com'}
+        self._do_user_login(openid_req, openid_resp)
+        response = self.client.get('/getuser/')
+
+        # If OPENID_FOLLOW_RENAMES, attempt to change username to 'testuser'
+        # but since that username is already taken by someone else, we go through
+        # the process of adding +i to it.  Even though it looks like the username
+        # follows the nickname+i scheme, it has non-numbers in the suffix, so 
+        # it's not an auto-generated one.  The regular process of renaming to 
+        # 'testuser' has a conflict, so we get +2 at the end.
+        self.assertEquals(response.content, 'testuser2')
+
+        # The user's full name and email have been updated.
+        user = User.objects.get(username=response.content)
+        self.assertEquals(user.first_name, 'Rename')
+        self.assertEquals(user.last_name, 'User')
+        self.assertEquals(user.email, 'rename@example.com')
+        
+    def test_login_follow_rename_conflict_onlyonce(self):
+        settings.OPENID_FOLLOW_RENAMES = True
+        settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
+        # Setup existing user who's name we're going to switch to
+        user = User.objects.create_user('testuser', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=user,
+            claimed_id='http://example.com/existing_identity',
+            display_id='http://example.com/existing_identity')
+
+        # Setup user who is going to try to change username to 'testuser'
+        renamed_user = User.objects.create_user('testuser2000', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=renamed_user,
+            claimed_id='http://example.com/identity',
+            display_id='http://example.com/identity')
+
+        # identity url is for 'testuser2000'
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        # but returned username is for 'testuser', which already exists for another identity
+        openid_resp =  {'nickname': 'testuser', 'fullname': 'Rename User',
+                 'email': 'rename@example.com'}
+        self._do_user_login(openid_req, openid_resp)
+        response = self.client.get('/getuser/')
+
+        # If OPENID_FOLLOW_RENAMES, attempt to change username to 'testuser'
+        # but since that username is already taken by someone else, we go through
+        # the process of adding +i to it.  Since the user for this identity url
+        # already has a name matching that pattern, check if first.
+        self.assertEquals(response.content, 'testuser2000')
+
+        # The user's full name and email have been updated.
+        user = User.objects.get(username=response.content)
+        self.assertEquals(user.first_name, 'Rename')
+        self.assertEquals(user.last_name, 'User')
+        self.assertEquals(user.email, 'rename@example.com')
+        
+    def test_login_follow_rename_false_conflict(self):
+        settings.OPENID_FOLLOW_RENAMES = True
+        settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
+        # Setup existing user who's username matches the name+i pattern
+        user = User.objects.create_user('testuser2', 'someone@example.com')
+        UserOpenID.objects.get_or_create(
+            user=user,
+            claimed_id='http://example.com/identity',
+            display_id='http://example.com/identity')
+
+        # identity url is for 'testuser2'
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        # but returned username is for 'testuser', which looks like we've done
+        # a username+1 for them already, but 'testuser' isn't actually taken
+        openid_resp =  {'nickname': 'testuser', 'fullname': 'Same User',
+                 'email': 'same@example.com'}
+        self._do_user_login(openid_req, openid_resp)
+        response = self.client.get('/getuser/')
+
+        # If OPENID_FOLLOW_RENAMES, username should be changed to 'testuser'
+        # because it wasn't currently taken
+        self.assertEquals(response.content, 'testuser')
+
+        # The user's full name and email have been updated.
+        user = User.objects.get(username=response.content)
+        self.assertEquals(user.first_name, 'Same')
+        self.assertEquals(user.last_name, 'User')
+        self.assertEquals(user.email, 'same@example.com')
+        
     def test_strict_username_no_nickname(self):
         settings.OPENID_CREATE_USERS = True
         settings.OPENID_STRICT_USERNAMES = True
@@ -354,31 +564,17 @@ class RelyingPartyTests(TestCase):
             display_id='http://example.com/identity')
         useropenid.save()
 
-        # Posting in an identity URL begins the authentication request:
-        response = self.client.post('/openid/login/',
-            {'openid_identifier': 'http://example.com/identity',
-             'next': '/getuser/'})
-        self.assertContains(response, 'OpenID transaction in progress')
-
-        # Complete the request, passing back some simple registration
-        # data.  The user is redirected to the next URL.
-        openid_request = self.provider.parseFormPost(response.content)
-        sreg_request = sreg.SRegRequest.fromOpenIDRequest(openid_request)
-        openid_response = openid_request.answer(True)
-        sreg_response = sreg.SRegResponse.extractResponse(
-            sreg_request, {'nickname': 'someuser', 'fullname': 'Some User',
-                           'email': 'foo@example.com'})
-        openid_response.addExtension(sreg_response)
-        response = self.complete(openid_response)
-        self.assertRedirects(response, 'http://testserver/getuser/')
-
-        # And they are now logged in as testuser (the passed in
-        # nickname has not caused the username to change).
+        openid_req = {'openid_identifier': 'http://example.com/identity',
+               'next': '/getuser/'}
+        openid_resp =  {'nickname': 'testuser', 'fullname': 'Some User',
+                 'email': 'foo@example.com'}
+        self._do_user_login(openid_req, openid_resp)
         response = self.client.get('/getuser/')
+
         self.assertEquals(response.content, 'testuser')
 
         # The user's full name and email have been updated.
-        user = User.objects.get(username='testuser')
+        user = User.objects.get(username=response.content)
         self.assertEquals(user.first_name, 'Some')
         self.assertEquals(user.last_name, 'User')
         self.assertEquals(user.email, 'foo@example.com')
@@ -473,6 +669,7 @@ class RelyingPartyTests(TestCase):
         self.assertEquals(user.email, 'foo@example.com')
 
     def test_login_teams(self):
+        settings.OPENID_LAUNCHPAD_TEAMS_MAPPING_AUTO = False
         settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = {'teamname': 'groupname',
                                                    'otherteam': 'othergroup'}
         user = User.objects.create_user('testuser', 'someone@example.com')