From 455ab8384cc8a33be25c1a90087aca2673b96b69 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Tue, 23 Mar 2010 15:52:16 -0400 Subject: Bug 544332 - New bug_check_can_change_field hook for Bugzilla/Bug.pm r/a=mkanat --- Bugzilla/Bug.pm | 42 ++++++++++++++++++++++++------------ Bugzilla/Constants.pm | 14 ++++++++++++ Bugzilla/Hook.pm | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 14 deletions(-) (limited to 'Bugzilla') 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{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 + +L - The current bug object that this field is changing on. + +=item C + +The name (from the C table) of the field that we are checking. + +=item C + +The new value that the field is being changed to. + +=item C + +The old value that the field is being changed from. + +=item C + +C - 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 + +No privileges required. This explicitly B a change. + +=item C + +User is not the reporter, assignee or an empowered user, so B. + +=item C + +User is not the assignee or an empowered user, so B. + +=item C + +User is not a sufficiently empowered user, so B. + +=back + +=back + =head2 bug_fields Allows the addition of database fields from the bugs table to the standard -- cgit v1.2.3-24-g4f1b