From c39803cc45e621f01e0598c7fb875f5e494ebd14 Mon Sep 17 00:00:00 2001
From: "bbaetz%student.usyd.edu.au" <>
Date: Sat, 9 Nov 2002 09:58:02 +0000
Subject: Bug 114696 - permission checking in queries not optimal Patch by
joel, dkl + me r=myk, a=justdave
---
Bugzilla/Search.pm | 54 +++++++++++++++++++-------------
CGI.pl | 7 +++--
buglist.cgi | 29 +++++++++++++++--
template/en/default/list/table.html.tmpl | 2 +-
4 files changed, 65 insertions(+), 27 deletions(-)
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index a7c329307..cea9294ca 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -63,7 +63,7 @@ sub init {
my @fields;
my @supptables;
my @wherepart;
- my @having = ("(cntuseringroups = cntbugingroups OR canseeanyway)");
+ my @having;
@fields = @$fieldsref if $fieldsref;
my @specialchart;
my @andlist;
@@ -910,26 +910,38 @@ sub init {
# Make sure we create a legal SQL query.
@andlist = ("1 = 1") if !@andlist;
-
- my $query = ("SELECT " . join(', ', @fields) .
- ", COUNT(DISTINCT ugmap.group_id) AS cntuseringroups, " .
- " COUNT(DISTINCT bgmap.group_id) AS cntbugingroups, " .
- " ((COUNT(DISTINCT ccmap.who) AND cclist_accessible) " .
- " OR ((bugs.reporter = $::userid) AND bugs.reporter_accessible) " .
- " OR bugs.assigned_to = $::userid ) AS canseeanyway " .
- " FROM $suppstring" .
- " LEFT JOIN bug_group_map AS bgmap " .
- " ON bgmap.bug_id = bugs.bug_id " .
- " LEFT JOIN user_group_map AS ugmap " .
- " ON bgmap.group_id = ugmap.group_id " .
- " AND ugmap.user_id = $::userid " .
- " AND ugmap.isbless = 0" .
- " LEFT JOIN cc AS ccmap " .
- " ON ccmap.who = $::userid AND ccmap.bug_id = bugs.bug_id " .
- " WHERE " . join(' AND ', (@wherepart, @andlist)) .
- " GROUP BY bugs.bug_id" .
- " HAVING " . join(" AND ", @having));
-
+
+ my $query = "SELECT " . join(', ', @fields) .
+ " FROM $suppstring" .
+ " LEFT JOIN bug_group_map " .
+ " ON bug_group_map.bug_id = bugs.bug_id ";
+
+ if (defined @{$::vars->{user}{groupids}} && @{$::vars->{user}{groupids}} > 0) {
+ $query .= " AND bug_group_map.group_id NOT IN (" . join(',', @{$::vars->{user}{groupids}}) . ") ";
+ }
+
+ if ($::vars->{user}{userid}) {
+ $query .= " LEFT JOIN cc ON cc.bug_id = bugs.bug_id AND cc.who = $::userid ";
+ }
+
+ $query .= " WHERE " . join(' AND ', (@wherepart, @andlist)) .
+ " AND ((bug_group_map.group_id IS NULL)";
+
+ if ($::vars->{user}{userid}) {
+ $query .= " OR (bugs.reporter_accessible = 1 AND bugs.reporter = $::userid) " .
+ " OR (bugs.cclist_accessible = 1 AND cc.who IS NOT NULL) " .
+ " OR (bugs.assigned_to = $::userid) ";
+ if (Param('useqacontact')) {
+ $query .= "OR (bugs.qa_contact = $::userid) ";
+ }
+ }
+
+ $query .= ") GROUP BY bugs.bug_id";
+
+ if (@having) {
+ $query .= " HAVING " . join(" AND ", @having);
+ }
+
if ($debug) {
print "
" . value_quote($query) . "
\n";
exit;
diff --git a/CGI.pl b/CGI.pl
index 2069d9235..9ff485265 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -280,6 +280,7 @@ sub GetUserInfo {
my %user;
my @queries;
my %groups;
+ my @groupids;
# No info if not logged in
return \%user if ($userid == 0);
@@ -304,16 +305,18 @@ sub GetUserInfo {
$user{'canblessany'} = UserCanBlessAnything();
- SendSQL("SELECT name FROM groups, user_group_map " .
+ SendSQL("SELECT DISTINCT id, name FROM groups, user_group_map " .
"WHERE groups.id = user_group_map.group_id " .
"AND user_id = $userid " .
"AND NOT isbless");
while (MoreSQLData()) {
- my ($name) = FetchSQLData();
+ my ($id, $name) = FetchSQLData();
+ push(@groupids,$id);
$groups{$name} = 1;
}
$user{'groups'} = \%groups;
+ $user{'groupids'} = \@groupids;
return \%user;
}
diff --git a/buglist.cgi b/buglist.cgi
index 1f91bd322..0f33bee7b 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -619,6 +619,7 @@ SendSQL($query);
my $bugowners = {};
my $bugproducts = {};
my $bugstatuses = {};
+my @bugidlist;
my @bugs; # the list of records
@@ -628,7 +629,7 @@ while (my @row = FetchSQLData()) {
# Slurp the row of data into the record.
# The second from last column in the record is the number of groups
# to which the bug is restricted.
- foreach my $column (@selectcolumns, 'dummy', 'groupset', 'dummy' ) {
+ foreach my $column (@selectcolumns) {
$bug->{$column} = shift @row;
}
@@ -645,8 +646,13 @@ while (my @row = FetchSQLData()) {
$bugproducts->{$bug->{'product'}} = 1 if $bug->{'product'};
$bugstatuses->{$bug->{'status'}} = 1 if $bug->{'status'};
+ $bug->{isingroups} = 0;
+
# Add the record to the list.
push(@bugs, $bug);
+
+ # Add id to list for checking for bug privacy later
+ push(@bugidlist, $bug->{id});
}
# Switch back from the shadow database to the regular database so PutFooter()
@@ -654,6 +660,23 @@ while (my @row = FetchSQLData()) {
# in the shadow database.
SendSQL("USE $::db_name");
+# Check for bug privacy and set $bug->{isingroups} = 1 if private
+# to 1 or more groups
+my %privatebugs;
+if (@bugidlist) {
+ SendSQL("SELECT DISTINCT bugs.bug_id FROM bugs, bug_group_map " .
+ "WHERE bugs.bug_id = bug_group_map.bug_id " .
+ "AND bugs.bug_id IN (" . join(',',@bugidlist) . ")");
+ while (MoreSQLData()) {
+ my ($id) = FetchSQLData();
+ $privatebugs{$id} = 1;
+ }
+ foreach my $bug (@bugs) {
+ if ($privatebugs{$bug->{id}}) {
+ $bug->{isingroups} = 1;
+ }
+ }
+}
################################################################################
# Template Variable Definition
@@ -662,7 +685,7 @@ SendSQL("USE $::db_name");
# Define the variables and functions that will be passed to the UI template.
$vars->{'bugs'} = \@bugs;
-$vars->{'buglist'} = join(',', map($_->{id}, @bugs));
+$vars->{'buglist'} = join(',', @bugidlist);
$vars->{'columns'} = $columns;
$vars->{'displaycolumns'} = \@displaycolumns;
@@ -767,7 +790,7 @@ if ($format->{'extension'} eq "html") {
my $qorder = url_quote($order);
print "Set-Cookie: LASTORDER=$qorder ; path=$cookiepath; expires=Sun, 30-Jun-2029 00:00:00 GMT\n";
}
- my $bugids = join(":", map( $_->{'id'}, @bugs));
+ my $bugids = join(":", @bugidlist);
# See also Bug 111999
if (length($bugids) < 4000) {
print "Set-Cookie: BUGLIST=$bugids ; path=$cookiepath; expires=Sun, 30-Jun-2029 00:00:00 GMT\n";
diff --git a/template/en/default/list/table.html.tmpl b/template/en/default/list/table.html.tmpl
index 8c79b5d40..1de7f4efa 100644
--- a/template/en/default/list/table.html.tmpl
+++ b/template/en/default/list/table.html.tmpl
@@ -128,7 +128,7 @@
[% tableheader %]
[% END %]
-