Merge lp:~mhall119/django-openid-auth/fixes-642132 into lp:~django-openid-auth/django-openid-auth/trunk
- fixes-642132
- Merge into trunk
Status: | Superseded | ||||
---|---|---|---|---|---|
Proposed branch: | lp:~mhall119/django-openid-auth/fixes-642132 | ||||
Merge into: | lp:~django-openid-auth/django-openid-auth/trunk | ||||
Diff against target: |
391 lines (+267/-31) 2 files modified
django_openid_auth/auth.py (+50/-10) django_openid_auth/tests/test_views.py (+217/-21) |
||||
To merge this branch: | bzr merge lp:~mhall119/django-openid-auth/fixes-642132 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Henstridge | Needs Fixing | ||
Review via email: mp+38335@code.launchpad.net |
This proposal has been superseded by a proposal from 2011-03-21.
Commit message
Description of the change
Adds a new settings paramater: OPENID_
When set to True, and when OPENID_
Michael Hall (mhall119) wrote : | # |
Anthony Lenton (elachuni) wrote : | # |
Hi Michael,
Thanks for your work on this. The code around lines 8-10 of the diff would need to be a bit more robust when updating the User's username, as there might be some other user in the system with the new username already.
I think the underlying problem is what we really want to do in this case (ie if 'joesmith' changes his openid username to 'joebobs', but you already have a 'joebobs' in your system). Skipping the username update in that case would work but it'll leave you with out-of-sync data. Refusing to sign the user in would be quite violent, and merging the two accounts would be definitely a bad idea. What would work in your use case?
Sites with a single OpenID provider would be less affected by this issue, but it could still bite you.
Otoh, I'm not sure why django-openid-auth doesn't currently update username together with fullname and email when OPENID_
Michael Hall (mhall119) wrote : | # |
I think in the case of an existing username conflict, then we could go back to $username+1 increments until we find a unique one. Unfortunately it would do this incrementing process every time the user logged in, until their new LP username is available in Django, but I don't think that would be such a performance hit.
James Henstridge (jamesh) wrote : | # |
This should do the check to see if anyone else has the same user name, and should ideally use the same name checking code as create_
I would suggest only changing the user name if the existing user name does not have the sreg nickname as a prefix. This should prevent us from trying to change the nickname on every login if there is a duplicate nickname.
With respect to the tests, how about factoring out the body of test_login_
Lastly, a more basic question: do we want to have a setting for this behaviour? If it has minimal overhead and is something most people would want, is there any reason not to always do this?
Michael Hall (mhall119) wrote : | # |
There may be cases where renames aren't desirable, for example if the username is used to link tables, rather than a constant user id.
- 75. By Anthony Lenton
-
Merged in lp:~stuartmetcalfe/django-openid-auth/staff-assignment
- 76. By Anthony Lenton
- 77. By Anthony Lenton
-
Merged in lp:~michael.nelson/django-openid-auth/701484-optional-sreg-fields
- 78. By Anthony Lenton
-
Merged lp:~michael.nelson/django-openid-auth/701489-fire-event-with-sreg-response
- 79. By Anthony Lenton
-
Merged in lp:~michael.nelson/django-openid-auth/701489-rename-signal-provide-openid-response
Ronnie (ronnie.vd.c) wrote : | # |
I would suggest checking the OpenId-Provider (url like: https:/
If the new username exists, but claimed_id does not match (but is from the same provider), The old username should be called OpenIdUserX,
OPENIDPROVIDER1 (Google)
- {'nickname': 'Ben', 'claimed_id': 'http://
OPENIDPROVIDER2 (Launchpad)
- {'nickname': 'Ben', 'claimed_id': 'https:/
- {'nickname': 'Cees', 'claimed_id': 'https:/
======
- Ben deletes his account on launchpad
- Cees changed name to Ben
OPENIDCONSUMER
- Ben (http://
- Ben (https:/
- Cees (https:/
=======
- Ben (previous Cees | https:/
-- user 'Ben1' should be renamed to 'OpenIdUserX'
-- user Cees should be renamed to 'Ben1'
small catch: What to do if Ben (https:/
Ronnie (ronnie.vd.c) wrote : | # |
A more general approach, which also solves the small catch:
The username should always be renamed if it has changed, one exception for this is when the same username exists for another openid-provider. If there is already an user with that username (same provider) then the old_user should be renamed to the name without number (if available) else to the name + first_available
If the same username is kept by another openid-provider, the user should keep its old username (if exists), if a new user is created, it should be username1 (username2 etc which first is available)
Therefore in the example above:
OPENIDPROVIDER
User Ben(https:/
User Cees(https:/
OPENIDCONSUMER
Ben (previous Cees | https:/
user 'Ben1' (https:/
user 'Cees' should be renamed to 'Ben1'
Ben2 (https:/
user 'Ben1' (previous Cees | https:/
user 'Ben2' should be renamed to 'Ben1'
All next logins for Ben3 (Cees/Ben (https:/
- 80. By Michael Hall
-
Merge from trunk
- 81. By Michael Hall
-
Extra renaming considerations and tests for conflicts and false positives
- 82. By Michael Hall
-
Add another test to make sure renaming doesn't get confused by not-quite nickna me+1 usernames
- 83. By Michael Hall
-
Finish new test and altered comments
- 84. By Michael Hall
-
Add new option to README.txt and removed an unecessary check in _get_available_
username( ) - 85. By Michael Hall
-
Updates from trunk
- 86. By Michael Hall
-
Check for STRICT_USERNAMES before defaulting the nickname to openiduser
- 87. By Michael Hall
-
Make sure auto-mapping is turned off when testing teams->group
Unmerged revisions
Preview Diff
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-21 11:58:29 +0000 | |||
4 | @@ -81,7 +81,7 @@ | |||
5 | 81 | 81 | ||
6 | 82 | if getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False): | 82 | if getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False): |
7 | 83 | details = self._extract_user_details(openid_response) | 83 | details = self._extract_user_details(openid_response) |
9 | 84 | self.update_user_details(user, details) | 84 | self.update_user_details(user, details, openid_response) |
10 | 85 | 85 | ||
11 | 86 | teams_response = teams.TeamsResponse.fromSuccessResponse( | 86 | teams_response = teams.TeamsResponse.fromSuccessResponse( |
12 | 87 | openid_response) | 87 | openid_response) |
13 | @@ -98,7 +98,6 @@ | |||
14 | 98 | email = sreg_response.get('email') | 98 | email = sreg_response.get('email') |
15 | 99 | fullname = sreg_response.get('fullname') | 99 | fullname = sreg_response.get('fullname') |
16 | 100 | nickname = sreg_response.get('nickname') | 100 | nickname = sreg_response.get('nickname') |
17 | 101 | |||
18 | 102 | # If any attributes are provided via Attribute Exchange, use | 101 | # If any attributes are provided via Attribute Exchange, use |
19 | 103 | # them in preference. | 102 | # them in preference. |
20 | 104 | fetch_response = ax.FetchResponse.fromSuccessResponse(openid_response) | 103 | fetch_response = ax.FetchResponse.fromSuccessResponse(openid_response) |
21 | @@ -137,10 +136,36 @@ | |||
22 | 137 | return dict(email=email, nickname=nickname, | 136 | return dict(email=email, nickname=nickname, |
23 | 138 | first_name=first_name, last_name=last_name) | 137 | first_name=first_name, last_name=last_name) |
24 | 139 | 138 | ||
29 | 140 | def create_user_from_openid(self, openid_response): | 139 | def _get_available_username(self, nickname, identity_url): |
30 | 141 | details = self._extract_user_details(openid_response) | 140 | nickname = nickname or 'openiduser' |
31 | 142 | nickname = details['nickname'] or 'openiduser' | 141 | # See if we already have this nickname assigned to a username |
32 | 143 | email = details['email'] or '' | 142 | try: |
33 | 143 | user = User.objects.get(username__exact=nickname) | ||
34 | 144 | except User.DoesNotExist: | ||
35 | 145 | # No conflict, we can use this nickname | ||
36 | 146 | return nickname | ||
37 | 147 | |||
38 | 148 | # Check if we already have nickname+i for this identity_url | ||
39 | 149 | try: | ||
40 | 150 | user_openid = UserOpenID.objects.get( | ||
41 | 151 | claimed_id__exact=identity_url, | ||
42 | 152 | user__username__startswith=nickname) | ||
43 | 153 | # No exception means we have an existing user for this identity | ||
44 | 154 | # that starts with this nickname, so it's possible we've had to | ||
45 | 155 | # assign them to nickname+i already. | ||
46 | 156 | oid_username = user_openid.user.username | ||
47 | 157 | if len(oid_username) > len(nickname): | ||
48 | 158 | try: | ||
49 | 159 | # check that it ends with a number | ||
50 | 160 | int(oid_username[len(nickname):]) | ||
51 | 161 | return oid_username | ||
52 | 162 | except ValueError: | ||
53 | 163 | # username starts with nickname, but isn't nickname+# | ||
54 | 164 | pass | ||
55 | 165 | except UserOpenID.DoesNotExist: | ||
56 | 166 | # No user associated with this identity_url | ||
57 | 167 | pass | ||
58 | 168 | |||
59 | 144 | 169 | ||
60 | 145 | # Pick a username for the user based on their nickname, | 170 | # Pick a username for the user based on their nickname, |
61 | 146 | # checking for conflicts. | 171 | # checking for conflicts. |
62 | @@ -150,15 +175,27 @@ | |||
63 | 150 | if i > 1: | 175 | if i > 1: |
64 | 151 | username += str(i) | 176 | username += str(i) |
65 | 152 | try: | 177 | try: |
67 | 153 | User.objects.get(username__exact=username) | 178 | user = User.objects.get(username__exact=username) |
68 | 179 | if user.useropenid_set.filter(claimed_id__exact=identity_url).count() > 0: | ||
69 | 180 | # username already belongs to this openid user, so it's okay | ||
70 | 181 | return username | ||
71 | 182 | |||
72 | 154 | except User.DoesNotExist: | 183 | except User.DoesNotExist: |
73 | 155 | break | 184 | break |
74 | 156 | i += 1 | 185 | i += 1 |
75 | 186 | return username | ||
76 | 187 | |||
77 | 188 | def create_user_from_openid(self, openid_response): | ||
78 | 189 | details = self._extract_user_details(openid_response) | ||
79 | 190 | nickname = details['nickname'] or 'openiduser' | ||
80 | 191 | email = details['email'] or '' | ||
81 | 192 | |||
82 | 193 | username = self._get_available_username(details['nickname'], openid_response.identity_url) | ||
83 | 157 | 194 | ||
84 | 158 | user = User.objects.create_user(username, email, password=None) | 195 | user = User.objects.create_user(username, email, password=None) |
85 | 159 | self.update_user_details(user, details) | ||
86 | 160 | |||
87 | 161 | self.associate_openid(user, openid_response) | 196 | self.associate_openid(user, openid_response) |
88 | 197 | self.update_user_details(user, details, openid_response) | ||
89 | 198 | |||
90 | 162 | return user | 199 | return user |
91 | 163 | 200 | ||
92 | 164 | def associate_openid(self, user, openid_response): | 201 | def associate_openid(self, user, openid_response): |
93 | @@ -181,7 +218,7 @@ | |||
94 | 181 | 218 | ||
95 | 182 | return user_openid | 219 | return user_openid |
96 | 183 | 220 | ||
98 | 184 | def update_user_details(self, user, details): | 221 | def update_user_details(self, user, details, openid_response): |
99 | 185 | updated = False | 222 | updated = False |
100 | 186 | if details['first_name']: | 223 | if details['first_name']: |
101 | 187 | user.first_name = details['first_name'] | 224 | user.first_name = details['first_name'] |
102 | @@ -192,6 +229,9 @@ | |||
103 | 192 | if details['email']: | 229 | if details['email']: |
104 | 193 | user.email = details['email'] | 230 | user.email = details['email'] |
105 | 194 | updated = True | 231 | updated = True |
106 | 232 | if getattr(settings, 'OPENID_FOLLOW_RENAMES', False): | ||
107 | 233 | user.username = self._get_available_username(details['nickname'], openid_response.identity_url) | ||
108 | 234 | updated = True | ||
109 | 195 | 235 | ||
110 | 196 | if updated: | 236 | if updated: |
111 | 197 | user.save() | 237 | user.save() |
112 | 198 | 238 | ||
113 | === modified file 'django_openid_auth/tests/test_views.py' | |||
114 | --- django_openid_auth/tests/test_views.py 2011-01-13 08:38:01 +0000 | |||
115 | +++ django_openid_auth/tests/test_views.py 2011-03-21 11:58:29 +0000 | |||
116 | @@ -135,12 +135,14 @@ | |||
117 | 135 | self.old_sso_server_url = getattr(settings, 'OPENID_SSO_SERVER_URL', None) | 135 | self.old_sso_server_url = getattr(settings, 'OPENID_SSO_SERVER_URL', None) |
118 | 136 | self.old_teams_map = getattr(settings, 'OPENID_LAUNCHPAD_TEAMS_MAPPING', {}) | 136 | self.old_teams_map = getattr(settings, 'OPENID_LAUNCHPAD_TEAMS_MAPPING', {}) |
119 | 137 | self.old_use_as_admin_login = getattr(settings, 'OPENID_USE_AS_ADMIN_LOGIN', False) | 137 | self.old_use_as_admin_login = getattr(settings, 'OPENID_USE_AS_ADMIN_LOGIN', False) |
120 | 138 | self.old_follow_renames = getattr(settings, 'OPENID_FOLLOW_RENAMES', False) | ||
121 | 138 | 139 | ||
122 | 139 | settings.OPENID_CREATE_USERS = False | 140 | settings.OPENID_CREATE_USERS = False |
123 | 140 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = False | 141 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = False |
124 | 141 | settings.OPENID_SSO_SERVER_URL = None | 142 | settings.OPENID_SSO_SERVER_URL = None |
125 | 142 | settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = {} | 143 | settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = {} |
126 | 143 | settings.OPENID_USE_AS_ADMIN_LOGIN = False | 144 | settings.OPENID_USE_AS_ADMIN_LOGIN = False |
127 | 145 | settings.OPENID_FOLLOW_RENAMES = False | ||
128 | 144 | 146 | ||
129 | 145 | def tearDown(self): | 147 | def tearDown(self): |
130 | 146 | settings.LOGIN_REDIRECT_URL = self.old_login_redirect_url | 148 | settings.LOGIN_REDIRECT_URL = self.old_login_redirect_url |
131 | @@ -149,6 +151,7 @@ | |||
132 | 149 | settings.OPENID_SSO_SERVER_URL = self.old_sso_server_url | 151 | settings.OPENID_SSO_SERVER_URL = self.old_sso_server_url |
133 | 150 | settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = self.old_teams_map | 152 | settings.OPENID_LAUNCHPAD_TEAMS_MAPPING = self.old_teams_map |
134 | 151 | settings.OPENID_USE_AS_ADMIN_LOGIN = self.old_use_as_admin_login | 153 | settings.OPENID_USE_AS_ADMIN_LOGIN = self.old_use_as_admin_login |
135 | 154 | settings.OPENID_FOLLOW_RENAMES = self.old_follow_renames | ||
136 | 152 | 155 | ||
137 | 153 | setDefaultFetcher(None) | 156 | setDefaultFetcher(None) |
138 | 154 | super(RelyingPartyTests, self).tearDown() | 157 | super(RelyingPartyTests, self).tearDown() |
139 | @@ -286,6 +289,213 @@ | |||
140 | 286 | self.assertEquals(user.last_name, 'User') | 289 | self.assertEquals(user.last_name, 'User') |
141 | 287 | self.assertEquals(user.email, 'foo@example.com') | 290 | self.assertEquals(user.email, 'foo@example.com') |
142 | 288 | 291 | ||
143 | 292 | def _do_user_login(self, openid_req, openid_resp): | ||
144 | 293 | # Posting in an identity URL begins the authentication request: | ||
145 | 294 | response = self.client.post('/openid/login/', openid_req) | ||
146 | 295 | self.assertContains(response, 'OpenID transaction in progress') | ||
147 | 296 | |||
148 | 297 | # Complete the request, passing back some simple registration | ||
149 | 298 | # data. The user is redirected to the next URL. | ||
150 | 299 | openid_request = self.provider.parseFormPost(response.content) | ||
151 | 300 | sreg_request = sreg.SRegRequest.fromOpenIDRequest(openid_request) | ||
152 | 301 | openid_response = openid_request.answer(True) | ||
153 | 302 | sreg_response = sreg.SRegResponse.extractResponse( | ||
154 | 303 | sreg_request, openid_resp) | ||
155 | 304 | openid_response.addExtension(sreg_response) | ||
156 | 305 | response = self.complete(openid_response) | ||
157 | 306 | self.assertRedirects(response, 'http://testserver/getuser/') | ||
158 | 307 | |||
159 | 308 | def test_login_without_nickname(self): | ||
160 | 309 | settings.OPENID_CREATE_USERS = True | ||
161 | 310 | |||
162 | 311 | openid_req = {'openid_identifier': 'http://example.com/identity', | ||
163 | 312 | 'next': '/getuser/'} | ||
164 | 313 | openid_resp = {'nickname': '', 'fullname': 'Openid User', | ||
165 | 314 | 'email': 'foo@example.com'} | ||
166 | 315 | self._do_user_login(openid_req, openid_resp) | ||
167 | 316 | response = self.client.get('/getuser/') | ||
168 | 317 | |||
169 | 318 | # username defaults to 'openiduser' | ||
170 | 319 | self.assertEquals(response.content, 'openiduser') | ||
171 | 320 | |||
172 | 321 | # The user's full name and email have been updated. | ||
173 | 322 | user = User.objects.get(username=response.content) | ||
174 | 323 | self.assertEquals(user.first_name, 'Openid') | ||
175 | 324 | self.assertEquals(user.last_name, 'User') | ||
176 | 325 | self.assertEquals(user.email, 'foo@example.com') | ||
177 | 326 | |||
178 | 327 | def test_login_follow_rename(self): | ||
179 | 328 | settings.OPENID_FOLLOW_RENAMES = True | ||
180 | 329 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True | ||
181 | 330 | user = User.objects.create_user('testuser', 'someone@example.com') | ||
182 | 331 | useropenid = UserOpenID( | ||
183 | 332 | user=user, | ||
184 | 333 | claimed_id='http://example.com/identity', | ||
185 | 334 | display_id='http://example.com/identity') | ||
186 | 335 | useropenid.save() | ||
187 | 336 | |||
188 | 337 | openid_req = {'openid_identifier': 'http://example.com/identity', | ||
189 | 338 | 'next': '/getuser/'} | ||
190 | 339 | openid_resp = {'nickname': 'someuser', 'fullname': 'Some User', | ||
191 | 340 | 'email': 'foo@example.com'} | ||
192 | 341 | self._do_user_login(openid_req, openid_resp) | ||
193 | 342 | response = self.client.get('/getuser/') | ||
194 | 343 | |||
195 | 344 | # If OPENID_FOLLOW_RENAMES, they are logged in as | ||
196 | 345 | # someuser (the passed in nickname has changed the username) | ||
197 | 346 | self.assertEquals(response.content, 'someuser') | ||
198 | 347 | |||
199 | 348 | # The user's full name and email have been updated. | ||
200 | 349 | user = User.objects.get(username=response.content) | ||
201 | 350 | self.assertEquals(user.first_name, 'Some') | ||
202 | 351 | self.assertEquals(user.last_name, 'User') | ||
203 | 352 | self.assertEquals(user.email, 'foo@example.com') | ||
204 | 353 | |||
205 | 354 | def test_login_follow_rename_conflict(self): | ||
206 | 355 | settings.OPENID_FOLLOW_RENAMES = True | ||
207 | 356 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True | ||
208 | 357 | # Setup existing user who's name we're going to switch to | ||
209 | 358 | user = User.objects.create_user('testuser', 'someone@example.com') | ||
210 | 359 | UserOpenID.objects.get_or_create( | ||
211 | 360 | user=user, | ||
212 | 361 | claimed_id='http://example.com/existing_identity', | ||
213 | 362 | display_id='http://example.com/existing_identity') | ||
214 | 363 | |||
215 | 364 | # Setup user who is going to try to change username to 'testuser' | ||
216 | 365 | renamed_user = User.objects.create_user('renameuser', 'someone@example.com') | ||
217 | 366 | UserOpenID.objects.get_or_create( | ||
218 | 367 | user=renamed_user, | ||
219 | 368 | claimed_id='http://example.com/identity', | ||
220 | 369 | display_id='http://example.com/identity') | ||
221 | 370 | |||
222 | 371 | # identity url is for 'renameuser' | ||
223 | 372 | openid_req = {'openid_identifier': 'http://example.com/identity', | ||
224 | 373 | 'next': '/getuser/'} | ||
225 | 374 | # but returned username is for 'testuser', which already exists for another identity | ||
226 | 375 | openid_resp = {'nickname': 'testuser', 'fullname': 'Rename User', | ||
227 | 376 | 'email': 'rename@example.com'} | ||
228 | 377 | self._do_user_login(openid_req, openid_resp) | ||
229 | 378 | response = self.client.get('/getuser/') | ||
230 | 379 | |||
231 | 380 | # If OPENID_FOLLOW_RENAMES, attempt to change username to 'testuser' | ||
232 | 381 | # but since that username is already taken by someone else, we go through | ||
233 | 382 | # the process of adding +i to it, and get testuser2. | ||
234 | 383 | self.assertEquals(response.content, 'testuser2') | ||
235 | 384 | |||
236 | 385 | # The user's full name and email have been updated. | ||
237 | 386 | user = User.objects.get(username=response.content) | ||
238 | 387 | self.assertEquals(user.first_name, 'Rename') | ||
239 | 388 | self.assertEquals(user.last_name, 'User') | ||
240 | 389 | self.assertEquals(user.email, 'rename@example.com') | ||
241 | 390 | |||
242 | 391 | def test_login_follow_rename_false_onlyonce(self): | ||
243 | 392 | settings.OPENID_FOLLOW_RENAMES = True | ||
244 | 393 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True | ||
245 | 394 | # Setup existing user who's name we're going to switch to | ||
246 | 395 | user = User.objects.create_user('testuser', 'someone@example.com') | ||
247 | 396 | UserOpenID.objects.get_or_create( | ||
248 | 397 | user=user, | ||
249 | 398 | claimed_id='http://example.com/existing_identity', | ||
250 | 399 | display_id='http://example.com/existing_identity') | ||
251 | 400 | |||
252 | 401 | # Setup user who is going to try to change username to 'testuser' | ||
253 | 402 | renamed_user = User.objects.create_user('testuser2000eight', 'someone@example.com') | ||
254 | 403 | UserOpenID.objects.get_or_create( | ||
255 | 404 | user=renamed_user, | ||
256 | 405 | claimed_id='http://example.com/identity', | ||
257 | 406 | display_id='http://example.com/identity') | ||
258 | 407 | |||
259 | 408 | # identity url is for 'testuser2000eight' | ||
260 | 409 | openid_req = {'openid_identifier': 'http://example.com/identity', | ||
261 | 410 | 'next': '/getuser/'} | ||
262 | 411 | # but returned username is for 'testuser', which already exists for another identity | ||
263 | 412 | openid_resp = {'nickname': 'testuser2', 'fullname': 'Rename User', | ||
264 | 413 | 'email': 'rename@example.com'} | ||
265 | 414 | self._do_user_login(openid_req, openid_resp) | ||
266 | 415 | response = self.client.get('/getuser/') | ||
267 | 416 | |||
268 | 417 | # If OPENID_FOLLOW_RENAMES, attempt to change username to 'testuser' | ||
269 | 418 | # but since that username is already taken by someone else, we go through | ||
270 | 419 | # the process of adding +i to it. Even though it looks like the username | ||
271 | 420 | # follows the nickname+i scheme, it has non-numbers in the suffix, so | ||
272 | 421 | # it's not an auto-generated one. The regular process of renaming to | ||
273 | 422 | # 'testuser' has a conflict, so we get +2 at the end. | ||
274 | 423 | self.assertEquals(response.content, 'testuser2') | ||
275 | 424 | |||
276 | 425 | # The user's full name and email have been updated. | ||
277 | 426 | user = User.objects.get(username=response.content) | ||
278 | 427 | self.assertEquals(user.first_name, 'Rename') | ||
279 | 428 | self.assertEquals(user.last_name, 'User') | ||
280 | 429 | self.assertEquals(user.email, 'rename@example.com') | ||
281 | 430 | |||
282 | 431 | def test_login_follow_rename_conflict_onlyonce(self): | ||
283 | 432 | settings.OPENID_FOLLOW_RENAMES = True | ||
284 | 433 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True | ||
285 | 434 | # Setup existing user who's name we're going to switch to | ||
286 | 435 | user = User.objects.create_user('testuser', 'someone@example.com') | ||
287 | 436 | UserOpenID.objects.get_or_create( | ||
288 | 437 | user=user, | ||
289 | 438 | claimed_id='http://example.com/existing_identity', | ||
290 | 439 | display_id='http://example.com/existing_identity') | ||
291 | 440 | |||
292 | 441 | # Setup user who is going to try to change username to 'testuser' | ||
293 | 442 | renamed_user = User.objects.create_user('testuser2000', 'someone@example.com') | ||
294 | 443 | UserOpenID.objects.get_or_create( | ||
295 | 444 | user=renamed_user, | ||
296 | 445 | claimed_id='http://example.com/identity', | ||
297 | 446 | display_id='http://example.com/identity') | ||
298 | 447 | |||
299 | 448 | # identity url is for 'testuser2000' | ||
300 | 449 | openid_req = {'openid_identifier': 'http://example.com/identity', | ||
301 | 450 | 'next': '/getuser/'} | ||
302 | 451 | # but returned username is for 'testuser', which already exists for another identity | ||
303 | 452 | openid_resp = {'nickname': 'testuser', 'fullname': 'Rename User', | ||
304 | 453 | 'email': 'rename@example.com'} | ||
305 | 454 | self._do_user_login(openid_req, openid_resp) | ||
306 | 455 | response = self.client.get('/getuser/') | ||
307 | 456 | |||
308 | 457 | # If OPENID_FOLLOW_RENAMES, attempt to change username to 'testuser' | ||
309 | 458 | # but since that username is already taken by someone else, we go through | ||
310 | 459 | # the process of adding +i to it. Since the user for this identity url | ||
311 | 460 | # already has a name matching that pattern, check if first. | ||
312 | 461 | self.assertEquals(response.content, 'testuser2000') | ||
313 | 462 | |||
314 | 463 | # The user's full name and email have been updated. | ||
315 | 464 | user = User.objects.get(username=response.content) | ||
316 | 465 | self.assertEquals(user.first_name, 'Rename') | ||
317 | 466 | self.assertEquals(user.last_name, 'User') | ||
318 | 467 | self.assertEquals(user.email, 'rename@example.com') | ||
319 | 468 | |||
320 | 469 | def test_login_follow_rename_false_conflict(self): | ||
321 | 470 | settings.OPENID_FOLLOW_RENAMES = True | ||
322 | 471 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True | ||
323 | 472 | # Setup existing user who's username matches the name+i pattern | ||
324 | 473 | user = User.objects.create_user('testuser2', 'someone@example.com') | ||
325 | 474 | UserOpenID.objects.get_or_create( | ||
326 | 475 | user=user, | ||
327 | 476 | claimed_id='http://example.com/identity', | ||
328 | 477 | display_id='http://example.com/identity') | ||
329 | 478 | |||
330 | 479 | # identity url is for 'testuser2' | ||
331 | 480 | openid_req = {'openid_identifier': 'http://example.com/identity', | ||
332 | 481 | 'next': '/getuser/'} | ||
333 | 482 | # but returned username is for 'testuser', which looks like we've done | ||
334 | 483 | # a username+1 for them already, but 'testuser' isn't actually taken | ||
335 | 484 | openid_resp = {'nickname': 'testuser', 'fullname': 'Same User', | ||
336 | 485 | 'email': 'same@example.com'} | ||
337 | 486 | self._do_user_login(openid_req, openid_resp) | ||
338 | 487 | response = self.client.get('/getuser/') | ||
339 | 488 | |||
340 | 489 | # If OPENID_FOLLOW_RENAMES, username should be changed to 'testuser' | ||
341 | 490 | # because it wasn't currently taken | ||
342 | 491 | self.assertEquals(response.content, 'testuser') | ||
343 | 492 | |||
344 | 493 | # The user's full name and email have been updated. | ||
345 | 494 | user = User.objects.get(username=response.content) | ||
346 | 495 | self.assertEquals(user.first_name, 'Same') | ||
347 | 496 | self.assertEquals(user.last_name, 'User') | ||
348 | 497 | self.assertEquals(user.email, 'same@example.com') | ||
349 | 498 | |||
350 | 289 | def test_login_update_details(self): | 499 | def test_login_update_details(self): |
351 | 290 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True | 500 | settings.OPENID_UPDATE_DETAILS_FROM_SREG = True |
352 | 291 | user = User.objects.create_user('testuser', 'someone@example.com') | 501 | user = User.objects.create_user('testuser', 'someone@example.com') |
353 | @@ -295,31 +505,17 @@ | |||
354 | 295 | display_id='http://example.com/identity') | 505 | display_id='http://example.com/identity') |
355 | 296 | useropenid.save() | 506 | useropenid.save() |
356 | 297 | 507 | ||
377 | 298 | # Posting in an identity URL begins the authentication request: | 508 | openid_req = {'openid_identifier': 'http://example.com/identity', |
378 | 299 | response = self.client.post('/openid/login/', | 509 | 'next': '/getuser/'} |
379 | 300 | {'openid_identifier': 'http://example.com/identity', | 510 | openid_resp = {'nickname': 'testuser', 'fullname': 'Some User', |
380 | 301 | 'next': '/getuser/'}) | 511 | 'email': 'foo@example.com'} |
381 | 302 | self.assertContains(response, 'OpenID transaction in progress') | 512 | self._do_user_login(openid_req, openid_resp) |
362 | 303 | |||
363 | 304 | # Complete the request, passing back some simple registration | ||
364 | 305 | # data. The user is redirected to the next URL. | ||
365 | 306 | openid_request = self.provider.parseFormPost(response.content) | ||
366 | 307 | sreg_request = sreg.SRegRequest.fromOpenIDRequest(openid_request) | ||
367 | 308 | openid_response = openid_request.answer(True) | ||
368 | 309 | sreg_response = sreg.SRegResponse.extractResponse( | ||
369 | 310 | sreg_request, {'nickname': 'someuser', 'fullname': 'Some User', | ||
370 | 311 | 'email': 'foo@example.com'}) | ||
371 | 312 | openid_response.addExtension(sreg_response) | ||
372 | 313 | response = self.complete(openid_response) | ||
373 | 314 | self.assertRedirects(response, 'http://testserver/getuser/') | ||
374 | 315 | |||
375 | 316 | # And they are now logged in as testuser (the passed in | ||
376 | 317 | # nickname has not caused the username to change). | ||
382 | 318 | response = self.client.get('/getuser/') | 513 | response = self.client.get('/getuser/') |
383 | 514 | |||
384 | 319 | self.assertEquals(response.content, 'testuser') | 515 | self.assertEquals(response.content, 'testuser') |
385 | 320 | 516 | ||
386 | 321 | # The user's full name and email have been updated. | 517 | # The user's full name and email have been updated. |
388 | 322 | user = User.objects.get(username='testuser') | 518 | user = User.objects.get(username=response.content) |
389 | 323 | self.assertEquals(user.first_name, 'Some') | 519 | self.assertEquals(user.first_name, 'Some') |
390 | 324 | self.assertEquals(user.last_name, 'User') | 520 | self.assertEquals(user.last_name, 'User') |
391 | 325 | self.assertEquals(user.email, 'foo@example.com') | 521 | self.assertEquals(user.email, 'foo@example.com') |
Can somebody please review this, it is needed for loco.ubuntu.com and summit.ubuntu.com