From 6fac08b94b6cd67d8dd7ae8f7eeb5de5233c59d7 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Sat, 4 Dec 2010 02:22:49 +0100 Subject: Bug 529974: Let users with local editcomponents privs manage flags for products they can administer a=LpSolit (module owner) --- Bugzilla/FlagType.pm | 63 +++++++++++++++++++++++++++++++++------------------- Bugzilla/User.pm | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 23 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index d6a122609..bd3f7b054 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -112,6 +112,9 @@ use constant UPDATE_VALIDATORS => { sub create { my $class = shift; + my $dbh = Bugzilla->dbh; + + $dbh->bz_start_transaction(); $class->check_required_create_fields(@_); my $params = $class->run_create_validators(@_); @@ -126,6 +129,9 @@ sub create { $flagtype->set_clusions({ inclusions => $inclusions, exclusions => $exclusions }); + $flagtype->update(); + + $dbh->bz_commit_transaction(); return $flagtype; } @@ -161,7 +167,7 @@ sub update { FROM flags INNER JOIN bugs ON flags.bug_id = bugs.bug_id - LEFT OUTER JOIN flaginclusions AS i + LEFT JOIN flaginclusions AS i ON (flags.type_id = i.type_id AND (bugs.product_id = i.product_id OR i.product_id IS NULL) @@ -351,7 +357,6 @@ sub set_request_group { $_[0]->set('request_group_id', $_[1]); } sub set_clusions { my ($self, $list) = @_; - my $dbh = Bugzilla->dbh; my %products; foreach my $category (keys %$list) { @@ -360,22 +365,33 @@ sub set_clusions { foreach my $prod_comp (@{$list->{$category} || []}) { my ($prod_id, $comp_id) = split(':', $prod_comp); - my $component; + my $prod_name = '__Any__'; + my $comp_name = '__Any__'; # Does the product exist? - if ($prod_id && detaint_natural($prod_id)) { - $products{$prod_id} ||= new Bugzilla::Product($prod_id); - next unless defined $products{$prod_id}; + if ($prod_id) { + $products{$prod_id} ||= Bugzilla::Product->check({ id => $prod_id }); + detaint_natural($prod_id); + $prod_name = $products{$prod_id}->name; # Does the component belong to this product? - if ($comp_id && detaint_natural($comp_id)) { - ($component) = grep { $_->id == $comp_id } @{$products{$prod_id}->components}; - next unless $component; + if ($comp_id) { + detaint_natural($comp_id) + || ThrowCodeError('param_must_be_numeric', + { function => 'Bugzilla::FlagType::set_clusions' }); + + my ($component) = grep { $_->id == $comp_id } @{$products{$prod_id}->components} + or ThrowUserError('product_unknown_component', + { product => $prod_name, comp_id => $comp_id }); + $comp_name = $component->name; } + else { + $comp_id = 0; + } + } + else { + $prod_id = 0; + $comp_id = 0; } - $prod_id ||= 0; - $comp_id ||= 0; - my $prod_name = $prod_id ? $products{$prod_id}->name : '__Any__'; - my $comp_name = $comp_id ? $component->name : '__Any__'; $clusions{"$prod_name:$comp_name"} = "$prod_id:$comp_id"; $clusions_as_hash{$prod_id}->{$comp_id} = 1; } @@ -520,15 +536,16 @@ sub get_clusions { my $dbh = Bugzilla->dbh; my $list = - $dbh->selectall_arrayref("SELECT products.id, products.name, " . - " components.id, components.name " . - "FROM flagtypes, flag${type}clusions " . - "LEFT OUTER JOIN products " . - " ON flag${type}clusions.product_id = products.id " . - "LEFT OUTER JOIN components " . - " ON flag${type}clusions.component_id = components.id " . - "WHERE flagtypes.id = ? " . - " AND flag${type}clusions.type_id = flagtypes.id", + $dbh->selectall_arrayref("SELECT products.id, products.name, + components.id, components.name + FROM flagtypes + INNER JOIN flag${type}clusions + ON flag${type}clusions.type_id = flagtypes.id + LEFT JOIN products + ON flag${type}clusions.product_id = products.id + LEFT JOIN components + ON flag${type}clusions.component_id = components.id + WHERE flagtypes.id = ?", undef, $id); my (%clusions, %clusions_as_hash); foreach my $data (@$list) { @@ -667,7 +684,7 @@ sub sqlify_criteria { $join_clause .= "AND (e.component_id = $component_id OR e.component_id IS NULL) "; } else { - $addl_join_clause = "AND e.component_id IS NULL OR (i.component_id != e.component_id) "; + $addl_join_clause = "AND e.component_id IS NULL OR (i.component_id = e.component_id) "; } $join_clause .= "AND ((e.product_id = $product_id $addl_join_clause) OR e.product_id IS NULL)"; diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index de2d0dcc7..47f923f20 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -1006,6 +1006,49 @@ sub check_can_admin_product { return $product; } +sub check_can_admin_flagtype { + my ($self, $flagtype_id) = @_; + + my $flagtype = Bugzilla::FlagType->check({ id => $flagtype_id }); + my $can_fully_edit = 1; + + if (!$self->in_group('editcomponents')) { + my $products = $self->get_products_by_permission('editcomponents'); + # You need editcomponents privs for at least one product to have + # a chance to edit the flagtype. + scalar(@$products) + || ThrowUserError('auth_failure', {group => 'editcomponents', + action => 'edit', + object => 'flagtypes'}); + my $can_admin = 0; + my $i = $flagtype->inclusions_as_hash; + my $e = $flagtype->exclusions_as_hash; + + # If there is at least one product for which the user doesn't have + # editcomponents privs, then don't allow him to do everything with + # this flagtype, independently of whether this product is in the + # exclusion list or not. + my %product_ids; + map { $product_ids{$_->id} = 1 } @$products; + $can_fully_edit = 0 if grep { !$product_ids{$_} } keys %$i; + + unless ($e->{0}->{0}) { + foreach my $product (@$products) { + my $id = $product->id; + next if $e->{$id}->{0}; + # If we are here, the product has not been explicitly excluded. + # Check whether it's explicitly included, or at least one of + # its components. + $can_admin = ($i->{0}->{0} || $i->{$id}->{0} + || scalar(grep { !$e->{$id}->{$_} } keys %{$i->{$id}})); + last if $can_admin; + } + } + $can_admin || ThrowUserError('flag_type_not_editable', { flagtype => $flagtype }); + } + return wantarray ? ($flagtype, $can_fully_edit) : $flagtype; +} + sub can_request_flag { my ($self, $flag_type) = @_; @@ -2261,6 +2304,21 @@ not be aware of the existence of the product. Returns: On success, a product object. On failure, an error is thrown. +=item C + + Description: Checks whether the user is allowed to edit properties of the flag type. + If the flag type is also used by some products for which the user + hasn't editcomponents privs, then the user is only allowed to edit + the inclusion and exclusion lists for products he can administrate. + + Params: $flagtype_id - a flag type ID. + + Returns: On success, a flag type object. On failure, an error is thrown. + In list context, a boolean indicating whether the user can edit + all properties of the flag type is also returned. The boolean + is false if the user can only edit the inclusion and exclusions + lists. + =item C Description: Checks whether the user can request flags of the given type. -- cgit v1.2.3-24-g4f1b