Skip to content

bugzilla
bugzilla
Commit 6fa5c6ec authored by gerv%gerv.net's avatar gerv%gerv.net
Browse files
Options

Bug 168804 - Document CheckCanChangeField so sites can modify it for local…

Bug 168804 - Document CheckCanChangeField so sites can modify it for local needs. Patch by gerv; r=bbaetz, joel.
parent 02b923a6
Hide whitespace changes
Inline Side-by-side
Showing with 300 additions and 40 deletions
docs/sgml/administration.sgml
View file @ 6fa5c6ec
...@@ -1176,6 +1176,100 @@ ...@@ -1176,6 +1176,100 @@
</section> </section>
<section id="cust-change-permissions">
<title>Change Permission Customisation</title>
<warning>
<para>
This feature should be considered experimental; the Bugzilla code you
will be changing is not stable, and could change or move between
versions. Be aware that if you make modifications to it, you may have
to re-make them or port them if Bugzilla changes internally between
versions.
</para>
</warning>
<para>
Companies often have rules about which employees, or classes of employees,
are allowed to change certain things in the bug system. For example,
only the bug's designated QA Contact may be allowed to VERIFY the bug.
Bugzilla has been
designed to make it easy for you to write your own custom rules to define
who is allowed to make what sorts of value transition.
</para>
<para>
For maximum flexibility, customising 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 grep for
"sub CheckCanChangeField", you'll find it.
</para>
<para>
This function has been carefully commented to allow you to see exactly
how it works, and give you an idea of how to make changes to it. Certain
marked sections should not be changed - these are the "plumbing" which
makes the rest of the function work. In between those sections, you'll
find snippets of code like:
<programlisting> # Allow the owner to change anything.
if ($ownerid eq $whoid) {
return 1;
}</programlisting>
It's fairly obvious what this piece of code does.
</para>
<para>
So, how does one go about changing this function? Well, simple changes
can be made just be removing pieces - for example, if you wanted to
prevent any user adding a comment to a bug, just remove the lines marked
"Allow anyone to change comments." And if you want the reporter to have
no special rights on bugs they have filed, just remove the entire section
which refers to him.
</para>
<para>
More complex customisations are not much harder. Basically, you add
a check in the right place in the function, i.e. after all the variables
you are using have been set up. So, don't look at $ownerid before
$ownerid has been obtained from the database. You can either add a
positive check, which returns 1 (allow) if certain conditions are true,
or a negative check, which returns 0 (deny.) E.g.:
<programlisting> if ($field eq "qacontact") {
if (UserInGroup("quality_assurance")) {
return 1;
}
else {
return 0;
}
}</programlisting>
This says that only users in the group "quality_assurance" can change
the QA Contact field of a bug. Getting more weird:
<programlisting> if (($field eq "priority") &&
($vars->{'user'}{'login'} =~ /.*\@example\.com$/))
{
if ($oldvalue eq "P1") {
return 1;
}
else {
return 0;
}
}</programlisting>
This says that if the user is trying to change the priority field,
and their email address is @example.com, they can only do so if the
old value of the field was "P1". Not very useful, but illustrative.
</para>
<para>
For a list of possible field names, look in
<filename>data/versioncache</filename> for the list called
<filename>@::log_columns</filename>. If you need help writing custom
rules for your organisation, ask in the newsgroup.
</para>
</section>
<section id="upgrading"> <section id="upgrading">
<title>Upgrading to New Releases</title> <title>Upgrading to New Releases</title>
......
docs/xml/administration.xml
View file @ 6fa5c6ec
...@@ -1176,6 +1176,100 @@ ...@@ -1176,6 +1176,100 @@
</section> </section>
<section id="cust-change-permissions">
<title>Change Permission Customisation</title>
<warning>
<para>
This feature should be considered experimental; the Bugzilla code you
will be changing is not stable, and could change or move between
versions. Be aware that if you make modifications to it, you may have
to re-make them or port them if Bugzilla changes internally between
versions.
</para>
</warning>
<para>
Companies often have rules about which employees, or classes of employees,
are allowed to change certain things in the bug system. For example,
only the bug's designated QA Contact may be allowed to VERIFY the bug.
Bugzilla has been
designed to make it easy for you to write your own custom rules to define
who is allowed to make what sorts of value transition.
</para>
<para>
For maximum flexibility, customising 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 grep for
"sub CheckCanChangeField", you'll find it.
</para>
<para>
This function has been carefully commented to allow you to see exactly
how it works, and give you an idea of how to make changes to it. Certain
marked sections should not be changed - these are the "plumbing" which
makes the rest of the function work. In between those sections, you'll
find snippets of code like:
<programlisting> # Allow the owner to change anything.
if ($ownerid eq $whoid) {
return 1;
}</programlisting>
It's fairly obvious what this piece of code does.
</para>
<para>
So, how does one go about changing this function? Well, simple changes
can be made just be removing pieces - for example, if you wanted to
prevent any user adding a comment to a bug, just remove the lines marked
"Allow anyone to change comments." And if you want the reporter to have
no special rights on bugs they have filed, just remove the entire section
which refers to him.
</para>
<para>
More complex customisations are not much harder. Basically, you add
a check in the right place in the function, i.e. after all the variables
you are using have been set up. So, don't look at $ownerid before
$ownerid has been obtained from the database. You can either add a
positive check, which returns 1 (allow) if certain conditions are true,
or a negative check, which returns 0 (deny.) E.g.:
<programlisting> if ($field eq "qacontact") {
if (UserInGroup("quality_assurance")) {
return 1;
}
else {
return 0;
}
}</programlisting>
This says that only users in the group "quality_assurance" can change
the QA Contact field of a bug. Getting more weird:
<programlisting> if (($field eq "priority") &&
($vars->{'user'}{'login'} =~ /.*\@example\.com$/))
{
if ($oldvalue eq "P1") {
return 1;
}
else {
return 0;
}
}</programlisting>
This says that if the user is trying to change the priority field,
and their email address is @example.com, they can only do so if the
old value of the field was "P1". Not very useful, but illustrative.
</para>
<para>
For a list of possible field names, look in
<filename>data/versioncache</filename> for the list called
<filename>@::log_columns</filename>. If you need help writing custom
rules for your organisation, ask in the newsgroup.
</para>
</section>
<section id="upgrading"> <section id="upgrading">
<title>Upgrading to New Releases</title> <title>Upgrading to New Releases</title>
......
process_bug.cgi
View file @ 6fa5c6ec
...@@ -260,9 +260,34 @@ my $ownerid; ...@@ -260,9 +260,34 @@ my $ownerid;
my $reporterid; my $reporterid;
my $qacontactid; 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.
#
# 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 { sub CheckCanChangeField {
my ($f, $bugid, $oldvalue, $newvalue) = (@_); # START DO_NOT_CHANGE
if ($f eq "assigned_to" || $f eq "reporter" || $f eq "qa_contact") { my ($field, $bugid, $oldvalue, $newvalue) = (@_);
# Convert email IDs into addresses for $oldvalue
if (($field eq "assigned_to") ||
($field eq "reporter") ||
($field eq "qa_contact"))
{
if ($oldvalue =~ /^\d+$/) { if ($oldvalue =~ /^\d+$/) {
if ($oldvalue == 0) { if ($oldvalue == 0) {
$oldvalue = ""; $oldvalue = "";
...@@ -271,64 +296,106 @@ sub CheckCanChangeField { ...@@ -271,64 +296,106 @@ sub CheckCanChangeField {
} }
} }
} }
# Return true if they haven't changed this field at all.
if ($oldvalue eq $newvalue) { if ($oldvalue eq $newvalue) {
return 1; return 1;
} }
if (trim($oldvalue) eq trim($newvalue)) { elsif (trim($oldvalue) eq trim($newvalue)) {
return 1; return 1;
} }
if ($f =~ /^longdesc/) {
return 1; # A resolution change is always accompanied by a status change. So, we
# always OK resolution changes; if they really can't do this, we will
# notice it when status is checked.
if ($field eq "resolution") {
return 1;
} }
if ($f eq "resolution") { # always OK this. if they really can't, # END DO_NOT_CHANGE
return 1; # it'll flag it when "status" is checked.
# Allow anyone to change comments.
if ($field =~ /^longdesc/) {
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) { if ($UserInEditGroupSet < 0) {
$UserInEditGroupSet = UserInGroup("editbugs"); $UserInEditGroupSet = UserInGroup("editbugs");
} }
if ($UserInCanConfirmGroupSet < 0) {
$UserInCanConfirmGroupSet = UserInGroup("canconfirm");
}
# END DO_NOT_CHANGE
# Allow anyone with "editbugs" to change anything.
if ($UserInEditGroupSet) { if ($UserInEditGroupSet) {
return 1; return 1;
} }
# Allow anyone with "canconfirm" to confirm bugs.
if (($field eq "bug_status") &&
($oldvalue eq $::unconfirmedstate) &&
IsOpenedState($newvalue) &&
$UserInCanConfirmGroupSet)
{
return 1;
}
# START DO_NOT_CHANGE
# $reporterid, $ownerid and $qacontactid are caches of the results of
# the call to find out the owner, reporter and qacontact of the current bug.
if ($lastbugid != $bugid) { if ($lastbugid != $bugid) {
SendSQL("SELECT reporter, assigned_to, qa_contact FROM bugs " . SendSQL("SELECT reporter, assigned_to, qa_contact FROM bugs
"WHERE bug_id = $bugid"); WHERE bug_id = $bugid");
($reporterid, $ownerid, $qacontactid) = (FetchSQLData()); ($reporterid, $ownerid, $qacontactid) = (FetchSQLData());
}
# END DO_NOT_CHANGE
# Allow the owner to change anything.
if ($ownerid eq $whoid) {
return 1;
} }
# Let reporter change bug status, even if they can't edit bugs.
# If reporter can't re-open their bug they will just file a duplicate. # Allow the QA contact to change anything.
# While we're at it, let them close their own bugs as well. if ($qacontactid eq $whoid) {
if ( ($f eq "bug_status") && ($whoid eq $reporterid) ) {
return 1; return 1;
} }
if ($f eq "bug_status" && $newvalue ne $::unconfirmedstate &&
IsOpenedState($newvalue)) { # The reporter's a more complicated case...
if ($reporterid eq $whoid) {
# Hmm. They are trying to set this bug to some opened state # Reporter may not:
# that isn't the UNCONFIRMED state. Are they in the right # - confirm his own bugs (this assumes he doesn't have canconfirm, or we
# group? Or, has it ever been confirmed? If not, then this # would have returned "1" earlier)
# isn't legal. if (($field eq "bug_status") &&
($oldvalue eq $::unconfirmedstate) &&
if ($UserInCanConfirmGroupSet < 0) { IsOpenedState($newvalue))
$UserInCanConfirmGroupSet = UserInGroup("canconfirm"); {
} return 0;
if ($UserInCanConfirmGroupSet) {
return 1;
} }
SendSQL("SELECT everconfirmed FROM bugs WHERE bug_id = $bugid");
my $everconfirmed = FetchOneColumn(); # - change the target milestone
if ($everconfirmed) { if ($field eq "target_milestone") {
return 1; return 0;
}
# - change the priority (unless he could have set it originally)
if (($field eq "priority") &&
!Param('letsubmitterchoosepriority'))
{
return 0;
} }
} elsif ($reporterid eq $whoid || $ownerid eq $whoid ||
$qacontactid eq $whoid) { # Allow reporter to change anything else.
return 1; return 1;
} }
# The user doesn't have the necessary permissions to change this field. # If we haven't returned by this point, then the user doesn't have the
$vars->{'oldvalue'} = $oldvalue; # necessary permissions to change this field.
$vars->{'newvalue'} = $newvalue; return 0;
$vars->{'field'} = $f;
ThrowUserError("illegal_change", "abort");
} }
# Confirm that the reporter of the current bug can access the bug we are duping to. # Confirm that the reporter of the current bug can access the bug we are duping to.
...@@ -989,7 +1056,12 @@ foreach my $id (@idlist) { ...@@ -989,7 +1056,12 @@ foreach my $id (@idlist) {
$oldvalues[$i] ||= ''; $oldvalues[$i] ||= '';
$oldhash{$col} = $oldvalues[$i]; $oldhash{$col} = $oldvalues[$i];
if (exists $::FORM{$col}) { if (exists $::FORM{$col}) {
CheckCanChangeField($col, $id, $oldvalues[$i], $::FORM{$col}); if (!CheckCanChangeField($col, $id, $oldvalues[$i], $::FORM{$col})) {
$vars->{'oldvalue'} = $oldvalues[$i];
$vars->{'newvalue'} = $::FORM{$col};
$vars->{'field'} = $col;
ThrowUserError("illegal_change", "abort");
}
} }
$i++; $i++;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment