Merged in lp:~mhall119/django-openid-auth/strict-username-requirements

README.txt

@@ -125,3 +125,13 @@ If you require openid authentication into the admin application, add the followi
 It is worth noting that a user needs to be be marked as a "staff user" to be able to access the admin interface.  A new openid user will not normally be a "staff user".  
 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.
+
+== Require a valid nickname ==
+
+If you must have a valid, unique nickname in order to create a user accont, add the following setting:
+
+        OPENID_STRICT_USERNAMES = True
+        
+This will cause an OpenID login attempt to fail if the provider does not return a 'nickname' (username) for the user, or if the nickname conflicts with an existing user with a different openid identiy url.
+Without this setting, logins without a nickname will be given the username 'openiduser', and upon conflicts with existing username, an incrementing number will be appended to the username until it is unique.
+

django_openid_auth/auth.py

@@ -42,6 +42,8 @@ from django_openid_auth.models import UserOpenID
 class IdentityAlreadyClaimed(Exception):
     pass
 
+class StrictUsernameViolation(Exception):
+    pass
     
 class OpenIDBackend:
     """A django.contrib.auth backend that authenticates the user based on
@@ -72,7 +74,10 @@ class OpenIDBackend:
                 claimed_id__exact=openid_response.identity_url)
         except UserOpenID.DoesNotExist:
             if getattr(settings, 'OPENID_CREATE_USERS', False):
+                try:
                     user = self.create_user_from_openid(openid_response)
+                except StrictUsernameViolation:
+                    return None
         else:
             user = user_openid.user
 
@@ -142,6 +147,12 @@ class OpenIDBackend:
         nickname = details['nickname'] or 'openiduser'
         email = details['email'] or ''
 
+        if getattr(settings, 'OPENID_STRICT_USERNAMES', False):
+            if details['nickname'] is None or details['nickname'] == '':
+                raise StrictUsernameViolation("No username")
+            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

django_openid_auth/tests/test_views.py

@@ -131,12 +131,14 @@ class RelyingPartyTests(TestCase):
 
         self.old_login_redirect_url = getattr(settings, 'LOGIN_REDIRECT_URL', '/accounts/profile/')
         self.old_create_users = getattr(settings, 'OPENID_CREATE_USERS', False)
+        self.old_strict_usernames = getattr(settings, 'OPENID_STRICT_USERNAMES', False)
         self.old_update_details = getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False)
         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)
 
         settings.OPENID_CREATE_USERS = False
+        settings.OPENID_STRICT_USERNAMES = False
         settings.OPENID_UPDATE_DETAILS_FROM_SREG = False
         settings.OPENID_SSO_SERVER_URL = None
         settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = {}
@@ -145,6 +147,7 @@ class RelyingPartyTests(TestCase):
     def tearDown(self):
         settings.LOGIN_REDIRECT_URL = self.old_login_redirect_url
         settings.OPENID_CREATE_USERS = self.old_create_users
+        settings.OPENID_STRICT_USERNAMES = self.old_strict_usernames
         settings.OPENID_UPDATE_DETAILS_FROM_SREG = self.old_update_details
         settings.OPENID_SSO_SERVER_URL = self.old_sso_server_url
         settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = self.old_teams_map
@@ -286,6 +289,62 @@ class RelyingPartyTests(TestCase):
         self.assertEquals(user.last_name, 'User')
         self.assertEquals(user.email, 'foo@example.com')
 
+    def test_strict_username_no_nickname(self):
+        settings.OPENID_CREATE_USERS = True
+        settings.OPENID_STRICT_USERNAMES = True
+
+        # 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': '', # No nickname
+                           'fullname': 'Some User',
+                           'email': 'foo@example.com'})
+        openid_response.addExtension(sreg_response)
+        response = self.complete(openid_response)
+        
+        # Status code should be 403: Forbidden
+        self.assertEquals(403, response.status_code)
+
+    def test_strict_username_duplicate_user(self):
+        settings.OPENID_CREATE_USERS = True
+        settings.OPENID_STRICT_USERNAMES = True
+        # Create a user with the same name as we'll pass back via sreg.
+        user = User.objects.create_user('someuser', 'someone@example.com')
+        useropenid = UserOpenID(
+            user=user,
+            claimed_id='http://example.com/different_identity',
+            display_id='http://example.com/different_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)
+        
+        # Status code should be 403: Forbidden
+        self.assertEquals(403, response.status_code)
+
     def test_login_update_details(self):
         settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
         user = User.objects.create_user('testuser', 'someone@example.com')