Commit 9e7ad08c authored by Simon Green's avatar Simon Green

Bug 452525 - Allow the option of "OR" groups ("any of the groups" instead of "all of the groups")

r=gerv, a=sgreen
parent 8465c056
......@@ -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;
......
......@@ -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;
}
......
......@@ -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.
......
......@@ -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
......@@ -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
......
......@@ -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."
}
%]
......@@ -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>.
......
......@@ -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>
......
......@@ -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>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment