From 1e087ef9416da23989f9134866586ea02df156da Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Sun, 2 Jul 2006 03:01:51 +0000 Subject: Bug 340278: Move CheckCanChangeField() from process_bug.cgi to Bug.pm - Patch by Frédéric Buclin r=mkanat a=justdave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- process_bug.cgi | 211 ++++++++------------------------------------------------ 1 file changed, 30 insertions(+), 181 deletions(-) (limited to 'process_bug.cgi') diff --git a/process_bug.cgi b/process_bug.cgi index ee263bfbf..645a076ce 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -41,11 +41,6 @@ use strict; -my $UserInEditGroupSet = -1; -my $UserInCanConfirmGroupSet = -1; -my $PrivilegesRequired = 0; -my $lastbugid = 0; - use lib qw(.); use Bugzilla; @@ -78,6 +73,7 @@ $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count(); my @editable_bug_fields = editable_bug_fields(); my $requiremilestone = 0; +my $PrivilegesRequired = 0; ###################################################################### # Subroutines @@ -143,6 +139,11 @@ if (defined $cgi->param('id')) { # Make sure there are bugs to process. scalar(@idlist) || ThrowUserError("no_bugs_chosen"); +# Build a bug object using $cgi->param('id') as ID. +# If there are more than one bug changed at once, the bug object will be +# empty, which doesn't matter. +my $bug = new Bugzilla::Bug(scalar $cgi->param('id'), $whoid); + # Make sure form param 'dontchange' is defined so it can be compared to easily. $cgi->param('dontchange','') unless defined $cgi->param('dontchange'); @@ -176,7 +177,6 @@ ValidateComment(scalar $cgi->param('comment')); # during validation. foreach my $field ("dependson", "blocked") { if ($cgi->param('id')) { - my $bug = new Bugzilla::Bug($cgi->param('id'), $user->id); my @old = @{$bug->$field}; my @new; foreach my $id (split(/[\s,]+/, $cgi->param($field))) { @@ -198,8 +198,9 @@ foreach my $field ("dependson", "blocked") { } } } - if ((@$added || @$removed) - && (!CheckCanChangeField($field, $bug->bug_id, 0, 1))) { + if ((@$added || @$removed) + && !$bug->check_can_change_field($field, 0, 1, \$PrivilegesRequired)) + { $vars->{'privs'} = $PrivilegesRequired; $vars->{'field'} = $field; ThrowUserError("illegal_change", $vars); @@ -307,8 +308,9 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct) && CheckonComment( "reassignbycomponent" )) { # Check to make sure they actually have the right to change the product - if (!CheckCanChangeField('product', scalar $cgi->param('id'), $oldproduct, - $cgi->param('product'))) { + if (!$bug->check_can_change_field('product', $oldproduct, $cgi->param('product'), + \$PrivilegesRequired)) + { $vars->{'oldvalue'} = $oldproduct; $vars->{'newvalue'} = $cgi->param('product'); $vars->{'field'} = 'product'; @@ -410,169 +412,6 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct) } } - -# Checks that the user is allowed to change the given field. Actually, right -# now, the rules are pretty simple, and don't look at the field itself very -# much, but that could be enhanced. - -my $ownerid; -my $reporterid; -my $qacontactid; - -################################################################################ -# CheckCanChangeField() defines what users are allowed to change what bugs. You -# can add code here for site-specific policy changes, according to the -# instructions given in the Bugzilla Guide and below. Note that you may also -# have to update the Bugzilla::Bug::user() function to give people access to the -# options that they are permitted to change. -# -# CheckCanChangeField() should return true if the user is allowed to change this -# field, and false if they are not. -# -# The parameters to this function are as follows: -# $field - name of the field in the bugs table the user is trying to change -# $bugid - the ID of the bug they are changing -# $oldvalue - what they are changing it from -# $newvalue - what they are changing it to -# -# Note that this function is currently not called for dependency changes -# (bug 141593) or CC changes, which means anyone can change those fields. -# -# Do not change the sections between START DO_NOT_CHANGE and END DO_NOT_CHANGE. -################################################################################ -sub CheckCanChangeField { - # START DO_NOT_CHANGE - my ($field, $bugid, $oldvalue, $newvalue) = (@_); - - $oldvalue = defined($oldvalue) ? $oldvalue : ''; - $newvalue = defined($newvalue) ? $newvalue : ''; - - # Return true if they haven't changed this field at all. - if ($oldvalue eq $newvalue) { - return 1; - } elsif (trim($oldvalue) eq trim($newvalue)) { - return 1; - # numeric fields need to be compared using == - } elsif (($field eq "estimated_time" || $field eq "remaining_time") - && $newvalue ne $cgi->param('dontchange') - && $oldvalue == $newvalue) - { - return 1; - } - # END DO_NOT_CHANGE - - # Allow anyone to change comments. - if ($field =~ /^longdesc/) { - return 1; - } - - # Ignore the assigned_to field if the bug is not being reassigned - if ($field eq "assigned_to" - && $cgi->param('knob') ne "reassignbycomponent" - && $cgi->param('knob') ne "reassign") - { - return 1; - } - - # START DO_NOT_CHANGE - # Find out whether the user is a member of the "editbugs" and/or - # "canconfirm" groups. $UserIn*GroupSet are caches of the return value of - # the UserInGroup calls. - if ($UserInEditGroupSet < 0) { - $UserInEditGroupSet = UserInGroup("editbugs"); - } - - if ($UserInCanConfirmGroupSet < 0) { - $UserInCanConfirmGroupSet = UserInGroup("canconfirm"); - } - # END DO_NOT_CHANGE - - # If the user isn't allowed to change a field, we must tell him who can. - # We store the required permission set into the $PrivilegesRequired - # variable which gets passed to the error template. - # - # $PrivilegesRequired = 0 : no privileges required; - # $PrivilegesRequired = 1 : the reporter, assignee or an empowered user; - # $PrivilegesRequired = 2 : the assignee or an empowered user; - # $PrivilegesRequired = 3 : an empowered user. - - # Allow anyone with "editbugs" privs to change anything. - if ($UserInEditGroupSet) { - return 1; - } - - # *Only* users with "canconfirm" privs can confirm bugs. - if ($field eq "canconfirm" - || ($field eq "bug_status" - && $oldvalue eq 'UNCONFIRMED' - && is_open_state($newvalue))) - { - $PrivilegesRequired = 3; - return $UserInCanConfirmGroupSet; - } - - # START DO_NOT_CHANGE - # $reporterid, $ownerid and $qacontactid are caches of the results of - # the call to find out the assignee, reporter and qacontact of the current bug. - if ($lastbugid != $bugid) { - ($reporterid, $ownerid, $qacontactid) = $dbh->selectrow_array( - q{SELECT reporter, assigned_to, qa_contact FROM bugs - WHERE bug_id = ? }, undef, $bugid); - $lastbugid = $bugid; - } - # END DO_NOT_CHANGE - - # Allow the assignee to change anything else. - if ($ownerid == $whoid) { - return 1; - } - - # Allow the QA contact to change anything else. - if (Param('useqacontact') && $qacontactid && ($qacontactid == $whoid)) { - return 1; - } - - # At this point, the user is either the reporter or an - # unprivileged user. We first check for fields the reporter - # is not allowed to change. - - # The reporter may not: - # - reassign bugs, unless the bugs are assigned to him; - # in that case we will have already returned 1 above - # when checking for the assignee of the bug. - if ($field eq "assigned_to") { - $PrivilegesRequired = 2; - return 0; - } - # - change the QA contact - if ($field eq "qa_contact") { - $PrivilegesRequired = 2; - return 0; - } - # - change the target milestone - if ($field eq "target_milestone") { - $PrivilegesRequired = 2; - return 0; - } - # - change the priority (unless he could have set it originally) - if ($field eq "priority" - && !Param('letsubmitterchoosepriority')) - { - $PrivilegesRequired = 2; - return 0; - } - - # The reporter is allowed to change anything else. - if ($reporterid == $whoid) { - return 1; - } - - # If we haven't returned by this point, then the user doesn't - # have the necessary permissions to change this field. - $PrivilegesRequired = 1; - return 0; -} - # Confirm that the reporter of the current bug can access the bug we are duping to. sub DuplicateUserConfirm { # if we've already been through here, then exit @@ -775,7 +614,7 @@ sub DoComma { # confirming the bug or not. my $everconfirmed; sub DoConfirm { - if (CheckCanChangeField("canconfirm", scalar $cgi->param('id'), 0, 1)) { + if ($bug->check_can_change_field("canconfirm", 0, 1, \$PrivilegesRequired)) { DoComma(); $::query .= "everconfirmed = 1"; $everconfirmed = 1; @@ -860,7 +699,9 @@ sub ChangeResolution { $dbh->selectrow_array('SELECT resolution FROM bugs WHERE bug_id = ?', undef, $bug_id); } - unless (CheckCanChangeField('resolution', $bug_id, $old_resolution, $str)) { + unless ($bug->check_can_change_field('resolution', $old_resolution, $str, + \$PrivilegesRequired)) + { $vars->{'oldvalue'} = $old_resolution; $vars->{'newvalue'} = $str; $vars->{'field'} = 'resolution'; @@ -873,7 +714,7 @@ sub ChangeResolution { trick_taint($str); push(@values, $str); # We define this variable here so that customized installations - # may set rules based on the resolution in CheckCanChangeField. + # may set rules based on the resolution in Bug::check_can_change_field(). $cgi->param('resolution', $str); } } @@ -1541,7 +1382,8 @@ foreach my $id (@idlist) { $oldvalues[$i] = defined($oldvalues[$i]) ? $oldvalues[$i] : ''; # Convert the deadline taken from the DB into the YYYY-MM-DD format # for consistency with the deadline provided by the user, if any. - # Else CheckCanChangeField() would see them as different in all cases. + # Else Bug::check_can_change_field() would see them as different + # in all cases. if ($col eq 'deadline') { $oldvalues[$i] = format_time($oldvalues[$i], "%Y-%m-%d"); } @@ -1562,12 +1404,18 @@ foreach my $id (@idlist) { $formhash{'bug_status'} = $oldhash{'bug_status'}; } } + # This hash is required by Bug::check_can_change_field(). + my $cgi_hash = { + 'dontchange' => scalar $cgi->param('dontchange'), + 'knob' => scalar $cgi->param('knob') + }; foreach my $col (@editable_bug_fields) { # The 'resolution' field is checked by ChangeResolution(), # i.e. only if we effectively use it. next if ($col eq 'resolution'); if (exists $formhash{$col} - && !CheckCanChangeField($col, $id, $oldhash{$col}, $formhash{$col})) + && !$old_bug_obj->check_can_change_field($col, $oldhash{$col}, $formhash{$col}, + \$PrivilegesRequired, $cgi_hash)) { my $vars; if ($col eq 'component_id') { @@ -1592,14 +1440,15 @@ foreach my $id (@idlist) { # 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 - # the list hasn't changed. To fix that, we have to call CheckCanChangeField + # Bug::check_can_change_field will fail to check permissions because it thinks + # the list hasn't changed. To fix that, we have to call Bug::check_can_change_field # again with old!=new if the keyword action is "delete" and old=new. if ($keywordaction eq "delete" && defined $cgi->param('keywords') && length(@keywordlist) > 0 && $cgi->param('keywords') eq $oldhash{keywords} - && !CheckCanChangeField("keywords", $id, "old is not", "equal to new")) + && !$old_bug_obj->check_can_change_field("keywords", "old is not", "equal to new", + \$PrivilegesRequired)) { $vars->{'oldvalue'} = $oldhash{keywords}; $vars->{'newvalue'} = "no keywords"; -- cgit v1.2.3-24-g4f1b