Merge lp:~rharding/launchpad/filter_more_products into lp:launchpad

[Richard Harding]

Update product filters to prevent leaking non-public products in the translationgroup code.

= Summary =

A search for products brought up several places to look to make sure we filter
products so that non-public ones are not leaked.

== Pre Implementation ==

None, straight forward case of add filters.

== Implementation Notes ==

ITranslationGroup.products

The query needed to be adjusted to allow for use of the
getproductPrivacyFilter. Added tests for it.

ITranslationGroup.projects

This is actually querying for a list of ProjectGroups which do not have an
information type.

     ProjectGroup.selectBy(translationgroup=self.id, active=True)

For any returned project group you'd then have to query for the products
within it and those are all filtered appropriately.

ITranslationGroup.fetchProjectsForDisplay()

This needed to be updated to add the extra filter. Tests added to verify it
filters correctly.

ITranslationGroup.fetchProjectGroupsForDisplay()

This again is querying for ProjectGroups which fall under the .projects case
above.

ITranslationGroup.top_projects

This uses self.products, self.projects to get data. Since they've been checked
and updated it's safe to use.

== Tests ==

lib/lp/translations/tests/test_translationgroup.py

[Richard Harding (rharding)]

I've updated the one method that was changed to take a user argument and updated the template and view to provide the user so that it doesn't use the launchbag. However, changing the other ones, which are properties, will cascade into a larger change that I'd prefer was done as a second branch.

[Aaron Bentley (abentley)]

Thanks!

review: Approve

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-11-19 06:26:32 +0000
+++ lib/lp/registry/model/product.py	2012-11-20 18:19:28 +0000
@@ -300,7 +300,7 @@
     def composeLicensesColumn(cls, for_class=None):
         """Compose a Storm column specification for licences.
 
-        Use this to render a list of `Product` linkes without querying
+        Use this to render a list of `Product` links without querying
         licences for each one individually.
 
         It lets you prefetch the licensing information in the same

=== modified file 'lib/lp/translations/browser/translationgroup.py'
--- lib/lp/translations/browser/translationgroup.py	2012-01-01 02:58:52 +0000
+++ lib/lp/translations/browser/translationgroup.py	2012-11-20 18:19:28 +0000
@@ -25,6 +25,7 @@
     )
 from lp.app.errors import NotFoundError
 from lp.registry.browser.objectreassignment import ObjectReassignmentView
+from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
     GetitemNavigation,
@@ -72,6 +73,18 @@
         self.translation_groups = getUtility(ITranslationGroupSet)
         self.user_can_edit = check_permission('launchpad.Edit', self.context)
 
+    @cachedproperty
+    def distributions(self):
+        return self.context.fetchDistrosForDisplay()
+
+    @cachedproperty
+    def projectgroups(self):
+        return self.context.fetchProjectGroupsForDisplay()
+
+    @cachedproperty
+    def projects(self):
+        return self.context.fetchProjectsForDisplay(self.user)
+
     @property
     def label(self):
         return "%s translation group" % self.context.title

=== modified file 'lib/lp/translations/interfaces/translationgroup.py'
--- lib/lp/translations/interfaces/translationgroup.py	2012-04-16 23:02:44 +0000
+++ lib/lp/translations/interfaces/translationgroup.py	2012-11-20 18:19:28 +0000
@@ -138,7 +138,7 @@
             ordered by language name in English.
         """
 
-    def fetchProjectsForDisplay():
+    def fetchProjectsForDisplay(user):
         """Fetch `Product`s using this group, for display purposes.
 
         Prefetches display-related properties.

=== modified file 'lib/lp/translations/model/translationgroup.py'
--- lib/lp/translations/model/translationgroup.py	2011-12-30 06:14:56 +0000
+++ lib/lp/translations/model/translationgroup.py	2012-11-20 18:19:28 +0000
@@ -24,6 +24,7 @@
     )
 from storm.store import Store
 from zope.interface import implements
+from zope.component import getUtility
 
 from lp.app.errors import NotFoundError
 from lp.registry.interfaces.person import validate_public_person
@@ -41,6 +42,7 @@
     LibraryFileAlias,
     LibraryFileContent,
     )
+from lp.services.webapp.interfaces import ILaunchBag
 from lp.services.worlddata.model.language import Language
 from lp.translations.interfaces.translationgroup import (
     ITranslationGroup,
@@ -109,9 +111,17 @@
     def products(self):
         """See `ITranslationGroup`."""
         # Avoid circular imports.
-        from lp.registry.model.product import Product
-
-        return Product.selectBy(translationgroup=self.id, active=True)
+        from lp.registry.model.product import (
+            Product,
+            ProductSet,
+        )
+        user = getUtility(ILaunchBag).user
+        results = Store.of(self).find(
+            Product,
+            Product.active==True,
+            Product.translationgroup==self.id,
+            ProductSet.getProductPrivacyFilter(user))
+        return results
 
     @property
     def projects(self):
@@ -182,17 +192,20 @@
         mapper = lambda row: row[slice(0, 3)]
         return DecoratedResultSet(translator_data, mapper)
 
-    def fetchProjectsForDisplay(self):
+    def fetchProjectsForDisplay(self, user):
         """See `ITranslationGroup`."""
         # Avoid circular imports.
         from lp.registry.model.product import (
             Product,
+            ProductSet,
             ProductWithLicenses,
             )
 
         using = [
             Product,
-            LeftJoin(LibraryFileAlias, LibraryFileAlias.id == Product.iconID),
+            LeftJoin(
+                LibraryFileAlias,
+                LibraryFileAlias.id == Product.iconID),
             LeftJoin(
                 LibraryFileContent,
                 LibraryFileContent.id == LibraryFileAlias.contentID),
@@ -205,7 +218,9 @@
             )
         product_data = ISlaveStore(Product).using(*using).find(
             columns,
-            Product.translationgroupID == self.id, Product.active == True)
+            Product.translationgroup == self.id,
+            Product.active == True,
+            ProductSet.getProductPrivacyFilter(user))
         product_data = product_data.order_by(Product.displayname)
 
         return [

=== modified file 'lib/lp/translations/templates/translationgroup-portlet-projects.pt'
--- lib/lp/translations/templates/translationgroup-portlet-projects.pt	2010-09-03 06:37:26 +0000
+++ lib/lp/translations/templates/translationgroup-portlet-projects.pt	2012-11-20 18:19:28 +0000
@@ -13,30 +13,26 @@
     <a href="#teams">translation teams</a>.
   </p>
 
-  <div id="related-projects"
-       tal:define="
-        distributions context/fetchDistrosForDisplay;
-        projectgroups context/fetchProjectGroupsForDisplay;
-        projects context/fetchProjectsForDisplay">
-    <div tal:condition="distributions" style="margin-top:1em;">
+  <div id="related-projects">
+    <div tal:condition="view/distributions" style="margin-top:1em;">
       <h3 style="display: inline;">Distributions:</h3>
-      <tal:distribution repeat="distribution distributions">
+      <tal:distribution repeat="distribution view/distributions">
         <a href="#" tal:replace="structure distribution/fmt:link">Ubuntu
         </a><tal:comma condition="not:repeat/distribution/end">, </tal:comma>
       </tal:distribution>
     </div>
 
-    <div tal:condition="projectgroups" style="margin-top:1em;">
+    <div tal:condition="view/projectgroups" style="margin-top:1em;">
       <h3 style="display: inline;">Project groups:</h3>
-      <tal:projectgroup repeat="projectgroup projectgroups">
+      <tal:projectgroup repeat="projectgroup view/projectgroups">
         <a href="#" tal:replace="structure projectgroup/fmt:link">GNOME
         </a><tal:comma condition="not:repeat/projectgroup/end">, </tal:comma>
       </tal:projectgroup>
     </div>
 
-    <div tal:condition="projects" style="margin-top:1em;">
+    <div tal:condition="view/projects" style="margin-top:1em;">
       <h3 style="display: inline;">Projects:</h3>
-      <tal:project repeat="project projects">
+      <tal:project repeat="project view/projects">
         <a href="#" tal:replace="structure project/fmt:link">Firefox
         </a><tal:comma condition="not:repeat/project/end">, </tal:comma>
       </tal:project>

=== modified file 'lib/lp/translations/tests/test_translationgroup.py'
--- lib/lp/translations/tests/test_translationgroup.py	2012-01-01 02:58:52 +0000
+++ lib/lp/translations/tests/test_translationgroup.py	2012-11-20 18:19:28 +0000
@@ -9,11 +9,13 @@
 import transaction
 from zope.component import getUtility
 
+from lp.app.enums import InformationType
 from lp.registry.interfaces.teammembership import (
     ITeamMembershipSet,
     TeamMembershipStatus,
     )
 from lp.testing import (
+    person_logged_in,
     TestCaseWithFactory,
     WebServiceTestCase,
     )
@@ -21,6 +23,65 @@
 from lp.translations.interfaces.translationgroup import ITranslationGroupSet
 
 
+class TestTranslationGroup(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def _setup_products(self):
+        """Helper to setup one public one non-public product."""
+        user = self.factory.makePerson()
+        private_owner = self.factory.makePerson()
+        group = self.factory.makeTranslationGroup()
+
+        with person_logged_in(user):
+            public_product = self.factory.makeProduct()
+            public_product.translationgroup = group
+
+        with person_logged_in(private_owner):
+            private_product = self.factory.makeProduct(
+                information_type=InformationType.PROPRIETARY,
+                owner=private_owner)
+            private_product.translationgroup = group
+
+        return group, user, private_owner
+
+    def test_non_public_products_hidden(self):
+        """Non Public products are not returned via products attribute."""
+        group, public_user, private_user = self._setup_products()
+
+        with person_logged_in(public_user):
+            self.assertEqual(
+                1,
+                group.products.count(),
+                'There is only one public product for this user')
+
+        with person_logged_in(private_user):
+            self.assertEqual(
+                2,
+                group.products.count(),
+                'There are two for the private user.')
+
+    def test_non_public_products_hidden_for_display(self):
+        """Non Public products are not returned via fetchProjectsForDisplay."""
+        group, public_user, private_user = self._setup_products()
+
+        # Magical transaction so our data shows up via ISlaveStore
+        import transaction
+        transaction.commit()
+
+        with person_logged_in(public_user):
+            self.assertEqual(
+                1,
+                len(group.fetchProjectsForDisplay(public_user)),
+                'There is only one public product for the public user')
+
+        with person_logged_in(private_user):
+            self.assertEqual(
+                2,
+                len(group.fetchProjectsForDisplay(private_user)),
+                'Both products show for the private user.')
+
+
 class TestTranslationGroupSet(TestCaseWithFactory):
     layer = ZopelessDatabaseLayer