mozilla / django-browserid

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

Removal of /info/ endpoint causes problems for sites not using Django templating #285

Closed edmorley closed 9 years ago

edmorley commented 9 years ago

The Treeherder service (https://github.com/mozilla/treeherder) uses Django, however we do not make use of Django templating for the front-end - and instead have a static AngularJS based UI (https://github.com/mozilla/treeherder-ui).

In https://bugzilla.mozilla.org/show_bug.cgi?id=1118023 I was trying to update from django-browserid e8d1d57145 to v0.11.1: https://github.com/mozilla/django-browserid/compare/e8d1d57145401cf3f42993fe84edc32e29e5903f...v0.11.1

However one of the changes is the switch back to using the browserid_info tag and the removal of the /info/ endpoint in: https://github.com/mozilla/django-browserid/commit/a021621b9adabac4d722880cce6e1f90b2908a8e#diff-57a33c8268383e09bc76e6970e423ca3L124

Since we're not using Django templates, we don't have access to this tag, and so were wondering whether you'd be receptive to adding back /info/ or equivalent?

Our current UI implementation is at: https://github.com/mozilla/treeherder-ui/blob/5b87e9693f43b50b3069d3b06d0f14fc47a055b5/webapp/app/js/services/main.js#L91 https://github.com/mozilla/treeherder-ui/blob/2f7f9d92f5bcbdaf62d232447f787adbbbfa169e/webapp/app/js/directives/persona.js

Thanks! :-)

CC @mozilla/treeherder

Osmose commented 9 years ago

So to avoid getting the Persona popup blocked by popup blocking while still fetching the URLs and arguments that /info/ provided, you have to either:

  1. Embed the data into the HTML (the path we chose).
  2. Fetch the data before the user clicks a login button via AJAX.

The second one is problematic because, depending on how long it takes to fetch the data, a user might click the login button and be blocked on the AJAX request finishing. I'm pretty sure (but not 100% sure) that waiting for that request will cause you to lose the "user-triggered action" bit and cause the popup to be blocked anyway, so I don't think adding the /info/ view back to django-browserid will help.

The goal is really just to tell our library's JS a few URLs and some arguments for the call to navigator.id.request. Given that you control the backend URLs yourself (whereas django-browserid the library doesn't), it's probably fine to just have treeherder-ui manually add a <div id="browserid-info"> tag to its pages with the right URLs and arguments.

(And if you're worried about those URLs changing in a future version of django-browserid, don't be. Sometime within the next month or two I'm going to enable Django 1.8 testing, ensure that the library works on Django 1.8 with no warnings, and then roll a 1.0 release and call the API frozen.)

Would that be a decent solution?

edmorley commented 9 years ago

I thought /info/ contained more than just URLs, since https://treeherder.mozilla.org/browserid/info/ returns: ..., "userEmail": "emorley@<snip>.com", ... ...when I am logged in, so it seemed like that was the only way to figure out the user's email (I'm really not familiar with browserid, or django-browserid in the slightest).

If that's not the case, then great :-)

maurodoglio commented 9 years ago

@edmorley we can get some user info from https://treeherder.mozilla.org/api/user/ so manually adding the browserid bits in a tag seems to cover our case :smiley: thanks @Osmose !

Osmose commented 9 years ago

I thought /info/ contained more than just URLs

It used to, but doesn't anymore. We used to need the user's email to tell Persona what state the user was in for some weird auto-login behavior (like if a user was logged in to your site but not into Persona they'd auto-trigger a logout and some other weird crap), but Persona added a stateless mode that we use which is much simpler.

Nowadays it just has three URLs and some kwargs for navigator.id.request. Check out https://github.com/mozilla/django-browserid/blob/master/django_browserid/helpers.py#L47 for details.

Glad this seems to work for you, feel free to reopen if you run into more issues. :D

edmorley commented 9 years ago

It used to, but doesn't anymore. We used to need the user's email to tell Persona what state the user was in for some weird auto-login behavior (like if a user was logged in to your site but not into Persona they'd auto-trigger a logout and some other weird crap), but Persona added a stateless mode that we use which is much simpler.

Ah, that makes sense, thank you :-)