271596 - editcomponents priv allows you to see/edit products you don't have access to

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[timeless]

Access Denied
	  	
You do not have the permissions necessary to access that product.

--
That's kinda funny, since I can load:
https://bugzilla.mozilla.org/editcomponents.cgi?product=Talkback and read the
list there.

Comment 1

[Frédéric Buclin]

Attachment: allow users in the editcomponents group to view all products, v1

Comment 2

[Dave Miller [:justdave]]

The correct fix for this situation is to prevent the person with editcomponents
from being able to see/edit products that they don't have access to in
editproducts.cgi.  Or better yet, a product-level editcomponents priv.

There are things on b.m.o that timeless shouldn't be able to see (despite his
ability to know where the places are that he can't get to)

I'm not outright WONTFIXing this, because it might make sense for the global
editcomponents priv once we have product-level editcomponents that we can hand
out.  But there's no way I'm going to allow this to land prior to that.

Comment 3

[timeless]

i don't see the problem, when you rework editcomponents to be per product, you'd
just rework one more cgi. either the priv disappears globally in which case you
remove it from one more script, or it's supplemented by a new permission which
is per product and would need to be applied to this script too in about the same
place as the current bit. the fact is that this really only applies to me anyway
:), and it's really just a convenience, i can just as easily get the info i want
from editcomponents.

Comment 4

[Dave Miller [:justdave]]

ok, we discussed this on IRC...  we're going to fix editcomponents so unless
you're a member of group 'admin', 'editcomponents' only lets you edit products
you can otherwise see anyway.

Comment 5

[Dave Miller [:justdave]]

Comment on attachment 172159 [details] [diff] [review]
allow users in the editcomponents group to view all products, v1

review- per previous discussion

Comment 6

[Dave Miller [:justdave]]

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

Comment 7

[Andreas Höfler]

Is there any chance that this fix can also land on 2.18.2? 
For us it would be quite important that 'admins' of a product can edit all
aspects of their own product but not those of all others.

I would even see that as a security-blocker...

Comment 8

[Frédéric Buclin]

I won't backport this patch for 2.18 or even 2.20 as it implies a lot of new
code and files which do not exist for earlier versions, mainly Product.pm.

Note to self: when bug 293524 is fixed, the single remaining place to fix will
be editflagtypes.cgi (the list of products available for flag inclusions and
exclusions).

Comment 9

[Frédéric Buclin]

(In reply to comment #4)
> ok, we discussed this on IRC...  we're going to fix editcomponents so unless
> you're a member of group 'admin', 'editcomponents' only lets you edit products
> you can otherwise see anyway.

editcomponents.cgi is not the only one file involved in the discussion.
editversions.cgi, editmilestones.cgi, editproducts.cgi and
editclassifications.cgi should also be considered.

But I'm not sure hiding products I'm not allowed to see is the right solution as
I could create a product which has the same name as an existing product (for
editproducts.cgi). Better would be to display product names, but forbid to
edit/get any information on them. comments?

Comment 10

[Dave Miller [:justdave]]

How about if we only allow admins to create new products, or add a separate priv for that?  And then not let someone who can't see all products create new ones?

What we really need here is product-specific admin rights, but that's definitely a new feature.

Comment 11

[Frédéric Buclin]

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

Comment 12

[jessn]

Bug #315068 

What I had in mind, was that a "global" admin or even "(global) Edit component" right was used to create the product initially. Then a "local" Edit component right was granted to one of the persons, which is member of the current product.

Then they will be able to maintain their own product without the need of any global admin rights and they will not be able to access any products that they dont have permission to currently.

Comment 13

[timeless]

Again, at bmo i'm likely to be allowed to create new products, so your restriction is going ot hurt things a lot more than it's going ot help. do it the other way.

Comment 14

[Frédéric Buclin]

Attachment: patch, v2

Only allows the user to edit properties of a product (including its versions, milestones and components) he is allowed to see. Per discussion with justdave on IRC, the user is trustable enough to let him know if a product he is not allowed to edit exists or not (because he could guess it anyway when creating or renaming a product to an existing name).

editclassifications.cgi uses 'editclassifications' privs and so doesn't need to be considered here.

And editflagtypes.cgi will be fixed separately...

(17:39:05) LpSolit: justdave: if a flag type is applied to a product you cannot see, I suppose editflagtypes.cgi should not allow you to edit/rename/delete this flag type?
(17:39:33) justdave: you know, this is really a huge can of worms
(17:39:40) LpSolit: yes I know
(17:39:41) justdave: make joel do it ;)
(17:39:44) LpSolit: :-D
(17:39:51) LpSolit: can I split this bug then?
(17:40:00) justdave: yeah, go for it

Comment 15

[Max Kanat-Alexander]

Please make sure that the "you cannot see all components unless you are in 'admin'" is documented somewhere. :-)

Comment 16

[Frédéric Buclin]

(In reply to comment #15)
> Please make sure that the "you cannot see all components unless you are in
> 'admin'" is documented somewhere. :-)

Actually, this comment is wrong! Depending on how group inheritance is configured, you could be in the 'admin' group but not be allowed to see some products (a good example is our QA installations on landfill ;)).

Comment 17

[Frédéric Buclin]

Checking in editcomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v  <--  editcomponents.cgi
new revision: 1.65; previous revision: 1.64
done
Checking in editmilestones.cgi;
/cvsroot/mozilla/webtools/bugzilla/editmilestones.cgi,v  <--  editmilestones.cgi
new revision: 1.47; previous revision: 1.46
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.108; previous revision: 1.107
done
Checking in editversions.cgi;
/cvsroot/mozilla/webtools/bugzilla/editversions.cgi,v  <--  editversions.cgi
new revision: 1.42; previous revision: 1.41
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.97; previous revision: 1.96
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.143; previous revision: 1.142
done

Comment 18

[Max Kanat-Alexander]

Added to the Bugzilla 2.22 Release Notes in bug 322960, including data from comment 16 and the other relevant comments.