launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:snap-oci-build-privacy-banner into launchpad:master.

Commit message:
Add privacy banners for snap and OCI recipe builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/405339
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:snap-oci-build-privacy-banner into launchpad:master.

diff --git a/lib/lp/oci/interfaces/ocirecipebuild.py b/lib/lp/oci/interfaces/ocirecipebuild.py
index 22799e7..ad659ea 100644
--- a/lib/lp/oci/interfaces/ocirecipebuild.py
+++ b/lib/lp/oci/interfaces/ocirecipebuild.py
@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2019-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for a build record for OCI recipes."""
@@ -47,6 +47,7 @@ from zope.schema import (
     )
 
 from lp import _
+from lp.app.interfaces.launchpad import IPrivacy
 from lp.buildmaster.interfaces.buildfarmjob import ISpecificBuildFarmJobSource
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
 from lp.oci.interfaces.ocirecipe import (
@@ -141,7 +142,7 @@ class OCIRecipeBuildSetRegistryUploadStatus(EnumeratedType):
     """)
 
 
-class IOCIRecipeBuildView(IPackageBuild):
+class IOCIRecipeBuildView(IPackageBuild, IPrivacy):
     """`IOCIRecipeBuild` attributes that require launchpad.View permission."""
 
     build_request = Reference(
diff --git a/lib/lp/oci/model/ocirecipebuild.py b/lib/lp/oci/model/ocirecipebuild.py
index 9ebb2e0..ed3d19d 100644
--- a/lib/lp/oci/model/ocirecipebuild.py
+++ b/lib/lp/oci/model/ocirecipebuild.py
@@ -267,10 +267,13 @@ class OCIRecipeBuild(PackageBuildMixin, StormBase):
         # https://code.launchpad.net/
         # ~cjwatson/launchpad/snap-build-record-code/+merge/365356
         return (
+            self.recipe.private or
             self.recipe.owner.private or
             self.recipe.git_repository is None or
             self.recipe.git_repository.private)
 
+    private = is_private
+
     def retry(self):
         """See `IOCIRecipeBuild`."""
         assert self.can_be_retried, "Build %s cannot be retried" % self.id
diff --git a/lib/lp/oci/tests/test_ocirecipebuild.py b/lib/lp/oci/tests/test_ocirecipebuild.py
index c78e1ba..05f9e12 100644
--- a/lib/lp/oci/tests/test_ocirecipebuild.py
+++ b/lib/lp/oci/tests/test_ocirecipebuild.py
@@ -28,6 +28,7 @@ from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
 from lp.app.errors import NotFoundError
+from lp.app.interfaces.launchpad import IPrivacy
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
@@ -46,6 +47,10 @@ from lp.oci.interfaces.ocirecipebuild import (
 from lp.oci.interfaces.ocirecipebuildjob import IOCIRegistryUploadJobSource
 from lp.oci.model.ocirecipebuild import OCIRecipeBuildSet
 from lp.oci.tests.helpers import OCIConfigHelperMixin
+from lp.registry.enums import (
+    PersonVisibility,
+    TeamMembershipPolicy,
+    )
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.authserver.xmlrpc import AuthServerAPIView
 from lp.services.config import config
@@ -113,6 +118,7 @@ class TestOCIRecipeBuild(OCIConfigHelperMixin, TestCaseWithFactory):
         with admin_logged_in():
             self.assertProvides(self.build, IOCIRecipeBuild)
             self.assertProvides(self.build, IPackageBuild)
+            self.assertProvides(self.build, IPrivacy)
 
     def test_addFile(self):
         lfa = self.factory.makeLibraryFileAlias()
@@ -259,6 +265,20 @@ class TestOCIRecipeBuild(OCIConfigHelperMixin, TestCaseWithFactory):
         self.assertIsNotNone(bq.processor)
         self.assertEqual(bq, self.build.buildqueue_record)
 
+    def test_is_private(self):
+        # An OCIRecipeBuild is private if its owner is.
+        self.assertFalse(self.build.is_private)
+        self.assertFalse(self.build.private)
+        private_team = self.factory.makeTeam(
+            membership_policy=TeamMembershipPolicy.MODERATED,
+            visibility=PersonVisibility.PRIVATE)
+        with person_logged_in(private_team.teamowner):
+            build = self.factory.makeOCIRecipeBuild(
+                requester=private_team.teamowner, owner=private_team,
+                information_type=InformationType.USERDATA)
+            self.assertTrue(build.is_private)
+            self.assertTrue(build.private)
+
     def test_updateStatus_triggers_webhooks(self):
         # Updating the status of an OCIRecipeBuild triggers webhooks on the
         # corresponding OCIRecipe.
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 6108048..5983e3a 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -3569,10 +3569,15 @@ class OCIRecipeSubscriptionView(AuthorizationBase):
         return self.obj.recipe.visibleByUser(user.person)
 
 
-class ViewOCIRecipeBuild(AnonymousAuthorization):
-    """Anyone can view an `IOCIRecipe`."""
+class ViewOCIRecipeBuild(DelegatedAuthorization):
+    permission = 'launchpad.View'
     usedfor = IOCIRecipeBuild
 
+    def iter_objects(self):
+        yield self.obj.recipe
+        if self.obj.recipe.git_repository is not None:
+            yield self.obj.recipe.git_repository
+
 
 class EditOCIRecipeBuild(AdminByBuilddAdmin):
     permission = 'launchpad.Edit'
diff --git a/lib/lp/snappy/interfaces/snapbuild.py b/lib/lp/snappy/interfaces/snapbuild.py
index f5a6dbc..ec81231 100644
--- a/lib/lp/snappy/interfaces/snapbuild.py
+++ b/lib/lp/snappy/interfaces/snapbuild.py
@@ -50,6 +50,7 @@ from zope.schema import (
     )
 
 from lp import _
+from lp.app.interfaces.launchpad import IPrivacy
 from lp.buildmaster.interfaces.buildfarmjob import ISpecificBuildFarmJobSource
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
 from lp.registry.interfaces.person import IPerson
@@ -130,7 +131,7 @@ class SnapBuildStoreUploadStatus(EnumeratedType):
         """)
 
 
-class ISnapBuildView(IPackageBuild):
+class ISnapBuildView(IPackageBuild, IPrivacy):
     """`ISnapBuild` attributes that require launchpad.View permission."""
 
     build_request = Reference(
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index 76ccc0e..85afe4b 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -232,6 +232,8 @@ class SnapBuild(PackageBuildMixin, Storm):
             self.archive.private
         )
 
+    private = is_private
+
     def __repr__(self):
         return "<SnapBuild ~%s/+snap/%s/+build/%d>" % (
             self.snap.owner.name, self.snap.name, self.id)
diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
index 521cfca..dab737f 100644
--- a/lib/lp/snappy/tests/test_snapbuild.py
+++ b/lib/lp/snappy/tests/test_snapbuild.py
@@ -31,7 +31,10 @@ from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
 from lp.app.errors import NotFoundError
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.app.interfaces.launchpad import (
+    ILaunchpadCelebrities,
+    IPrivacy,
+    )
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
@@ -112,9 +115,10 @@ class TestSnapBuild(TestCaseWithFactory):
         self.build = self.factory.makeSnapBuild()
 
     def test_implements_interfaces(self):
-        # SnapBuild implements IPackageBuild and ISnapBuild.
+        # SnapBuild implements IPackageBuild, ISnapBuild, and IPrivacy.
         self.assertProvides(self.build, IPackageBuild)
         self.assertProvides(self.build, ISnapBuild)
+        self.assertProvides(self.build, IPrivacy)
 
     def test___repr__(self):
         # SnapBuild has an informative __repr__.
@@ -173,8 +177,9 @@ class TestSnapBuild(TestCaseWithFactory):
         self.assertEqual("main", build.current_component.name)
 
     def test_is_private(self):
-        # A SnapBuild is private iff its Snap and archive are.
+        # A SnapBuild is private iff its Snap or owner or archive are.
         self.assertFalse(self.build.is_private)
+        self.assertFalse(self.build.private)
         private_team = self.factory.makeTeam(
             membership_policy=TeamMembershipPolicy.MODERATED,
             visibility=PersonVisibility.PRIVATE)
@@ -183,10 +188,12 @@ class TestSnapBuild(TestCaseWithFactory):
                 requester=private_team.teamowner, owner=private_team,
                 private=True)
             self.assertTrue(build.is_private)
+            self.assertTrue(build.private)
         private_archive = self.factory.makeArchive(private=True)
         with person_logged_in(private_archive.owner):
             build = self.factory.makeSnapBuild(archive=private_archive)
             self.assertTrue(build.is_private)
+            self.assertTrue(build.private)
 
     def test_can_be_retried(self):
         ok_cases = [