From dace6ab711a16731f1015cd9bd47f12f25165212 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Wed, 30 Oct 2013 15:29:08 +0800 Subject: Bug 927778: users without canconfirm cannot set needinfo, and can clear needinfo requests which aren't targeted at them --- extensions/Needinfo/Extension.pm | 48 ++++++++++++---------- .../template/en/default/bug/needinfo.html.tmpl | 13 +++++- .../hook/global/user-error-errors.html.tmpl | 13 ++++++ 3 files changed, 50 insertions(+), 24 deletions(-) create mode 100644 extensions/Needinfo/template/en/default/hook/global/user-error-errors.html.tmpl (limited to 'extensions/Needinfo') diff --git a/extensions/Needinfo/Extension.pm b/extensions/Needinfo/Extension.pm index 622221507..b30750488 100644 --- a/extensions/Needinfo/Extension.pm +++ b/extensions/Needinfo/Extension.pm @@ -10,9 +10,10 @@ use strict; use base qw(Bugzilla::Extension); -use Bugzilla::User; +use Bugzilla::Error; use Bugzilla::Flag; use Bugzilla::FlagType; +use Bugzilla::User; our $VERSION = '0.01'; @@ -57,7 +58,7 @@ sub bug_start_of_update { my $cgi = Bugzilla->cgi; my $params = Bugzilla->input_params; - if ($user->in_group('canconfirm') && $params->{needinfo}) { + if ($params->{needinfo}) { # do a match if applicable Bugzilla::User::match_field({ 'needinfo_from' => { 'type' => 'multi' } @@ -69,7 +70,7 @@ sub bug_start_of_update { $params->{needinfo_done} = 1; Bugzilla->input_params($params); - my $needinfo = delete $params->{needinfo}; + my $add_needinfo = delete $params->{needinfo}; my $needinfo_from = delete $params->{needinfo_from}; my $needinfo_role = delete $params->{needinfo_role}; my $is_private = $params->{'comment_is_private'}; @@ -85,7 +86,7 @@ sub bug_start_of_update { my @new_flags; my $needinfo_requestee; - if ($user->in_group('canconfirm') && $needinfo) { + if ($add_needinfo) { foreach my $type (@{ $bug->flag_types }) { next if $type->name ne 'needinfo'; my %requestees; @@ -99,7 +100,7 @@ sub bug_start_of_update { $requestees{$bug->assigned_to->login} = 1; } # Use reporter as requestee - elsif ( $needinfo_role eq 'reporter') { + elsif ($needinfo_role eq 'reporter') { $requestees{$bug->reporter->login} = 1; } # Use qa_contact as requestee @@ -138,28 +139,14 @@ sub bug_start_of_update { } } - # Clear the flag if additional information was given as requested my @flags; foreach my $flag (@{ $bug->flags }) { next if $flag->type->name ne 'needinfo'; - my $clear_needinfo = 0; - # Clear if somehow the flag has been set to +/- - $clear_needinfo = 1 if $flag->status ne '?'; - - # Clear if current user has selected override - $clear_needinfo = 1 if grep($_ == $flag->id, @needinfo_overrides); - - # Clear if comment provided by the proper requestee - if ($bug->{added_comments} - && (!$flag->requestee || $flag->requestee->login eq Bugzilla->user->login) - && (!$is_private || $flag->setter->is_insider) - && grep($_ == $flag->id, @needinfo_overrides)) + # or if the "clear needinfo" override checkbox is selected + if ($flag->status ne '?' + or grep { $_ == $flag->id } @needinfo_overrides) { - $clear_needinfo = 1; - } - - if ($clear_needinfo) { push(@flags, { id => $flag->id, status => 'X' }); } } @@ -169,4 +156,21 @@ sub bug_start_of_update { } } +sub object_before_delete { + my ($self, $args) = @_; + my $object = $args->{object}; + return unless $object->isa('Bugzilla::Flag') + && $object->type->name eq 'needinfo'; + my $user = Bugzilla->user; + + # Require canconfirm to clear requests targetted at someone else + if ($object->setter_id != $user->id + && $object->requestee + && $object->requestee->id != $user->id + && !$user->in_group('canconfirm')) + { + ThrowUserError('needinfo_illegal_change'); + } +} + __PACKAGE__->NAME; diff --git a/extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl b/extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl index 0e023fcc2..60a1b0a1c 100644 --- a/extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl +++ b/extensions/Needinfo/template/en/default/bug/needinfo.html.tmpl @@ -34,9 +34,11 @@ [% FOREACH flag = needinfo_flags %] [% IF !flag.requestee || flag.requestee.id == user.id %] + [%# needinfo targetted at the current user, or anyone %] + name="needinfo_override_[% flag.id FILTER html %]" value="1" + [% "checked" IF flag.requestee || user.in_group("canconfirm") %]> - [% ELSE %] + [% ELSIF user.in_group("canconfirm") || flag.setter_id == user.id %] + [%# needinfo targetted at someone else, but the user can clear %] @@ -55,6 +58,12 @@ (clears the needinfo request). + [% ELSE %] + [%# current user does not have permissions to clear needinfo %] +   + + Needinfo requested from [% flag.requestee.login FILTER html %]. + [% END %] [% END %] diff --git a/extensions/Needinfo/template/en/default/hook/global/user-error-errors.html.tmpl b/extensions/Needinfo/template/en/default/hook/global/user-error-errors.html.tmpl new file mode 100644 index 000000000..f1241bc61 --- /dev/null +++ b/extensions/Needinfo/template/en/default/hook/global/user-error-errors.html.tmpl @@ -0,0 +1,13 @@ +[%# 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 error == "needinfo_illegal_change" %] + [% title = 'Needinfo Illegal Change' %] + Only the requestee or a user with the required permissions can clear a + needinfo flag. +[% END %] -- cgit v1.2.3-24-g4f1b