189627 - Add limitations to userpermissions so privileges are not global

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Jens Dueholm Christensen]

User-Agent:       Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
Build Identifier: 

With many different projects in a single bugzilla install it would be a nice 
feature if some users were project-admins, allowing them to modify privileges 
for other users in the same project group and also modify components in their 
own project.

As it is now, users with the "Editcomponents" priviledge, can edit ALL 
components for every product which is not a good thing.

What's needed is a pr-product administration.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Jacob Steenhagen]

You'd think somebody would have requested this already, but I can't find
anything in my brief search. I agree, this could be very helpful even here on
bmo.  FE, Dave could be given the ablility to modify components in the Bugzilla
product instead of filing bugs asking other people to do it :)

Comment 2

[Jens Dueholm Christensen]

Yes, it makes it impossible for us at sunsite.dk to use bugzilla as a 
bugtracker, since we can't seperate the different projects.

I've actually talked it over with joel ([e-mail]) on IRC and he 
suggested that I'd fill this report - so I did :)

Comment 3

[Dave Miller [:justdave]]

There's a bug somewhere titled "everything should be per-product", which I think
was originally intended for this sort of thing, although that probably applies
to things like feature on/off switches rather than permissions.

This would be killer to have.

Here's what I propose:

Since the number of groups has a "really big" limit now, we should go ahead and
use groups for this.

Each product (and maybe each component?) would automatically get a series of
groups created for it.
  1. role_editproduct_${product_id}
      - inherits membership from editcomponents (which inherits from admin already)
  2. role_canconfirm_${product_id}
      - inherits from canconfirm
  3. role_editbugs_${product_id}
      - inherits from editbugs
  4. role_canverify_${product_id}
      - inherits from canverify (which doesn't exist yet - it should)
  5. role_canclose_${product_id}
      - inherits from canclose (see above)
  6. etc?

Additionally, if we went the component-specific route as well (which would be
cool, too)
  1. role_editcomponent_${product_id}_${component_id}
      - inherits from role_editproduct_${product_id}
  2. role_canconfirm_${product_id}_${component_id}
      - inherits from role_canconfirm_${product_id}
  3. etc

Allowing all of this to be either product or component-specific would be really
cool.  Notice my use of ID numbers and not names....  this way we avoid having
to rename the groups anytime we rename a product or component.  Not only that,
but product and component names can be pretty long, and groups probably can't be.

This might make it hard to tell what you're looking at in editusers with all the
checkboxes though...  so it would be better to have a separate UI for these
instead of listing them with checkboxes.  (like a chart with boxes for each priv
type, and a list of the products/components the user has that access in, with
buttons to add/remove products/components from the lists).

Thoughts anyone?

How big is our key for groups.id?  If it's 32K, we probably better bump it to 8
million. :)

Comment 4

[Jens Dueholm Christensen]

What about using a selection method like on the query.cgi page with boxes 
containing the different users, projects and the components.

Selecting a user will display his current permissions in the different projects 
and components.
Selecting a project will display the users that's in the projectgroup and 
individually what those users can do to the project.
Selecting a component will display the users that can edit the component and 
what the users individually can do to the component.

All pages with the possibility to change settings/add another 
user/project/component..

Of course a projectadmin user would only be able to see/select/add/edit users 
and components in the projectgroup.

Comment 5

[Joel Peshkin]

WRT comment 4: That would be inventing a duplicate authorization scheme, which I
would prefer not to do.

WRT comment 3: The trick would be ....
To determine if a user can edit component X of product Y...

If the user has editcomponents --> YES
If role_editcomponent_Y_X is defined and the user is in it --> YES
If role_editcomponent_Y is defined and the user is in it --> YES
Otherwise --> NO

With this approach.....
Are Y and X Names or IDs in the DB?
If Y and X are IDs in the DB, are they names in the UI for editing groups?

If we are going to have to create new administrative UIs for it anyway, perhaps
the right approach is to create a Role table....

role {edit_it, add_to_it, edit_children ... ??? }
product { id or ALL }
component { id or ALL }
group_id 

(Yeah, I know those were horrible names)
Then, a group can be defined for a collection of people with a given authority
and associated with a permission using the role table.

For Jens's application, as I understand it, he would want a group of project
admins for each product to have the ability to add, delete, or edit components
within that entire product, but not the ability to change the names or group
controls for the product.  The same group that was permitted to add, delete, or
edit components would also (coincidentally) be able to bless the group of users
with access to the product's bugs.

So, I have a few questions....
What is the list of roles and what should they be called?
Does this scheme sound like the right thing to do?

My list of roles is as follows....
role_editproduct - can change EVERYTHING about the product
role_admincomponents - can add, delete, or edit components in the product
role_editcomponents - can only make changes to existing components

We already have bless groups, so user edits are covered.
There might be some merit in letting blessers change a group's regexp as well.

What else would need this scheme??

Comment 6

[Jens Dueholm Christensen]

As I see it, the roles needed are:

- Project Owners, who can change every aspect of the project - user privileges 
to different components in the project, add/delete components in the project, 
contents of the components, edit users (automatically add new users to the 
project and delete users already connected to the project)
- Project Admins, who can change/add/delete every component in the project
- Project Users, who can confirm and change bugs (regular users)

Then it's a simple matter for us to use our project-setup tool to add a new 
project and projectowner to bugzilla and let him do the rest of the work - like 
adding new users and making sure they get the correct permissions etc. etc. - 
thus reducing our time spend on administrating bugzilla for our projects.

Comment 7

[Gervase Markham [:gerv]]

My great concern about things like this is that, in making the permissions
system flexible enough to accommodate everyone's wishes, we make it too
complicated in the average case (and damage Bugzilla's performance also.) A page
full of checkboxes is a miserable way to try and manage permissions,
particularly if your needs are simple.

If you have seven "roles" per product, who is going to manage them all? How are
we going to keep performance sane when every last action has to be cross-checked
against some permissions matrix?

Gerv 

Comment 8

[Dave Miller [:justdave]]

There would not be a "premissions matrix" needing to be checked every time
something happened.  All of these are situations where only one of these roles
would apply in any given situation.

The key thing here is to set reasonable defaults when a new product is created,
so that most people never even have to touch them.  For example, the proposed
plan has everything inheriting from the global groups for those functions... if
all a given site uses is the global permissions, then they'll never have to
touch the product specific ones.

Do remember that because of the way group inheritance works, you still only have
to check the product-specific group, because it inherits from the global one. 
You don't have to check both the product-specific one and the global one.

Comment 9

[Dave Miller [:justdave]]

*** Bug 110947 has been marked as a duplicate of this bug. ***

Comment 10

[Dave Miller [:justdave]]

*** Bug 194684 has been marked as a duplicate of this bug. ***

Comment 11

[Dave Miller [:justdave]]

*** Bug 230894 has been marked as a duplicate of this bug. ***

Comment 12

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 13

[Frédéric Buclin]

(In reply to comment #5)
> To determine if a user can edit component X of product Y...

Do not forget to include classifications in the discussion.

Would we have something like component < product < classification < projet?

Comment 14

[Frédéric Buclin]

*** Bug 230895 has been marked as a duplicate of this bug. ***

Comment 15

[Frédéric Buclin]

*** Bug 296574 has been marked as a duplicate of this bug. ***

Comment 16

[Frédéric Buclin]

*** Bug 329902 has been marked as a duplicate of this bug. ***

Comment 17

[Frédéric Buclin]

I will *try* to have it implemented for Bugzilla 3.0. This lets me less than 2 months.

Comment 18

[Frédéric Buclin]

Hum... This is easier than expected. I should have a patch ready in the next few days.

Comment 19

[Frédéric Buclin]

Attachment: patch, v1

Each product has 3 product-specific groups: editcomponents_foo, editbugs_foo and canconfirm_foo, where foo is the product id, as suggested by justdave in a previous comment (to avoid problems when renaming a product).

Comment 20

[Max Kanat-Alexander]

Comment on attachment 230219 [details] [diff] [review]
patch, v1

Noooooooo.

Instead there should be new columns in group_control_map, for each one of these.

Comment 21

[Max Kanat-Alexander]

Okay, LpSolit has asked me to be more specific:

Instead of adding three new groups for every product (which is ridiculous and would quickly spiral out of control), you can just add three new boolean columns to group_control_map:

can_edit_components
can_edit_versions
can_edit_bugs
can_edit_groups

You may also want to rename the current "editbugs" field to "restrict_editbugs".

Obviously can_edit_components and can_edit_versions is easier, so you might want to do that first and then do can_edit_bugs in a separate bug.

Only users with editcomponents should be able to change can_edit_groups, probably.

Anyhow, per-product permissions is the whole point of group_control_map.

I already have a patch awaiting copyright release which allows specific groups to have can_admin privs, which allows them to edit components, versions, target milestones, and groups. However, when I bring it upstream I could separate these out, if you want.

Comment 22

[Frédéric Buclin]

I don't want to have can_edit_each_individual_bit_of_the_system_separately privs. This seems as "ridiculous" as having 3 groups per product (to quote yourself). IMO, editcomponents, editbugs and canconfirm are really what we need. At least, this is already several orders of magnitude better than what Bugzilla permits us to do currently.

If you add these 3 new columns to group_control_map, you would have to add the ID (for each cell) of the group having, say, canconfirm privs for the FooBar product. In the worst case, the super_big_admin_chief (e.g. me :)) may decide that I don't want to have a group to have two distinct roles, i.e. I want one different group per cell in the group_control_map table (which I would be allowed to do; why not?). In this case, I would end with the same number of groups as my actual patch! This is a edge-case, but which *could* happen (this is to make me feel not completely stupid with my "ridiculous" patch).

Your approach makes sense (unfortunately for me; this means I have to update my patch ;)), but I definitely prefer my molecular approach (editcomponents privs per product) than your subatomic approach (editmilestones, editversions, edit_sortkeys_of_the_milestones privs per product). IMO, we shouldn't implement product-specific privs which don't have corresponding global privs, i.e. don't implement product-specific editmilestones privs if we don't have a global editmilestones privilege. At least I would not do this in this bug.

So I will keep your idea of using group_control_map, but I will only add the 3 product-specific privs I used in my patch, i.e. editcomponents, canconfirm and editbugs.

Comment 23

[Frédéric Buclin]

Attachment: patch, v2

This patch v2 uses the group_control_map approach suggested by mkanat. Tested, works fine.

Comment 24

[Max Kanat-Alexander]

Comment on attachment 230266 [details] [diff] [review]
patch, v2

This patch is pretty significant. I hate to say this, but could we at least split out editbugs for another bug? That would allow me to do a good review on just the strucure of the implementation, without having to worry about all the details of three prefs.

Comment 25

[Max Kanat-Alexander]

Comment on attachment 230266 [details] [diff] [review]
patch, v2

>+# 2006-07-22 [e-mail] - Bug 189627
>+$dbh->bz_add_column('group_control_map', 'editcomponents',
>+                    {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});

  I'd rather call this something more descriptive, like can_admin.

>+$dbh->bz_add_column('group_control_map', 'editbugs',
>+                    {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});

  I'd prefer can_edit_bugs.

>+$dbh->bz_add_column('group_control_map', 'canconfirm',
>+                    {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});

  I'd prefer can_confirm_bugs.

>Index: Bugzilla/User.pm
> [snip]
> sub in_group {
>-    my ($self, $group) = @_;

  I really do not like this implementation. I'd much prefer a $user->can($permission, $bug). That would allow us a lot more flexibility in the future. Except we'd have to call it something other than "can", because that's a reserved method name.

  Product permisions and groups are different things.

> sub in_group_id {
>@@ -411,6 +432,19 @@
>     return exists $j{$id} ? 1 : 0;
> }
> 
>+sub get_specific_products_for_group {

  In the implementation I reviewed internally, we had a function called "get_adminable_products". I think that's probably better. You could also have a "get_canconfirm_products" and "get_editbugs_products" if you'd like, or perhaps we could come up with some more modular way to do it. If we could find some way to combine all get_X_products into a single method, and still keep it performant, that would be great.

  Either that or this method just needs a better name. I have no idea what "specific products for group" means.

  Also, I'd much rather have it return Product objects.

>+sub check_can_admin_product {

  I'd much rather have can_admin_product and let the caller decide what to do about it.

>+Any group having <b>editcomponents</b> selected allows users being
>+in this group to edit all aspects of this product, among which components,
>+milestones and versions. This does not give <i>editcomponents</i> privileges
>+for other products.

  They should also be able to edit group controls for the product, except for the editcomponents priv itself.

Comment 26

[Kevin Benton]

... much deleted ...

> >Index: Bugzilla/User.pm
> > [snip]
> > sub in_group {
> >-    my ($self, $group) = @_;
> 
>   I really do not like this implementation. I'd much prefer a
> $user->can($permission, $bug). That would allow us a lot more flexibility in
> the future. Except we'd have to call it something other than "can", because
> that's a reserved method name.
> 
>   Product permisions and groups are different things.

s/can/may/ ? :)

> > sub in_group_id {
> >@@ -411,6 +432,19 @@
> >     return exists $j{$id} ? 1 : 0;
> > }
> > 
> >+sub get_specific_products_for_group {

I like $user->may('see', $product) to do this better.  That way, we don't have a ton of special-purpose functions laying around when a single function will do nicely.

Also, please don't forget classifications...  This may be worth creating another bug for, however, I want to be sure we can segment classifications properly so they can follow these criteria as well.

Comment 27

[Max Kanat-Alexander]

> I like $user->may('see', $product) to do this better.  That way, we don't have
> a ton of special-purpose functions laying around when a single function will do
> nicely.

  Yeah. Of course, thinking about it, that isn't *too* different from LpSolit's in_group implementation, at the moment. I think in_group isn't as flexible, but I can see why he did it.

> Also, please don't forget classifications...  This may be worth creating
> another bug for, however, I want to be sure we can segment classifications
> properly so they can follow these criteria as well.

  Sure. You could make the function $user->may('see', { product => $product }). That way you could add classifications in the future.

  It could load up a hash called {permissions} that explained the permissions for the user against every product, and then it could search that hash to answer any question.

Comment 28

[Frédéric Buclin]

Note to self: per discussion on IRC with mkanat, all I have to do is to rename User::get_specific_products_for_group() to something more meaningful and to implement a cache in User::in_group() in case it's called twice with the same product name. I think that's all IIRC. :)

Comment 29

[Frédéric Buclin]

*** Bug 330886 has been marked as a duplicate of this bug. ***

Comment 30

[Max Kanat-Alexander]

This is a blocker, per our discussion in our most recent meeting.

Comment 31

[Frédéric Buclin]

Attachment: patch, v3

Fixed as per comment 28

Comment 32

[Max Kanat-Alexander]

Comment on attachment 242824 [details] [diff] [review]
patch, v3

  In general, I would rather in_group took a product object than a product id.

  I'd very much like you to remove editbugs and canconfirm from this patch, and do them in a separate patch.

>+++ Bugzilla/User.pm	20 Oct 2006 00:50:48 -0000
> sub in_group {
> [snip]
>+        my $valid_groups = ['editcomponents', 'editbugs', 'canconfirm'];

  This needs to be in a constant somewhere.

>+        if (!defined $self->{"product_$product_id"}->{$group}) {

  If $self->{"product_$product_id"} is undefined, this will throw an error.

>+                $dbh->selectrow_array("SELECT 1
>+                                         FROM group_control_map


>+                                        WHERE product_id = ?
>+                                          AND $group != 0
>+                                          AND group_id IN (" . $self->groups_as_string . ") " .

  Usually I prefer:

  WHERE condition
        AND condition
        AND condition

  Also, you could start the SELECT on the next line, and move the whole thing back and fit more per-line.

>+                                         $dbh->sql_limit(1),

  Can that really ever validly return more than one result?

>+        return $self->{"product_$product_id"}->{$group};
>+    }
>+    else {
>+        return 0;
>+    }

  Just leave off the "else" and "return 0" at the end of the function.

>@@ -419,6 +444,19 @@
>     return exists $j{$id} ? 1 : 0;
> }
> 
>+sub get_products_by_group {

  I think by_permission might be clearer.

>+    return [] if (lsearch($valid_groups, $group) < 0);

  If you add the constant for $valid_groups, you can just make it a qw(), and then this can be a grep.

  Also, if the user has canconfirm, editbugs, or editcomponents, then shouldn't this return all products that he can see?

  And I think we should return a list of Product objects, not a list of product ids.

>+sub check_can_admin_product {
>+    my ($self, $product_name) = @_;

  Wouldn't it be better to just pass in a Product object?

>+++ userprefs.cgi	20 Oct 2006 00:50:54 -0000
> [snip]
>+        my $prod_ids = $user->get_products_by_group($privs);
>+        my $products = Bugzilla::Product->new_from_list($prod_ids);
>+        # Do not mention products the user is not allowed to access.
>+        @$products = grep {$user->can_see_product($_->name)} @$products;

  Why is the function returning the products at all, if the user can't see them? If he can edit them, he can see them.

>Index: editcomponents.cgi
> [snip]
> unless ($product_name) {
>-    $vars->{'products'} = $user->get_selectable_products;
>+    my $selectable_products = $user->get_selectable_products;
>+    # If the user has editcomponents privs for some products only,
>+    # we have to restrict the list of products to display.
>+    unless ($user->in_group('editcomponents')) {

  When I did this, we just had get_adminable_products. That's why I think get_product_by should return everything the user has that permission for.

>@@ -584,19 +589,23 @@
> 
>     my $sth_Insert = $dbh->prepare('INSERT INTO group_control_map
>                                     (group_id, product_id, entry, membercontrol,
>-                                     othercontrol, canedit)
>-                                    VALUES (?, ?, ?, ?, ?, ?)');
>+                                     othercontrol, canedit, editcomponents,
>+                                     canconfirm, editbugs)
>+                                    VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)');

  Users should only be able to update "editcomponents" on a product if they have global editcomponents. For the other two permissions, though, they shouldn't need global editcomponents.

  Maybe the code does this somewhere already and I just missed it.


templates:
>+The following settings control permit you to give some privileges

  "permit you" is unclear.

>+on a <b>per product basis</b>,

  Should be "per-product"

> this means without affecting other products.

  People know what per-product means.

>+<p>
>+Any group having <b>editcomponents</b> selected allows users being
>+in this group

  "who are in this group" (instead of "being in...")

> to edit all aspects of this product, among which components,

  "including" instead of "among which"

>+milestones and versions. This does not give <i>editcomponents</i> privileges
>+for other products.

  I think that last sentence is not necessary. (This applies to all the paragraphs, as do my other comments.)

>+Any group having <b>canconfirm</b> selected allows users being
>+in this group to confirm [% terms.bugs %] being in this product.

  Remove "being" before "in this product."

>+Any group having <b>editbugs</b> selected allows users being
>+in this group to edit all fields of [% terms.bugs %] being in this product.

  Remove "being" before "in this product."


  I didn't test the code. The above are just my comments from the code inspection.

Comment 33

[Frédéric Buclin]

(In reply to comment #32)
>   In general, I would rather in_group took a product object than a product id.

I disagree! In all cases is the product ID available in the caller and permits to do the required checks. Building a product object to extract its ID again is meaningless. I definitely against this idea. ;)


>   I'd very much like you to remove editbugs and canconfirm from this patch, and
> do them in a separate patch.

/me sighs.

> >+                                         $dbh->sql_limit(1),
> 
>   Can that really ever validly return more than one result?

Of course, yes. As soon as more than one group applies to a given product.


>   And I think we should return a list of Product objects, not a list of product
> ids.

As I said above, there is no need to return objects if all we need is IDs.


> >+sub check_can_admin_product {
> >+    my ($self, $product_name) = @_;
> 
>   Wouldn't it be better to just pass in a Product object?

edit*.cgi get a product name as param. I prefer to let this method build the product object than doing it 20 times in admin pages.


> >+        my $prod_ids = $user->get_products_by_group($privs);
> >+        my $products = Bugzilla::Product->new_from_list($prod_ids);
> >+        # Do not mention products the user is not allowed to access.
> >+        @$products = grep {$user->can_see_product($_->name)} @$products;
> 
>   Why is the function returning the products at all, if the user can't see
> them? If he can edit them, he can see them.

No. products returned by can_edit_product() are not necessarily a subclass of products returned by can_see_product(). That's the current behavior, and this is not the goal of this patch to change this behavior.

Comment 34

[Myk Melez [:myk] [@mykmelez]]

mozdev would find this very useful:

http://bugzilla.mozdev.org/show_bug.cgi?id=15643

Any chance an updated patch would make Bugzilla 3.0?

Comment 35

[Max Kanat-Alexander]

(In reply to comment #34)
> Any chance an updated patch would make Bugzilla 3.0?

  Yeah, absolutely. It's one of my highest priorities.

Comment 36

[Frédéric Buclin]

(In reply to comment #34)
> Any chance an updated patch would make Bugzilla 3.0?

Don't worry. It's a blocker for 3.0, and the assignee is pretty active. ;)

Comment 37

[Frédéric Buclin]

Attachment: patch, v4 part 1 (editcomponents)

This patch contains all the code specific to editcomponents + DB changes which are common to editcomponents, editbugs and canconfirm as well as changes in editproducts.cgi which are also common to these per-product privs.

As I said in my previous comment, I don't want to pass product objects as that's not what we want in most cases (and the path product ID -> object -> ID is a waste of time).

The patch specific to editbugs and canconfirm will follow in a few seconds...

Comment 38

[Frédéric Buclin]

Attachment: patch, v4 part 2 (editbugs + canconfirm)

And here is the code specific to editbugs and canconfirm. The patch v4 part1 is required before applying this one (due to shared code which is in part 1).

Comment 39

[Max Kanat-Alexander]

Comment on attachment 244755 [details] [diff] [review]
patch, v4 part 1 (editcomponents)

>+sub get_products_by_permission {
>+    my ($self, $group) = @_;
>+    # Make sure $group exists on a per-product basis.
>+    return [] unless (grep {$_ eq $group} PER_PRODUCT_PRIVILEGES);
>+
>+    my $dbh = Bugzilla->dbh;
>+    return $dbh->selectcol_arrayref("SELECT DISTINCT product_id
>+                                       FROM group_control_map
>+                                      WHERE $group != 0
>+                                        AND group_id IN(" . $self->groups_as_string . ")");
>+}

  Okay. This one *must* return Product objects. I know that maybe you don't need them now, but we almost certainly will in the future.

  Also, I'd think that people *never* have a permission on a product they can't access. Wouldn't it simplify calling code if we just checked that here? (Or somewhere around here.)

>+sub check_can_admin_product {
>+    my ($self, $product_name) = @_;
>+    my $specific_product_ids = [];

  Wouldn't it be better to just do check_has_perm_on_product (that is, a more generic version)?

>+# Privileges which can be per-product.
>+use constant PER_PRODUCT_PRIVILEGES => ('editcomponents', 'editbugs', 'canconfirm');

  Nit: That could be a qw().

>Index: template/en/default/admin/users/confirm-delete.html.tmpl
>+              [% IF user.in_group("editcomponents", component.productname) %]

  productname? That doesn't look right. Same thing below.

>Index: template/en/default/admin/products/groupcontrol/edit.html.tmpl
>@@ -119,6 +122,18 @@
>+            <input type=checkbox value=1 name=editcomponents_[% group.id %]
>+            [% " checked=\"checked\"" IF group.editcomponents %]>

  Nit: You can do ' checked="checked"' which allows you to avoid the \".

>Index: template/en/default/account/prefs/permissions.html.tmpl
>+        [% FOREACH privs = ["editcomponents", "canconfirm", "editbugs"] %]

  Should be from constants instead.

Comment 40

[Frédéric Buclin]

Attachment: patch, v5 part 1 (editcomponents)

OK, I fixed all your comments except nits. About the list in permissions.html.tmpl, TT seems unable to handle arrays correctly (it seems to only like references to arrays). I don't want to mess the rest of the code only to make TT happy.

Comment 41

[Frédéric Buclin]

Attachment: patch, v5 part 2 (editbugs + canconfirm)

Comment 42

[Frédéric Buclin]

Attachment: patch, v5 part 1 (editcomponents)

oops, my other patch had some code from another patch in it. :(

Comment 43

[Max Kanat-Alexander]

Comment on attachment 245118 [details] [diff] [review]
patch, v5 part 1 (editcomponents)

This looks fine. My only comment is that somebody with product-specific editcomponents shouldn't be able to grant editcomponents to other groups, but they should otherwise be able to control groups.

We can handle that in another bug, if that's not already handled here.

Comment 44

[Max Kanat-Alexander]

Comment on attachment 245117 [details] [diff] [review]
patch, v5 part 2 (editbugs + canconfirm)

Okay, yeah, this looks fine. I don't have time to test it right now, though. However, I trust you, and you'll be the one leading QA on it anyway. :-)

Comment 45

[Frédéric Buclin]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.125; previous revision: 1.124
done
Checking in editcomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v  <--  editcomponents.cgi
new revision: 1.79; previous revision: 1.78
done
Checking in editmilestones.cgi;
/cvsroot/mozilla/webtools/bugzilla/editmilestones.cgi,v  <--  editmilestones.cgi
new revision: 1.56; previous revision: 1.55
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.131; previous revision: 1.130
done
Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.141; previous revision: 1.140
done
Checking in editversions.cgi;
/cvsroot/mozilla/webtools/bugzilla/editversions.cgi,v  <--  editversions.cgi
new revision: 1.53; previous revision: 1.52
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.151; previous revision: 1.150
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.109; previous revision: 1.108
done
Checking in Bugzilla/Attachment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Attachment.pm,v  <--  Attachment.pm
new revision: 1.41; previous revision: 1.40
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.164; previous revision: 1.163
done
Checking in Bugzilla/Component.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Component.pm,v  <--  Component.pm
new revision: 1.13; previous revision: 1.12
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.59; previous revision: 1.58
done
Checking in Bugzilla/Product.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Product.pm,v  <--  Product.pm
new revision: 1.21; previous revision: 1.20
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.141; previous revision: 1.140
done
Checking in Bugzilla/DB/Schema.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Schema.pm,v  <--  Schema.pm
new revision: 1.75; previous revision: 1.74
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.22; previous revision: 1.21
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.89; previous revision: 1.88
done
Checking in template/en/default/account/prefs/permissions.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/permissions.html.tmpl,v  <--  permissions.html.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/admin/products/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/admin/products/footer.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/footer.html.tmpl,v  <--  footer.html.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in template/en/default/admin/products/groupcontrol/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/groupcontrol/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/admin/users/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.15; previous revision: 1.14
done
Checking in template/en/default/attachment/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.31; previous revision: 1.30
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.69; previous revision: 1.68
done
Checking in template/en/default/global/site-navigation.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/site-navigation.html.tmpl,v  <--  site-navigation.html.tmpl
new revision: 1.21; previous revision: 1.20
done
Checking in template/en/default/global/useful-links.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/useful-links.html.tmpl,v  <--  useful-links.html.tmpl
new revision: 1.53; previous revision: 1.52
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.197; previous revision: 1.196
done

Comment 46

[Frédéric Buclin]

*** Bug 362116 has been marked as a duplicate of this bug. ***

Comment 47

[Frédéric Buclin]

*** Bug 363811 has been marked as a duplicate of this bug. ***

Comment 48

[Max Kanat-Alexander]

Added to the relnotes currently attached to bug 349423.