Bug 550732: Allow read-only JSON-RPC methods to be called with GET

r=dkl, a=mkanat

12 files changed, 219 insertions, 29 deletions

diff --git a/Bugzilla/Auth/Login.pm b/Bugzilla/Auth/Login.pm
index 4a4c5f26d..b3add7365 100644
--- a/Bugzilla/Auth/Login.pm
+++ b/Bugzilla/Auth/Login.pm
@@ -27,6 +27,7 @@ use constant can_login  => 1;
 use constant requires_persistence  => 1;
 use constant requires_verification => 1;
 use constant user_can_create_account => 0;
+use constant is_automatic => 0;
 
 sub new {
     my ($class) = @_;
@@ -122,4 +123,12 @@ got from this login method. Defaults to C<true>.
 Whether or not users can create accounts, if this login method is
 currently being used by the system. Defaults to C<false>.
 
+=item C<is_automatic>
+
+True if this login method requires no interaction from the user within
+Bugzilla. (For example, C<Env> auth is "automatic" because the webserver
+just passes us an environment variable on most page requests, and does not
+ask the user for authentication information directly in Bugzilla.) Defaults
+to C<false>.
+
 =back
diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm
index 570988f7e..91fb820fb 100644
--- a/Bugzilla/Auth/Login/Cookie.pm
+++ b/Bugzilla/Auth/Login/Cookie.pm
@@ -27,6 +27,7 @@ use List::Util qw(first);
 use constant requires_persistence  => 0;
 use constant requires_verification => 0;
 use constant can_login => 0;
+use constant is_automatic => 1;
 
 # Note that Cookie never consults the Verifier, it always assumes
 # it has a valid DB account or it fails.
diff --git a/Bugzilla/Auth/Login/Env.pm b/Bugzilla/Auth/Login/Env.pm
index 1f2739a88..f93034ef3 100644
--- a/Bugzilla/Auth/Login/Env.pm
+++ b/Bugzilla/Auth/Login/Env.pm
@@ -31,6 +31,7 @@ use constant can_logout => 0;
 use constant can_login  => 0;
 use constant requires_persistence  => 0;
 use constant requires_verification => 0;
+use constant is_automatic => 1;
 
 sub get_login_info {
     my ($self) = @_;
diff --git a/Bugzilla/Auth/Login/Stack.pm b/Bugzilla/Auth/Login/Stack.pm
index bef9171c9..f490d243b 100644
--- a/Bugzilla/Auth/Login/Stack.pm
+++ b/Bugzilla/Auth/Login/Stack.pm
@@ -52,6 +52,11 @@ sub get_login_info {
     my $self = shift;
     my $result;
     foreach my $object (@{$self->{_stack}}) {
+        # See Bugzilla::WebService::Server::JSONRPC for where and why
+        # auth_no_automatic_login is used.
+        if (Bugzilla->request_cache->{auth_no_automatic_login}) {
+            next if $object->is_automatic;
+        }
         $result = $object->get_login_info(@_);
         $self->{successful} = $object;
         last if !$result->{failure};
diff --git a/Bugzilla/WebService.pm b/Bugzilla/WebService.pm
index 2ba8e925e..fe7766ad1 100644
--- a/Bugzilla/WebService.pm
+++ b/Bugzilla/WebService.pm
@@ -29,6 +29,10 @@ use constant DATE_FIELDS => {};
 # For some methods, we shouldn't call Bugzilla->login before we call them
 use constant LOGIN_EXEMPT => { };
 
+# Used to allow methods to be called in the JSON-RPC WebService via GET.
+# Methods that can modify data MUST not be listed here.
+use constant READ_ONLY => ();
+
 sub login_exempt {
     my ($class, $method) = @_;
     return $class->LOGIN_EXEMPT->{$method};
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 240eee91e..58a39bf21 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -48,6 +48,16 @@ use constant DATE_FIELDS => {
     search   => ['last_change_time', 'creation_time'],
 };
 
+use constant READ_ONLY => qw(
+    attachments
+    comments
+    fields
+    get
+    history
+    legal_values
+    search
+);
+
 ######################################################
 # Add aliases here for old method name compatibility #
 ######################################################
diff --git a/Bugzilla/WebService/Bugzilla.pm b/Bugzilla/WebService/Bugzilla.pm
index c14cc7dea..2501fe60c 100644
--- a/Bugzilla/WebService/Bugzilla.pm
+++ b/Bugzilla/WebService/Bugzilla.pm
@@ -31,6 +31,13 @@ use constant LOGIN_EXEMPT => {
     version => 1,
 };
 
+use constant READ_ONLY => qw(
+    extensions
+    timezone
+    time
+    version
+);
+
 sub version {
     my $self = shift;
     return { version => $self->type('string', BUGZILLA_VERSION) };
diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 19d230759..0ada98ba4 100644
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -120,10 +120,11 @@ use constant WS_ERROR_CODE => {
     user_access_by_id_denied    => 505,
     user_access_by_match_denied => 505,
 
-    # RPC Server Errors. See the following URL:
-    # http://xmlrpc-epi.sourceforge.net/specs/rfc.fault_codes.php
-    xmlrpc_invalid_value        => -32600,
-    unknown_method              => -32601,
+    # Errors thrown by the WebService itself. The ones that are negative 
+    # conform to http://xmlrpc-epi.sourceforge.net/specs/rfc.fault_codes.php
+    xmlrpc_invalid_value => -32600,
+    unknown_method       => -32601,
+    json_rpc_post_only   => 32610,
 };
 
 # These are the fallback defaults for errors not in ERROR_CODE.
diff --git a/Bugzilla/WebService/Product.pm b/Bugzilla/WebService/Product.pm
index eaec012a4..90c81dcc0 100644
--- a/Bugzilla/WebService/Product.pm
+++ b/Bugzilla/WebService/Product.pm
@@ -23,6 +23,13 @@ use Bugzilla::Product;
 use Bugzilla::User;
 use Bugzilla::WebService::Util qw(validate);
 
+use constant READ_ONLY => qw(
+    get
+    get_accessible_products
+    get_enterable_products
+    get_selectable_products
+);
+
 ##################################################
 # Add aliases here for method name compatibility #
 ##################################################
diff --git a/Bugzilla/WebService/Server/JSONRPC.pm b/Bugzilla/WebService/Server/JSONRPC.pm
index 3ec64c6bc..ae479ff5e 100644
--- a/Bugzilla/WebService/Server/JSONRPC.pm
+++ b/Bugzilla/WebService/Server/JSONRPC.pm
@@ -28,6 +28,12 @@ use Bugzilla::Error;
 use Bugzilla::WebService::Constants;
 use Bugzilla::WebService::Util qw(taint_data);
 
+use Bugzilla::Util qw(correct_urlbase);
+
+#####################################
+# Public JSON::RPC Method Overrides #
+#####################################
+
 sub new {
     my $class = shift;
     my $self = $class->SUPER::new(@_);
@@ -71,6 +77,81 @@ sub response {
     print $response->content;
 }
 
+# The JSON-RPC 1.1 GET specification is not so great--you can't specify
+# data structures as parameters. However, the JSON-RPC 2.0 "JSON-RPC over
+# HTTP" spec is excellent, so we are using that for GET requests, instead.
+# Spec: http://groups.google.com/group/json-rpc/web/json-rpc-over-http
+#
+# The one exception is that we don't require the "params" argument to be
+# Base64 encoded, because that is ridiculous and obnoxious for JavaScript
+# clients.
+sub retrieve_json_from_get {
+    my $self = shift;
+    my $cgi = $self->cgi;
+
+    my %input;
+
+    # Both version and id must be set before any errors are thrown.
+    if ($cgi->param('version')) {
+        $self->version(scalar $cgi->param('version'));
+        $input{version} = $cgi->param('version');
+    }
+    else {
+        $self->version('1.0');
+    }
+
+    # The JSON-RPC 2.0 spec says that any request that omits an id doesn't
+    # want a response. However, in an HTTP GET situation, it's stupid to
+    # expect all clients to specify some id parameter just to get a response,
+    # so we don't require it.
+    my $id;
+    if (defined $cgi->param('id')) {
+        $id = $cgi->param('id');
+    }
+    # However, JSON::RPC does require that an id exist in most cases, in
+    # order to throw proper errors. We use the installation's urlbase as
+    # the id, in this case.
+    else {
+        $id = correct_urlbase();
+    }
+    # Setting _bz_request_id here is required in case we throw errors early,
+    # before _handle.
+    $self->{_bz_request_id} = $input{id} = $id;
+
+    if (!$cgi->param('method')) {
+        ThrowUserError('json_rpc_get_method_required');
+    }
+    $input{method} = $cgi->param('method');
+
+    my $params;
+    if (defined $cgi->param('params')) {
+        local $@;
+        $params = eval { 
+            $self->json->decode(scalar $cgi->param('params')) 
+        };
+        if ($@) {
+            ThrowUserError('json_rpc_invalid_params',
+                           { params => scalar $cgi->param('params'),
+                             err_msg  => $@ });
+        }
+    }
+    elsif (!$self->version or $self->version ne '1.1') {
+        $params = [];
+    }
+    else {
+        $params = {};
+    }
+
+    $input{params} = $params;
+
+    my $json = $self->json->encode(\%input);
+    return $json;
+}
+
+#######################################
+# Bugzilla::WebService Implementation #
+#######################################
+
 sub type {
     my ($self, $type, $value) = @_;
     
@@ -108,6 +189,35 @@ sub datetime_format_outbound {
     return $self->SUPER::datetime_format_outbound(@_) . 'Z';
 }
 
+sub handle_login {
+    my $self = shift;
+
+    # If we're being called using GET, we don't allow cookie-based or Env
+    # login, because GET requests can be done cross-domain, and we don't
+    # want private data showing up on another site unless the user
+    # explicitly gives that site their username and password. (This is
+    # particularly important for JSONP, which would allow a remote site
+    # to use private data without the user's knowledge, unless we had this
+    # protection in place.)
+    if ($self->request->method ne 'POST') {
+        # XXX There's no particularly good way for us to get a parameter
+        # to Bugzilla->login at this point, so we pass this information
+        # around using request_cache, which is a bit of a hack. The
+        # implementation of it is in Bugzilla::Auth::Login::Stack.
+        Bugzilla->request_cache->{auth_no_automatic_login} = 1;
+    }
+
+    my $path = $self->path_info;
+    my $class = $self->{dispatch_path}->{$path};
+    my $full_method = $self->_bz_method_name;
+    $full_method =~ /^\S+\.(\S+)/;
+    my $method = $1;
+    $self->SUPER::handle_login($class, $method, $full_method);
+}
+
+######################################
+# Private JSON::RPC Method Overrides #
+######################################
 
 # Store the ID of the current call, because Bugzilla::Error will need it.
 sub _handle {
@@ -154,21 +264,11 @@ sub _error {
     return $json;
 }
 
-##################
-# Login Handling #
-##################
-
 # This handles dispatching our calls to the appropriate class based on
 # the name of the method.
 sub _find_procedure {
     my $self = shift;
 
-    # This is also a good place to deny GET requests, since we can
-    # safely call ThrowUserError at this point.
-    if ($self->request->method ne 'POST') {
-        ThrowUserError('json_rpc_post_only');
-    }
-
     my $method = shift;
     $self->{_bz_method_name} = $method;
 
@@ -217,6 +317,16 @@ sub _argument_type_check {
 
     Bugzilla->input_params($params);
 
+    if ($self->request->method ne 'POST') {
+        # When being called using GET, we don't allow calling
+        # methods that can change data. This protects us against cross-site
+        # request forgeries.
+        if (!grep($_ eq $method, $pkg->READ_ONLY)) {
+            ThrowUserError('json_rpc_post_only', 
+                           { method => $self->_bz_method_name });
+        }
+    }
+
     # This is the best time to do login checks.
     $self->handle_login();
 
@@ -235,17 +345,6 @@ sub _argument_type_check {
     return $params;
 }
 
-sub handle_login {
-    my $self = shift;
-
-    my $path = $self->path_info;
-    my $class = $self->{dispatch_path}->{$path};
-    my $full_method = $self->_bz_method_name;
-    $full_method =~ /^\S+\.(\S+)/;
-    my $method = $1;
-    $self->SUPER::handle_login($class, $method, $full_method);
-}
-
 # _bz_method_name is stored by _find_procedure for later use.
 sub _bz_method_name {
     return $_[0]->{_bz_method_name}; 
@@ -285,8 +384,40 @@ your Bugzilla installation. For example, if your Bugzilla is at
 C<bugzilla.yourdomain.com>, then your JSON-RPC client would access the
 API via: C<http://bugzilla.yourdomain.com/jsonrpc.cgi>
 
-Bugzilla only allows JSON-RPC requests over C<POST>. C<GET> requests
-(or any other type of request, such as C<HEAD>) will be denied.
+=head2 Connecting via GET
+
+The most powerful way to access the JSON-RPC interface is by HTTP POST.
+However, for convenience, you can also access certain methods by using GET
+(a normal webpage load). Methods that modify the database or cause some
+action to happen in Bugzilla cannot be called over GET. Only methods that
+simply return data can be used over GET.
+
+For security reasons, when you connect over GET, cookie authentication
+is not accepted. If you want to authenticate using GET, you have to
+use the C<Bugzilla_login> and C<Bugzilla_password> method described at
+L<Bugzilla::WebService/LOGGING IN>.
+
+To connect over GET, simply send the values that you'd normally send for
+each JSON-RPC argument as URL parameters, with the C<params> item being
+a JSON string. 
+
+The simplest example is a call to C<Bugzilla.time>:
+
+ jsonrpc.cgi?method=Bugzilla.time
+
+Here's a call to C<User.get>, with several parameters:
+
+ jsonrpc.cgi?method=User.get&params=[ { "ids": [1,2], "names": ["user@domain.com"] } ]
+
+Although in reality you would url-encode the C<params> argument, so it would
+look more like this:
+
+ jsonrpc.cgi?method=User.get&params=%5B+%7B+%22ids%22%3A+%5B1%2C2%5D%2C+%22names%22%3A+%5B%22user%40domain.com%22%5D+%7D+%5D
+
+You can also specify C<version> as a URL parameter, if you want to specify
+what version of the JSON-RPC protocol you're using, and C<id> as a URL
+parameter if you want there to be a specific C<id> value in the returned
+JSON-RPC response.
 
 =head1 PARAMETERS
 
diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm
index 76d4d3e37..4aa152969 100644
--- a/Bugzilla/WebService/User.pm
+++ b/Bugzilla/WebService/User.pm
@@ -36,6 +36,10 @@ use constant LOGIN_EXEMPT => {
     offer_account_by_email => 1,
 };
 
+use constant READ_ONLY => qw(
+    get
+);
+
 ##############
 # User Login #
 ##############
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 033410ee0..d866f301d 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -979,9 +979,19 @@
       [%+ constants.LOGIN_LOCKOUT_INTERVAL FILTER html %] minutes.
     [% END %]
 
+  [% ELSIF error == "json_rpc_get_method_required" %]
+    When using JSON-RPC over GET, you must specify a 'method'
+    parameter. See the documentation at
+    [%+ docs_urlbase FILTER html %]api/Bugzilla/WebService/Server/JSONRPC.html
+
+  [% ELSIF error == "json_rpc_invalid_params" %]
+    Could not parse the 'params' argument as valid JSON.
+    Error: [% err_msg FILTER html %]
+    Value: [% params FILTER html %]
+
   [% ELSIF error == "json_rpc_post_only" %]
-    For security reasons, you may only use JSON-RPC with the POST
-    HTTP method.
+    For security reasons, you may only call the '[% method FILTER html %]'
+    method via HTTP POST.
 
   [% ELSIF error == "keyword_already_exists" %]
     [% title = "Keyword Already Exists" %]