summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rwxr-xr-xBugzilla/Bug.pm134
-rw-r--r--docs/xml/customization.xml10
-rwxr-xr-xprocess_bug.cgi211
3 files changed, 168 insertions, 187 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 8b6302b92..91a92cbe2 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -100,7 +100,7 @@ sub initBug {
my ($bug_id, $user_id) = (@_);
my $dbh = Bugzilla->dbh;
- $bug_id = trim($bug_id);
+ $bug_id = trim($bug_id || 0);
my $old_bug_id = $bug_id;
@@ -1166,6 +1166,138 @@ sub CheckIfVotedConfirmed {
return $ret;
}
+################################################################################
+# check_can_change_field() defines what users are allowed to change. 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.
+#
+# check_can_change_field() returns true if the user is allowed to change this
+# field, and false if they are not.
+#
+# The parameters to this method are as follows:
+# $field - name of the field in the bugs table the user is trying to change
+# $oldvalue - what they are changing it from
+# $newvalue - what they are changing it to
+# $PrivilegesRequired - return the reason of the failure, if any
+# $data - hash containing relevant parameters, e.g. from the CGI object
+################################################################################
+sub check_can_change_field {
+ my $self = shift;
+ my ($field, $oldvalue, $newvalue, $PrivilegesRequired, $data) = (@_);
+ my $user = $self->{'who'};
+
+ $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 $data->{'dontchange'}
+ && $oldvalue == $newvalue)
+ {
+ return 1;
+ }
+
+ # 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'
+ && $data->{'knob'} ne 'reassignbycomponent'
+ && $data->{'knob'} ne 'reassign')
+ {
+ return 1;
+ }
+
+ # 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 ($user->in_group('editbugs')) {
+ 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 $user->in_group('canconfirm');
+ }
+
+ # Make sure that a valid bug ID has been given.
+ if (!$self->{'error'}) {
+ # Allow the assignee to change anything else.
+ if ($self->{'assigned_to_id'} == $user->id) {
+ return 1;
+ }
+
+ # Allow the QA contact to change anything else.
+ if (Bugzilla->params->{'useqacontact'}
+ && $self->{'qa_contact_id'}
+ && ($self->{'qa_contact_id'} == $user->id))
+ {
+ 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 (!$self->{'error'} && $self->{'reporter_id'} == $user->id) {
+ 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;
+}
+
#
# Field Validation
#
diff --git a/docs/xml/customization.xml b/docs/xml/customization.xml
index aefacda67..4a832c614 100644
--- a/docs/xml/customization.xml
+++ b/docs/xml/customization.xml
@@ -673,11 +673,11 @@
<para>
For maximum flexibility, customizing this means editing Bugzilla's Perl
code. This gives the administrator complete control over exactly who is
- allowed to do what. The relevant function is called
- <filename>CheckCanChangeField()</filename>,
- and is found in <filename>process_bug.cgi</filename> in your
- Bugzilla directory. If you open that file and search for
- <quote>sub CheckCanChangeField</quote>, you'll find it.
+ allowed to do what. The relevant method is called
+ <filename>check_can_change_field()</filename>,
+ and is found in <filename>Bug.pm</filename> in your
+ Bugzilla/ directory. If you open that file and search for
+ <quote>sub check_can_change_field</quote>, you'll find it.
</para>
<para>
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";
generated by cgit v1.2.3-24-g4f1b (git 2.32.0) at 2024-11-26 21:33:44 +0100