550727 - (bz-jsonp) Add JSONP Support to the JSON-RPC WebService Interface

Status: RESOLVED FIXED

(Bugzilla :: WebService, enhancement, P1)

Description

[Max Kanat-Alexander]

JSONP is a method of doing cross-domain requests involving JSON. The original proposal is described here:

  http://bob.pythonmac.org/archives/2005/12/05/remote-json-jsonp/

  That is exactly (more or less) what we will be implementing. We will not be implementing any JSONP extensions, like JSONPP or something like that.

  There are significant security concerns associated with JSONP. Here are the ones I'm aware of and how they will be dealt with:

  CSRF: When using JSONP, you cannot use any JSON-RPC method that will modify data. You may only use methods that retrieve data.

  Information Leak (security bug exposure): Because JSONP involves using script tags, cookies will be sent to the remote site, meaning that a site could silently retrieve secure data without the user's knowledge, and then send it to itself, thus effectively stealing private data. The solution to this is to not allow cookie-based login when using JSONP. We will only allow explicit login using method parameters. So a remote site will only be able to access private information if a user explicitly types in their password at that site.

  Code Insertion: If you let people specify any arbitrary thing for the "padding", they could insert all sorts of code that runs in the context of your domain, which is bad. The solution is that we only allow function names that match \w+.

  Are there any other security issues that I should be aware of, or I have I covered them all?

Comment 1

[Max Kanat-Alexander]

  Oh, and to be clear, the form of JSONP that we will be implementing is the sort that wraps the returned data in a callback function, because I think that's the most flexible. Theoretically, our data could also be assigned to a variable (like var callback = { };) but the function seems more flexible and less-likely to pollute somebody's namespace with endless variables.

Comment 2

[Max Kanat-Alexander]

Attachment: v1

Here we go! After implementing the GET stuff, the remaining stuff for implementing JSONP was pretty simple.

  Note that this requires the patch from the blocker.

Comment 3

[Gervase Markham [:gerv]]

One comment: when I looked into it for the BzAPI, a common name for the URL parameter giving the name of the JavaScript function to be called is "callback" (i.e. "callback=myfunction") - although there didn't seem to be a standard. So that's what I used. It would be good to have the name the same in both APIs. If there's a standard, point me at it. Otherwise, we should agree something :-)

Gerv

Comment 4

[Max Kanat-Alexander]

  "callback" seems reasonable. That does seem to be what most things are using. I'll re-post the patch with "callback" as the argument.

Comment 5

[Max Kanat-Alexander]

Attachment: v2

Okay, this version uses "callback" as the URL argument.

Comment 6

[Max Kanat-Alexander]

Attachment: v3

And you know, we might as well allow brackets in the callback, too. The actual example in the original JSONP spec uses brackets.

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 431626 [details] [diff] [review]
v3

With the patch from 550732 applied and this one together I get the following warning from JSONRPC.pm:

Subroutine response redefined at Bugzilla/WebService/Server/JSONRPC.pm line 155.
Bugzilla/WebService/Server/JSONRPC.pm syntax OK

response() is present in two places and needs to be combined into one.

I combined them in my code like below and my testing worked well. Fix this and then r=dkl.

sub response {
    my ($self, $response) = @_;
    my $headers = $response->headers;
    my @header_args;
    foreach my $name ($headers->header_field_names) {
        my @values = $headers->header($name);
        $name =~ s/-/_/g;
        foreach my $value (@values) {
            push(@header_args, "-$name", $value);
        }
    }

    if (my $callback = $self->_bz_callback) {
        my $content = $response->content;
        $response->content("$callback($content)");

    }

    my $cgi = $self->cgi;
    print $cgi->header(-status => $response->code, @header_args);
    print $response->content;
}

Comment 8

[Max Kanat-Alexander]

Attachment: v4

Ah, thanks for catching that! Here's the patch with it fixed.

Comment 9

[David Lawrence [:dkl]]

Comment on attachment 440838 [details] [diff] [review]
v4

Looks good and works as expected. r=dkl

Comment 10

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/WebService/Server/JSONRPC.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7145.

Comment 11

[Max Kanat-Alexander]

Added to the release notes in bug 604256.