diff options
| -rw-r--r-- | Bugzilla/Config.pm | 3 | ||||
| -rw-r--r-- | Bugzilla/Config/GroupSecurity.pm | 6 | ||||
| -rw-r--r-- | Bugzilla/Search.pm | 20 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 241 | ||||
| -rwxr-xr-x | request.cgi | 21 | ||||
| -rw-r--r-- | template/en/default/admin/params/groupsecurity.html.tmpl | 13 | ||||
| -rw-r--r-- | template/en/default/admin/products/groupcontrol/edit.html.tmpl | 14 | ||||
| -rw-r--r-- | template/en/default/bug/create/create.html.tmpl | 2 | ||||
| -rw-r--r-- | template/en/default/bug/edit.html.tmpl | 5 |
9 files changed, 228 insertions, 97 deletions
diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index 69becbf95..5d9a8061c 100644 --- a/Bugzilla/Config.pm +++ b/Bugzilla/Config.pm @@ -196,6 +196,9 @@ sub update_params { $param->{'utf8'} = 1 if $new_install; + # Bug 452525: OR based groups are on by default for new installations + $param->{'or_groups'} = 1 if $new_install; + # --- REMOVE OLD PARAMS --- my %oldparams; diff --git a/Bugzilla/Config/GroupSecurity.pm b/Bugzilla/Config/GroupSecurity.pm index 1d3878415..076f841fd 100644 --- a/Bugzilla/Config/GroupSecurity.pm +++ b/Bugzilla/Config/GroupSecurity.pm @@ -83,6 +83,12 @@ sub get_param_list { name => 'strict_isolation', type => 'b', default => 0 + }, + + { + name => 'or_groups', + type => 'b', + default => 0 } ); return @param_list; } diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index e89a9b361..e546be6d9 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -1228,9 +1228,12 @@ sub _standard_joins { push(@joins, $security_join); if ($user->id) { - $security_join->{extra} = - ["NOT (" . $user->groups_in_sql('security_map.group_id') . ")"]; - + # See also _standard_joins for the other half of the below statement + if (!Bugzilla->params->{'or_groups'}) { + $security_join->{extra} = + ["NOT (" . $user->groups_in_sql('security_map.group_id') . ")"]; + } + my $security_cc_join = { table => 'cc', as => 'security_cc', @@ -1304,10 +1307,17 @@ sub _standard_where { # until their group controls are set. So if a bug has a NULL creation_ts, # it shouldn't show up in searches at all. my @where = ('bugs.creation_ts IS NOT NULL'); - - my $security_term = 'security_map.group_id IS NULL'; my $user = $self->_user; + my $security_term = ''; + # See also _standard_joins for the other half of the below statement + if (Bugzilla->params->{'or_groups'}) { + $security_term .= " (security_map.group_id IS NULL OR security_map.group_id IN (" . $user->groups_as_string . "))"; + } + else { + $security_term = 'security_map.group_id IS NULL'; + } + if ($user->id) { my $userid = $user->id; # This indentation makes the resulting SQL more readable. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 5c875918f..1bd6c0b19 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -875,15 +875,34 @@ sub can_edit_product { my ($self, $prod_id) = @_; my $dbh = Bugzilla->dbh; - my $has_external_groups = - $dbh->selectrow_array('SELECT 1 - FROM group_control_map - WHERE product_id = ? - AND canedit != 0 - AND group_id NOT IN(' . $self->groups_as_string . ')', - undef, $prod_id); + if (Bugzilla->params->{'or_groups'}) { + my $groups = $self->groups_as_string; + # For or-groups, we check if there are any can_edit groups for the + # product, and if the user is in any of them. If there are none or + # the user is in at least one of them, they can edit the product + my ($cnt_can_edit, $cnt_group_member) = $dbh->selectrow_array( + "SELECT SUM(p.cnt_can_edit), + SUM(p.cnt_group_member) + FROM (SELECT CASE WHEN canedit = 1 THEN 1 ELSE 0 END AS cnt_can_edit, + CASE WHEN canedit = 1 AND group_id IN ($groups) THEN 1 ELSE 0 END AS cnt_group_member + FROM group_control_map + WHERE product_id = $prod_id) AS p"); + return ($cnt_can_edit == 0 or $cnt_group_member > 0); + } + else { + # For and-groups, a user needs to be in all canedit groups. Therefore + # if the user is not in a can_edit group for the product, they cannot + # edit the product. + my $has_external_groups = + $dbh->selectrow_array('SELECT 1 + FROM group_control_map + WHERE product_id = ? + AND canedit != 0 + AND group_id NOT IN(' . $self->groups_as_string . ')', + undef, $prod_id); - return !$has_external_groups; + return !$has_external_groups; + } } sub can_see_bug { @@ -904,9 +923,6 @@ sub visible_bugs { my @check_ids = grep(!exists $visible_cache->{$_}, @bug_ids); if (@check_ids) { - my $dbh = Bugzilla->dbh; - my $user_id = $self->id; - foreach my $id (@check_ids) { my $orig_id = $id; detaint_natural($id) @@ -914,56 +930,113 @@ sub visible_bugs { function => 'Bugzilla::User->visible_bugs'}); } - my $sth; - # Speed up the can_see_bug case. - if (scalar(@check_ids) == 1) { - $sth = $self->{_sth_one_visible_bug}; - } - $sth ||= $dbh->prepare( - # This checks for groups that the bug is in that the user - # *isn't* in. Then, in the Perl code below, we check if - # the user can otherwise access the bug (for example, by being - # the assignee or QA Contact). - # - # The DISTINCT exists because the bug could be in *several* - # groups that the user isn't in, but they will all return the - # same result for bug_group_map.bug_id (so DISTINCT filters - # out duplicate rows). - "SELECT DISTINCT bugs.bug_id, reporter, assigned_to, qa_contact, - reporter_accessible, cclist_accessible, cc.who, - bug_group_map.bug_id - FROM bugs - LEFT JOIN cc - ON cc.bug_id = bugs.bug_id - AND cc.who = $user_id - LEFT JOIN bug_group_map - ON bugs.bug_id = bug_group_map.bug_id - AND bug_group_map.group_id NOT IN (" - . $self->groups_as_string . ') - WHERE bugs.bug_id IN (' . join(',', ('?') x @check_ids) . ') - AND creation_ts IS NOT NULL '); - if (scalar(@check_ids) == 1) { - $self->{_sth_one_visible_bug} = $sth; - } - - $sth->execute(@check_ids); - my $use_qa_contact = Bugzilla->params->{'useqacontact'}; - while (my $row = $sth->fetchrow_arrayref) { - my ($bug_id, $reporter, $owner, $qacontact, $reporter_access, - $cclist_access, $isoncclist, $missinggroup) = @$row; - $visible_cache->{$bug_id} ||= - ((($reporter == $user_id) && $reporter_access) - || ($use_qa_contact - && $qacontact && ($qacontact == $user_id)) - || ($owner == $user_id) - || ($isoncclist && $cclist_access) - || !$missinggroup) ? 1 : 0; - } + Bugzilla->params->{'or_groups'} + ? $self->_visible_bugs_check_or(\@check_ids) + : $self->_visible_bugs_check_and(\@check_ids); } return [grep { $visible_cache->{blessed $_ ? $_->id : $_} } @$bugs]; } +sub _visible_bugs_check_or { + my ($self, $check_ids) = @_; + my $visible_cache = $self->{_visible_bugs_cache}; + my $dbh = Bugzilla->dbh; + my $user_id = $self->id; + + my $sth; + # Speed up the can_see_bug case. + if (scalar(@$check_ids) == 1) { + $sth = $self->{_sth_one_visible_bug}; + } + my $query = qq{ + SELECT DISTINCT bugs.bug_id + FROM bugs + LEFT JOIN bug_group_map AS security_map ON bugs.bug_id = security_map.bug_id + LEFT JOIN cc AS security_cc ON bugs.bug_id = security_cc.bug_id AND security_cc.who = $user_id + WHERE bugs.bug_id IN (} . join(',', ('?') x @$check_ids) . qq{) + AND ((security_map.group_id IS NULL OR security_map.group_id IN (} . $self->groups_as_string . qq{)) + OR (bugs.reporter_accessible = 1 AND bugs.reporter = $user_id) + OR (bugs.cclist_accessible = 1 AND security_cc.who IS NOT NULL) + OR bugs.assigned_to = $user_id + }; + + if (Bugzilla->params->{'useqacontact'}) { + $query .= " OR bugs.qa_contact = $user_id"; + } + $query .= ')'; + + $sth ||= $dbh->prepare($query); + if (scalar(@$check_ids) == 1) { + $self->{_sth_one_visible_bug} = $sth; + } + + # Set all bugs as non visible + foreach my $bug_id (@$check_ids) { + $visible_cache->{$bug_id} = 0; + } + + # Now get the bugs the user can see + my $visible_bug_ids = $dbh->selectcol_arrayref($sth, undef, @$check_ids); + foreach my $bug_id (@$visible_bug_ids) { + $visible_cache->{$bug_id} = 1; + } +} + +sub _visible_bugs_check_and { + my ($self, $check_ids) = @_; + my $visible_cache = $self->{_visible_bugs_cache}; + my $dbh = Bugzilla->dbh; + my $user_id = $self->id; + + my $sth; + # Speed up the can_see_bug case. + if (scalar(@$check_ids) == 1) { + $sth = $self->{_sth_one_visible_bug}; + } + $sth ||= $dbh->prepare( + # This checks for groups that the bug is in that the user + # *isn't* in. Then, in the Perl code below, we check if + # the user can otherwise access the bug (for example, by being + # the assignee or QA Contact). + # + # The DISTINCT exists because the bug could be in *several* + # groups that the user isn't in, but they will all return the + # same result for bug_group_map.bug_id (so DISTINCT filters + # out duplicate rows). + "SELECT DISTINCT bugs.bug_id, reporter, assigned_to, qa_contact, + reporter_accessible, cclist_accessible, cc.who, + bug_group_map.bug_id + FROM bugs + LEFT JOIN cc + ON cc.bug_id = bugs.bug_id + AND cc.who = $user_id + LEFT JOIN bug_group_map + ON bugs.bug_id = bug_group_map.bug_id + AND bug_group_map.group_id NOT IN (" + . $self->groups_as_string . ') + WHERE bugs.bug_id IN (' . join(',', ('?') x @$check_ids) . ') + AND creation_ts IS NOT NULL '); + if (scalar(@$check_ids) == 1) { + $self->{_sth_one_visible_bug} = $sth; + } + + $sth->execute(@$check_ids); + my $use_qa_contact = Bugzilla->params->{'useqacontact'}; + while (my $row = $sth->fetchrow_arrayref) { + my ($bug_id, $reporter, $owner, $qacontact, $reporter_access, + $cclist_access, $isoncclist, $missinggroup) = @$row; + $visible_cache->{$bug_id} ||= + ((($reporter == $user_id) && $reporter_access) + || ($use_qa_contact + && $qacontact && ($qacontact == $user_id)) + || ($owner == $user_id) + || ($isoncclist && $cclist_access) + || !$missinggroup) ? 1 : 0; + } + +} + sub clear_product_cache { my $self = shift; delete $self->{enterable_products}; @@ -983,14 +1056,25 @@ sub get_selectable_products { my $class_restricted = Bugzilla->params->{'useclassification'} && $class_id; if (!defined $self->{selectable_products}) { - my $query = "SELECT id " . - " FROM products " . - "LEFT JOIN group_control_map " . - "ON group_control_map.product_id = products.id " . - " AND group_control_map.membercontrol = " . CONTROLMAPMANDATORY . - " AND group_id NOT IN(" . $self->groups_as_string . ") " . - " WHERE group_id IS NULL " . - "ORDER BY name"; + my $query = + Bugzilla->params->{'or_groups'} + ? "SELECT id + FROM products + WHERE id NOT IN ( + SELECT product_id + FROM group_control_map + WHERE group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " + AND group_id NOT IN (" . $self->groups_as_string . ") + ) + ORDER BY name" + : "SELECT id + FROM products + LEFT JOIN group_control_map + ON group_control_map.product_id = products.id + AND group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " + AND group_id NOT IN(" . $self->groups_as_string . ") + WHERE group_id IS NULL + ORDER BY name"; my $prod_ids = Bugzilla->dbh->selectcol_arrayref($query); $self->{selectable_products} = Bugzilla::Product->new_from_list($prod_ids); @@ -1084,14 +1168,21 @@ sub get_enterable_products { } # All products which the user has "Entry" access to. - my $enterable_ids = $dbh->selectcol_arrayref( + my $query = 'SELECT products.id FROM products - LEFT JOIN group_control_map - ON group_control_map.product_id = products.id - AND group_control_map.entry != 0 - AND group_id NOT IN (' . $self->groups_as_string . ') - WHERE group_id IS NULL - AND products.isactive = 1'); + LEFT JOIN group_control_map + ON group_control_map.product_id = products.id + AND group_control_map.entry != 0'; + + if (Bugzilla->params->{'or_groups'}) { + $query .= " WHERE (group_id IN (" . $self->groups_as_string . ")" . + " OR group_id IS NULL)"; + } else { + $query .= " AND group_id NOT IN (" . $self->groups_as_string . ")" . + " WHERE group_id IS NULL" + } + $query .= " AND products.isactive = 1"; + my $enterable_ids = $dbh->selectcol_arrayref($query); if (scalar @$enterable_ids) { # And all of these products must have at least one component @@ -2476,6 +2567,12 @@ Returns 1 if the specified user account exists and is visible to the user, Determines if, given a product id, the user can edit bugs in this product at all. +=item C<visible_bugs($bugs)> + +Description: Determines if a list of bugs are visible to the user. +Params: C<$bugs> - An arrayref of Bugzilla::Bug objects or bug ids +Returns: An arrayref of the bug ids that the user can see + =item C<can_see_bug(bug_id)> Determines if the user can see the specified bug. @@ -2846,8 +2943,6 @@ L<Bugzilla|Bugzilla> =item extern_id -=item visible_bugs - =item UPDATE_COLUMNS =back diff --git a/request.cgi b/request.cgi index 118935092..b771cee37 100755 --- a/request.cgi +++ b/request.cgi @@ -118,20 +118,27 @@ sub queue { ON bugs.product_id = products.id INNER JOIN components ON bugs.component_id = components.id - LEFT JOIN bug_group_map AS bgmap - ON bgmap.bug_id = bugs.bug_id - AND bgmap.group_id NOT IN (" . - $user->groups_as_string . ") LEFT JOIN bug_group_map AS privs ON privs.bug_id = bugs.bug_id LEFT JOIN cc AS ccmap ON ccmap.who = $userid AND ccmap.bug_id = bugs.bug_id - " . + LEFT JOIN bug_group_map AS bgmap + ON bgmap.bug_id = bugs.bug_id + "; + + if (Bugzilla->params->{or_groups}) { + $query .= " AND bgmap.group_id IN (" . $user->groups_as_string . ")"; + $query .= " WHERE (privs.group_id IS NULL OR bgmap.group_id IS NOT NULL OR"; + } + else { + $query .= " AND bgmap.group_id NOT IN (" . $user->groups_as_string . ")"; + $query .= " WHERE (bgmap.group_id IS NULL OR"; + } # Weed out bug the user does not have access to - " WHERE ((bgmap.group_id IS NULL) OR - (ccmap.who IS NOT NULL AND cclist_accessible = 1) OR + $query .= + " (ccmap.who IS NOT NULL AND cclist_accessible = 1) OR (bugs.reporter = $userid AND bugs.reporter_accessible = 1) OR (bugs.assigned_to = $userid) " . (Bugzilla->params->{'useqacontact'} ? "OR diff --git a/template/en/default/admin/params/groupsecurity.html.tmpl b/template/en/default/admin/params/groupsecurity.html.tmpl index 4f0b4919b..c9dbf5b4b 100644 --- a/template/en/default/admin/params/groupsecurity.html.tmpl +++ b/template/en/default/admin/params/groupsecurity.html.tmpl @@ -37,13 +37,20 @@ usevisibilitygroups => "Do you wish to restrict visibility of users to members of " _ "specific groups?", - + strict_isolation => "Don't allow users to be assigned to, " _ "be qa-contacts on, " _ "be added to CC list, " _ "or make or remove dependencies " _ "involving any bug that is in a product on which that " _ - "user is forbidden to edit.", - + "user is forbidden to edit.", + + or_groups => "Define the visibility of a $terms.bug which is in multiple " _ + "groups. If this is on (recommended), a user only needs to " _ + "be a member of one of the $terms.bug's groups in order to " _ + "view it. If it is off, a user needs to be a member of all " _ + "the $terms.bug's groups. Note that in either case, if the " _ + "user has a role on the $terms.bug (e.g. reporter) that may " _ + "also affect their permissions." } %] diff --git a/template/en/default/admin/products/groupcontrol/edit.html.tmpl b/template/en/default/admin/products/groupcontrol/edit.html.tmpl index 889647e7e..1ba600271 100644 --- a/template/en/default/admin/products/groupcontrol/edit.html.tmpl +++ b/template/en/default/admin/products/groupcontrol/edit.html.tmpl @@ -129,15 +129,17 @@ product. </p> <p> If any group has <b>Entry</b> selected, then this product will -restrict [% terms.bug %] entry to only those users who are members of all the -groups with entry selected. +restrict [% terms.bug %] entry to only those users who are members of +[%+ IF Param('or_groups') %]at least one of[% ELSE %]all[% END %] the groups +with entry selected. </p> <p> If any group has <b>Canedit</b> selected, then this product -will be read-only for any users who are not members of all of -the groups with Canedit selected. ONLY users who are members of -all the canedit groups will be able to edit. This is an additional -restriction that further restricts what can be edited by a user. +will be read-only for any users who are not members of +[%+ IF Param('or_groups') %]one[% ELSE %]all[% END %] of the groups with +Canedit selected. ONLY users who are members of +[%+ IF Param('or_groups') %]at least one of[% ELSE %]all[% END %] the canedit groups +will be able to edit. This is an additional restriction that further restricts what can be edited by a user. </p> <p> The following settings control let you choose privileges on a <b>per-product basis</b>. diff --git a/template/en/default/bug/create/create.html.tmpl b/template/en/default/bug/create/create.html.tmpl index 3d150bf89..f4c60ad24 100644 --- a/template/en/default/bug/create/create.html.tmpl +++ b/template/en/default/bug/create/create.html.tmpl @@ -634,7 +634,7 @@ TUI_hide_default('attachment_text_field'); <td colspan="3"> <br> <strong> - Only users in all of the selected groups can view this + Only users in [%+ IF Param('or_groups') %]at least one[% ELSE %]all[% END %] of the selected groups can view this [%+ terms.bug %]: </strong> <br> diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl index 202a981ea..0ddc7cc06 100644 --- a/template/en/default/bug/edit.html.tmpl +++ b/template/en/default/bug/edit.html.tmpl @@ -651,8 +651,9 @@ [% IF NOT emitted_description %] [% emitted_description = 1 %] <div id="bz_restrict_group_visibility_help"> - <b>Only users in all of the selected groups can view this - [%+ terms.bug %]:</b> + <b>Only users in + [%+ IF Param('or_groups') %]at least one[% ELSE %]all[% END %] + of the selected groups can view this [% terms.bug %]:</b> <p class="instructions"> Unchecking all boxes makes this a more public [% terms.bug %]. </p> |