mozilla / django-browserid

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

Authenticating using a different email than User.email causes infinite redirect loop #210

Closed seocam closed 10 years ago

seocam commented 10 years ago

In my use case users are allowed to add many email addresses to their account and they should be able to authenticate using all of them.

By extending BrowserIDBackend and overriding filter_users_by_email, django-browserid allows the user to authenticate with a different email address than just the one set in his user account User.email.

To do so I've implemented the following Backend:

from django_browserid.auth import BrowserIDBackend

class ColabBrowserIDBackend(BrowserIDBackend):
    def filter_users_by_email(self, email):
        return self.User.objects.filter(emails__address=email)

User.emails is defined by a foreign relation were each model instance has an address defined in `email.address.

The logging process works fine with all emails but if I logging using an email address different than user.email it causes the web site to start an infinite redirect to LOGIN_REDIRECT_URL. It doesn't log me out, just constantly send me back to LOGIN_REDIRECT_URL.

I suppose user.email is hard coded somewhere inside django-browserid.

seocam commented 10 years ago

CC @arloc

Osmose commented 10 years ago

I suppose user.email is hard coded somewhere inside django-browserid.

Yep: https://github.com/mozilla/django-browserid/blob/master/django_browserid/helpers.py#L46

What happens is that email is passed to navigator.id.watch on the client-side, which compares it to the email Persona thinks you're logged in as. So, if you logged in as a@example.com but your User.email value is b@example.com, Persona will be told you are b@example.com and say "Hey, this person is actually a@example.com, let's call the logout/login methods to get them logged in as the right user!".

The good news is that Persona is releasing a variation of the JS API that doesn't do any of this auto-login magic in about a week or so (see https://github.com/mozilla/browserid/pull/3961 for details) and we're going to switch to it immediately (this is not the first time we've been burned by the auto-login). That should solve your issue. :D

PS: That's a really cool use of the filter_users_by_email function. I might add it as an example when I get around to redoing the docs with examples.

seocam commented 10 years ago

I've found that in helpers and implemented a simple fix.

First I set the email used to login into the user's session:

session = getattr(request, 'session', None)
if session:
    session['browserid_email'] = email

And after I've used that instead of user.email:

email = request.session.get('browserid_email', '')

I'm already writing/fixing the tests affected to pull request. Is that an acceptable fix?

seocam commented 10 years ago

The first block of code is done just after calling verify inside authenticate.

Osmose commented 10 years ago

It would be a great fix if that code wasn't going to go away completely in a week. Not only does the new API not do auto-login, it doesn't need the email address at all, so fetching the email will be removed from the helper completely.

In the interim, you could just run your site off your fork with your fix. Given the big changes in #208 that are hopefully going to be merged before we switch anyway, that might be a good idea so you can keep things working until you can port over to the new setup instructions (I'd be happy to put together a list of changes once #208 lands if that'd help you, I'm going to have to anyway for the documentation before the next release).

seocam commented 10 years ago

Roger that.

I might not be able to update this code any time soon. I'll working as a temporary contractor and after my contract is over I won't have any access to the code or servers. So, probably the best solution here is to fork and patch 0.9 and make my requirements.txt pointing there.

[off topic] BTW this project is probably a cool case for django-browserid and persona. The portal it's a tool sponsored by the UN to encourage collaborations among employees of city councils all over Brazil.

As soon as we launch I'm gonna let people know about it on identity mailing list. [/off topic]

seocam commented 10 years ago

@Osmose the #208 merge seems to work fine with multiple emails.

I've just deployed it here: https://colab.interlegis.leg.br/

Osmose commented 10 years ago

Interesting! That seems to indicate we're not quite handling auto-login correctly anymore.

Well, it's going away anyway, as I think the new API is out and it's time to switch over. Glad things are working for you! :D