launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/projects-index-improve-redaction into lp:launchpad.

Commit message:
Fix redaction in pillar listings of projects for which the user only has LimitedView.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1650430 in Launchpad itself: "Project creation duplicate search checks for LimitedView but then crashes when it tries to use View"
  https://bugs.launchpad.net/launchpad/+bug/1650430

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/projects-index-improve-redaction/+merge/365367

For search, we show the name but redact the summary.  For the list of latest projects, we just skip those where the user only has an artifact grant, since that's quite unlikely and not really worth bothering to show.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/projects-index-improve-redaction into lp:launchpad.

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2018-08-17 11:46:36 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2019-04-01 18:00:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for product views."""
@@ -11,6 +11,7 @@
 from urlparse import urlsplit
 
 from lazr.restful.interfaces import IJSONRequestCache
+from six.moves.urllib.parse import urlencode
 from soupmatchers import (
     HTMLContains,
     Tag,
@@ -65,7 +66,10 @@
     LaunchpadFunctionalLayer,
     )
 from lp.testing.matchers import HasQueryCount
-from lp.testing.pages import find_tag_by_id
+from lp.testing.pages import (
+    extract_text,
+    find_tag_by_id,
+    )
 from lp.testing.service_usage_helpers import set_service_usage
 from lp.testing.views import (
     create_initialized_view,
@@ -880,6 +884,82 @@
             self.assertIn(public.name, browser.contents)
             self.assertIn(proprietary.name, browser.contents)
 
+    def test_latest_redaction(self):
+        # The "Latest projects registered" portlet includes only private
+        # projects for which the user has a policy grant.
+        owner = self.factory.makePerson()
+        public = self.factory.makeProduct(
+            owner=owner, information_type=InformationType.PUBLIC)
+        policy_granted = self.factory.makeProduct(
+            owner=owner, information_type=InformationType.PROPRIETARY)
+        artifact_granted = self.factory.makeProduct(
+            owner=owner, information_type=InformationType.PROPRIETARY)
+        with person_logged_in(owner):
+            artifact_branch = self.factory.makeProductBranch(
+                product=artifact_granted, owner=owner,
+                information_type=InformationType.PROPRIETARY)
+        viewer = self.factory.makePerson()
+        self.factory.makeAccessPolicyGrant(
+            policy=self.factory.makeAccessPolicy(
+                pillar=policy_granted, check_existing=True),
+            grantee=viewer, grantor=owner)
+        self.factory.makeAccessArtifactGrant(
+            concrete_artifact=artifact_branch, grantee=viewer, grantor=owner)
+        browser = self.getViewBrowser(getUtility(IProductSet), user=viewer)
+        with person_logged_in(viewer):
+            results = [
+                re.sub('\nregistered\n.*', '', extract_text(td))
+                for td in find_tag_by_id(
+                    browser.contents, 'latest-registered').findAll('td')]
+            self.assertIn(public.display_name, results)
+            self.assertIn(policy_granted.display_name, results)
+            self.assertNotIn(artifact_granted.display_name, results)
+
+    def test_search_redaction(self):
+        # A project search includes private projects for which the user has
+        # a policy or an artifact grant, but if the user doesn't have a
+        # policy grant (and hence doesn't have launchpad.View) then the
+        # summary is
+        # redacted.
+        prefix = self.factory.getUniqueString()
+        owner = self.factory.makePerson()
+        public = self.factory.makeProduct(
+            name=self.factory.getUniqueString(prefix), owner=owner,
+            information_type=InformationType.PUBLIC)
+        policy_granted = self.factory.makeProduct(
+            name=self.factory.getUniqueString(prefix), owner=owner,
+            information_type=InformationType.PROPRIETARY)
+        artifact_granted = self.factory.makeProduct(
+            name=self.factory.getUniqueString(prefix), owner=owner,
+            information_type=InformationType.PROPRIETARY)
+        with person_logged_in(owner):
+            artifact_branch = self.factory.makeProductBranch(
+                product=artifact_granted, owner=owner,
+                information_type=InformationType.PROPRIETARY)
+        viewer = self.factory.makePerson()
+        self.factory.makeAccessPolicyGrant(
+            policy=self.factory.makeAccessPolicy(
+                pillar=policy_granted, check_existing=True),
+            grantee=viewer, grantor=owner)
+        self.factory.makeAccessArtifactGrant(
+            concrete_artifact=artifact_branch, grantee=viewer, grantor=owner)
+        search_url = (
+            canonical_url(getUtility(IProductSet)) + '?' +
+            urlencode({'text': prefix}))
+        browser = self.getUserBrowser(search_url, user=viewer)
+        with person_logged_in(viewer):
+            expected_results = [
+                '%s\n%s' % (public.display_name, public.summary),
+                '%s\n%s' % (
+                    policy_granted.display_name, policy_granted.summary),
+                artifact_granted.display_name,
+                ]
+            self.assertContentEqual(
+                expected_results,
+                [extract_text(td)
+                 for td in find_tag_by_id(
+                     browser.contents, 'search-results').findAll('td')])
+
 
 class TestProductSetBranchView(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/templates/pillar-listing-simple.pt'
--- lib/lp/registry/templates/pillar-listing-simple.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/pillar-listing-simple.pt	2019-04-01 18:00:33 +0000
@@ -6,7 +6,8 @@
 <tr>
   <td>
     <div tal:content="structure context/fmt:link" />
-    <div style="max-width: 60em;" >
+    <div style="max-width: 60em;"
+         tal:condition="context/required:launchpad.View">
       <tal:summary replace="context/summary/fmt:shorten/240" />
     </div><br/>
   </td>

=== modified file 'lib/lp/registry/templates/products-index.pt'
--- lib/lp/registry/templates/products-index.pt	2012-10-05 20:56:28 +0000
+++ lib/lp/registry/templates/products-index.pt	2019-04-01 18:00:33 +0000
@@ -131,14 +131,16 @@
 
       <div class="portlet">
         <h2>Latest projects registered</h2>
-        <table>
-          <tr tal:repeat="product view/latest">
-            <td>
-              <tal:link replace="structure product/fmt:link" />
-              registered
-              <tal:date replace="product/datecreated/fmt:displaydate" />
-            </td>
-          </tr>
+        <table id="latest-registered">
+          <tal:product repeat="product view/latest">
+            <tr tal:condition="product/required:launchpad.View">
+              <td>
+                <tal:link replace="structure product/fmt:link" />
+                registered
+                <tal:date replace="product/datecreated/fmt:displaydate" />
+              </td>
+            </tr>
+          </tal:product>
         </table>
         <p><a href="/projects/+all">&raquo; Show all projects</a></p>
       </div>