launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:hide-creds-on-recipe-non-owners into launchpad:master.

Commit message:
Hide credentials on recipe for non-owners

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/387507

When viewing OCI push rules on the recipe page we want to hide the credentials part of the table instead of crashing completely.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:hide-creds-on-recipe-non-owners into launchpad:master.

diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 356ac1c..c9b8153 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -190,6 +190,10 @@ class OCIRecipeView(LaunchpadView):
             getUtility(IOCIPushRuleSet).findByRecipe(self.context))
 
     @property
+    def is_recipe_owner(self):
+        return self.user == self.context.owner
+
+    @property
     def has_push_rules(self):
         return len(self.push_rules) > 0
 
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index e25d463..db6ab82 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -35,11 +35,16 @@ from lp.oci.browser.ocirecipe import (
     OCIRecipeEditView,
     OCIRecipeView,
     )
+from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet
 from lp.oci.interfaces.ocirecipe import (
     CannotModifyOCIRecipeProcessor,
     IOCIRecipeSet,
     OCI_RECIPE_ALLOW_CREATE,
     )
+from lp.oci.interfaces.ociregistrycredentials import (
+    IOCIRegistryCredentialsSet,
+    )
+from lp.oci.tests.helpers import OCIConfigHelperMixin
 from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
 from lp.services.propertycache import get_property_cache
@@ -831,6 +836,80 @@ class TestOCIRecipeRequestBuildsView(BaseTestOCIRecipeView):
             extract_text(find_main_content(browser.contents)))
 
 
+class TestOCIRecipePushRulesView(OCIConfigHelperMixin,
+                                 BaseTestOCIRecipeView):
+    def setUp(self):
+        super(TestOCIRecipePushRulesView, self).setUp()
+        self.ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        self.distroseries = self.factory.makeDistroSeries(
+            distribution=self.ubuntu, name="shiny", displayname="Shiny")
+        self.architectures = []
+        for processor, architecture in ("386", "i386"), ("amd64", "amd64"):
+            das = self.factory.makeDistroArchSeries(
+                distroseries=self.distroseries, architecturetag=architecture,
+                processor=getUtility(IProcessorSet).getByName(processor))
+            das.addOrUpdateChroot(self.factory.makeLibraryFileAlias())
+            self.architectures.append(das)
+        self.useFixture(FeatureFixture({
+            OCI_RECIPE_ALLOW_CREATE: "on",
+            "oci.build_series.%s" % self.distroseries.distribution.name:
+                self.distroseries.name,
+        }))
+        oci_project = self.factory.makeOCIProject(
+            pillar=self.distroseries.distribution,
+            ociprojectname="oci-project-name")
+        self.recipe = self.factory.makeOCIRecipe(
+            name="recipe-name", registrant=self.person, owner=self.person,
+            oci_project=oci_project)
+
+        self.setConfig()
+
+    def test_view_oci_push_rules(self):
+        url = unicode(self.factory.getUniqueURL())
+        credentials = {'username': 'foo', 'password': 'bar'}
+        registry_credentials = getUtility(IOCIRegistryCredentialsSet).new(
+            owner=self.person,
+            url=url,
+            credentials=credentials)
+        image_name = self.factory.getUniqueUnicode()
+        getUtility(IOCIPushRuleSet).new(
+            recipe=self.recipe,
+            registry_credentials=registry_credentials,
+            image_name=image_name)
+        browser = self.getViewBrowser(self.recipe, user=self.person)
+        main_text = extract_text(find_main_content(browser.contents))
+
+        # Display the Registry URL and the Username
+        # for the recipe owner
+        with person_logged_in(self.person):
+            self.assertIn(image_name, main_text)
+            self.assertIn(registry_credentials.url, main_text)
+            self.assertIn(registry_credentials.username, main_text)
+
+    def test_view_oci_push_rules_non_owner(self):
+        url = unicode(self.factory.getUniqueURL())
+        credentials = {'username': 'foo', 'password': 'bar'}
+        registry_credentials = getUtility(IOCIRegistryCredentialsSet).new(
+            owner=self.person,
+            url=url,
+            credentials=credentials)
+        image_name = self.factory.getUniqueUnicode()
+        getUtility(IOCIPushRuleSet).new(
+            recipe=self.recipe,
+            registry_credentials=registry_credentials,
+            image_name=image_name)
+        non_owner = self.factory.makePerson()
+        login_person(non_owner)
+        browser = self.getViewBrowser(self.recipe, user=non_owner)
+        main_text = extract_text(find_main_content(browser.contents))
+
+        # Display only the image name for users
+        # who are not the recipe owner
+        with person_logged_in(self.person):
+            self.assertIn(image_name, main_text)
+            self.assertNotIn(registry_credentials.url, main_text)
+
+
 class TestOCIProjectRecipesView(BaseTestOCIRecipeView):
     def setUp(self):
         super(TestOCIProjectRecipesView, self).setUp()
diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt
index cf7c549..9c43183 100644
--- a/lib/lp/oci/templates/ocirecipe-index.pt
+++ b/lib/lp/oci/templates/ocirecipe-index.pt
@@ -123,8 +123,8 @@
            style="margin-bottom: 1em; ">
       <thead>
         <tr>
-          <th>Registry URL</th>
-          <th>Username</th>
+          <th tal:condition="view/is_recipe_owner">Registry URL</th>
+          <th tal:condition="view/is_recipe_owner">Username</th>
           <th>Image Name</th>
         </tr>
       </thead>
@@ -132,8 +132,10 @@
         <tal:recipe-push-rules repeat="item view/push_rules">
           <tr tal:define="rule item"
               tal:attributes="id string:rule-${rule/id}">
-            <td tal:content="rule/registry_credentials/url"/>
-            <td tal:content="rule/registry_credentials/username"/>
+            <td tal:condition="view/is_recipe_owner"
+              tal:content="rule/registry_credentials/url"/>
+            <td tal:condition="view/is_recipe_owner"
+              tal:content="rule/registry_credentials/username"/>
             <td tal:content="rule/image_name"/>
           </tr>
         </tal:recipe-push-rules>