From b54625a0ef75f691220cdb319ea2eea3ec27e04a Mon Sep 17 00:00:00 2001 From: "bugreport%peshkin.net" <> Date: Wed, 4 Aug 2004 23:17:09 +0000 Subject: Bug 186093: Move CanSeeBug to User.pm and make User.pm usable by templates r=kiko a=justdave --- Bugzilla/Bug.pm | 4 +-- Bugzilla/BugMail.pm | 4 +-- Bugzilla/Flag.pm | 4 +-- Bugzilla/FlagType.pm | 2 +- Bugzilla/User.pm | 80 +++++++++++++++++++++++++++++++++++++++++++++++++ CGI.pl | 2 +- globals.pl | 46 ++-------------------------- long_list.cgi | 2 +- process_bug.cgi | 5 ++-- showdependencygraph.cgi | 2 +- showdependencytree.cgi | 2 +- votes.cgi | 2 +- 12 files changed, 97 insertions(+), 58 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 2cac77ed3..01d2321c4 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -133,7 +133,7 @@ sub initBug { } } - $self->{'whoid'} = $user_id; + $self->{'who'} = new Bugzilla::User($user_id); my $query = " SELECT @@ -156,7 +156,7 @@ sub initBug { &::SendSQL($query); my @row = (); - if ((@row = &::FetchSQLData()) && &::CanSeeBug($bug_id, $self->{'whoid'})) { + if ((@row = &::FetchSQLData()) && $self->{'who'}->can_see_bug($bug_id)) { my $count = 0; my %fields; foreach my $field ("bug_id", "alias", "product_id", "product", "version", diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index 40a40dc2b..8731a9f72 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -720,7 +720,7 @@ sub NewProcessOnePerson ($$$$$$$$$$$$$) { # see the action of restricting the bug itself; the bug will just # quietly disappear from their radar. # - return unless CanSeeBug($id, $userid); + return unless $user->can_see_bug($id); # Drop any non-insiders if the comment is private return if (Param("insidergroup") && @@ -733,7 +733,7 @@ sub NewProcessOnePerson ($$$$$$$$$$$$$) { my $save_id = $dep_id; detaint_natural($dep_id) || warn("Unexpected Error: \@depbugs contains a non-numeric value: '$save_id'") && return; - return unless CanSeeBug($dep_id, $userid); + return unless $user->can_see_bug($dep_id); } my %mailhead = %defmailhead; diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index d4dac9053..3b2ae36c4 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -185,7 +185,7 @@ sub validate { my $requestee = Bugzilla::User->new_from_login($requestee_email); # Throw an error if the user can't see the bug. - if (!&::CanSeeBug($bug_id, $requestee->id)) + if (!$requestee->can_see_bug($bug_id)) { ThrowUserError("flag_requestee_unauthorized", { flag_type => $flag->{'type'}, @@ -592,7 +592,7 @@ sub notify { || next; next if $flag->{'target'}->{'bug'}->{'restricted'} - && !&::CanSeeBug($flag->{'target'}->{'bug'}->{'id'}, $ccuser->id); + && !$ccuser->can_see_bug($flag->{'target'}->{'bug'}->{'id'}); next if $flag->{'target'}->{'attachment'}->{'isprivate'} && Param("insidergroup") && !$ccuser->in_group(Param("insidergroup")); diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index f1cb00c5d..687a01768 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -226,7 +226,7 @@ sub validate { my $requestee = Bugzilla::User->new_from_login($requestee_email); # Throw an error if the user can't see the bug. - if (!&::CanSeeBug($bug_id, $requestee->id)) + if (!$requestee->can_see_bug($bug_id)) { ThrowUserError("flag_requestee_unauthorized", { flag_type => $flag_type, diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 8396d183f..66087b81c 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -244,6 +244,75 @@ sub in_group { return defined($res); } +sub can_see_bug { + my ($self, $bugid) = @_; + my $dbh = Bugzilla->dbh; + my $sth = $self->{sthCanSeeBug}; + my $userid = $self->{id}; + # Get fields from bug, presence of user on cclist, and determine if + # the user is missing any groups required by the bug. The prepared query + # is cached because this may be called for every row in buglists or + # every bug in a dependency list. + unless ($sth) { + $sth = $dbh->prepare("SELECT reporter, assigned_to, qa_contact, + reporter_accessible, cclist_accessible, + COUNT(cc.who), COUNT(bug_group_map.bug_id) + FROM bugs + LEFT JOIN cc + ON cc.bug_id = bugs.bug_id + AND cc.who = $userid + LEFT JOIN bug_group_map + ON bugs.bug_id = bug_group_map.bug_id + AND bug_group_map.group_ID NOT IN(" . + join(',',(-1, values(%{$self->groups}))) . + ") WHERE bugs.bug_id = ? GROUP BY bugs.bug_id"); + } + $sth->execute($bugid); + my ($reporter, $owner, $qacontact, $reporter_access, $cclist_access, + $isoncclist, $missinggroup) = $sth->fetchrow_array(); + $self->{sthCanSeeBug} = $sth; + return ( (($reporter == $userid) && $reporter_access) + || (Param('qacontact') && ($qacontact == $userid) && $userid) + || ($owner == $userid) + || ($isoncclist && $cclist_access) + || (!$missinggroup) ); +} + +sub get_selectable_products { + my ($self, $by_id) = @_; + + if (defined $self->{SelectableProducts}) { + my %list = @{$self->{SelectableProducts}}; + return \%list if $by_id; + return values(%list); + } + + my $query = "SELECT id, name " . + "FROM products " . + "LEFT JOIN group_control_map " . + "ON group_control_map.product_id = products.id "; + if (Param('useentrygroupdefault')) { + $query .= "AND group_control_map.entry != 0 "; + } else { + $query .= "AND group_control_map.membercontrol = " . + CONTROLMAPMANDATORY . " "; + } + $query .= "AND group_id NOT IN(" . + join(',', (-1,values(%{Bugzilla->user->groups}))) . ") " . + "WHERE group_id IS NULL ORDER BY name"; + my $dbh = Bugzilla->dbh; + my $sth = $dbh->prepare($query); + $sth->execute(); + my @products = (); + while (my @row = $sth->fetchrow_array) { + push(@products, @row); + } + $self->{SelectableProducts} = \@products; + my %list = @products; + return \%list if $by_id; + return values(%list); +} + # visible_groups_inherited returns a reference to a list of all the groups # whose members are visible to this user. sub visible_groups_inherited { @@ -939,6 +1008,10 @@ intended for cases where we are not looking at the currently logged in user, and only need to make a quick check for the group, where calling C and getting all of the groups would be overkill. +=item C + +Determines if the user can see the specified bug. + =item C Bugzilla allows for group inheritance. When data about the user (or any of the @@ -947,6 +1020,13 @@ care of by the constructor. However, when updating the email address, the user may be placed into different groups, based on a new email regexp. This method should be called in such a case to force reresolution of these groups. +=item C + +Returns an alphabetical list of product names from which +the user can select bugs. If the $by_id parameter is true, it returns +a hash where the keys are the product ids and the values are the +product names. + =item C Returns a list of all groups whose members should be visible to this user. diff --git a/CGI.pl b/CGI.pl index 5be0261d0..4f5b79f72 100644 --- a/CGI.pl +++ b/CGI.pl @@ -172,7 +172,7 @@ sub ValidateBugID { return if $skip_authorization; - return if CanSeeBug($id, $::userid); + return if Bugzilla->user->can_see_bug($id); # The user did not pass any of the authorization tests, which means they # are not authorized to see the bug. Display an error and stop execution. diff --git a/globals.pl b/globals.pl index d1680959e..9872dff70 100644 --- a/globals.pl +++ b/globals.pl @@ -630,48 +630,6 @@ sub GetFieldDefs { } -sub CanSeeBug { - - my ($id, $userid) = @_; - - # Query the database for the bug, retrieving a boolean value that - # represents whether or not the user is authorized to access the bug. - - # if no groups are found --> user is permitted to access - # if no user is found for any group --> user is not permitted to access - my $query = "SELECT bugs.bug_id, reporter, assigned_to, qa_contact," . - " reporter_accessible, cclist_accessible," . - " cc.who IS NOT NULL," . - " COUNT(DISTINCT(bug_group_map.group_id)) as cntbugingroups," . - " COUNT(DISTINCT(user_group_map.group_id)) as cntuseringroups" . - " FROM bugs" . - " LEFT JOIN cc ON bugs.bug_id = cc.bug_id" . - " AND cc.who = $userid" . - " LEFT JOIN bug_group_map ON bugs.bug_id = bug_group_map.bug_id" . - " LEFT JOIN user_group_map ON" . - " user_group_map.group_id = bug_group_map.group_id" . - " AND user_group_map.isbless = 0" . - " AND user_group_map.user_id = $userid" . - " WHERE bugs.bug_id = $id GROUP BY bugs.bug_id"; - PushGlobalSQLState(); - SendSQL($query); - my ($found_id, $reporter, $assigned_to, $qa_contact, - $rep_access, $cc_access, - $found_cc, $found_groups, $found_members) - = FetchSQLData(); - PopGlobalSQLState(); - return ( - ($found_groups == 0) - || (($userid > 0) && - ( - ($assigned_to == $userid) - || (Param('useqacontact') && $qa_contact == $userid) - || (($reporter == $userid) && $rep_access) - || ($found_cc && $cc_access) - || ($found_groups == $found_members) - )) - ); -} sub ValidatePassword { # Determines whether or not a password is valid (i.e. meets Bugzilla's @@ -947,7 +905,7 @@ sub GetAttachmentLink { my ($bugid, $isobsolete, $desc) = FetchSQLData(); my $title = ""; my $className = ""; - if (CanSeeBug($bugid, $::userid)) { + if (Bugzilla->user->can_see_bug($bugid)) { $title = $desc; } if ($isobsolete) { @@ -1018,7 +976,7 @@ sub GetBugLink { $title .= " $bug_res"; $post = ''; } - if (CanSeeBug($bug_num, $::userid)) { + if (Bugzilla->user->can_see_bug($bug_num)) { $title .= " - $bug_desc"; } $::buglink{$bug_num} = [$pre, value_quote($title), $post]; diff --git a/long_list.cgi b/long_list.cgi index 5644a5323..757d00239 100755 --- a/long_list.cgi +++ b/long_list.cgi @@ -75,7 +75,7 @@ my @bugs; foreach my $bug_id (split(/[:,]/, $buglist)) { detaint_natural($bug_id) || next; - CanSeeBug($bug_id, $::userid) || next; + Bugzilla->user->can_see_bug($bug_id) || next; SendSQL("$generic_query AND bugs.bug_id = $bug_id"); my %bug; diff --git a/process_bug.cgi b/process_bug.cgi index b5d641f77..2810d3b39 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -493,8 +493,9 @@ sub DuplicateUserConfirm { SendSQL("SELECT reporter FROM bugs WHERE bug_id = " . SqlQuote($dupe)); my $reporter = FetchOneColumn(); + my $rep_user = Bugzilla::User->new($reporter); - if (CanSeeBug($original, $reporter)) { + if ($rep_user->can_see_bug($original)) { $::FORM{'confirm_add_duplicate'} = "1"; return; } @@ -1773,7 +1774,7 @@ foreach my $id (@idlist) { # now show the next bug if ($next_bug) { - if (detaint_natural($next_bug) && CanSeeBug($next_bug, $::userid)) { + if (detaint_natural($next_bug) && Bugzilla->user->can_see_bug($next_bug)) { my $bug = new Bugzilla::Bug($next_bug, $::userid); ThrowCodeError("bug_error", { bug => $bug }) if $bug->error; diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi index b11562e1e..0d33d316d 100755 --- a/showdependencygraph.cgi +++ b/showdependencygraph.cgi @@ -170,7 +170,7 @@ foreach my $k (keys(%seen)) { $summary ||= ''; # Resolution and summary are shown only if user can see the bug - if (!CanSeeBug($k, $::userid)) { + if (!Bugzilla->user->can_see_bug($k)) { $resolution = $summary = ''; } diff --git a/showdependencytree.cgi b/showdependencytree.cgi index 202043acd..f1a495a6d 100755 --- a/showdependencytree.cgi +++ b/showdependencytree.cgi @@ -146,7 +146,7 @@ sub GetBug { my ($id) = @_; my $bug = {}; - if (CanSeeBug($id, $::userid)) { + if (Bugzilla->user->can_see_bug($id)) { SendSQL("SELECT 1, bug_status, short_desc, diff --git a/votes.cgi b/votes.cgi index 8bd007d60..f2590b324 100755 --- a/votes.cgi +++ b/votes.cgi @@ -185,7 +185,7 @@ sub show_user { # and they can see there are votes 'missing', but not on what bug # they are. This seems a reasonable compromise; the alternative is # to lie in the totals. - next if !CanSeeBug($id, $userid); + next if !Bugzilla->user->can_see_bug($id); push (@bugs, { id => $id, summary => $summary, -- cgit v1.2.3-24-g4f1b