From 5bb84b3c6efc503652a14e59efbe2b7c548608dd Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Thu, 22 Apr 2010 11:52:45 -0700 Subject: Bug 550732: Allow read-only JSON-RPC methods to be called with GET r=dkl, a=mkanat --- Bugzilla/Auth/Login.pm | 9 ++ Bugzilla/Auth/Login/Cookie.pm | 1 + Bugzilla/Auth/Login/Env.pm | 1 + Bugzilla/Auth/Login/Stack.pm | 5 + Bugzilla/WebService.pm | 4 + Bugzilla/WebService/Bug.pm | 10 ++ Bugzilla/WebService/Bugzilla.pm | 7 ++ Bugzilla/WebService/Constants.pm | 9 +- Bugzilla/WebService/Product.pm | 7 ++ Bugzilla/WebService/Server/JSONRPC.pm | 177 +++++++++++++++++++++++++++++----- Bugzilla/WebService/User.pm | 4 + 11 files changed, 207 insertions(+), 27 deletions(-) (limited to 'Bugzilla') 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. Whether or not users can create accounts, if this login method is currently being used by the system. Defaults to C. +=item C + +True if this login method requires no interaction from the user within +Bugzilla. (For example, C 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. + =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, then your JSON-RPC client would access the API via: C -Bugzilla only allows JSON-RPC requests over C. C requests -(or any other type of request, such as C) 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 and C method described at +L. + +To connect over GET, simply send the values that you'd normally send for +each JSON-RPC argument as URL parameters, with the C item being +a JSON string. + +The simplest example is a call to C: + + jsonrpc.cgi?method=Bugzilla.time + +Here's a call to C, with several parameters: + + jsonrpc.cgi?method=User.get¶ms=[ { "ids": [1,2], "names": ["user@domain.com"] } ] + +Although in reality you would url-encode the C argument, so it would +look more like this: + + jsonrpc.cgi?method=User.get¶ms=%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 as a URL parameter, if you want to specify +what version of the JSON-RPC protocol you're using, and C as a URL +parameter if you want there to be a specific C 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 # ############## -- cgit v1.2.3-24-g4f1b