From fa7ae98d1b2e83e88b2861f0826dc89e5b3bbd63 Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Tue, 11 Aug 2015 13:31:49 -0400 Subject: Bug 1184332 - Add Restricted API calls for MozReview --- Bugzilla/Auth.pm | 6 +++ Bugzilla/Auth/Login/APIKey.pm | 13 +++++ Bugzilla/User/APIKey.pm | 11 +++++ Bugzilla/WebService/Server/REST.pm | 1 + extensions/MozReview/Extension.pm | 55 +++++++++++++++------- extensions/MozReview/lib/Config.pm | 54 +++++++++++++++++++++ .../en/default/admin/params/mozreview.html.tmpl | 20 ++++++++ .../params/editparams-current_panel.html.tmpl | 12 ----- template/en/default/global/code-error.html.tmpl | 3 ++ 9 files changed, 145 insertions(+), 30 deletions(-) create mode 100644 extensions/MozReview/lib/Config.pm create mode 100644 extensions/MozReview/template/en/default/admin/params/mozreview.html.tmpl delete mode 100644 extensions/MozReview/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm index c502ffc35..6583d4e8b 100644 --- a/Bugzilla/Auth.pm +++ b/Bugzilla/Auth.pm @@ -96,6 +96,12 @@ sub login { return $self->_handle_login_result($login_info, $type); } +sub successful_info_getter { + my ($self) = @_; + + return $self->{_info_getter}->{successful}; +} + sub can_change_password { my ($self) = @_; my $verifier = $self->{_verifier}->{successful}; diff --git a/Bugzilla/Auth/Login/APIKey.pm b/Bugzilla/Auth/Login/APIKey.pm index 902ce4da7..4038cc8b9 100644 --- a/Bugzilla/Auth/Login/APIKey.pm +++ b/Bugzilla/Auth/Login/APIKey.pm @@ -22,6 +22,18 @@ use constant requires_verification => 0; use constant can_login => 0; use constant can_logout => 0; +use fields qw(app_id); + +sub set_app_id { + my ($self, $app_id) = @_; + $self->{app_id} = $app_id; +} + +sub app_id { + my ($self) = @_; + return $self->{app_id}; +} + # This method is only available to web services. An API key can never # be used to authenticate a Web request. sub get_login_info { @@ -45,6 +57,7 @@ sub get_login_info { } $api_key->update_last_used(); + $self->set_app_id($api_key->app_id); return { user_id => $api_key->user_id }; } diff --git a/Bugzilla/User/APIKey.pm b/Bugzilla/User/APIKey.pm index 75a4a6beb..c37cccb92 100644 --- a/Bugzilla/User/APIKey.pm +++ b/Bugzilla/User/APIKey.pm @@ -14,6 +14,7 @@ use parent qw(Bugzilla::Object); use Bugzilla::User; use Bugzilla::Util qw(generate_random_password trim); +use Bugzilla::Error; ##################################################################### # Overriden Constants that are used as methods @@ -24,6 +25,7 @@ use constant DB_COLUMNS => qw( id user_id api_key + app_id description revoked last_used @@ -32,6 +34,7 @@ use constant DB_COLUMNS => qw( use constant UPDATE_COLUMNS => qw(description revoked last_used); use constant VALIDATORS => { api_key => \&_check_api_key, + app_id => \&_check_app_id, description => \&_check_description, revoked => \&Bugzilla::Object::check_boolean, }; @@ -48,6 +51,7 @@ use constant { AUDIT_CREATES => 0, sub id { return $_[0]->{id} } sub user_id { return $_[0]->{user_id} } sub api_key { return $_[0]->{api_key} } +sub app_id { return $_[0]->{app_id} } sub description { return $_[0]->{description} } sub revoked { return $_[0]->{revoked} } sub last_used { return $_[0]->{last_used} } @@ -74,6 +78,13 @@ sub set_revoked { $_[0]->set('revoked', $_[1]); } # Validators sub _check_api_key { return generate_random_password(40); } sub _check_description { return trim($_[1]) || ''; } +sub _check_app_id { + my ($invocant, $app_id) = @_; + + ThrowCodeError("invalid_app_id", { app_id => $app_id }) unless $app_id =~ /^[[:xdigit:]]+$/; + + return $app_id; +} 1; __END__ diff --git a/Bugzilla/WebService/Server/REST.pm b/Bugzilla/WebService/Server/REST.pm index 1af41fe16..858375247 100644 --- a/Bugzilla/WebService/Server/REST.pm +++ b/Bugzilla/WebService/Server/REST.pm @@ -187,6 +187,7 @@ sub handle_login { my $class = $self->bz_class_name; my $method = $self->bz_method_name; my $full_method = $class . "." . $method; + $full_method =~ s/^Bugzilla::WebService:://; # Bypass JSONRPC::handle_login Bugzilla::WebService::Server->handle_login($class, $method, $full_method); diff --git a/extensions/MozReview/Extension.pm b/extensions/MozReview/Extension.pm index 4e1951ed4..5745cf219 100644 --- a/extensions/MozReview/Extension.pm +++ b/extensions/MozReview/Extension.pm @@ -13,10 +13,22 @@ use warnings; use parent qw(Bugzilla::Extension); use Bugzilla::Attachment; -use Bugzilla::Config::Common; +use Bugzilla::Error; +use List::MoreUtils qw( any ); our $VERSION = '0.01'; +my @METHOD_WHITELIST = ( + 'User.get', + 'User.login', + 'User.valid_login', + 'Bug.add_comment', + 'Bug.add_attachment', + 'Bug.attachments', + 'Bug.get', + 'Bug.update_attachment', +); + sub template_before_process { my ($self, $args) = @_; my $file = $args->{'file'}; @@ -65,25 +77,32 @@ sub auth_delegation_confirm { } } -sub config_modify_panels { +sub config_add_panels { my ($self, $args) = @_; - push @{ $args->{panels}->{advanced}->{params} }, { - name => 'mozreview_base_url', - type => 't', - default => '', - checker => \&check_urlbase - }; - push @{ $args->{panels}->{advanced}->{params} }, { - name => 'mozreview_auth_callback_url', - type => 't', - default => '', - checker => sub { - my ($url) = (@_); - - return 'must be an HTTP/HTTPS absolute URL' unless $url =~ m{^https?://}; - return ''; + my $modules = $args->{panel_modules}; + $modules->{MozReview} = "Bugzilla::Extension::MozReview::Config"; +} + +sub webservice_before_call { + my ($self, $args) = @_; + my ($method, $full_method) = ($args->{method}, $args->{full_method}); + my $mozreview_app_id = Bugzilla->params->{mozreview_app_id}; + my $user = Bugzilla->user; + + return unless $mozreview_app_id; + return unless $user->authorizer; + + my $getter = $user->authorizer->successful_info_getter() + or return; + + return unless $getter->can("app_id") && $getter->app_id; + + my $app_id = $getter->app_id; + if ($app_id eq $mozreview_app_id) { + unless (any { $full_method eq $_ } @METHOD_WHITELIST) { + ThrowCodeError('unknown_method', { method => $full_method }); } - }; + } } __PACKAGE__->NAME; diff --git a/extensions/MozReview/lib/Config.pm b/extensions/MozReview/lib/Config.pm new file mode 100644 index 000000000..ab6b8c7c3 --- /dev/null +++ b/extensions/MozReview/lib/Config.pm @@ -0,0 +1,54 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::MozReview::Config; + +use strict; +use warnings; + +use Bugzilla::Config::Common; + +our $sortkey = 1300; + +sub get_param_list { + my ($class) = @_; + + my @params = ( + { + name => 'mozreview_base_url', + type => 't', + default => '', + checker => \&check_urlbase + }, + { + name => 'mozreview_auth_callback_url', + type => 't', + default => '', + checker => sub { + my ($url) = (@_); + + return 'must be an HTTP/HTTPS absolute URL' unless $url =~ m{^https?://}; + return ''; + } + }, + { + name => 'mozreview_app_id', + type => 't', + default => '', + checker => sub { + my ($app_id) = (@_); + + return 'must be a hex number' unless $app_id =~ /^[[:xdigit:]]+$/; + return ''; + }, + }, + ); + + return @params; +} + +1; diff --git a/extensions/MozReview/template/en/default/admin/params/mozreview.html.tmpl b/extensions/MozReview/template/en/default/admin/params/mozreview.html.tmpl new file mode 100644 index 000000000..4a35555a4 --- /dev/null +++ b/extensions/MozReview/template/en/default/admin/params/mozreview.html.tmpl @@ -0,0 +1,20 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% + title = "MozReview" + desc = "Configure MozReview" +%] + +[% + param_descs = { + mozreview_base_url => 'MozReview Base URL', + mozreview_auth_callback_url => 'MozReview Auth Delegation URL', + mozreview_app_id => 'app_id for API Keys delegated to MozReview', + } +%] diff --git a/extensions/MozReview/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl b/extensions/MozReview/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl deleted file mode 100644 index eb08f26eb..000000000 --- a/extensions/MozReview/template/en/default/hook/admin/params/editparams-current_panel.html.tmpl +++ /dev/null @@ -1,12 +0,0 @@ -[%# This Source Code Form is subject to the terms of the Mozilla Public - # License, v. 2.0. If a copy of the MPL was not distributed with this - # file, You can obtain one at http://mozilla.org/MPL/2.0/. - # - # This Source Code Form is "Incompatible With Secondary Licenses", as - # defined by the Mozilla Public License, v. 2.0. - #%] - -[% IF panel.name == "advanced" %] - [% panel.param_descs.mozreview_base_url = 'MozReview Base URL' %] - [% panel.param_descs.mozreview_auth_callback_url = 'MozReview Auth Delegation URL' %] -[% END -%] diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 40b59801c..316e98450 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -207,6 +207,9 @@ [% ELSIF error == "illegal_field" %] A legal [% field FILTER html %] was not set. + [% ELSIF error == "invalid_app_id" %] + The app_id generated was invalid. [% app_id FILTER html %] is not a hexadecimal string. + [% ELSIF error == "invalid_attach_id_to_obsolete" %] The attachment number of one of the attachments you wanted to obsolete, [%+ attach_id FILTER html %], is invalid. -- cgit v1.2.3-24-g4f1b