Fix for bug 179510: takes group restrictions into account when sending request notifications

r=bbaetz,jpreed
a=justdave

4 files changed, 150 insertions, 18 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index 322a3b2ba..ea5cd531c 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -48,10 +48,12 @@ sub new {
     bless($self, $class);
     
     &::PushGlobalSQLState();
-    &::SendSQL("SELECT 1, description, bug_id FROM attachments " . 
+    &::SendSQL("SELECT 1, description, bug_id, isprivate FROM attachments " . 
                "WHERE attach_id = $id");
-    ($self->{'exists'}, $self->{'summary'}, $self->{'bug_id'}) = 
-      &::FetchSQLData();
+    ($self->{'exists'},
+     $self->{'summary'},
+     $self->{'bug_id'},
+     $self->{'isprivate'}) = &::FetchSQLData();
     &::PopGlobalSQLState();
 
     return $self;
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index 41cc18071..a327f2922 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -33,9 +33,12 @@ use Bugzilla::FlagType;
 use Bugzilla::User;
 use Bugzilla::Config;
 use Bugzilla::Util;
+use Bugzilla::Error;
 
 use Attachment;
 
+use constant TABLES_ALREADY_LOCKED => 1;
+
 # Note that this line doesn't actually import these variables for some reason,
 # so I have to use them as $::template and $::vars in the package code.
 use vars qw($template $vars); 
@@ -135,8 +138,8 @@ sub count {
 
 sub validate {
     # Validates fields containing flag modifications.
-    
-    my ($data) = @_;
+
+    my ($data, $bug_id) = @_;
   
     # Get a list of flags to validate.  Uses the "map" function
     # to extract flag IDs from form field names by matching fields
@@ -152,13 +155,62 @@ sub validate {
         my $flag = get($id);
         $flag || &::ThrowCodeError("flag_nonexistent", { id => $id });
 
-        # Don't bother validating flags the user didn't change.
-        next if $status eq $flag->{'status'};
-
         # Make sure the user chose a valid status.
         grep($status eq $_, qw(X + - ?))
           || &::ThrowCodeError("flag_status_invalid", 
                                { id => $id , status => $status });
+                
+        # Make sure the user didn't request the flag unless it's requestable.
+        if ($status eq '?' && !$flag->{type}->{is_requestable}) {
+            ThrowCodeError("flag_status_invalid", 
+                              { id => $id , status => $status });
+        }
+        
+        # Make sure the requestee is authorized to access the bug.
+        # (and attachment, if this installation is using the "insider group"
+        # feature and the attachment is marked private).
+        if ($status eq '?'
+            && $flag->{type}->{is_requesteeble}
+            && trim($data->{"requestee-$id"}))
+        {
+            my $requestee_email = trim($data->{"requestee-$id"});
+            if ($requestee_email ne $flag->{'requestee'}->{'email'}) {
+                # We know the requestee exists because we ran
+                # Bugzilla::User::match_field before getting here.
+                # ConfirmGroup makes sure their group settings
+                # are up-to-date or calls DeriveGroups to update them.
+                my $requestee_id = &::DBname_to_id($requestee_email);
+                &::ConfirmGroup($requestee_id);
+
+                # Throw an error if the user can't see the bug.
+                if (!&::CanSeeBug($bug_id, $requestee_id))
+                {
+                    ThrowUserError("flag_requestee_unauthorized",
+                                   { flag_type => $flag->{'type'},
+                                     requestee =>
+                                       new Bugzilla::User($requestee_id),
+                                     bug_id => $bug_id,
+                                     attach_id =>
+                                       $flag->{target}->{attachment}->{id} });
+                }
+
+                # Throw an error if the target is a private attachment and
+                # the requestee isn't in the group of insiders who can see it.
+                if ($flag->{target}->{attachment}->{exists}
+                    && $data->{'isprivate'}
+                    && &::Param("insidergroup")
+                    && !&::UserInGroup(&::Param("insidergroup"), $requestee_id))
+                {
+                    ThrowUserError("flag_requestee_unauthorized_attachment",
+                                   { flag_type => $flag->{'type'},
+                                     requestee =>
+                                       new Bugzilla::User($requestee_id),
+                                     bug_id    => $bug_id,
+                                     attach_id =>
+                                      $flag->{target}->{attachment}->{id} });
+                }
+            }
+        }
     }
 }
 
@@ -457,14 +509,17 @@ sub GetBug {
     # Save the currently running query (if any) so we do not overwrite it.
     &::PushGlobalSQLState();
 
-    &::SendSQL("SELECT  1, short_desc, product_id, component_id
-                  FROM  bugs
-                 WHERE  bug_id = $id");
+    &::SendSQL("SELECT    1, short_desc, product_id, component_id,
+                          COUNT(bug_group_map.group_id)
+                FROM      bugs LEFT JOIN bug_group_map
+                            ON (bugs.bug_id = bug_group_map.bug_id)
+                WHERE     bugs.bug_id = $id
+                GROUP BY  bugs.bug_id");
 
     my $bug = { 'id' => $id };
     
     ($bug->{'exists'}, $bug->{'summary'}, $bug->{'product_id'}, 
-     $bug->{'component_id'}) = &::FetchSQLData();
+     $bug->{'component_id'}, $bug->{'restricted'}) = &::FetchSQLData();
 
     # Restore the previously running query (if any).
     &::PopGlobalSQLState();
@@ -504,6 +559,28 @@ sub notify {
     
     my ($flag, $template_file) = @_;
     
+    # If the target bug is restricted to one or more groups, then we need
+    # to make sure we don't send email about it to unauthorized users
+    # on the request type's CC: list, so we have to trawl the list for users
+    # not in those groups or email addresses that don't have an account.
+    if ($flag->{'target'}->{'bug'}->{'restricted'}
+        || $flag->{'target'}->{'attachment'}->{'isprivate'})
+    {
+        my @new_cc_list;
+        foreach my $cc (split(/[, ]+/, $flag->{'type'}->{'cc_list'})) {
+            my $user_id = &::DBname_to_id($cc) || next;
+            # re-derive permissions if necessary
+            &::ConfirmGroup($user_id, TABLES_ALREADY_LOCKED);
+            next if $flag->{'target'}->{'bug'}->{'restricted'}
+              && !&::CanSeeBug($flag->{'target'}->{'bug'}->{'id'}, $user_id);
+            next if $flag->{'target'}->{'attachment'}->{'isprivate'}
+              && Param("insidergroup")
+              && !&::UserInGroup(Param("insidergroup"), $user_id);
+            push(@new_cc_list, $cc);
+        }
+        $flag->{'type'}->{'cc_list'} = join(", ", @new_cc_list);
+    }
+
     $::vars->{'flag'} = $flag;
     
     my $message;
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 2e272f67c..523f60190 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -32,6 +32,9 @@ package Bugzilla::FlagType;
 # Use Bugzilla's User module which contains utilities for handling users.
 use Bugzilla::User;
 
+use Bugzilla::Error;
+use Bugzilla::Util;
+
 # Note!  This module requires that its caller have said "require CGI.pl" 
 # to import relevant functions from that script and its companion globals.pl.
 
@@ -177,9 +180,9 @@ sub count {
 }
 
 sub validate {
-    my ($data) = @_;
+    my ($data, $bug_id, $attach_id) = @_;
   
-    # Get a list of flags types to validate.  Uses the "map" function
+    # Get a list of flag types to validate.  Uses the "map" function
     # to extract flag type IDs from form field names by matching columns
     # whose name looks like "flag_type-nnn", where "nnn" is the ID,
     # and returning just the ID portion of matching field names.
@@ -192,14 +195,62 @@ sub validate {
         # Don't bother validating types the user didn't touch.
         next if $status eq "X";
 
-        # Make sure the flag exists.
-        get($id) 
+        # Make sure the flag type exists.
+        my $flag_type = get($id);
+        $flag_type 
           || &::ThrowCodeError("flag_type_nonexistent", { id => $id });
 
         # Make sure the value of the field is a valid status.
         grep($status eq $_, qw(X + - ?))
           || &::ThrowCodeError("flag_status_invalid", 
                                { id => $id , status => $status });
+                
+        # Make sure the user didn't request the flag unless it's requestable.
+        if ($status eq '?' && !$flag_type->{is_requestable}) {
+            ThrowCodeError("flag_status_invalid", 
+                           { id => $id , status => $status });
+        }
+        
+        # Make sure the requestee is authorized to access the bug
+        # (and attachment, if this installation is using the "insider group"
+        # feature and the attachment is marked private).
+        if ($status eq '?'
+            && $flag_type->{is_requesteeble}
+            && trim($data->{"requestee_type-$id"}))
+        {
+            my $requestee_email = trim($data->{"requestee_type-$id"});
+            my $requestee_id = &::DBname_to_id($requestee_email);
+
+            # We know the requestee exists because we ran
+            # Bugzilla::User::match_field before getting here.
+            # ConfirmGroup makes sure their group settings
+            # are up-to-date or calls DeriveGroups to update them.
+            &::ConfirmGroup($requestee_id);
+
+            # Throw an error if the user can't see the bug.
+            if (!&::CanSeeBug($bug_id, $requestee_id))
+            {
+                ThrowUserError("flag_requestee_unauthorized",
+                               { flag_type => $flag_type,
+                                 requestee => new Bugzilla::User($requestee_id),
+                                 bug_id    => $bug_id,
+                                 attach_id => $attach_id });
+            }
+            
+            # Throw an error if the target is a private attachment and
+            # the requestee isn't in the group of insiders who can see it.
+            if ($attach_id
+                && &::Param("insidergroup")
+                && $data->{'isprivate'}
+                && !&::UserInGroup(&::Param("insidergroup"), $requestee_id))
+            {
+                ThrowUserError("flag_requestee_unauthorized_attachment",
+                               { flag_type => $flag_type,
+                                 requestee => new Bugzilla::User($requestee_id),
+                                 bug_id    => $bug_id,
+                                 attach_id => $attach_id });
+            }
+        }
     }
 }
 
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 5aecb5ad9..511ba2592 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -129,8 +129,10 @@ sub min {
 
 sub trim {
     my ($str) = @_;
-    $str =~ s/^\s+//g;
-    $str =~ s/\s+$//g;
+    if ($str) {
+      $str =~ s/^\s+//g;
+      $str =~ s/\s+$//g;
+    }
     return $str;
 }