launchpad-reviewers team mailing list archive

[Ines Almeida <mp+440317@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:prevent-bug-reporter-from-accessing-bug-in-disabled-product into launchpad:master.

Commit message:
Exclude bugs from inactive product series in queries

Update query to exclude BugTasks from inactive product series from a user's reported bugs list

LP: #1321055

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1321055 in Launchpad itself: "Deactivating a product doesn't hide its productseries' bugs"
  https://bugs.launchpad.net/launchpad/+bug/1321055

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/440317

Update a query and introduced 2 unit tests that verify the change:
 - one that ensures the user can't see the bug they reported on an inactive product (which passed before and after the change)
 - one that ensures the user can't see the bug they reported on an inactive product *series* (which failed before the change)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:prevent-bug-reporter-from-accessing-bug-in-disabled-product into launchpad:master.

diff --git a/lib/lp/bugs/model/bugtasksearch.py b/lib/lp/bugs/model/bugtasksearch.py
index 19598bc..1cebcef 100644
--- a/lib/lp/bugs/model/bugtasksearch.py
+++ b/lib/lp/bugs/model/bugtasksearch.py
@@ -72,7 +72,7 @@ from lp.registry.model.distribution import Distribution
 from lp.registry.model.milestone import Milestone
 from lp.registry.model.milestonetag import MilestoneTag
 from lp.registry.model.person import Person
-from lp.registry.model.product import Product, ProductSet
+from lp.registry.model.product import Product, ProductSeries, ProductSet
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database.bulk import load
 from lp.services.database.decoratedresultset import DecoratedResultSet
@@ -84,6 +84,7 @@ from lp.services.database.sqlbase import (
 from lp.services.database.stormexpr import (
     ArrayAgg,
     ArrayIntersects,
+    IsTrue,
     Unnest,
     WithMaterialized,
     fti_search,
@@ -609,15 +610,30 @@ def _build_query(params):
         and params.productseries is None
         and params.distroseries is None
     ):
-        extra_clauses.append(
-            Or(BugTaskFlat.product == None, Product.active == True)
+        extra_clauses.append(IsTrue(Product.active))
+
+        join_tables.append(
+            (
+                ProductSeries,
+                LeftJoin(
+                    ProductSeries,
+                    BugTaskFlat.productseries_id == ProductSeries.id,
+                ),
+            )
         )
+
         join_tables.append(
             (
                 Product,
                 LeftJoin(
                     Product,
-                    And(BugTaskFlat.product_id == Product.id, Product.active),
+                    And(
+                        Or(
+                            BugTaskFlat.product_id == Product.id,
+                            ProductSeries.productID == Product.id,
+                        ),
+                        Product.active,
+                    ),
                 ),
             )
         )
diff --git a/lib/lp/registry/tests/test_private_team_visibility.py b/lib/lp/registry/tests/test_private_team_visibility.py
index 6f68e2f..1a7bdab 100644
--- a/lib/lp/registry/tests/test_private_team_visibility.py
+++ b/lib/lp/registry/tests/test_private_team_visibility.py
@@ -24,6 +24,7 @@ from lp.registry.interfaces.teammembership import (
     ITeamMembershipSet,
     TeamMembershipStatus,
 )
+from lp.services.propertycache import clear_property_cache
 from lp.services.webapp.authorization import (
     check_permission,
     clear_cache,
@@ -33,6 +34,7 @@ from lp.services.webapp.interaction import ANONYMOUS
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
     TestCaseWithFactory,
+    admin_logged_in,
     login,
     login_celebrity,
     login_person,
@@ -412,3 +414,45 @@ class TestPrivateTeamVisibility(TestCaseWithFactory):
 
     def test_team_assigned_to_private_bug(self):
         self._test_team_assigned_to_bug()
+
+    def test_user_cant_see_bug_disabled_product(self, product_series=False):
+        """A user that reports a bug in a private project, can't see the bug
+        once the project is deactivated
+        """
+        private_product = self.factory.makeProduct(
+            owner=self.priv_team,
+            information_type=InformationType.PROPRIETARY,
+            registrant=self.priv_owner,
+        )
+        if product_series:
+            private_series = self.factory.makeProductSeries(
+                product=private_product
+            )
+        # Report a bug within the private product
+        login_person(self.priv_member)
+        bug = self.factory.makeBug(
+            owner=self.priv_member, target=private_product
+        )
+
+        # Change target to the product series
+        if product_series:
+            nomination = bug.addNomination(
+                target=private_series, owner=self.priv_member
+            )
+            nomination.approve(self.priv_owner)
+
+        # Bug reporter user should be able to see the bug initially
+        self.assertTrue(bug.userCanView(self.priv_member))
+
+        # Remove bug reporter from the team
+        with admin_logged_in():
+            private_product.active = False
+        clear_property_cache(bug)
+        # Bug reporter user should no longer have access to the bug
+        self.assertFalse(bug.userCanView(self.priv_member))
+
+    def test_user_cant_see_bug_disabled_product_series(self):
+        """A user that reports a bug in a private project series, can't see
+        the bug once the series's project is deactivated
+        """
+        self.test_user_cant_see_bug_disabled_product(product_series=True)