django-openid-auth

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

strict-username-requirements
Merge into trunk

Proposed by Michael Hall on 2011-03-18

Status:

Merged

Approved by:

James Henstridge on 2011-04-12

Approved revision:

Merged at revision:

Proposed branch:

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

Merge into:

lp://staging/~django-openid-auth/django-openid-auth/trunk

Diff against target:

147 lines (+82/-2)

3 files modified

README.txt (+10/-0)
django_openid_auth/auth.py (+13/-2)
django_openid_auth/tests/test_views.py (+59/-0)

To merge this branch:

bzr merge lp://staging/~mhall119/django-openid-auth/strict-username-requirements

Undecided

Fix Released

Link a bug report

Reviewer	Date Requested	Status
James Henstridge	2011-03-18	Approve on 2011-04-12
Anthony Lenton	2011-03-18	Approve on 2011-03-23
Review via email: mp+54065@code.staging.launchpad.net

This proposal supersedes a proposal from 2010-10-15.

Description of the change

Adds an extra OPENID_STRICT_USERNAMES setting that will cause the authentication to fail if the OpenID response does not contain a nickname (used for User.username in Django), or if a Django User with that username already exists but doesn't have the same OpenID identity_url

Revision history for this message

Anthony Lenton (elachuni) wrote on 2010-12-06: Posted in a previous version of this proposal

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

review: Approve

Revision history for this message

James Henstridge (jamesh) wrote on 2010-12-22: Posted in a previous version of this proposal

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

Revision history for this message

Anthony Lenton (elachuni) wrote on 2011-03-23:

Hi Michael,

With the tests this now looks fine. Could you add a brief snippet to README.txt to mention this new setting and what it does?

lp://staging/~mhall119/django-openid-auth/strict-username-requirements updated on 2011-03-23

81. By Michael Hall on 2011-03-23: Added notes about the new settings option to README.txt

Revision history for this message

Anthony Lenton (elachuni) on 2011-03-23:

review: Approve

Revision history for this message

James Henstridge (jamesh) wrote on 2011-04-12:

Looks good. In test_strict_username_no_nickname, I'd leave out the nickname from the SReg response entirely though, since that is what you'd see from a provider that doesn't support nicknames.

review: Approve