diff --git a/Bugzilla/API/V1/Teams.pm b/Bugzilla/API/V1/Teams.pm index a88ef895b7..49bdd0373b 100644 --- a/Bugzilla/API/V1/Teams.pm +++ b/Bugzilla/API/V1/Teams.pm @@ -14,25 +14,22 @@ use JSON::MaybeXS qw(decode_json); sub setup_routes { my ($class, $r) = @_; $r->get('/config/component_teams')->to('V1::Teams#component_teams'); - $r->get('/config/component_security_teams')->to('V1::Teams#component_security_teams'); + $r->get('/config/component_security_teams') + ->to('V1::Teams#component_security_teams'); } sub component_teams { my ($self) = @_; $self->bugzilla->login(LOGIN_REQUIRED) - || return $self->render(status => 401, text => 'Unauthorized'); - $self->render( - json => decode_json(Bugzilla->params->{report_component_teams}) - ); + || return $self->user_error('invalid_username'); + $self->render(json => decode_json(Bugzilla->params->{report_component_teams})); } sub component_security_teams { my ($self) = @_; $self->bugzilla->login(LOGIN_REQUIRED) - || return $self->render(status => 401, text => 'Unauthorized'); - $self->render( - json => decode_json(Bugzilla->params->{report_secbugs_teams}) - ); + || return $self->user_error('invalid_username'); + $self->render(json => decode_json(Bugzilla->params->{report_secbugs_teams})); } 1; diff --git a/Bugzilla/API/V1/User.pm b/Bugzilla/API/V1/User.pm index 191f4731cb..23f6392473 100644 --- a/Bugzilla/API/V1/User.pm +++ b/Bugzilla/API/V1/User.pm @@ -35,7 +35,7 @@ sub user_profile { ); } else { - $self->render(status => 401, text => 'Unauthorized'); + return $self->user_error('invalid_username'); } } diff --git a/Bugzilla/App.pm b/Bugzilla/App.pm index 414f78f168..b6ff12c5d4 100644 --- a/Bugzilla/App.pm +++ b/Bugzilla/App.pm @@ -46,6 +46,7 @@ sub startup { $self->plugin('Bugzilla::App::Plugin::BlockIP'); $self->plugin('Bugzilla::App::Plugin::Glue'); $self->plugin('Bugzilla::App::Plugin::Login'); + $self->plugin('Bugzilla::App::Plugin::Error'); $self->plugin('Bugzilla::App::Plugin::Hostage') unless $ENV{BUGZILLA_DISABLE_HOSTAGE}; $self->plugin('Bugzilla::App::Plugin::SizeLimit') diff --git a/Bugzilla/App/API.pm b/Bugzilla/App/API.pm index 44d7e206e4..b971df783b 100644 --- a/Bugzilla/App/API.pm +++ b/Bugzilla/App/API.pm @@ -8,13 +8,15 @@ package Bugzilla::App::API; use 5.10.1; -use Bugzilla::Constants; -use Bugzilla::Logging; -use Module::Runtime qw(require_module); use Mojo::Base qw( Mojolicious::Controller ); + use Mojo::Loader qw( find_modules ); +use Module::Runtime qw(require_module); use Try::Tiny; +use Bugzilla::Constants; +use Bugzilla::Logging; + use constant SUPPORTED_VERSIONS => qw(V1); sub setup_routes { diff --git a/Bugzilla/App/OAuth2/Clients.pm b/Bugzilla/App/OAuth2/Clients.pm index 66a5dad405..11d8b53777 100644 --- a/Bugzilla/App/OAuth2/Clients.pm +++ b/Bugzilla/App/OAuth2/Clients.pm @@ -22,9 +22,10 @@ sub setup_routes { my $client_route = $r->under( '/admin/oauth' => sub { my ($c) = @_; + Bugzilla->usage_mode(USAGE_MODE_MOJO); my $user = $c->bugzilla->login(LOGIN_REQUIRED) || return undef; $user->in_group('admin') - || ThrowUserError('auth_failure', + || return $c->user_error('auth_failure', {group => 'admin', action => 'edit', object => 'oauth_clients'}); return 1; } @@ -67,31 +68,32 @@ sub create { my $description = $self->param('description'); my $id = $self->param('id'); my $secret = $self->param('secret'); - my @scopes = $self->param('scopes'); - $description or ThrowCodeError('param_required', {param => 'description'}); - $id or ThrowCodeError('param_required', {param => 'id'}); - $secret or ThrowCodeError('param_required', {param => 'secret'}); - any { $_ > 0 } @scopes or ThrowCodeError('param_required', {param => 'scopes'}); + my $scopes = $self->every_param('scopes'); + $description + or return $self->code_error('param_required', {param => 'description'}); + $id or return $self->code_error('param_required', {param => 'id'}); + $secret or return $self->code_error('param_required', {param => 'secret'}); + any { $_ > 0 } @{$scopes} + or return $self->code_error('param_required', {param => 'scopes'}); my $token = $self->param('token'); check_token_data($token, 'create_oauth_client'); - $dbh->do('INSERT INTO oauth2_client (client_id, description, secret) VALUES (?, ?, ?)', + $dbh->do( + 'INSERT INTO oauth2_client (client_id, description, secret) VALUES (?, ?, ?)', undef, $id, $description, $secret); my $client_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE client_id = ?', undef, $id); - foreach my $scope_id (@scopes) { + foreach my $scope_id (@{$scopes}) { $scope_id = $dbh->selectrow_array('SELECT id FROM oauth2_scope WHERE id = ?', undef, $scope_id); if (!$scope_id) { - ThrowCodeError('param_required', {param => 'scopes'}); + return $self->code_error('param_required', {param => 'scopes'}); } - $dbh->do( - 'INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', - undef, $client_data->{id}, $scope_id - ); + $dbh->do('INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', + undef, $client_data->{id}, $scope_id); } delete_token($token); @@ -114,8 +116,9 @@ sub delete { my $dbh = Bugzilla->dbh; my $vars = {}; - my $id = $self->param('id'); - my $client_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', + my $id = $self->param('id'); + my $client_data + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', undef, $id); if (!$self->param('deleteme')) { @@ -156,14 +159,15 @@ sub edit { my $vars = {}; my $id = $self->param('id'); - my $client_data = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', + my $client_data + = $dbh->selectrow_hashref('SELECT * FROM oauth2_client WHERE id = ?', undef, $id); my $client_scopes = $dbh->selectall_arrayref( 'SELECT scope_id FROM oauth2_client_scope WHERE client_id = ?', undef, $client_data->{id}); $client_data->{scopes} = [map { $_->[0] } @{$client_scopes}]; - $vars->{client} = $client_data; + $vars->{client} = $client_data; # All scopes my $all_scopes @@ -183,7 +187,7 @@ sub edit { my $description = $self->param('description'); my $active = $self->param('active'); - my @scopes = $self->param('scopes'); + my $scopes = $self->every_param('scopes'); if ($description ne $client_data->{description}) { $dbh->do('UPDATE oauth2_client SET description = ? WHERE id = ?', @@ -196,11 +200,9 @@ sub edit { } $dbh->do('DELETE FROM oauth2_client_scope WHERE client_id = ?', undef, $id); - foreach my $scope_id (@scopes) { - $dbh->do( - 'INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', - undef, $client_data->{id}, $scope_id - ); + foreach my $scope_id (@{$scopes}) { + $dbh->do('INSERT INTO oauth2_client_scope (client_id, scope_id) VALUES (?, ?)', + undef, $client_data->{id}, $scope_id); } delete_token($token); diff --git a/Bugzilla/App/Plugin/Error.pm b/Bugzilla/App/Plugin/Error.pm new file mode 100644 index 0000000000..d9e6f30fe1 --- /dev/null +++ b/Bugzilla/App/Plugin/Error.pm @@ -0,0 +1,88 @@ +# 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::App::Plugin::Error; +use 5.10.1; +use Mojo::Base 'Mojolicious::Plugin'; + +use Bugzilla::Constants; +use Bugzilla::Logging; +use Bugzilla::WebService::Constants; + +sub register { + my ($self, $app) = @_; + $app->helper('code_error' => sub { _render_error('code', @_); }); + $app->helper('user_error' => sub { _render_error('user', @_); }); +} + +sub _render_error { + my ($type, $c, $error, $vars) = @_; + my $logfunc = _make_logfunc(ucfirst($type)); + + # Errors displayed in a web page + if (Bugzilla->error_mode == ERROR_MODE_MOJO + || Bugzilla->error_mode == ERROR_MODE_WEBPAGE) + { + $logfunc->("webpage error: $error"); + + $c->render( + handler => 'bugzilla', + template => "global/$type-error", + format => 'html', + error => $error, + %{$vars} + ); + } + + # Errors returned in an API request + elsif (Bugzilla->error_mode == ERROR_MODE_REST) { + my %error_map = %{WS_ERROR_CODE()}; + my $code = $error_map{$error}; + + if (!$code) { + $code = ERROR_UNKNOWN_FATAL if $type =~ /code/i; + $code = ERROR_UNKNOWN_TRANSIENT if $type =~ /user/i; + } + + my %status_code_map = %{REST_STATUS_CODE_MAP()}; + my $status_code = $status_code_map{$code} || $status_code_map{'_default'}; + + $logfunc->("REST error: $error (HTTP $status_code, internal code $code)"); + + # Find the full error message + my $message; + $vars->{error} = $error; + my $template = Bugzilla->template; + $template->process("global/$type-error.html.tmpl", $vars, \$message) + || die $template->error(); + + my $error = { + error => 1, + code => $code, + message => $message, + documentation => 'https://bmo.readthedocs.io/en/latest/api/', + }; + + $c->render(json => $error, status => $status_code); + } +} + +sub _make_logfunc { + my ($type) = @_; + my $logger = Log::Log4perl->get_logger("Bugzilla.Error.$type"); + return sub { + local $Log::Log4perl::caller_depth = $Log::Log4perl::caller_depth + 3; + if ($type eq 'User') { + $logger->warn(@_); + } + else { + $logger->error(@_); + } + }; +} + +1; diff --git a/Bugzilla/App/Plugin/Glue.pm b/Bugzilla/App/Plugin/Glue.pm index 2958883c70..43c0ab5010 100644 --- a/Bugzilla/App/Plugin/Glue.pm +++ b/Bugzilla/App/Plugin/Glue.pm @@ -67,23 +67,6 @@ sub register { } ); - $app->helper( - 'bugzilla.error_page' => sub { - my ($c, $error) = @_; - if (blessed $error && $error->isa('Bugzilla::Error::Base')) { - $c->render( - handler => 'bugzilla', - template => $error->template, - error => $error->message, - %{$error->vars} - ); - } - else { - $c->reply->exception($error); - } - } - ); - $app->helper( 'url_is_attachment_base' => sub { my ($c, $id) = @_;