From: Kohei Yoshino <kohei.yoshino@gmail.com>
Date: Mon, 24 Feb 2020 19:50:30 +0000 (-0500)
Subject: Bug 1591549 - Hide bugs in dependencies and regression fields from users without... 
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=778bdcf283411d88fd265da8d664a61d61e32316;p=thirdparty%2Fbugzilla.git

Bug 1591549 - Hide bugs in dependencies and regression fields from users without access
---

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index dd5763da9..e18d6da12 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -4514,9 +4514,12 @@ sub list_relationship {
             ORDER BY is_open DESC, $target_field"
   );
 
-  return $dbh->selectcol_arrayref(
+  my $bug_ids = $dbh->selectcol_arrayref(
     $cache->{"${target_field}_sth_$exclude_resolved"},
     undef, $bug_id);
+
+  # List only bugs visible to the user
+  return Bugzilla->user->visible_bugs(\@$bug_ids);
 }
 
 # Creates a lot of bug objects in the same order as the input array.
@@ -4546,7 +4549,8 @@ sub _bugs_in_order {
 # This routine assumes Bugzilla::Bug->check has been previously called.
 sub GetBugActivity {
   my ($bug_id, $attach_id, $starttime, $include_comment_activity) = @_;
-  my $dbh = Bugzilla->dbh;
+  my $dbh  = Bugzilla->dbh;
+  my $user = Bugzilla->user;
 
   # Arguments passed to the SQL query.
   my @args = ($bug_id);
@@ -4560,7 +4564,7 @@ sub GetBugActivity {
   # Only includes attachments the user is allowed to see.
   my $suppjoins = "";
   my $suppwhere = "";
-  if (!Bugzilla->user->is_insider) {
+  if (!$user->is_insider) {
     $suppjoins = "LEFT JOIN attachments
                    ON attachments.attach_id = bugs_activity.attach_id";
     $suppwhere = "AND COALESCE(attachments.isprivate, 0) = 0";
@@ -4601,7 +4605,7 @@ sub GetBugActivity {
     # Only includes comment tag activity for comments the user is allowed to see.
     $suppjoins = "";
     $suppwhere = "";
-    if (!Bugzilla->user->is_insider) {
+    if (!$user->is_insider) {
       $suppjoins = "INNER JOIN longdescs
                           ON longdescs.comment_id = longdescs_tags_activity.comment_id";
       $suppwhere = "AND longdescs.isprivate = 0";
@@ -4683,10 +4687,10 @@ sub GetBugActivity {
       || $fieldname eq 'work_time'
       || $fieldname eq 'deadline')
     {
-      $activity_visible = Bugzilla->user->is_timetracker;
+      $activity_visible = $user->is_timetracker;
     }
     elsif ($fieldname eq 'longdescs.isprivate'
-      && !Bugzilla->user->is_insider
+      && !$user->is_insider
       && $added)
     {
       $activity_visible = 0;
@@ -4734,6 +4738,13 @@ sub GetBugActivity {
         $added = _join_activity_entries($fieldname, $old_change->{'added'}, $added);
       }
 
+      # List only bugs visible to the user
+      if ($fieldname =~ /^(?:dependson|blocked|regress(?:ed_by|es))$/) {
+        $removed = join(', ', @{$user->visible_bugs([split(/,\s*/, $removed)])});
+        $added   = join(', ', @{$user->visible_bugs([split(/,\s*/, $added)])});
+        next if !$removed && !$added;
+      }
+
       $operation->{'who'}       = $who;
       $operation->{'when'}      = $when;
       $operation->{'fieldname'} = $change{'fieldname'} = $fieldname;
diff --git a/qa/t/lib/QA/RPC.pm b/qa/t/lib/QA/RPC.pm
index 8c82a39b7..92fc755de 100644
--- a/qa/t/lib/QA/RPC.pm
+++ b/qa/t/lib/QA/RPC.pm
@@ -149,13 +149,14 @@ sub _string_array {
 }
 
 sub bz_create_test_bugs {
-  my ($self, $second_private) = @_;
+  my ($self, $second_private, $no_cc) = @_;
   my $config = $self->bz_config;
 
   my @whiteboard_strings = _string_array(3);
   my @summary_strings    = _string_array(3);
 
   my $public_bug = create_bug_fields($config);
+  delete $public_bug->{cc} if $no_cc;
   $public_bug->{alias}      = random_string(40);
   $public_bug->{whiteboard} = join(' ', @whiteboard_strings);
   $public_bug->{summary}    = join(' ', @summary_strings);
diff --git a/qa/t/test_security.t b/qa/t/test_security.t
index 1919912bd..dd1d1bef0 100644
--- a/qa/t/test_security.t
+++ b/qa/t/test_security.t
@@ -139,13 +139,15 @@ log_in($sel, $config, 'editbugs');
 go_to_bug($sel, $bug1_id);
 ok(!$sel->is_text_present("secret_qa_bug_$bug2_id"),
   "The alias 'secret_qa_bug_$bug2_id' is not visible for unauthorized users");
-$sel->is_text_present_ok($bug2_id);
+ok(!$sel->is_text_present($bug2_id),
+  "Even the bug ID is not visible for unauthorized users");
 logout($sel);
 
 go_to_bug($sel, $bug1_id, 1);
 ok(!$sel->is_text_present("secret_qa_bug_$bug2_id"),
   "The alias 'secret_qa_bug_$bug2_id' is not visible for logged out users");
-$sel->is_text_present_ok($bug2_id);
+ok(!$sel->is_text_present($bug2_id),
+  "Even the bug ID is not visible for logged out users");
 
 #######################################################################
 # Security bug 472206.
diff --git a/qa/t/webservice_bug_get.t b/qa/t/webservice_bug_get.t
index 5a6918a14..e02ab0f5d 100644
--- a/qa/t/webservice_bug_get.t
+++ b/qa/t/webservice_bug_get.t
@@ -16,12 +16,12 @@ use Data::Dumper;
 use DateTime;
 use QA::Util;
 use QA::Tests qw(bug_tests PRIVATE_BUG_USER);
-use Test::More tests => 1036;
+use Test::More tests => 1009;
 my ($config, @clients) = get_rpc_clients();
 
 my $xmlrpc = $clients[0];
 our $creation_time = DateTime->now();
-our ($public_bug, $private_bug) = $xmlrpc->bz_create_test_bugs('private');
+our ($public_bug, $private_bug) = $xmlrpc->bz_create_test_bugs('private', 'no_cc');
 my $private_id = $private_bug->{id};
 my $public_id  = $public_bug->{id};
 
@@ -67,7 +67,6 @@ $private_bug->{see_also}              = [
 $private_bug->{cf_qa_status} = ['in progress', 'verified'];
 $private_bug->{cf_single_select} = 'two';
 
-$public_bug->{depends_on}            = [$private_id];
 $public_bug->{dupe_of}               = undef;
 $public_bug->{resolution}            = '';
 $public_bug->{is_open}               = 1;
@@ -108,6 +107,8 @@ sub post_success {
 
   is(scalar @{$call->result->{bugs}}, 1, "Got exactly one bug");
   my $bug = $call->result->{bugs}->[0];
+  my $is_private_bug  = $bug->{id} == $private_bug->{id};
+  my $is_private_user = $t->{user} && $t->{user} eq PRIVATE_BUG_USER;
 
   if ($t->{user} && $t->{user} eq 'admin') {
     ok(exists $bug->{estimated_time} && exists $bug->{remaining_time},
@@ -123,6 +124,18 @@ sub post_success {
     );
   }
 
+  if (exists $bug->{depends_on}) {
+    is_deeply(
+      $bug->{depends_on},
+      $is_private_bug ? [] : $is_private_user ? [$private_id] : [],
+      $is_private_bug
+        ? 'depends_on value is correct'
+        : $is_private_user
+        ? 'Private bug ID in depends_on is returned to private bug user'
+        : 'Private bug ID in depends_on is not returned to non-private bug user (' . $t->{user} . ')'
+    );
+  }
+
   if ($t->{user}) {
     ok($bug->{update_token}, 'Update token returned for logged-in user');
   }
@@ -131,7 +144,7 @@ sub post_success {
       'Update token not returned for logged-out users');
   }
 
-  my $expect = $bug->{id} == $private_bug->{id} ? $private_bug : $public_bug;
+  my $expect = $is_private_bug ? $private_bug : $public_bug;
 
   my @fields = sort keys %$expect;
   push(@fields, 'creation_time', 'last_change_time');
diff --git a/qa/t/webservice_bug_get_bugs.t b/qa/t/webservice_bug_get_bugs.t
index cdb261f36..9dda170b8 100644
--- a/qa/t/webservice_bug_get_bugs.t
+++ b/qa/t/webservice_bug_get_bugs.t
@@ -16,12 +16,12 @@ use Data::Dumper;
 use DateTime;
 use QA::Util;
 use QA::Tests qw(bug_tests PRIVATE_BUG_USER);
-use Test::More tests => 1036;
+use Test::More tests => 1009;
 my ($config, @clients) = get_rpc_clients();
 
 my $xmlrpc = $clients[0];
 our $creation_time = DateTime->now();
-our ($public_bug, $private_bug) = $xmlrpc->bz_create_test_bugs('private');
+our ($public_bug, $private_bug) = $xmlrpc->bz_create_test_bugs('private', 'no_cc');
 my $private_id = $private_bug->{id};
 my $public_id  = $public_bug->{id};
 
@@ -59,7 +59,6 @@ $private_bug->{see_also}         = ["${base_url}show_bug.cgi?id=$public_id"];
 $private_bug->{cf_qa_status}     = ['in progress', 'verified'];
 $private_bug->{cf_single_select} = 'two';
 
-$public_bug->{depends_on}            = [$private_id];
 $public_bug->{dupe_of}               = undef;
 $public_bug->{resolution}            = '';
 $public_bug->{is_open}               = 1;
@@ -98,6 +97,8 @@ sub post_success {
 
   is(scalar @{$call->result->{bugs}}, 1, "Got exactly one bug");
   my $bug = $call->result->{bugs}->[0];
+  my $is_private_bug  = $bug->{id} == $private_bug->{id};
+  my $is_private_user = $t->{user} && $t->{user} eq PRIVATE_BUG_USER;
 
   if ($t->{user} && $t->{user} eq 'admin') {
     ok(
@@ -120,6 +121,18 @@ sub post_success {
     );
   }
 
+  if (exists $bug->{depends_on}) {
+    is_deeply(
+      $bug->{depends_on},
+      $is_private_bug ? [] : $is_private_user ? [$private_id] : [],
+      $is_private_bug
+        ? 'depends_on value is correct'
+        : $is_private_user
+        ? 'Private bug ID in depends_on is returned to private bug user'
+        : 'Private bug ID in depends_on is not returned to non-private bug user (' . $t->{user} . ')'
+    );
+  }
+
   if ($t->{user}) {
     ok($bug->{update_token}, 'Update token returned for logged-in user');
   }
@@ -128,7 +141,7 @@ sub post_success {
       'Update token not returned for logged-out users');
   }
 
-  my $expect = $bug->{id} == $private_bug->{id} ? $private_bug : $public_bug;
+  my $expect = $is_private_bug ? $private_bug : $public_bug;
 
   my @fields = sort keys %$expect;
   push(@fields, 'creation_time', 'last_change_time');