Merge lp:~mhall119/django-openid-auth/strict-username-requirements into lp:~django-openid-auth/django-openid-auth/trunk

[Michael Hall]

I'm not sure how to write proper tests for this, but I have tested that it works. The error page isn't real nice, but it's not turned on by default, and we're really going to need this for loco-directory and summit.

[Anthony Lenton (elachuni)]

Tests would be nice, but it looks ok to me :)

review: Approve

[James Henstridge (jamesh)]

I agree with Anthony here.

Here is a quick sketch for the test:

1. add a test to test_views.py. Using test_login_create_users() as a base would probably be easiest.
2. create a User and UserOpenID object with a given nickname and identity.
3. set OPENID_STRICT_USERNAMES, and try to log in using a different identity URL but the same nickname
4. verify that the login has failed.

After adding the test, this change looks fine.

review: Needs Fixing

[lp:~mhall119/django-openid-auth/strict-username-requirements]

75. By Anthony Lenton on 2011-01-05

Merged in lp:~stuartmetcalfe/django-openid-auth/staff-assignment

76. By Anthony Lenton on 2011-01-07

Merged in lp:~elachuni/django-openid-auth/django1.1-support

77. By Anthony Lenton on 2011-01-12

Merged in lp:~michael.nelson/django-openid-auth/701484-optional-sreg-fields

78. By Anthony Lenton on 2011-01-12

Merged lp:~michael.nelson/django-openid-auth/701489-fire-event-with-sreg-response

79. By Anthony Lenton on 2011-01-13

Merged in lp:~michael.nelson/django-openid-auth/701489-rename-signal-provide-openid-response

80. By Michael Hall on 2011-03-18

Add tests for strict username restrictions

81. By Michael Hall on 2011-03-23

Added notes about the new settings option to README.txt

Unmerged revisions

=== modified file 'django_openid_auth/auth.py'
--- django_openid_auth/auth.py	2010-10-15 09:08:10 +0000
+++ django_openid_auth/auth.py	2011-03-18 20:03:27 +0000
@@ -42,7 +42,9 @@
 class IdentityAlreadyClaimed(Exception):
     pass
 
-
+class StrictUsernameViolation(Exception):
+    pass
+    
 class OpenIDBackend:
     """A django.contrib.auth backend that authenticates the user based on
     an OpenID response."""
@@ -72,7 +74,10 @@
                 claimed_id__exact=openid_response.identity_url)
         except UserOpenID.DoesNotExist:
             if getattr(settings, 'OPENID_CREATE_USERS', False):
-                user = self.create_user_from_openid(openid_response)
+                try:
+                    user = self.create_user_from_openid(openid_response)
+                except StrictUsernameViolation:
+                    return None
         else:
             user = user_openid.user
 
@@ -142,6 +147,12 @@
         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
 
=== modified file 'django_openid_auth/tests/test_views.py'
--- django_openid_auth/tests/test_views.py	2011-01-13 08:38:01 +0000
+++ django_openid_auth/tests/test_views.py	2011-03-18 20:03:27 +0000
@@ -131,12 +131,14 @@
 
         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 @@
     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 @@
         self.assertEquals(user.last_name, 'User')
         self.assertEquals(user.email, '[e-mail]')
 
+    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': '[e-mail]'})
+        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', '[e-mail]')
+        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': '[e-mail]'})
+        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', '[e-mail]')