launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/launchpad/bug-828914-test into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #828914 in Launchpad itself: "+request-daily-build oops with an AttributeError: 'NoneType' object has no attribute 'published_archives'"
  https://bugs.launchpad.net/launchpad/+bug/828914

For more details, see:
https://code.launchpad.net/~bac/launchpad/bug-828914-test/+merge/73585

= Summary =

When a user is not a member of the team owning a private PPA, but does
have upload rights via an ArchivePermission, tries to request a recipe
build there is an error in the security adapter which causes an OOPS.
The problem is the wrong security adapter is being used.

== Proposed fix ==

Use getAdapter to select the proper adapter.

== Pre-implementation notes ==

Talks with Julian, Aaron, and William.

== Tests ==

bin/test -vvt test_sourcepackagerecipe

== Demo and Q/A ==

* Create a private PPA for a team you are a member.
* Using the API, create a new component uploader archive permission
  for your user.

  team = lp.people['team']
  p3a = team.ppas[0]
  p3a.newComponentUploader(person=lp.me, component_name="main")

* Leave the team.
* Now create a recipe for one of your branches and use the private
  team PPA as the build destination.
* Select 'Request build'.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/lp/buildmaster/model/packagebuild.py
  lib/lp/code/model/tests/test_sourcepackagerecipe.py
  lib/lp/code/browser/tests/test_sourcepackagerecipe.py

Will fix these lint issues later.

./lib/lp/code/model/tests/test_sourcepackagerecipe.py
     647: local variable 'build' is assigned to but never used
    1100: local variable 'build' is assigned to but never used
     105: E251 no spaces around keyword / parameter equals
     307: E225 missing whitespace around operator
     313: E225 missing whitespace around operator
     500: E225 missing whitespace around operator
     642: E225 missing whitespace around operator
    1072: E225 missing whitespace around operator
    1092: E225 missing whitespace around operator
./lib/lp/code/browser/tests/test_sourcepackagerecipe.py
     483: Line exceeds 78 characters.
    1155: Line exceeds 78 characters.
     483: E501 line too long (80 characters)
     595: E225 missing whitespace around operator
    1042: E225 missing whitespace around operator
-- 
https://code.launchpad.net/~bac/launchpad/bug-828914-test/+merge/73585
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/bug-828914-test into lp:launchpad.

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2011-08-28 07:29:11 +0000
+++ lib/canonical/launchpad/security.py	2011-08-31 20:03:32 +0000
@@ -1649,7 +1649,8 @@
 
     def _checkBuildPermission(self, user=None):
         """Check access to `IPackageBuild` for this job."""
-        permission = ViewBinaryPackageBuild(self.obj.build)
+        permission = getAdapter(
+            self.obj.build, IAuthorization, self.permission)
         if user is None:
             return permission.checkUnauthenticated()
         else:

=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py	2011-05-16 23:41:48 +0000
+++ lib/lp/buildmaster/model/packagebuild.py	2011-08-31 20:03:32 +0000
@@ -280,10 +280,12 @@
             specific_job.job.suspend()
 
         duration_estimate = self.estimateDuration()
+        job = specific_job.job
+        processor = specific_job.processor
         queue_entry = BuildQueue(
             estimated_duration=duration_estimate,
             job_type=self.build_farm_job_type,
-            job=specific_job.job, processor=specific_job.processor,
+            job=job, processor=processor,
             virtualized=specific_job.virtualized)
         Store.of(self).add(queue_entry)
         return queue_entry

=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-08-31 12:23:13 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py	2011-08-31 20:03:32 +0000
@@ -1529,7 +1529,7 @@
 
 
 class TestSourcePackageRecipeBuildView(BrowserTestCase):
-    """Test behaviour of SourcePackageReciptBuildView."""
+    """Test behaviour of SourcePackageRecipeBuildView."""
 
     layer = LaunchpadFunctionalLayer
 

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2011-08-29 07:55:48 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2011-08-31 20:03:32 +0000
@@ -418,6 +418,39 @@
         recipe.requestBuild(archive, recipe.owner, series,
                 PackagePublishingPocket.RELEASE)
 
+    def test_requestBuildPrivatePPAWithArchivePermission(self):
+        """User is not in PPA owner team but has ArchivePermission.
+
+        The case where the user is not in the PPA owner team but is allowed to
+        upload to the PPA via an explicit ArchivePermission takes a different
+        security path than if he were part of the team.
+        """
+
+        # Create a team private PPA.
+        team_owner = self.factory.makePerson()
+        team = self.factory.makeTeam(owner=team_owner)
+        team_p3a = self.factory.makeArchive(
+            owner=team, displayname='Private PPA', name='p3a',
+            private=True)
+
+        # Create a recipe with the team P3A as the build destination.
+        recipe = self.factory.makeSourcePackageRecipe()
+
+        # Add upload component rights for the non-team person.
+        with person_logged_in(team_owner):
+            team_p3a.newComponentUploader(
+                person=recipe.owner, component_name="main")
+        (distroseries,) = list(recipe.distroseries)
+
+        # Try to request a build.  It should work.
+        with person_logged_in(recipe.owner):
+            build = recipe.requestBuild(
+                team_p3a, recipe.owner, distroseries,
+                PackagePublishingPocket.RELEASE)
+            self.assertEqual(build.archive, team_p3a)
+            self.assertEqual(build.distroseries, distroseries)
+            self.assertEqual(build.requester, recipe.owner)
+
     def test_sourcepackagerecipe_description(self):
         """Ensure that the SourcePackageRecipe has a proper description."""
         description = u'The whoozits and whatzits.'