Bug 271596: editcomponents priv allows you to see/edit products you don't have access to - Patch by Frédéric Buclin <LpSolit@gmail.com> r=wicked a=justdave

6 files changed, 88 insertions, 41 deletions

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index d35077a4b..9f6c415ef 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -452,12 +452,15 @@ sub can_see_product {
 
 sub get_selectable_products {
     my $self = shift;
+    my $classification_id = shift;
 
     if (defined $self->{selectable_products}) {
         return $self->{selectable_products};
     }
 
     my $dbh = Bugzilla->dbh;
+    my @params = ();
+
     my $query = "SELECT id " .
                 "FROM products " .
                 "LEFT JOIN group_control_map " .
@@ -470,9 +473,17 @@ sub get_selectable_products {
     }
     $query .= "AND group_id NOT IN(" . 
                $self->groups_as_string . ") " .
-              "WHERE group_id IS NULL ORDER BY name";
+              "WHERE group_id IS NULL ";
+
+    if (Param('useclassification') && $classification_id) {
+        $query .= "AND classification_id = ? ";
+        detaint_natural($classification_id);
+        push(@params, $classification_id);
+    }
 
-    my $prod_ids = $dbh->selectcol_arrayref($query);
+    $query .= "ORDER BY name";
+
+    my $prod_ids = $dbh->selectcol_arrayref($query, undef, @params);
     my @products;
     foreach my $prod_id (@$prod_ids) {
         push(@products, new Bugzilla::Product($prod_id));
@@ -1603,9 +1614,12 @@ method should be called in such a case to force reresolution of these groups.
 
 =item C<get_selectable_products>
 
- Description: Returns all products the user is allowed to access.
+ Description: Returns all products the user is allowed to access. This list
+              is restricted to some given classification if $classification_id
+              is given.
 
- Params:      none
+ Params:      $classification_id - (optional) The ID of the classification
+                                   the products belong to.
 
  Returns:     An array of product objects, sorted by the product name.
 
diff --git a/editcomponents.cgi b/editcomponents.cgi
index 60074cb40..d514fb3bf 100755
--- a/editcomponents.cgi
+++ b/editcomponents.cgi
@@ -20,6 +20,7 @@
 #
 # Contributor(s): Holger Schurig <holgerschurig@nikocity.de>
 #                 Terry Weissman <terry@mozilla.org>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 #
 # Direct any questions on this source code to
 #
@@ -71,21 +72,22 @@ my $showbugcounts = (defined $cgi->param('showbugcounts'));
 #
 
 unless ($product_name) {
-
-    my @products = Bugzilla::Product::get_all_products();
-
+    $vars->{'products'} = $user->get_selectable_products;
     $vars->{'showbugcounts'} = $showbugcounts;
-    $vars->{'products'} = \@products;
-    $template->process("admin/components/select-product.html.tmpl",
-                       $vars)
-      || ThrowTemplateError($template->error());
-
 
+    $template->process("admin/components/select-product.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
     exit;
 }
 
+# First make sure the product name is valid.
 my $product = Bugzilla::Product::check_product($product_name);
 
+# Then make sure the user is allowed to edit properties of this product.
+$user->can_see_product($product->name)
+  || ThrowUserError('product_access_denied', {product => $product->name});
+
+
 #
 # action='' -> Show nice list of components
 #
diff --git a/editmilestones.cgi b/editmilestones.cgi
index 95babd737..c87828526 100755
--- a/editmilestones.cgi
+++ b/editmilestones.cgi
@@ -60,20 +60,22 @@ my $showbugcounts = (defined $cgi->param('showbugcounts'));
 #
 
 unless ($product_name) {
-    
-    my @products = Bugzilla::Product::get_all_products();
-
+    $vars->{'products'} = $user->get_selectable_products;
     $vars->{'showbugcounts'} = $showbugcounts;
-    $vars->{'products'} = \@products;
-    $template->process("admin/milestones/select-product.html.tmpl",
-                       $vars)
-      || ThrowTemplateError($template->error());
 
+    $template->process("admin/milestones/select-product.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
     exit;
 }
 
+# First make sure the product name is valid.
 my $product = Bugzilla::Product::check_product($product_name);
 
+# Then make sure the user is allowed to edit properties of this product.
+$user->can_see_product($product->name)
+  || ThrowUserError('product_access_denied', {product => $product->name});
+
+
 #
 # action='' -> Show nice list of milestones
 #
diff --git a/editproducts.cgi b/editproducts.cgi
index b4007a2f4..2b7c5dc5d 100755
--- a/editproducts.cgi
+++ b/editproducts.cgi
@@ -82,15 +82,10 @@ if (Param('useclassification')
     && !$classification_name
     && !$product_name)
 {
-    my @classifications =
-        Bugzilla::Classification::get_all_classifications();
+    $vars->{'classifications'} = $user->get_selectable_classifications;
     
-    $vars->{'classifications'} = \@classifications;
-    
-    $template->process("admin/products/list-classifications.html.tmpl",
-                       $vars)
+    $template->process("admin/products/list-classifications.html.tmpl", $vars)
         || ThrowTemplateError($template->error());
-
     exit;
 }
 
@@ -101,19 +96,19 @@ if (Param('useclassification')
 #
 
 if (!$action && !$product_name) {
-    my @products;
+    my $products;
 
     if (Param('useclassification')) {
         my $classification = 
             Bugzilla::Classification::check_classification($classification_name);
 
-        @products = @{$classification->products};
+        $products = $user->get_selectable_products($classification->id);
         $vars->{'classification'} = $classification;
     } else {
-        @products = Bugzilla::Product::get_all_products;
+        $products = $user->get_selectable_products;
     }
 
-    $vars->{'products'} = \@products;
+    $vars->{'products'} = $products;
     $vars->{'showbugcounts'} = $showbugcounts;
 
     $template->process("admin/products/list.html.tmpl", $vars)
@@ -327,9 +322,13 @@ if ($action eq 'new') {
 #
 
 if ($action eq 'del') {
-    
+    # First make sure the product name is valid.
     my $product = Bugzilla::Product::check_product($product_name);
 
+    # Then make sure the user is allowed to edit properties of this product.
+    $user->can_see_product($product->name)
+      || ThrowUserError('product_access_denied', {product => $product->name});
+
     if (Param('useclassification')) {
         my $classification = 
             Bugzilla::Classification::check_classification($classification_name);
@@ -353,8 +352,12 @@ if ($action eq 'del') {
 #
 
 if ($action eq 'delete') {
-    
+    # First make sure the product name is valid.
     my $product = Bugzilla::Product::check_product($product_name);
+
+    # Then make sure the user is allowed to edit properties of this product.
+    $user->can_see_product($product->name)
+      || ThrowUserError('product_access_denied', {product => $product->name});
     
     $vars->{'product'} = $product;
 
@@ -425,9 +428,13 @@ if ($action eq 'delete') {
 #
 
 if ($action eq 'edit' || (!$action && $product_name)) {
-
+    # First make sure the product name is valid.
     my $product = Bugzilla::Product::check_product($product_name);
 
+    # Then make sure the user is allowed to edit properties of this product.
+    $user->can_see_product($product->name)
+      || ThrowUserError('product_access_denied', {product => $product->name});
+
     if (Param('useclassification')) {
         my $classification; 
         if (!$classification_name) {
@@ -476,8 +483,13 @@ if ($action eq 'edit' || (!$action && $product_name)) {
 #
 
 if ($action eq 'updategroupcontrols') {
-
+    # First make sure the product name is valid.
     my $product = Bugzilla::Product::check_product($product_name);
+
+    # Then make sure the user is allowed to edit properties of this product.
+    $user->can_see_product($product->name)
+      || ThrowUserError('product_access_denied', {product => $product->name});
+
     my @now_na = ();
     my @now_mandatory = ();
     foreach my $f ($cgi->param()) {
@@ -739,8 +751,13 @@ if ($action eq 'update') {
 
     my $checkvotes = 0;
 
+    # First make sure the product name is valid.
     my $product_old = Bugzilla::Product::check_product($product_old_name);
 
+    # Then make sure the user is allowed to edit properties of this product.
+    $user->can_see_product($product_old->name)
+      || ThrowUserError('product_access_denied', {product => $product_old->name});
+
     if (Param('useclassification')) {
         my $classification; 
         if (!$classification_name) {
@@ -971,7 +988,13 @@ if ($action eq 'update') {
 #
 
 if ($action eq 'editgroupcontrols') {
+    # First make sure the product name is valid.
     my $product = Bugzilla::Product::check_product($product_name);
+
+    # Then make sure the user is allowed to edit properties of this product.
+    $user->can_see_product($product->name)
+      || ThrowUserError('product_access_denied', {product => $product->name});
+
     # Display a group if it is either enabled or has bugs for this product.
     my $groups = $dbh->selectall_arrayref(
         'SELECT id, name, entry, membercontrol, othercontrol, canedit,
diff --git a/editversions.cgi b/editversions.cgi
index eae1001ca..be2c8a3c6 100755
--- a/editversions.cgi
+++ b/editversions.cgi
@@ -69,20 +69,22 @@ my $showbugcounts = (defined $cgi->param('showbugcounts'));
 #
 
 unless ($product_name) {
-
-    my @products = Bugzilla::Product::get_all_products();
-
+    $vars->{'products'} = $user->get_selectable_products;
     $vars->{'showbugcounts'} = $showbugcounts;
-    $vars->{'products'} = \@products;
-    $template->process("admin/versions/select-product.html.tmpl",
-                       $vars)
-      || ThrowTemplateError($template->error());
 
+    $template->process("admin/versions/select-product.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
     exit;
 }
 
+# First make sure the product name is valid.
 my $product = Bugzilla::Product::check_product($product_name);
 
+# Then make sure the user is allowed to edit properties of this product.
+$user->can_see_product($product->name)
+  || ThrowUserError('product_access_denied', {product => $product->name});
+
+
 #
 # action='' -> Show nice list of versions
 #
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index e911b39d2..350f2c8a0 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1015,6 +1015,10 @@
     create the milestone '[% defaultmilestone FILTER html %]'</a> before
     it can be made the default milestone for product '[% product FILTER html %]'.
 
+  [% ELSIF error == "product_access_denied" %]
+    [% title = "Product Access Denied" %]
+    You are not allowed to edit properties of product '[% product FILTER html %]'.
+
   [% ELSIF error == "product_blank_name" %]
     [% title = "Blank Product Name Not Allowed" %]
     You must enter a name for the new product.