diff options
| -rw-r--r-- | Bugzilla/Bug.pm | 42 | ||||
| -rw-r--r-- | Bugzilla/Constants.pm | 14 | ||||
| -rw-r--r-- | Bugzilla/Hook.pm | 60 | ||||
| -rw-r--r-- | extensions/Example/Extension.pm | 39 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 4 |
5 files changed, 143 insertions, 16 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 9282bc3d2..c7c168125 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -49,7 +49,7 @@ use Bugzilla::Group; use Bugzilla::Status; use Bugzilla::Comment; -use List::Util qw(min); +use List::Util qw(min first); use Storable qw(dclone); use URI; use URI::QueryParam; @@ -3315,6 +3315,20 @@ sub check_can_change_field { return 1; } + my @priv_results; + Bugzilla::Hook::process('bug_check_can_change_field', + { bug => $self, field => $field, + new_value => $newvalue, old_value => $oldvalue, + priv_results => \@priv_results }); + if (my $priv_required = first { $_ > 0 } @priv_results) { + $$PrivilegesRequired = $priv_required; + return 0; + } + my $allow_found = first { $_ == 0 } @priv_results; + if (defined $allow_found) { + return 1; + } + # Allow anyone to change comments. if ($field =~ /^longdesc/) { return 1; @@ -3324,15 +3338,15 @@ sub check_can_change_field { # 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. + # $PrivilegesRequired = PRIVILEGES_REQUIRED_NONE : no privileges required; + # $PrivilegesRequired = PRIVILEGES_REQUIRED_REPORTER : the reporter, assignee or an empowered user; + # $PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE : the assignee or an empowered user; + # $PrivilegesRequired = PRIVILEGES_REQUIRED_EMPOWERED : an empowered user. # Only users in the time-tracking group can change time-tracking fields. if ( grep($_ eq $field, qw(deadline estimated_time remaining_time)) ) { if (!$user->is_timetracker) { - $$PrivilegesRequired = 3; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_EMPOWERED; return 0; } } @@ -3344,7 +3358,7 @@ sub check_can_change_field { # *Only* users with (product-specific) "canconfirm" privs can confirm bugs. if ($self->_changes_everconfirmed($field, $oldvalue, $newvalue)) { - $$PrivilegesRequired = 3; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_EMPOWERED; return $user->in_group('canconfirm', $self->{'product_id'}); } @@ -3375,36 +3389,36 @@ sub check_can_change_field { # 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; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE; return 0; } # - change the QA contact if ($field eq 'qa_contact') { - $$PrivilegesRequired = 2; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE; return 0; } # - change the target milestone if ($field eq 'target_milestone') { - $$PrivilegesRequired = 2; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE; return 0; } # - change the priority (unless he could have set it originally) if ($field eq 'priority' && !Bugzilla->params->{'letsubmitterchoosepriority'}) { - $$PrivilegesRequired = 2; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE; return 0; } # - unconfirm bugs (confirming them is handled above) if ($field eq 'everconfirmed') { - $$PrivilegesRequired = 2; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE; return 0; } # - change the status from one open state to another if ($field eq 'bug_status' && is_open_state($oldvalue) && is_open_state($newvalue)) { - $$PrivilegesRequired = 2; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_ASSIGNEE; return 0; } @@ -3415,7 +3429,7 @@ sub check_can_change_field { # If we haven't returned by this point, then the user doesn't # have the necessary permissions to change this field. - $$PrivilegesRequired = 1; + $$PrivilegesRequired = PRIVILEGES_REQUIRED_REPORTER; return 0; } diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index d38f123fd..87210ffd4 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -175,6 +175,11 @@ use File::Basename; PASSWORD_SALT_LENGTH CGI_URI_LIMIT + + PRIVILEGES_REQUIRED_NONE + PRIVILEGES_REQUIRED_REPORTER + PRIVILEGES_REQUIRED_ASSIGNEE + PRIVILEGES_REQUIRED_EMPOWERED ); @Bugzilla::Constants::EXPORT_OK = qw(contenttypes); @@ -522,6 +527,15 @@ use constant PASSWORD_SALT_LENGTH => 8; # can be safely done or not based on the web server's URI length setting. use constant CGI_URI_LIMIT => 8000; +# 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. + +use constant PRIVILEGES_REQUIRED_NONE => 0; +use constant PRIVILEGES_REQUIRED_REPORTER => 1; +use constant PRIVILEGES_REQUIRED_ASSIGNEE => 2; +use constant PRIVILEGES_REQUIRED_EMPOWERED => 3; + sub bz_locations { # We know that Bugzilla/Constants.pm must be in %INC at this point. # So the only question is, what's the name of the directory diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index a5be1d38d..af73814ce 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -249,6 +249,66 @@ C<$changes-E<gt>{field} = [old, new]> =back +=head2 bug_check_can_change_field + +This hook controls what fields users are allowed to change. You can add code here for +site-specific policy changes and other customizations. This hook is only +executed if the field's new and old values differ. Any denies take priority over any allows. +So, if another extension denies a change but yours allows the change, the other extension's +deny will override your extension's allow. + +Params: + +=over + +=item C<bug> + +L<Bugzilla::Bug> - The current bug object that this field is changing on. + +=item C<field> + +The name (from the C<fielddefs> table) of the field that we are checking. + +=item C<new_value> + +The new value that the field is being changed to. + +=item C<old_value> + +The old value that the field is being changed from. + +=item C<priv_results> + +C<array> - This is how you explicitly allow or deny a change. You should only +push something into this array if you want to explicitly allow or explicitly +deny the change, and thus skip all other permission checks that would otherwise +happen after this hook is called. If you don't care about the field change, +then don't push anything into the array. + +The pushed value should be a choice from the following constants: + +=over + +=item C<PRIVILEGES_REQUIRED_NONE> + +No privileges required. This explicitly B<allows> a change. + +=item C<PRIVILEGES_REQUIRED_REPORTER> + +User is not the reporter, assignee or an empowered user, so B<deny>. + +=item C<PRIVILEGES_REQUIRED_ASSIGNEE> + +User is not the assignee or an empowered user, so B<deny>. + +=item C<PRIVILEGES_REQUIRED_EMPOWERED> + +User is not a sufficiently empowered user, so B<deny>. + +=back + +=back + =head2 bug_fields Allows the addition of database fields from the bugs table to the standard diff --git a/extensions/Example/Extension.pm b/extensions/Example/Extension.pm index 6acf3e135..398ddbd56 100644 --- a/extensions/Example/Extension.pm +++ b/extensions/Example/Extension.pm @@ -29,6 +29,7 @@ use Bugzilla::Error; use Bugzilla::Group; use Bugzilla::User; use Bugzilla::Util qw(diff_arrays html_quote); +use Bugzilla::Status qw(is_open_state); # This is extensions/Example/lib/Util.pm. I can load this here in my # Extension.pm only because I have a Config.pm. @@ -587,6 +588,44 @@ sub template_before_process { } } +sub bug_check_can_change_field { + my ($self, $args) = @_; + + my ($bug, $field, $new_value, $old_value, $priv_results) + = @$args{qw(bug field new_value old_value priv_results)}; + + my $user = Bugzilla->user; + + # Disallow a bug from being reopened if currently closed unless user + # is in 'admin' group + if ($field eq 'bug_status' && $bug->product_obj->name eq 'Example') { + if (!is_open_state($old_value) && is_open_state($new_value) + && !$user->in_group('admin')) + { + push(@$priv_results, PRIVILEGES_REQUIRED_EMPOWERED); + return; + } + } + + # Disallow a bug's keywords from being edited unless user is the + # reporter of the bug + if ($field eq 'keywords' && $bug->product_obj->name eq 'Example' + && $user->login ne $bug->reporter->login) + { + push(@$priv_results, PRIVILEGES_REQUIRED_REPORTER); + return; + } + + # Allow updating of priority even if user cannot normally edit the bug + # and they are in group 'engineering' + if ($field eq 'priority' && $bug->product_obj->name eq 'Example' + && $user->in_group('engineering')) + { + push(@$priv_results, PRIVILEGES_REQUIRED_NONE); + return; + } +} + sub webservice { my ($self, $args) = @_; diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index d9872f1b0..6bf904a94 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -775,9 +775,9 @@ to <em>[% newvalue.join(', ') FILTER html %]</em> [% END %] , but only - [% IF privs < 3 %] + [% IF privs < constants.PRIVILEGES_REQUIRED_EMPOWERED %] the assignee - [% IF privs < 2 %] or reporter [% END %] + [% IF privs < constants.PRIVILEGES_REQUIRED_ASSIGNEE %] or reporter [% END %] of the [% terms.bug %], or [% END %] a user with the required permissions may change that field. |