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

Proposed by Michael Hall on 2010-10-15
Status: Superseded
Proposed branch: lp:~mhall119/django-openid-auth/strict-username-requirements
Merge into: lp:~django-openid-auth/django-openid-auth/trunk
Diff against target: 129 lines (+72/-2)
2 files modified
django_openid_auth/auth.py (+13/-2)
django_openid_auth/tests/test_views.py (+59/-0)
To merge this branch: bzr merge lp:~mhall119/django-openid-auth/strict-username-requirements
Reviewer Review Type Date Requested Status
James Henstridge Needs Fixing on 2010-12-22
Anthony Lenton 2010-10-15 Approve on 2010-12-06
Review via email: mp+38532@code.launchpad.net

This proposal has been superseded by a proposal from 2011-03-18.

Description of the change

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.

To post a comment you must log in.
Anthony Lenton (elachuni) wrote :

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

review: Approve
James Henstridge (jamesh) wrote :

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
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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'django_openid_auth/auth.py'
2--- django_openid_auth/auth.py 2010-10-15 09:08:10 +0000
3+++ django_openid_auth/auth.py 2011-03-18 20:03:27 +0000
4@@ -42,7 +42,9 @@
5 class IdentityAlreadyClaimed(Exception):
6 pass
7
8-
9+class StrictUsernameViolation(Exception):
10+ pass
11+
12 class OpenIDBackend:
13 """A django.contrib.auth backend that authenticates the user based on
14 an OpenID response."""
15@@ -72,7 +74,10 @@
16 claimed_id__exact=openid_response.identity_url)
17 except UserOpenID.DoesNotExist:
18 if getattr(settings, 'OPENID_CREATE_USERS', False):
19- user = self.create_user_from_openid(openid_response)
20+ try:
21+ user = self.create_user_from_openid(openid_response)
22+ except StrictUsernameViolation:
23+ return None
24 else:
25 user = user_openid.user
26
27@@ -142,6 +147,12 @@
28 nickname = details['nickname'] or 'openiduser'
29 email = details['email'] or ''
30
31+ if getattr(settings, 'OPENID_STRICT_USERNAMES', False):
32+ if details['nickname'] is None or details['nickname'] == '':
33+ raise StrictUsernameViolation("No username")
34+ if User.objects.filter(username__exact=nickname).count() > 0:
35+ raise StrictUsernameViolation("Duplicate username: %s" % nickname)
36+
37 # Pick a username for the user based on their nickname,
38 # checking for conflicts.
39 i = 1
40
41=== modified file 'django_openid_auth/tests/test_views.py'
42--- django_openid_auth/tests/test_views.py 2011-01-13 08:38:01 +0000
43+++ django_openid_auth/tests/test_views.py 2011-03-18 20:03:27 +0000
44@@ -131,12 +131,14 @@
45
46 self.old_login_redirect_url = getattr(settings, 'LOGIN_REDIRECT_URL', '/accounts/profile/')
47 self.old_create_users = getattr(settings, 'OPENID_CREATE_USERS', False)
48+ self.old_strict_usernames = getattr(settings, 'OPENID_STRICT_USERNAMES', False)
49 self.old_update_details = getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False)
50 self.old_sso_server_url = getattr(settings, 'OPENID_SSO_SERVER_URL', None)
51 self.old_teams_map = getattr(settings, 'OPENID_LAUNCHPAD_TEAMS_MAPPING', {})
52 self.old_use_as_admin_login = getattr(settings, 'OPENID_USE_AS_ADMIN_LOGIN', False)
53
54 settings.OPENID_CREATE_USERS = False
55+ settings.OPENID_STRICT_USERNAMES = False
56 settings.OPENID_UPDATE_DETAILS_FROM_SREG = False
57 settings.OPENID_SSO_SERVER_URL = None
58 settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = {}
59@@ -145,6 +147,7 @@
60 def tearDown(self):
61 settings.LOGIN_REDIRECT_URL = self.old_login_redirect_url
62 settings.OPENID_CREATE_USERS = self.old_create_users
63+ settings.OPENID_STRICT_USERNAMES = self.old_strict_usernames
64 settings.OPENID_UPDATE_DETAILS_FROM_SREG = self.old_update_details
65 settings.OPENID_SSO_SERVER_URL = self.old_sso_server_url
66 settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = self.old_teams_map
67@@ -286,6 +289,62 @@
68 self.assertEquals(user.last_name, 'User')
69 self.assertEquals(user.email, 'foo@example.com')
70
71+ def test_strict_username_no_nickname(self):
72+ settings.OPENID_CREATE_USERS = True
73+ settings.OPENID_STRICT_USERNAMES = True
74+
75+ # Posting in an identity URL begins the authentication request:
76+ response = self.client.post('/openid/login/',
77+ {'openid_identifier': 'http://example.com/identity',
78+ 'next': '/getuser/'})
79+ self.assertContains(response, 'OpenID transaction in progress')
80+
81+ # Complete the request, passing back some simple registration
82+ # data. The user is redirected to the next URL.
83+ openid_request = self.provider.parseFormPost(response.content)
84+ sreg_request = sreg.SRegRequest.fromOpenIDRequest(openid_request)
85+ openid_response = openid_request.answer(True)
86+ sreg_response = sreg.SRegResponse.extractResponse(
87+ sreg_request, {'nickname': '', # No nickname
88+ 'fullname': 'Some User',
89+ 'email': 'foo@example.com'})
90+ openid_response.addExtension(sreg_response)
91+ response = self.complete(openid_response)
92+
93+ # Status code should be 403: Forbidden
94+ self.assertEquals(403, response.status_code)
95+
96+ def test_strict_username_duplicate_user(self):
97+ settings.OPENID_CREATE_USERS = True
98+ settings.OPENID_STRICT_USERNAMES = True
99+ # Create a user with the same name as we'll pass back via sreg.
100+ user = User.objects.create_user('someuser', 'someone@example.com')
101+ useropenid = UserOpenID(
102+ user=user,
103+ claimed_id='http://example.com/different_identity',
104+ display_id='http://example.com/different_identity')
105+ useropenid.save()
106+
107+ # Posting in an identity URL begins the authentication request:
108+ response = self.client.post('/openid/login/',
109+ {'openid_identifier': 'http://example.com/identity',
110+ 'next': '/getuser/'})
111+ self.assertContains(response, 'OpenID transaction in progress')
112+
113+ # Complete the request, passing back some simple registration
114+ # data. The user is redirected to the next URL.
115+ openid_request = self.provider.parseFormPost(response.content)
116+ sreg_request = sreg.SRegRequest.fromOpenIDRequest(openid_request)
117+ openid_response = openid_request.answer(True)
118+ sreg_response = sreg.SRegResponse.extractResponse(
119+ sreg_request, {'nickname': 'someuser', 'fullname': 'Some User',
120+ 'email': 'foo@example.com'})
121+ openid_response.addExtension(sreg_response)
122+ response = self.complete(openid_response)
123+
124+ # Status code should be 403: Forbidden
125+ self.assertEquals(403, response.status_code)
126+
127 def test_login_update_details(self):
128 settings.OPENID_UPDATE_DETAILS_FROM_SREG = True
129 user = User.objects.create_user('testuser', 'someone@example.com')

Subscribers

People subscribed via source and target branches