From 1f9c83ae81c5c81d005fa0d9a428e23ea5126576 Mon Sep 17 00:00:00 2001 From: "bugreport%peshkin.net" <> Date: Tue, 18 Oct 2005 04:19:00 +0000 Subject: 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 r=joel, a=justdave --- Bugzilla/Bug.pm | 11 ++++++ Bugzilla/Config/GroupSecurity.pm | 6 +++ Bugzilla/User.pm | 25 ++++++++++++ attachment.cgi | 2 +- globals.pl | 21 ---------- process_bug.cgi | 45 ++++++++++++++++++++-- .../default/admin/params/groupsecurity.html.tmpl | 5 ++- template/en/default/global/user-error.html.tmpl | 5 +++ 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 + +Determines if, given a product id, the user can edit bugs in this product +at all. + =item C 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 [% name FILTER html %] is not a valid username. -- cgit v1.2.3-24-g4f1b