mozilla / django-browserid

Django application for adding BrowserID support.
Mozilla Public License 2.0
179 stars 80 forks source link

De-duplicate login/logout redirect behaviour #297

Closed alanbriolat closed 8 years ago

alanbriolat commented 8 years ago

As raised in #296, browserid_login and browserid_logout buttons set a default next which prevents the documented behaviour of Verify.success_url and Logout.redirect_url ever taking effect. The default values duplicate the default behaviour of those properties, causing confusion when attempts to customise the redirect behaviour of these views has no effect.

This change instead uses a blank next by default, letting the Verify.success_url and Logout.redirect_url behaviour be used as appropriate when no next has been explicitly supplied at the point of creating the button. The default behaviour of the library should be unaffected, but it could affect existing custom JS that mistakenly uses the data-next attribute of the button instead of the returned redirect from the API.

alanbriolat commented 8 years ago

I can't reproduce that test failure for py34_dj19 (running tox locally), and it looks like a spurious test failure due to assumptions about Python dictionary ordering...

Osmose commented 8 years ago

Thanks for the patch! This looks good to me, and the tests pass locally for me. r+!

I added a fix to master for the dict ordering issue you found: https://github.com/mozilla/django-browserid/commit/3c17d80cecff3bfbb03eae44dc83ceb693adcb04

Then I rebased your PR and merged it: https://github.com/mozilla/django-browserid/commit/40ec93a316e9bbb8d782d538a5bf3b87962b5a31

And then added you to the AUTHORS file (let me know if you don't wish to be listed and I'll remove you): https://github.com/mozilla/django-browserid/commit/6e75dbc2a5fab66d01e83fa2aaa44bfa85ada9a3