Merge lp:~abentley/launchpad/build-security into lp:launchpad

[Aaron Bentley]

Provide launchpad.View for SourcePackageRecipeBuild.

= Summary =
Fix bug 552976, "SourcePackageRecipeBuild security needs work"

== Proposed fix ==
Define launchpad.View such that it is only provided when the user has
launchpad.View on the recipe and the archive.

== Pre-implementation notes ==
None

== Implementation details ==
Extracted DerivedAuthorization from ViewSourcePackageRecipe, and reused it
for ViewSourcePackageRecipeBuild.

== Tests ==
bin/test -t test_view_private test_sourcepackagerecipebuild

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/tests/test_sourcepackagerecipebuild.py
  lib/canonical/launchpad/security.py
  lib/lp/testing/factory.py

== Pylint notices ==

lib/lp/code/tests/test_sourcepackagerecipebuild.py
    13: [F0401] Unable to import 'transaction'
    14: [F0401] Unable to import 'zope.component'
    16: [F0401] Unable to import 'canonical.testing.layers'
    18: [F0401] Unable to import 'canonical.launchpad.webapp.authorization'
    19: [F0401] Unable to import 'lp.buildmaster.interfaces.buildbase'
    20: [F0401] Unable to import 'lp.buildmaster.interfaces.buildqueue'
    21: [F0401] Unable to import 'lp.code.interfaces.sourcepackagerecipebuild'
    24: [F0401] Unable to import 'lp.testing'

[Tim Penhey (thumper)]

Yay

review: Approve

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-04-23 17:31:49 +0000
+++ lib/canonical/launchpad/security.py	2010-04-26 22:01:31 +0000
@@ -29,6 +29,8 @@
 from lp.code.interfaces.branchsubscription import (
     IBranchSubscription)
 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
+from lp.code.interfaces.sourcepackagerecipebuild import (
+    ISourcePackageRecipeBuild)
 from lp.bugs.interfaces.bug import IBug
 from lp.bugs.interfaces.bugattachment import IBugAttachment
 from lp.bugs.interfaces.bugbranch import IBugBranch
@@ -2172,14 +2174,19 @@
         return user.in_admin
 
 
-class ViewSourcePackageRecipe(AuthorizationBase):
-
-    permission = "launchpad.View"
-    usedfor = ISourcePackageRecipe
+class DerivedAuthorization(AuthorizationBase):
+    """An Authorization that is based on permissions for other objects.
+
+    Implementations must define permission, usedfor and iter_objects.
+    iter_objects should iterate through the objects to check permission on.
+
+    Failure on the permission check for any object causes an overall failure.
+    """
 
     def iter_adapters(self):
-        for branch in self.obj.getReferencedBranches():
-            yield getAdapter(branch, IAuthorization, self.permission)
+        return (
+            getAdapter(obj, IAuthorization, self.permission)
+            for obj in self.iter_objects())
 
     def checkAuthenticated(self, user):
         for adapter in self.iter_adapters():
@@ -2194,6 +2201,25 @@
         return True
 
 
+class ViewSourcePackageRecipe(DerivedAuthorization):
+
+    permission = "launchpad.View"
+    usedfor = ISourcePackageRecipe
+
+    def iter_objects(self):
+        return self.obj.getReferencedBranches()
+
+
+class ViewSourcePackageRecipeBuild(DerivedAuthorization):
+
+    permission = "launchpad.View"
+    usedfor = ISourcePackageRecipeBuild
+
+    def iter_objects(self):
+        yield self.obj.recipe
+        yield self.obj.archive
+
+
 class ViewSourcePackagePublishingHistory(ViewArchive):
     """Restrict viewing of source publications."""
     permission = "launchpad.View"

=== modified file 'lib/lp/code/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/tests/test_sourcepackagerecipebuild.py	2010-03-25 15:29:31 +0000
+++ lib/lp/code/tests/test_sourcepackagerecipebuild.py	2010-04-26 22:01:31 +0000
@@ -3,6 +3,8 @@
 
 """Tests for source package builds."""
 
+from __future__ import with_statement
+
 __metaclass__ = type
 
 import datetime
@@ -13,12 +15,13 @@
 
 from canonical.testing.layers import DatabaseFunctionalLayer
 
+from canonical.launchpad.webapp.authorization import check_permission
 from lp.buildmaster.interfaces.buildbase import IBuildBase
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,
     ISourcePackageRecipeBuildSource)
-from lp.testing import TestCaseWithFactory
+from lp.testing import ANONYMOUS, login, person_logged_in, TestCaseWithFactory
 
 
 class TestSourcePackageRecipeBuild(TestCaseWithFactory):
@@ -86,6 +89,31 @@
         spb = self.makeSourcePackageRecipeBuild()
         self.assertEqual(False, spb.is_private)
 
+    def test_view_private_branch(self):
+        """Recipebuilds with private branches are restricted."""
+        owner = self.factory.makePerson()
+        branch = self.factory.makeAnyBranch(owner=owner, private=True)
+        with person_logged_in(owner):
+            recipe = self.factory.makeSourcePackageRecipe(branches=[branch])
+            build = self.factory.makeSourcePackageRecipeBuild(recipe=recipe)
+            self.assertTrue(check_permission('launchpad.View', build))
+        with person_logged_in(self.factory.makePerson()):
+            self.assertFalse(check_permission('launchpad.View', build))
+        login(ANONYMOUS)
+        self.assertFalse(check_permission('launchpad.View', build))
+
+    def test_view_private_archive(self):
+        """Recipebuilds with private branches are restricted."""
+        owner = self.factory.makePerson()
+        archive = self.factory.makeArchive(owner=owner, private=True)
+        build = self.factory.makeSourcePackageRecipeBuild(archive=archive)
+        with person_logged_in(owner):
+            self.assertTrue(check_permission('launchpad.View', build))
+        with person_logged_in(self.factory.makePerson()):
+            self.assertFalse(check_permission('launchpad.View', build))
+        login(ANONYMOUS)
+        self.assertFalse(check_permission('launchpad.View', build))
+
     def test_estimateDuration(self):
         # The duration estimate is currently hard-coded as two minutes.
         spb = self.makeSourcePackageRecipeBuild()

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-04-25 03:40:36 +0000
+++ lib/lp/testing/factory.py	2010-04-26 22:01:31 +0000
@@ -1671,8 +1671,9 @@
             description=description)
 
         if private:
-            archive.private = True
-            archive.buildd_secret = "sekrit"
+            naked_archive = removeSecurityProxy(archive)
+            naked_archive.private = True
+            naked_archive.buildd_secret = "sekrit"
 
         return archive