Bug 927778: users without canconfirm cannot set needinfo, and can clear needinfo requests which aren't targeted at them

3 files changed, 50 insertions, 24 deletions

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 %]
       <tr>
         [% IF !flag.requestee || flag.requestee.id == user.id %]
+          [%# needinfo targetted at the current user, or anyone %]
           <td align="center">
             <input type="checkbox" id="needinfo_override_[% flag.id FILTER html %]"
-                   name="needinfo_override_[% flag.id FILTER html %]" value="1" checked>
+                   name="needinfo_override_[% flag.id FILTER html %]" value="1"
+                   [% "checked" IF flag.requestee || user.in_group("canconfirm") %]>
           </td>
           <td>
             <label for="needinfo_override_[% flag.id FILTER html %]">
@@ -44,7 +46,8 @@
               <em>[% IF !flag.requestee %]anyone[% ELSE %][% flag.requestee.login FILTER html %][% END %]</em>.
             </label>
           </td>
-        [% ELSE %]
+        [% ELSIF user.in_group("canconfirm") || flag.setter_id == user.id %]
+          [%# needinfo targetted at someone else, but the user can clear %]
           <td align="center">
             <input type="checkbox" id="needinfo_override_[% flag.id FILTER html %]"
                    name="needinfo_override_[% flag.id FILTER html %]" value="1">
@@ -55,6 +58,12 @@
               (clears the needinfo request).
             </label>
           </td>
+        [% ELSE %]
+          [%# current user does not have permissions to clear needinfo %]
+          <td>&nbsp;</td>
+          <td>
+            Needinfo requested from <em>[% flag.requestee.login FILTER html %]</em>.
+          </td>
         [% END %]
       </tr>
     [% 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 %]