launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-1063287 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1063287 in Launchpad itself: "Cannot retarget project because picker found a private project"
  https://bugs.launchpad.net/launchpad/+bug/1063287

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-1063287/+merge/130540

This branch fixes bug 1063287: Cannot retarget project because picker
found a private project

The reason is simple: class ProductVocabulary did not filter product
a user is not allowed to view.

I added the filter expression generated by the static method
ProdusctSet.getProductPrivacyFilter() to the SQL query in
ProductVocabulary.search().

This requires to convert the existing "self._table.select()" call
into a proper Storm query.

The replacement of '%%' with '%%%%' in

   like_query = "'%%%%' || %s || '%%%%'" % quote_like(like_query)

may look a bit bit odd. The reason for this replacement is that Storm first
generates an SQL query of the different bits and pieces, and then does
its own string formatting call. Here we need '%%' to avoid a
"TypeError: not enough arguments for format string" or a similar
expection.

tests:

./bin/test -vvt lp.registry.tests.test_product_vocabularies.TestProductVocabulary

no lint

-- 
https://code.launchpad.net/~adeuring/launchpad/bug-1063287/+merge/130540
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-1063287 into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_product_vocabularies.py'
--- lib/lp/registry/tests/test_product_vocabularies.py	2012-07-27 13:12:41 +0000
+++ lib/lp/registry/tests/test_product_vocabularies.py	2012-10-19 12:57:22 +0000
@@ -5,9 +5,16 @@
 
 __metaclass__ = type
 
+from zope.component import getUtility
+
+from lp.app.enums import InformationType
+from lp.app.interfaces.services import IService
+from lp.registry.enums import SharingPermission
 from lp.registry.vocabularies import ProductVocabulary
 from lp.testing import (
+    ANONYMOUS,
     celebrity_logged_in,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -92,3 +99,43 @@
             self.product.active = False
         result = self.vocabulary.search('bedbugs')
         self.assertEqual([], list(result))
+
+    def test_private_products(self):
+        # Embargoed and proprietary products are only returned if
+        # the current user can see them.
+        public_product = self.factory.makeProduct('quux-public')
+        embargoed_owner = self.factory.makePerson()
+        embargoed_product = self.factory.makeProduct(
+            name='quux-embargoed', owner=embargoed_owner,
+            information_type=InformationType.EMBARGOED)
+        proprietary_owner = self.factory.makePerson()
+        proprietary_product = self.factory.makeProduct(
+            name='quux-proprietary', owner=proprietary_owner,
+            information_type=InformationType.PROPRIETARY)
+
+        # Anonymous users see only the public product.
+        with person_logged_in(ANONYMOUS):
+            result = self.vocabulary.search('quux')
+            self.assertEqual([public_product], list(result))
+
+        # Ordinary logged in users see only the public product.
+        user = self.factory.makePerson()
+        with person_logged_in(user):
+            result = self.vocabulary.search('quux')
+            self.assertEqual([public_product], list(result))
+
+        # People with grants on a private product can see this product.
+        with person_logged_in(embargoed_owner):
+            getUtility(IService, 'sharing').sharePillarInformation(
+                embargoed_product, user, embargoed_owner,
+                {InformationType.EMBARGOED: SharingPermission.ALL})
+        with person_logged_in(user):
+            result = self.vocabulary.search('quux')
+            self.assertEqual([embargoed_product, public_product], list(result))
+
+        # Admins can see all products.
+        with celebrity_logged_in('admin'):
+            result = self.vocabulary.search('quux')
+            self.assertEqual(
+                [embargoed_product, proprietary_product, public_product],
+                list(result))

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2012-10-11 04:57:59 +0000
+++ lib/lp/registry/vocabularies.py	2012-10-19 12:57:22 +0000
@@ -148,7 +148,10 @@
     Person,
     )
 from lp.registry.model.pillar import PillarName
-from lp.registry.model.product import Product
+from lp.registry.model.product import (
+    Product,
+    ProductSet,
+    )
 from lp.registry.model.productrelease import ProductRelease
 from lp.registry.model.productseries import ProductSeries
 from lp.registry.model.projectgroup import ProjectGroup
@@ -288,17 +291,26 @@
         if query is None or an empty string.
         """
         if query:
+            store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
             query = ensure_unicode(query)
             like_query = query.lower()
-            like_query = "'%%' || %s || '%%'" % quote_like(like_query)
+            like_query = "'%%%%' || %s || '%%%%'" % quote_like(like_query)
             fti_query = quote(query)
-            sql = "active = 't' AND (name LIKE %s OR fti @@ ftq(%s))" % (
-                    like_query, fti_query)
-            order_by = (
+            where_clause = And(
+                SQL(
+                    "active = 't' AND (name LIKE %s OR fti @@ ftq(%s))" % (
+                        like_query, fti_query)),
+                ProductSet.getProductPrivacyFilter(
+                    getUtility(ILaunchBag).user))
+            order_by = SQL(
                 '(CASE name WHEN %s THEN 1 '
                 ' ELSE rank(fti, ftq(%s)) END) DESC, displayname, name'
                 % (fti_query, fti_query))
-            return self._table.select(sql, orderBy=order_by, limit=100)
+            result = store.find(self._table, where_clause)
+            result.order_by(order_by)
+            result.config(limit=100)
+            return result
+
         return self.emptySelectResults()