search

mozilla / django-browserid

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

Remove {% browserid_info %} #198

Closed Osmose closed 10 years ago

Osmose commented 11 years ago

{% browserid_info %} serves two purposes:

  1. Provide a <form> for submitting assertions, which we want to get rid of.
  2. Store values to use for customizing the Persona popup via the REQUEST_ARGS setting.

Given that 1 is going away, we should figure out a better way to solve 2 so we can get rid of the requirement of having {% browserid_info %} on any page that wants to have a login button.

One idea is to use a cookie that the JavaScript can read from.

peterbe commented 11 years ago

Agreed that point 1 can and should go away.

Regarding point 2, an important point of #browserid-info is currently .data('userEmail'), which I just found out, is optional!

Suppose you have an overridden verifier that does an additional LDAP check on your login, the loggedInUser will be null but persona will start the infamous infinite loop of invoking onlogin. However, if you just have a failure URL that contains bid_login_failed=1 that loop is easy to avoid.

All other artifacts that #browserid-info provides such as requestArgs can all be put onto the a.browserid-login button. After all, you can already do this:

<a href="#" data-next="/welcomein/" class="browserid-login dark">Sign in</a>

So, that could be upgraded to 

<a href="#" data-requestArgs="{\"siteName\": \"Site\"}" data-next="/welcomein/" class="browserid-login dark">Sign in</a>
Osmose commented 11 years ago

All other artifacts that #browserid-info provides such as requestArgs can all be put onto the a.browserid-login button.

It'd be nice if we could support specifying the request args for Persona popups without requiring you to use browserid_button. I wouldn't call it a blocker, but it's a nice little feature of using the django_browserid.login method instead of calling navigator.id.request directly.

Regarding point 2, an important point of #browserid-info is currently .data('userEmail'), which I just found out, is optional!

Optional in that, if you ignore it, you then have to catch the subsequent onlogin callback and discard it, which sucks. Of course the news that the new Persona API is agreed-upon brings hope that soon we won't even need to pass it or deal with unwanted auto-logins anyway.