Bug 1257662 - Disallow clearing a flag if the flag is set to allow granting by specifc group and changer is not in group

4 files changed, 20 insertions, 8 deletions

diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 07041bd79..c6c979fa7 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -782,13 +782,13 @@ sub _check_setter {
     # Make sure the user is authorized to modify flags, see bug 180879:
     # - The flag exists and is unchanged.
     # - The flag setter can unset flag.
-    # - Users in the request_group can clear pending requests and set flags
-    #   and can rerequest set flags.
-    # - Users in the grant_group can set/clear flags, including "+" and "-".
+    # - Users in the request_group can clear pending requests
+    # - Users in the grant_group can set/cleari/request flags, including "+" and "-".
     unless (($status eq $self->{_old_status})
             || ($status eq 'X' && $setter->id == Bugzilla->user->id)
             || (($status eq 'X' || $status eq '?')
                 && $setter->can_request_flag($self->type))
+            || $setter->can_unset_flag($self->type, $self->{_old_status})
             || $setter->can_set_flag($self->type))
     {
         ThrowUserError('flag_update_denied',
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 5386e70ca..3fe59fe76 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1529,6 +1529,13 @@ sub can_set_flag {
             || $self->in_group_id($flag_type->grant_group_id)) ? 1 : 0;
 }
 
+sub can_unset_flag {
+    my ($self, $flag_type, $flag_status) = @_;
+    return 1 if !$flag_type->grant_group_id;
+    return 1 if ($flag_status ne '+' && $flag_status ne '-');
+    return $self->in_group_id($flag_type->grant_group_id) ? 1 : 0;
+}
+
 # visible_groups_inherited returns a reference to a list of all the groups
 # whose members are visible to this user.
 sub visible_groups_inherited {
diff --git a/extensions/BugModal/template/en/default/bug_modal/flags.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/flags.html.tmpl
index 3e0da1173..d4c2d50b3 100644
--- a/extensions/BugModal/template/en/default/bug_modal/flags.html.tmpl
+++ b/extensions/BugModal/template/en/default/bug_modal/flags.html.tmpl
@@ -127,15 +127,17 @@
       <input type="hidden" id="[% flag_id FILTER html %]-dirty">
       <select id="[% flag_id FILTER html %]" name="[% flag_id FILTER html %]"
         title="[% t.description FILTER html %]"
-        [% UNLESS (t.is_requestable && user.can_request_flag(t)) || user.can_set_flag(t) %]
+        [% UNLESS (t.is_requestable && user.can_request_flag(t))
+                   || user.can_set_flag(t)
+                   || (f && user.can_unset_flag(t, f.status)) %]
           disabled
         [% END %]
         class="bug-flag">
-        [% IF !f || (user.can_request_flag(t)) || f.setter_id == user.id %]
+        [% IF !f || (user.can_unset_flag(t, f.status) && user.can_request_flag(t)) || f.setter_id == user.id %]
           <option value="X"></option>
         [% END %]
         [% IF t.is_active %]
-          [% IF (t.is_requestable && user.can_request_flag(t)) || (f && f.status == "?") %]
+          [% IF (!f && t.is_requestable && user.can_request_flag(t)) || (f && user.can_unset_flag(t, f.status) && f.status == "?") %]
             <option value="?" [% "selected" IF f && f.status == "?" %]>?</option>
           [% END %]
           [% IF user.can_set_flag(t) || (f && f.status == "+") %]
diff --git a/template/en/default/flag/list.html.tmpl b/template/en/default/flag/list.html.tmpl
index 0d84e9bff..7993a2a89 100644
--- a/template/en/default/flag/list.html.tmpl
+++ b/template/en/default/flag/list.html.tmpl
@@ -133,11 +133,14 @@
                 class="flag_select flag_type-[% type.id %]"
                 [% IF !can_edit_flag %] disabled="disabled"[% END %]>
         [%# Only display statuses the user is allowed to set. %]
-        [% IF !flag || (can_edit_flag && user.can_request_flag(type)) || flag.setter_id == user.id %]
+        [% IF !flag
+              || (can_edit_flag && user.can_unset_flag(type, flag.status) && user.can_request_flag(type))
+              || flag.setter_id == user.id %]
           <option value="X"></option>
         [% END %]
         [% IF type.is_active && can_edit_flag %]
-          [% IF (type.is_requestable && user.can_request_flag(type)) || (flag && flag.status == "?") %]
+          [% IF (!flag && type.is_requestable && user.can_request_flag(type))
+                 || (flag && user.can_unset_flag(type, flag.status) && flag.status == "?") %]
             <option value="?" [% "selected" IF flag && flag.status == "?" %]>?</option>
           [% END %]
           [% IF user.can_set_flag(type) || (flag && flag.status == "+") %]