mozilla / django-browserid

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

Submit assertions to server via AJAX instead of a `<form>` #197

Closed Osmose closed 10 years ago

Osmose commented 10 years ago

Currently, we submit the assertion given by Persona through a <form> that is embedded on the page via the {% browserid_info %} tag. This makes it difficult for users to do things like authenticate a user without refreshing the page, and also makes it difficult to remove the {% browserid_info %} tag.

Instead, our JavaScript should submit the assertion via AJAX, and the Verify view should return a blob of JSON with instructions on what to do next.

(I'm not entirely convinced JSON is the right response, as opposed to using HTTP codes to signify a redirect or some other action, and input on this would be appreciated.)

Osmose commented 10 years ago

I've played around a bit with this in a branch, and a few questions/ideas have come up:

How do we get the CSRF token to the JavaScript?

What should the Verify view return now that it's being called by AJAX?

Off the top of my head:

{
  "user": {
    "email": "a@example.com",
    "username": "cooldude02"
  },
  "redirect": "/post-login/page/"
}

user would be a JSON representation of the user, or null if verification failed. redirect would be a suggestion; the JavaScript could choose to not redirect if it doesn't make sense for the site. We could add an extra method to Verify for generating this object based on the logged-in user that could be overridden.

What does the JS API look like now?

I'd like to be able to support the default behavior of redirecting, as well as support passing a callback and handling verification manually. Here's some stuff I wrote down looking at what we have and what I think we'll need:

/* Login ***/
// Redirect to settings.LOGIN_REDIRECT_URL on success, settings.LOGIN_REDIRECT_URL_FAILURE on failure.
django_browserid.login({siteName: 'My Site'});

// Redirect to /next/url on success, settings.LOGIN_REDIRECT_URL_FAILURE on failure.
django_browserid.login('/next/url', {siteName: 'My Site'});

// Redirect to /next/url on success, /failure/url on failure.
django_browserid.login('/next/url', '/failure/url', {siteName: 'My Site'});

// Execute the callback when Verify returns.
django_browserid.login(function(result) {
    // result is the JSON returned by Verify.
}, {siteName: 'My Site'});

/* Logout ***/
// Redirect to settings.LOGOUT_REDIRECT_URL on success, settings.LOGOUT_REDIRECT_URL_FAILURE on failure.
django_browserid.logout();

// Redirect to /next/url on success, settings.LOGOUT_REDIRECT_URL_FAILURE.
django_browserid.logout('/next/url');

// Redirect to /next/url on success, /failure/url on failure.
django_browserid.logout('/next/url', '/failure/url');

// Execute the callback after logout returns.
django_browserid.logout(function(result) {
    // result is {error: null, redirect: '/some/url'}
});

/* getAssertion ***/
// Execute the callback after retrieving the assertion from Persona.
django_browserid.getAssertion(function(assertion) {
    // assertion is either a string or false/null/whatever Persona gives us.
}, {siteName: 'My Site'});  

/* verifyAssertion ***/
// Execute the callback once the assertion has been verified. Unlike login, this does not 
// authenticate the user.
django_browserid.verifyAssertion('myassertion', function(result) {
    // verified is {verified: true, email: 'a@example.com'}
});

Input on any or all of this would be greatly appreciated. :D

[1]: Including the token in a cookie and using that token for the CSRF check is a bad idea if you're on a subdomain, as another site on that domain could set a cookie that replaces the CSRF token, and then submit a request with that token. However, if you're just including the token in the cookie, but it's stored somewhere else (like the cache), this isn't an issue.

[2]: Because the method that we use to send the CSRF token will also end up being the method we send the Persona customization parameters for #198, we'd end up loading it right before we call navigator.id.request.

peterbe commented 10 years ago

Pardon me if I haven't thought enough about this but I think cookies are best avoided at all costs. I'm pro us doing a GET right before we do the POST but only if we know we're going to make POST some time soon.

Server-side and client-side cookies don't mix. Especially since the security people say so. And I can see this; a broken site with xss vulnerabilities could make it possible to leak out a sessionId. There's also the concerns there about sub-domains.

One common problem with generating the token upon page rendering and using it many seconds/minutes later in the POST means that if the cache that was used to generate the token is expired, people will get the dreaded 403 error. This happens a lot when you use LocMemcache as a backend but can easily happen if a server upgrade happens which comes with a memcache reset. (Apparently SUMO resets the memcache prefix on every deployment and they use continuous deployment)

I think I would much more favor something like this:

    ...
    var loginCallback = null; // Callback to run post-login.
    var token = null;

    // Public API
    window.django_browserid = {
        ...
        login: function login(next, requestArgs) {
            var defaults = $('#browserid-info').data('requestArgs');
            requestArgs = $.extend({}, defaults, requestArgs);

            loginRedirect = next;
            var preflight = $.getJSON($browseridInfo.data('token-url'));
            preflight.done(function(new_token) {
                 token = new_token;
                 navigator.id.request(requestArgs);
            }).fail(function() {
                // if we can't do a GET we won't be able to do a POST in a couple of seconds
                // need to deal with it somehow!
            });            
        },
     ...
Osmose commented 10 years ago

@st3fan mentioned on dev-webdev that it would be useful from the standpoint of security trying to automatically test logged-in pages on some sites to have the URLs for django-browserid be consistent across sites, which makes a stronger case for going with the preflight AJAX instead of cookies.

I'm more or less sold on that point, unless someone screams at me otherwise. :D