Launchpad itself

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

  1. build-security
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/build-security
Merge into: lp:launchpad
Diff against target: 139 lines (+64/-9)
3 files modified
lib/canonical/launchpad/security.py (+32/-6)
lib/lp/code/tests/test_sourcepackagerecipebuild.py (+29/-1)
lib/lp/testing/factory.py (+3/-2)
To merge this branch: bzr merge lp:~abentley/launchpad/build-security
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+23741@code.launchpad.net

Commit message

Provide launchpad.View for SourcePackageRecipeBuild.

Description of the change

= 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'

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Yay

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/security.py'
2--- lib/canonical/launchpad/security.py 2010-04-23 17:31:49 +0000
3+++ lib/canonical/launchpad/security.py 2010-04-26 22:01:31 +0000
4@@ -29,6 +29,8 @@
5 from lp.code.interfaces.branchsubscription import (
6 IBranchSubscription)
7 from lp.code.interfaces.sourcepackagerecipe import ISourcePackageRecipe
8+from lp.code.interfaces.sourcepackagerecipebuild import (
9+ ISourcePackageRecipeBuild)
10 from lp.bugs.interfaces.bug import IBug
11 from lp.bugs.interfaces.bugattachment import IBugAttachment
12 from lp.bugs.interfaces.bugbranch import IBugBranch
13@@ -2172,14 +2174,19 @@
14 return user.in_admin
15
16
17-class ViewSourcePackageRecipe(AuthorizationBase):
18-
19- permission = "launchpad.View"
20- usedfor = ISourcePackageRecipe
21+class DerivedAuthorization(AuthorizationBase):
22+ """An Authorization that is based on permissions for other objects.
23+
24+ Implementations must define permission, usedfor and iter_objects.
25+ iter_objects should iterate through the objects to check permission on.
26+
27+ Failure on the permission check for any object causes an overall failure.
28+ """
29
30 def iter_adapters(self):
31- for branch in self.obj.getReferencedBranches():
32- yield getAdapter(branch, IAuthorization, self.permission)
33+ return (
34+ getAdapter(obj, IAuthorization, self.permission)
35+ for obj in self.iter_objects())
36
37 def checkAuthenticated(self, user):
38 for adapter in self.iter_adapters():
39@@ -2194,6 +2201,25 @@
40 return True
41
42
43+class ViewSourcePackageRecipe(DerivedAuthorization):
44+
45+ permission = "launchpad.View"
46+ usedfor = ISourcePackageRecipe
47+
48+ def iter_objects(self):
49+ return self.obj.getReferencedBranches()
50+
51+
52+class ViewSourcePackageRecipeBuild(DerivedAuthorization):
53+
54+ permission = "launchpad.View"
55+ usedfor = ISourcePackageRecipeBuild
56+
57+ def iter_objects(self):
58+ yield self.obj.recipe
59+ yield self.obj.archive
60+
61+
62 class ViewSourcePackagePublishingHistory(ViewArchive):
63 """Restrict viewing of source publications."""
64 permission = "launchpad.View"
65
66=== modified file 'lib/lp/code/tests/test_sourcepackagerecipebuild.py'
67--- lib/lp/code/tests/test_sourcepackagerecipebuild.py 2010-03-25 15:29:31 +0000
68+++ lib/lp/code/tests/test_sourcepackagerecipebuild.py 2010-04-26 22:01:31 +0000
69@@ -3,6 +3,8 @@
70
71 """Tests for source package builds."""
72
73+from __future__ import with_statement
74+
75 __metaclass__ = type
76
77 import datetime
78@@ -13,12 +15,13 @@
79
80 from canonical.testing.layers import DatabaseFunctionalLayer
81
82+from canonical.launchpad.webapp.authorization import check_permission
83 from lp.buildmaster.interfaces.buildbase import IBuildBase
84 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
85 from lp.code.interfaces.sourcepackagerecipebuild import (
86 ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuild,
87 ISourcePackageRecipeBuildSource)
88-from lp.testing import TestCaseWithFactory
89+from lp.testing import ANONYMOUS, login, person_logged_in, TestCaseWithFactory
90
91
92 class TestSourcePackageRecipeBuild(TestCaseWithFactory):
93@@ -86,6 +89,31 @@
94 spb = self.makeSourcePackageRecipeBuild()
95 self.assertEqual(False, spb.is_private)
96
97+ def test_view_private_branch(self):
98+ """Recipebuilds with private branches are restricted."""
99+ owner = self.factory.makePerson()
100+ branch = self.factory.makeAnyBranch(owner=owner, private=True)
101+ with person_logged_in(owner):
102+ recipe = self.factory.makeSourcePackageRecipe(branches=[branch])
103+ build = self.factory.makeSourcePackageRecipeBuild(recipe=recipe)
104+ self.assertTrue(check_permission('launchpad.View', build))
105+ with person_logged_in(self.factory.makePerson()):
106+ self.assertFalse(check_permission('launchpad.View', build))
107+ login(ANONYMOUS)
108+ self.assertFalse(check_permission('launchpad.View', build))
109+
110+ def test_view_private_archive(self):
111+ """Recipebuilds with private branches are restricted."""
112+ owner = self.factory.makePerson()
113+ archive = self.factory.makeArchive(owner=owner, private=True)
114+ build = self.factory.makeSourcePackageRecipeBuild(archive=archive)
115+ with person_logged_in(owner):
116+ self.assertTrue(check_permission('launchpad.View', build))
117+ with person_logged_in(self.factory.makePerson()):
118+ self.assertFalse(check_permission('launchpad.View', build))
119+ login(ANONYMOUS)
120+ self.assertFalse(check_permission('launchpad.View', build))
121+
122 def test_estimateDuration(self):
123 # The duration estimate is currently hard-coded as two minutes.
124 spb = self.makeSourcePackageRecipeBuild()
125
126=== modified file 'lib/lp/testing/factory.py'
127--- lib/lp/testing/factory.py 2010-04-25 03:40:36 +0000
128+++ lib/lp/testing/factory.py 2010-04-26 22:01:31 +0000
129@@ -1671,8 +1671,9 @@
130 description=description)
131
132 if private:
133- archive.private = True
134- archive.buildd_secret = "sekrit"
135+ naked_archive = removeSecurityProxy(archive)
136+ naked_archive.private = True
137+ naked_archive.buildd_secret = "sekrit"
138
139 return archive
140