diff options
| -rw-r--r-- | Bugzilla/Flag.pm | 19 | ||||
| -rw-r--r-- | Bugzilla/FlagType.pm | 30 | ||||
| -rwxr-xr-x | checksetup.pl | 9 | ||||
| -rwxr-xr-x | editflagtypes.cgi | 39 | ||||
| -rwxr-xr-x | editgroups.cgi | 19 | ||||
| -rw-r--r-- | template/en/default/admin/flag-type/edit.html.tmpl | 19 | ||||
| -rw-r--r-- | template/en/default/admin/groups/delete.html.tmpl | 12 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 16 |
8 files changed, 154 insertions, 9 deletions
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index a58bc7e3a..0fd4b047f 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -139,6 +139,7 @@ sub count { sub validate { # Validates fields containing flag modifications. + my $user = Bugzilla->user; my ($data, $bug_id) = @_; # Get a list of flags to validate. Uses the "map" function @@ -213,6 +214,24 @@ sub validate { } } } + + # Make sure the user is authorized to modify flags, see bug 180879 + # - The flag is unchanged + next if ($status eq $flag->{status}); + + # - User can clear flags set by itself + next if (($status eq "X") && ($user->id eq $flag->{setter})); + + # - User in the $grant_gid group can set/clear flags, + # including "+" and "-" + next if (!$flag->{type}->{grant_gid} + || $user->in_group(&::GroupIdToName($flag->{type}->{grant_gid}))); + + # - Any other flag modification is denied + ThrowUserError("flag_update_denied", + { name => $flag->{type}->{name}, + status => $status, + old_status => $flag->{status} }); } } diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 687a01768..5b681dc0c 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -49,7 +49,8 @@ my @base_columns = ("1", "flagtypes.id", "flagtypes.name", "flagtypes.description", "flagtypes.cc_list", "flagtypes.target_type", "flagtypes.sortkey", "flagtypes.is_active", "flagtypes.is_requestable", - "flagtypes.is_requesteeble", "flagtypes.is_multiplicable"); + "flagtypes.is_requesteeble", "flagtypes.is_multiplicable", + "flagtypes.grant_group_id", "flagtypes.request_group_id"); # Note: when adding tables to @base_tables, make sure to include the separator # (i.e. a comma or words like "LEFT OUTER JOIN") before the table name, @@ -181,6 +182,7 @@ sub count { } sub validate { + my $user = Bugzilla->user; my ($data, $bug_id, $attach_id) = @_; # Get a list of flag types to validate. Uses the "map" function @@ -249,6 +251,22 @@ sub validate { attach_id => $attach_id }); } } + + # Make sure the user is authorized to modify flags, see bug 180879 + # - User in the $grant_gid group can set flags, including "+" and "-" + next if (!$flag_type->{grant_gid} + || $user->in_group(&::GroupIdToName($flag_type->{grant_gid}))); + + # - User in the $request_gid group can request flags + next if ($status eq '?' + && (!$flag_type->{request_gid} + || $user->in_group(&::GroupIdToName($flag_type->{request_gid})))); + + # - Any other flag modification is denied + ThrowUserError("flag_update_denied", + { name => $flag_type->{name}, + status => $status, + old_status => "X" }); } } @@ -348,6 +366,12 @@ sub sqlify_criteria { push(@$columns, "COUNT(flagexclusions.type_id) AS num_exclusions"); $$having = "num_exclusions = 0"; } + if ($criteria->{group}) { + my $gid = $criteria->{group}; + detaint_natural($gid); + push(@criteria, "(flagtypes.grant_group_id = $gid " . + " OR flagtypes.request_group_id = $gid)"); + } return @criteria; } @@ -368,7 +392,9 @@ sub perlify_record { $type->{'is_requestable'} = $_[8]; $type->{'is_requesteeble'} = $_[9]; $type->{'is_multiplicable'} = $_[10]; - $type->{'flag_count'} = $_[11]; + $type->{'grant_gid'} = $_[11]; + $type->{'request_gid'} = $_[12]; + $type->{'flag_count'} = $_[13]; return $type; } diff --git a/checksetup.pl b/checksetup.pl index 710140191..c8af512ba 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -1685,7 +1685,9 @@ $table{flagtypes} = is_requesteeble TINYINT NOT NULL DEFAULT 0 , is_multiplicable TINYINT NOT NULL DEFAULT 0 , - sortkey SMALLINT NOT NULL DEFAULT 0 + sortkey SMALLINT NOT NULL DEFAULT 0 , + grant_group_id MEDIUMINT NULL , + request_group_id MEDIUMINT NULL '; $table{flaginclusions} = @@ -4124,6 +4126,11 @@ if (GetFieldDef("group_group_map", "isbless")) { # login data source AddField("profiles", "extern_id", "varchar(64)"); +# 2004-11-20 - LpSolit@netscape.net - Bug 180879 +# Add grant and request groups for flags +AddField('flagtypes', 'grant_group_id', 'mediumint null'); +AddField('flagtypes', 'request_group_id', 'mediumint null'); + # If you had to change the --TABLE-- definition in any way, then add your # differential change code *** A B O V E *** this comment. # diff --git a/editflagtypes.cgi b/editflagtypes.cgi index a14f75680..8b51be326 100755 --- a/editflagtypes.cgi +++ b/editflagtypes.cgi @@ -90,9 +90,12 @@ exit; sub list { # Define the variables and functions that will be passed to the UI template. - $vars->{'bug_types'} = Bugzilla::FlagType::match({ 'target_type' => 'bug' }, 1); + $vars->{'bug_types'} = + Bugzilla::FlagType::match({ 'target_type' => 'bug', + 'group' => $::FORM{'group'} }, 1); $vars->{'attachment_types'} = - Bugzilla::FlagType::match({ 'target_type' => 'attachment' }, 1); + Bugzilla::FlagType::match({ 'target_type' => 'attachment', + 'group' => $::FORM{'group'} }, 1); # Return the appropriate HTTP response headers. print Bugzilla->cgi->header(); @@ -129,6 +132,13 @@ sub edit { $vars->{'type'} = Bugzilla::FlagType::get($::FORM{'id'}); $vars->{'type'}->{'inclusions'} = Bugzilla::FlagType::get_inclusions($::FORM{'id'}); $vars->{'type'}->{'exclusions'} = Bugzilla::FlagType::get_exclusions($::FORM{'id'}); + # Users want to see group names, not IDs + foreach my $group ("grant_gid", "request_gid") { + my $gid = $vars->{'type'}->{$group}; + next if (!$gid); + SendSQL("SELECT name FROM groups WHERE id = $gid"); + $vars->{'type'}->{$group} = FetchOneColumn(); + } } # Otherwise set the target type (the minimal information about the type # that the template needs to know) from the URL parameter and default @@ -208,6 +218,7 @@ sub insert { validateIsRequestable(); validateIsRequesteeble(); validateAllowMultiple(); + validateGroups(); my $name = SqlQuote($::FORM{'name'}); my $description = SqlQuote($::FORM{'description'}); @@ -224,11 +235,13 @@ sub insert { # Insert a record for the new flag type into the database. SendSQL("INSERT INTO flagtypes (id, name, description, cc_list, target_type, sortkey, is_active, is_requestable, - is_requesteeble, is_multiplicable) + is_requesteeble, is_multiplicable, + grant_group_id, request_group_id) VALUES ($id, $name, $description, $cc_list, '$target_type', $::FORM{'sortkey'}, $::FORM{'is_active'}, $::FORM{'is_requestable'}, $::FORM{'is_requesteeble'}, - $::FORM{'is_multiplicable'})"); + $::FORM{'is_multiplicable'}, $::FORM{'grant_gid'}, + $::FORM{'request_gid'})"); # Populate the list of inclusions/exclusions for this flag type. foreach my $category_type ("inclusions", "exclusions") { @@ -267,6 +280,7 @@ sub update { validateIsRequestable(); validateIsRequesteeble(); validateAllowMultiple(); + validateGroups(); my $name = SqlQuote($::FORM{'name'}); my $description = SqlQuote($::FORM{'description'}); @@ -282,7 +296,9 @@ sub update { is_active = $::FORM{'is_active'} , is_requestable = $::FORM{'is_requestable'} , is_requesteeble = $::FORM{'is_requesteeble'} , - is_multiplicable = $::FORM{'is_multiplicable'} + is_multiplicable = $::FORM{'is_multiplicable'} , + grant_group_id = $::FORM{'grant_gid'} , + request_group_id = $::FORM{'request_gid'} WHERE id = $::FORM{'id'}"); # Update the list of inclusions/exclusions for this flag type. @@ -499,3 +515,16 @@ sub validateAllowMultiple { $::FORM{'is_multiplicable'} = $::FORM{'is_multiplicable'} ? 1 : 0; } +sub validateGroups { + # Convert group names to group IDs + foreach my $col ("grant_gid", "request_gid") { + my $name = $::FORM{$col}; + $::FORM{$col} ||= "NULL"; + next if (!$name); + SendSQL("SELECT id FROM groups WHERE name = " . SqlQuote($name)); + $::FORM{$col} = FetchOneColumn(); + if (!$::FORM{$col}) { + ThrowUserError("group_unknown", { name => $name }); + } + } +} diff --git a/editgroups.cgi b/editgroups.cgi index b3b6135e4..bc22d518e 100755 --- a/editgroups.cgi +++ b/editgroups.cgi @@ -317,12 +317,20 @@ if ($action eq 'del') { $hasproduct = 1; } + my $hasflags = 0; + SendSQL("SELECT id FROM flagtypes + WHERE grant_group_id = $gid OR request_group_id = $gid"); + if (FetchOneColumn()) { + $hasflags = 1; + } + $vars->{'gid'} = $gid; $vars->{'name'} = $name; $vars->{'description'} = $desc; $vars->{'hasusers'} = $hasusers; $vars->{'hasbugs'} = $hasbugs; $vars->{'hasproduct'} = $hasproduct; + $vars->{'hasflags'} = $hasflags; $vars->{'buglist'} = $buglist; print Bugzilla->cgi->header(); @@ -365,8 +373,19 @@ if ($action eq 'delete') { $cantdelete = 1; } } + SendSQL("SELECT id FROM flagtypes + WHERE grant_group_id = $gid OR request_group_id = $gid"); + if (FetchOneColumn()) { + if (!defined $cgi->param('removeflags')) { + $cantdelete = 1; + } + } if (!$cantdelete) { + SendSQL("UPDATE flagtypes SET grant_group_id = NULL + WHERE grant_group_id = $gid"); + SendSQL("UPDATE flagtypes SET request_group_id = NULL + WHERE request_group_id = $gid"); SendSQL("DELETE FROM user_group_map WHERE group_id = $gid"); SendSQL("DELETE FROM group_group_map WHERE grantor_id = $gid"); SendSQL("DELETE FROM bug_group_map WHERE group_id = $gid"); diff --git a/template/en/default/admin/flag-type/edit.html.tmpl b/template/en/default/admin/flag-type/edit.html.tmpl index 1faaaf3b8..253a310ac 100644 --- a/template/en/default/admin/flag-type/edit.html.tmpl +++ b/template/en/default/admin/flag-type/edit.html.tmpl @@ -188,6 +188,25 @@ </tr> <tr> + <th>Grant Group:</th> + <td> + the group allowed to grant/deny flags of this type + (to allow all users to grant/deny these flags, leave this empty)<br> + <input type="text" name="grant_gid" value="[% type.grant_gid FILTER html %]" size="50" maxlength="255"> + </td> + </tr> + + <tr> + <th>Request Group:</th> + <td> + if flags of this type are requestable, the group allowed to request them + (to allow all users to request these flags, leave this empty)<br> + Note that the request group alone has no effect if the grant group is not defined!<br> + <input type="text" name="request_gid" value="[% type.request_gid FILTER html %]" size="50" maxlength="255"> + </td> + </tr> + + <tr> <th></th> <td> <input type="submit" value=" diff --git a/template/en/default/admin/groups/delete.html.tmpl b/template/en/default/admin/groups/delete.html.tmpl index 905f68cf3..842e2c6f1 100644 --- a/template/en/default/admin/groups/delete.html.tmpl +++ b/template/en/default/admin/groups/delete.html.tmpl @@ -29,6 +29,7 @@ # hasusers: boolean int. True if the group includes users in it. # hasbugs: boolean int. True if the group includes bugs in it. # hasproduct: boolean int. True if the group is binded to a product. + # hasflags: boolean int. True if the group is used by a flag type. # buglist: string. The list of bugs included in this group. #%] @@ -81,11 +82,20 @@ <br><input type="checkbox" name="unbind">Delete this group anyway, and make the <U>[% name FILTER html %]</U> publicly visible.</p> [% END %] + + [% IF hasflags %] + <p><b>This group restricts who can make changes to flags of certain types. + You cannot delete this group while there are flag types using it.</b> + + <br><a href="editflagtypes.cgi?action=list&group=[% gid FILTER html %]">Show + me which types</a> - <input type="checkbox" name="removeflags">Remove all + flag types from this group for me.</p> + [% END %] <h2>Confirmation</h2> <p>Do you really want to delete this group?</p> - [% IF (hasusers || hasbugs || hasproduct) %] + [% IF (hasusers || hasbugs || hasproduct || hasflags) %] <p><b>You must check all of the above boxes or correct the indicated problems first before you can proceed.</b></p> [% END %] diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 93c50b85a..c92824ad3 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -344,6 +344,17 @@ [% title = "Flag Type Name Invalid" %] The name <em>[% name FILTER html %]</em> must be 1-50 characters long. + [% ELSIF error == "flag_update_denied" %] + [% title = "Flag Modification Denied" %] + You tried to [% IF status == "+" %] grant [% ELSIF status == "-" %] deny + [% ELSIF status == "X" %] clear [% ELSE %] request [% END %] + <code>[% name FILTER html %]</code> + [% IF status == "?" && old_status != "X" %], but this flag is already + set[% END %]. + Only a sufficiently empowered user [% IF status == "X" %] or the user who + set <code>[% name FILTER html %][% old_status FILTER html %]</code> in + the first place [% END %] can make this change. + [% ELSIF error == "format_not_found" %] [% title = "Format Not Found" %] The requested format <em>[% format FILTER html %]</em> does not exist with @@ -362,6 +373,11 @@ [% title = "Group not specified" %] No group was specified. + [% ELSIF error == "group_unknown" %] + [% title = "Unknown Group" %] + The group [% name FILTER html %] does not exist. Please specify + a valid group name. Create it first if necessary! + [% ELSIF error == "illegal_at_least_x_votes" %] [% title = "Your Search Makes No Sense" %] The <em>At least ___ votes</em> field must be a simple number. |