Bug 309681 Prevent users from adding another user who shouldn't have access to a bug as assignee or CC member

Patch by Gabriel Sales de Oliveira <gabriel@async.com.br>
r=joel, a=justdave

8 files changed, 93 insertions, 27 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 526f002b0..c08703789 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1303,6 +1303,17 @@ sub ValidateDependencies {
     return %deps;
 }
 
+#Verify if the new assignee belongs to the group of 
+#the product that the bug(s) is in.
+sub can_add_user_to_bug {
+    my ($prod_id, $id, $uid) = @_;
+    my $user = new Bugzilla::User($uid);
+    if (!$user->can_edit_product($prod_id)) {
+        ThrowUserError("invalid_user_group", { 'user' =>
+        $user->login, bug_id => $id });
+   } 
+}
+
 sub AUTOLOAD {
   use vars qw($AUTOLOAD);
   my $attr = $AUTOLOAD;
diff --git a/Bugzilla/Config/GroupSecurity.pm b/Bugzilla/Config/GroupSecurity.pm
index e48cd4966..bd1aa3829 100644
--- a/Bugzilla/Config/GroupSecurity.pm
+++ b/Bugzilla/Config/GroupSecurity.pm
@@ -74,6 +74,12 @@ sub get_param_list {
    name => 'usevisibilitygroups',
    type => 'b',
    default => 0
+  }, 
+  
+  {
+   name => 'strict_isolation',
+   type => 'b',
+   default => 0
   } );
   return @param_list;
 }
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 85584d70c..9b99428a6 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -382,6 +382,26 @@ sub can_see_user {
     return Bugzilla->dbh->selectrow_array($query, undef, $otherUser->id);
 }
 
+sub can_edit_product {
+    my ($self, $prod_id) = @_;
+    my $dbh = Bugzilla->dbh;
+    my $sth = $self->{sthCanEditProductId};
+    my $userid = $self->{id};
+    my $query = q{SELECT group_id FROM group_control_map 
+                   WHERE product_id =? 
+                     AND canedit != 0 };
+    if (%{$self->groups}) {
+        my $groups = join(',', values(%{$self->groups}));
+        $query .= qq{AND group_id NOT IN($groups)};
+    }
+    unless ($sth) { $sth = $dbh->prepare($query); }
+    $sth->execute($prod_id);
+    $self->{sthCanEditProductId} = $sth;
+    my $result = $sth->fetchrow_array();
+    
+    return (!defined($result));
+}
+
 sub can_see_bug {
     my ($self, $bugid) = @_;
     my $dbh = Bugzilla->dbh;
@@ -1535,6 +1555,11 @@ that you need to be aware of a group in order to bless a group.
 Returns 1 if the specified user account exists and is visible to the user,
 0 otherwise.
 
+=item C<can_edit_product(prod_id)>
+
+Determines if, given a product id, the user can edit bugs in this product
+at all.
+
 =item C<can_see_bug(bug_id)>
 
 Determines if the user can see the specified bug.
diff --git a/attachment.cgi b/attachment.cgi
index 67272ae50..eafb31ea5 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -218,7 +218,7 @@ sub validateCanChangeAttachment
              ON bugs.bug_id = attachments.bug_id
              WHERE attach_id = $attachid");
     my $productid = FetchOneColumn();
-    CanEditProductId($productid)
+    Bugzilla->user->can_edit_product_id($productid)
       || ThrowUserError("illegal_attachment_edit",
                         { attach_id => $attachid });
 }
diff --git a/globals.pl b/globals.pl
index 07d12a984..ebbce4c16 100644
--- a/globals.pl
+++ b/globals.pl
@@ -372,27 +372,6 @@ sub AnyDefaultGroups {
     return $::CachedAnyDefaultGroups;
 }
 
-#
-# This function checks if, given a product id, the user can edit
-# bugs in this product at all.
-sub CanEditProductId {
-    my ($productid) = @_;
-    my $dbh = Bugzilla->dbh;
-    my $query = "SELECT group_id FROM group_control_map " .
-                "WHERE product_id = $productid " .
-                "AND canedit != 0 "; 
-    if (%{Bugzilla->user->groups}) {
-        $query .= "AND group_id NOT IN(" . 
-                   join(',', values(%{Bugzilla->user->groups})) . ") ";
-    }
-    $query .= $dbh->sql_limit(1);
-    PushGlobalSQLState();
-    SendSQL($query);
-    my ($result) = FetchSQLData();
-    PopGlobalSQLState();
-    return (!defined($result));
-}
-
 sub IsInClassification {
     my ($classification,$productname) = @_;
 
diff --git a/process_bug.cgi b/process_bug.cgi
index 9362af4a8..0cc4a224f 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -984,6 +984,11 @@ if (defined $cgi->param('qa_contact')
     # The QA contact cannot be deleted from show_bug.cgi for a single bug!
     if ($name ne $cgi->param('dontchange')) {
         $qacontact = DBNameToIdAndCheck($name) if ($name ne "");
+        if (Param("strict_isolation")) {
+                my $product_id = get_product_id($cgi->param('product'));
+                Bugzilla::Bug::can_add_user_to_bug(
+                    $product_id, $cgi->param('id'), $qacontact);
+        }
         DoComma();
         if($qacontact) {
             $::query .= "qa_contact = $qacontact";
@@ -1046,7 +1051,14 @@ SWITCH: for ($cgi->param('knob')) {
         }
         ChangeStatus('NEW');
         DoComma();
-        if (!defined $cgi->param('assigned_to')
+        if (defined $cgi->param('assigned_to')) { 
+            my $uid = DBNameToIdAndCheck($cgi->param('assigned_to'));
+            if (Param("strict_isolation")) {
+                my $product_id = get_product_id($cgi->param('product'));
+                Bugzilla::Bug::can_add_user_to_bug(
+                    $product_id, $cgi->param('id'), $uid);
+            }
+        } elsif (!defined $cgi->param('assigned_to')
             || trim($cgi->param('assigned_to')) eq "") {
             ThrowUserError("reassign_to_empty");
         }
@@ -1276,6 +1288,7 @@ sub LogDependencyActivity {
 # show_bug.cgi).
 #
 foreach my $id (@idlist) {
+    my $bug_obj = new Bugzilla::Bug($id, $whoid);
     my %dependencychanged;
     $bug_changed = 0;
     my $write = "WRITE";        # Might want to make a param to control
@@ -1350,7 +1363,16 @@ foreach my $id (@idlist) {
             ThrowUserError("illegal_change", $vars);
         }
     }
-
+    if ($cgi->param('assigned_to') && Param("strict_isolation")) { 
+        my $uid = DBNameToIdAndCheck($cgi->param('assigned_to'));
+        Bugzilla::Bug::can_add_user_to_bug(
+            $bug_obj->{'product_id'}, $id, $uid);
+    }
+    if ($cgi->param('qa_contact') && Param("strict_isolation")) { 
+        Bugzilla::Bug::can_add_user_to_bug(
+            $bug_obj->{'product_id'}, $id, $qacontact);
+    }
+    
     # When editing multiple bugs, users can specify a list of keywords to delete
     # from bugs.  If the list matches the current set of keywords on those bugs,
     # CheckCanChangeField above will fail to check permissions because it thinks
@@ -1370,7 +1392,7 @@ foreach my $id (@idlist) {
     }
 
     $oldhash{'product'} = get_product_name($oldhash{'product_id'});
-    if (!CanEditProductId($oldhash{'product_id'})) {
+    if (!Bugzilla->user->can_edit_product($oldhash{'product_id'})) {
         ThrowUserError("product_edit_denied",
                       { product => $oldhash{'product'} });
     }
@@ -1565,7 +1587,22 @@ foreach my $id (@idlist) {
             $oncc{FetchOneColumn()} = 1;
         }
 
-        my (@added, @removed) = ();
+        my (@added, @removed, @blocked_cc) = ();
+        
+        if (Param("strict_isolation")) {
+            foreach my $pid (keys %cc_add) {
+                my $user = Bugzilla::User->new($pid);
+                if (!$user->can_edit_product($bug_obj->{'product_id'})) {
+                    push (@blocked_cc, $cc_add{$pid});
+                }
+            }
+            if (scalar(@blocked_cc)) {
+                my $blocked_cc = join(", ", @blocked_cc);
+                ThrowUserError("invalid_user_group", 
+                    {'user' => $blocked_cc , bug_id => $id });
+            }
+        }
+ 
         foreach my $pid (keys %cc_add) {
             # If this person isn't already on the cc list, add them
             if (! $oncc{$pid}) {
diff --git a/template/en/default/admin/params/groupsecurity.html.tmpl b/template/en/default/admin/params/groupsecurity.html.tmpl
index fe986f387..d4e219a88 100644
--- a/template/en/default/admin/params/groupsecurity.html.tmpl
+++ b/template/en/default/admin/params/groupsecurity.html.tmpl
@@ -47,5 +47,8 @@
                        "information.",
 
   usevisibilitygroups => "Do you wish to restrict visibility of users to members of " _
-                         "specific groups?" }
+                         "specific groups?",
+  
+  strict_isolation => "Don't allow users to assign, be qa-contacts or add to CC list " _
+                      "any user that do not have permission to edit the bug." }
 %]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 56fedbed3..4de46f958 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -665,6 +665,11 @@
     [% title = "Invalid regular expression" %]
     The regular expression you entered is invalid.
 
+  [% ELSIF error == "invalid_user_group" %]
+    [% title = "Invalid User Group" %]
+    User '[% user FILTER html %]' is not able to edit the 
+    [% terms.bug %] '[% bug_id FILTER html %]'.
+
   [% ELSIF error == "invalid_username" %]
     [% title = "Invalid Username" %]
     The name <tt>[% name FILTER html %]</tt> is not a valid username.