launchpad-reviewers team mailing list archive

[Bayard 'kit' Randel <kit.randel@xxxxxxxxxxxxx>]

Bayard 'kit' Randel has proposed merging lp:~blr/launchpad/bug-1474592-check-branch-accessibility into lp:launchpad.

Commit message:
Ensure logged in user has launchpad.View permissions on branch/repo before rendering golang-import meta.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1474592 in Launchpad itself: "golang_import_spec needs to check that the branch is accessible"
  https://bugs.launchpad.net/launchpad/+bug/1474592

For more details, see:
https://code.launchpad.net/~blr/launchpad/bug-1474592-check-branch-accessibility/+merge/264787
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~blr/launchpad/bug-1474592-check-branch-accessibility into lp:launchpad.

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2015-07-09 20:06:17 +0000
+++ lib/lp/registry/browser/product.py	2015-07-15 04:27:10 +0000
@@ -1030,24 +1030,22 @@
         if self.context.vcs == VCSType.GIT:
             repo = getUtility(IGitRepositorySet).getDefaultRepository(
                 self.context)
-            if repo:
+            if check_permission('launchpad.View', repo):
                 return "{hostname}/{product} git {git_https_url}".format(
                     hostname=config.vhost.mainsite.hostname,
                     product=self.context.name,
                     git_https_url=repo.git_https_url)
-            else:
-                return None
-        elif (self.context.vcs == VCSType.BZR and
-        self.context.development_focus.branch):
-            return (
-                "{hostname}/{product} bzr "
-                "{root_url}{branch}").format(
-                    hostname=config.vhost.mainsite.hostname,
-                    root_url=allvhosts.configs['mainsite'].rooturl,
-                    product=self.context.name,
-                    branch=self.context.development_focus.branch.unique_name)
-        else:
-            return None
+        elif self.context.vcs == VCSType.BZR:
+            branch = self.context.development_focus.branch
+            if check_permission('launchpad.View', branch):
+                return (
+                    "{hostname}/{product} bzr "
+                    "{root_url}{branch}").format(
+                        hostname=config.vhost.mainsite.hostname,
+                        root_url=allvhosts.configs['mainsite'].rooturl,
+                        product=self.context.name,
+                        branch=branch.unique_name)
+        return None
 
     def browserLanguages(self):
         return browser_languages(self.request)

=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py	2015-07-08 16:05:11 +0000
+++ lib/lp/registry/browser/productseries.py	2015-07-15 04:27:10 +0000
@@ -385,7 +385,8 @@
         """Meta string for golang remote import path.
         See: https://golang.org/cmd/go/#hdr-Remote_import_paths
         """
-        if self.context.product.vcs == VCSType.BZR and self.context.branch:
+        if (self.context.product.vcs == VCSType.BZR and
+            self.user_branch_visible):
             return (
                 "{hostname}/{product}/{series} bzr {root_url}{branch}").format(
                     hostname=config.vhost.mainsite.hostname,

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2015-07-07 22:33:29 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2015-07-15 04:27:10 +0000
@@ -24,7 +24,6 @@
 from lp.app.browser.lazrjs import vocabulary_to_choice_edit_items
 from lp.app.enums import (
     InformationType,
-    PROPRIETARY_INFORMATION_TYPES,
     ServiceUsage,
     )
 from lp.code.interfaces.gitrepository import IGitRepositorySet
@@ -369,6 +368,25 @@
             repo.target.vcs = VCSType.GIT
         self.assertIsNone(view.golang_import_spec)
 
+    def test_golang_meta_no_permissions(self):
+        # ensure golang meta import path is not rendered if user does
+        # not have launchpad.View permissions on branch.
+        simple_user = self.factory.makePerson()
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(owner=owner)
+        branch = self.factory.makeBranch(
+            owner=owner, information_type=InformationType.PRIVATESECURITY)
+
+        with person_logged_in(owner):
+            product.development_focus.branch = branch
+            product.vcs = VCSType.BZR
+            view = create_initialized_view(product, '+index')
+            self.assertIsNot(None, view.golang_import_spec)
+
+        with person_logged_in(simple_user):
+            view = create_initialized_view(product, '+index')
+            self.assertIsNone(view.golang_import_spec)
+
     def test_show_programming_languages_without_languages(self):
         # show_programming_languages is false when there are no programming
         # languages set.

=== modified file 'lib/lp/registry/browser/tests/test_productseries_views.py'
--- lib/lp/registry/browser/tests/test_productseries_views.py	2015-07-07 04:20:30 +0000
+++ lib/lp/registry/browser/tests/test_productseries_views.py	2015-07-15 04:27:10 +0000
@@ -70,7 +70,27 @@
         with person_logged_in(series.product.owner):
             series.product.vcs = VCSType.BZR
 
-        self.assertEqual(None, view.golang_import_spec)
+        self.assertIsNone(view.golang_import_spec)
+
+    def test_golang_meta_no_permissions(self):
+        # ensure golang meta import path is not rendered if user does
+        # not have launchpad.View permissions on branch.
+        owner = self.factory.makePerson()
+        simple_user = self.factory.makePerson()
+        product = self.factory.makeProduct(owner=owner)
+        series = self.factory.makeProductSeries(owner=owner, product=product)
+        branch = self.factory.makeBranch(
+            owner=owner, information_type=InformationType.USERDATA)
+
+        with person_logged_in(owner):
+            series.branch = branch
+            series.product.vcs = VCSType.BZR
+            view = create_initialized_view(series, '+index')
+            self.assertIsNot(None, view.golang_import_spec)
+
+        with person_logged_in(simple_user):
+            view = create_initialized_view(series, '+index')
+            self.assertIsNone(view.golang_import_spec)
 
     def test_information_type_public(self):
         # A ProductSeries view should include its information_type,