diff options
| -rw-r--r-- | docs/sgml/administration.sgml | 94 | ||||
| -rw-r--r-- | docs/xml/administration.xml | 94 | ||||
| -rwxr-xr-x | process_bug.cgi | 152 |
3 files changed, 300 insertions, 40 deletions
diff --git a/docs/sgml/administration.sgml b/docs/sgml/administration.sgml index f932beb25..a82a659bf 100644 --- a/docs/sgml/administration.sgml +++ b/docs/sgml/administration.sgml @@ -1176,6 +1176,100 @@ </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"> <title>Upgrading to New Releases</title> diff --git a/docs/xml/administration.xml b/docs/xml/administration.xml index f932beb25..a82a659bf 100644 --- a/docs/xml/administration.xml +++ b/docs/xml/administration.xml @@ -1176,6 +1176,100 @@ </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"> <title>Upgrading to New Releases</title> diff --git a/process_bug.cgi b/process_bug.cgi index ab65c0da5..be661c629 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -260,9 +260,34 @@ 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. +# +# 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 { - my ($f, $bugid, $oldvalue, $newvalue) = (@_); - if ($f eq "assigned_to" || $f eq "reporter" || $f eq "qa_contact") { + # START DO_NOT_CHANGE + 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 == 0) { $oldvalue = ""; @@ -271,64 +296,106 @@ sub CheckCanChangeField { } } } + + # Return true if they haven't changed this field at all. if ($oldvalue eq $newvalue) { return 1; - } - if (trim($oldvalue) eq trim($newvalue)) { + } + elsif (trim($oldvalue) eq trim($newvalue)) { 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, - return 1; # it'll flag it when "status" is checked. + # END DO_NOT_CHANGE + + # 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) { $UserInEditGroupSet = UserInGroup("editbugs"); } + + if ($UserInCanConfirmGroupSet < 0) { + $UserInCanConfirmGroupSet = UserInGroup("canconfirm"); + } + # END DO_NOT_CHANGE + + # Allow anyone with "editbugs" to change anything. if ($UserInEditGroupSet) { 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) { - SendSQL("SELECT reporter, assigned_to, qa_contact FROM bugs " . - "WHERE bug_id = $bugid"); + SendSQL("SELECT reporter, assigned_to, qa_contact FROM bugs + WHERE bug_id = $bugid"); ($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. - # While we're at it, let them close their own bugs as well. - if ( ($f eq "bug_status") && ($whoid eq $reporterid) ) { + + # Allow the QA contact to change anything. + if ($qacontactid eq $whoid) { return 1; } - if ($f eq "bug_status" && $newvalue ne $::unconfirmedstate && - IsOpenedState($newvalue)) { - - # Hmm. They are trying to set this bug to some opened state - # that isn't the UNCONFIRMED state. Are they in the right - # group? Or, has it ever been confirmed? If not, then this - # isn't legal. - - if ($UserInCanConfirmGroupSet < 0) { - $UserInCanConfirmGroupSet = UserInGroup("canconfirm"); - } - if ($UserInCanConfirmGroupSet) { - return 1; + + # The reporter's a more complicated case... + if ($reporterid eq $whoid) { + # Reporter may not: + # - confirm his own bugs (this assumes he doesn't have canconfirm, or we + # would have returned "1" earlier) + if (($field eq "bug_status") && + ($oldvalue eq $::unconfirmedstate) && + IsOpenedState($newvalue)) + { + return 0; } - SendSQL("SELECT everconfirmed FROM bugs WHERE bug_id = $bugid"); - my $everconfirmed = FetchOneColumn(); - if ($everconfirmed) { - return 1; + + # - change the target milestone + if ($field eq "target_milestone") { + 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; } - - # The user doesn't have the necessary permissions to change this field. - $vars->{'oldvalue'} = $oldvalue; - $vars->{'newvalue'} = $newvalue; - $vars->{'field'} = $f; - ThrowUserError("illegal_change", "abort"); + + # If we haven't returned by this point, then the user doesn't have the + # necessary permissions to change this field. + return 0; } # Confirm that the reporter of the current bug can access the bug we are duping to. @@ -989,7 +1056,12 @@ foreach my $id (@idlist) { $oldvalues[$i] ||= ''; $oldhash{$col} = $oldvalues[$i]; 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++; } |