Launchpad itself

Merge lp:~stevenk/launchpad/subscribers-can-view-p3as into lp:launchpad

  1. subscribers-can-view-p3as
  2. Merge into devel
Proposed by Steve Kowalik
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 11057
Proposed branch: lp:~stevenk/launchpad/subscribers-can-view-p3as
Merge into: lp:launchpad
Diff against target: 227 lines (+166/-2)
4 files modified
lib/canonical/launchpad/security.py (+8/-1)
lib/lp/soyuz/browser/archive.py (+17/-1)
lib/lp/soyuz/browser/tests/test_archive_packages.py (+101/-0)
lib/lp/soyuz/tests/test_archive_privacy.py (+40/-0)
To merge this branch: bzr merge lp:~stevenk/launchpad/subscribers-can-view-p3as
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Approve
Review via email: mp+27806@code.launchpad.net

Description of the change

Allow subscribers to view P3As, but do not allow them to view +packages. Add tests that check that the links they receive are disabled, and that they receive 403 errors when they browse there directly.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Just a small note on the vertical whitespace mid function: PEP8 says
to use that sparingly to separate logically distinct areas of a single
function; the 'list of things to check' is one big area IMO - I would
delete the vertical white space there.

-Rob

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Thanks, good stuff.

There is only one empty line above TestPPAPackages, there should be two.

The test methods in TestP3APackages also contain the phrase p3a_packages again, this seems a bit superfluous given the test case name.

The comments on the first line of each test method explaining what the tests do could perhaps be docstrings.

review: Approve (code)

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-06-16 18:49:47 +0000
3+++ lib/canonical/launchpad/security.py 2010-06-24 14:19:31 +0000
4@@ -19,7 +19,7 @@
5 IArchivePermissionSet)
6 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthToken
7 from lp.soyuz.interfaces.archivesubscriber import (
8- IArchiveSubscriber, IPersonalArchiveSubscription)
9+ IArchiveSubscriber, IArchiveSubscriberSet, IPersonalArchiveSubscription)
10 from lp.code.interfaces.branch import (
11 IBranch, user_has_special_branch_access)
12 from lp.code.interfaces.branchmergeproposal import (
13@@ -2040,6 +2040,13 @@
14 if self.obj.is_ppa and self.obj.checkArchivePermission(user.person):
15 return True
16
17+ # Subscribers can view private PPAs.
18+ if self.obj.is_ppa and self.obj.private:
19+ archive_subs = getUtility(IArchiveSubscriberSet).getBySubscriber(
20+ user.person, self.obj).any()
21+ if archive_subs:
22+ return True
23+
24 return False
25
26 def checkUnauthenticated(self):
27
28=== modified file 'lib/lp/soyuz/browser/archive.py'
29--- lib/lp/soyuz/browser/archive.py 2010-06-14 09:31:26 +0000
30+++ lib/lp/soyuz/browser/archive.py 2010-06-24 14:19:31 +0000
31@@ -35,6 +35,7 @@
32 from zope.component import getUtility
33 from zope.formlib import form
34 from zope.interface import implements, Interface
35+from zope.security.interfaces import Unauthorized
36 from zope.security.proxy import removeSecurityProxy
37 from zope.schema import Choice, List, TextLine
38 from zope.schema.interfaces import IContextSourceBinder
39@@ -426,7 +427,12 @@
40
41 def packages(self):
42 text = 'View package details'
43- return Link('+packages', text, icon='info')
44+ link = Link('+packages', text, icon='info')
45+ # Disable the link for P3As if they don't have upload rights.
46+ if self.context.private:
47+ if not check_permission('launchpad.Append', self.context):
48+ link.enabled = False
49+ return link
50
51 @enabled_with_permission('launchpad.Edit')
52 def delete(self):
53@@ -500,6 +506,10 @@
54 """Common features for Archive view classes."""
55
56 @cachedproperty
57+ def private(self):
58+ return self.context.private
59+
60+ @cachedproperty
61 def has_sources(self):
62 """Whether or not this PPA has any sources for the view.
63
64@@ -960,6 +970,12 @@
65 """Detailed packages view for an archive."""
66 implements(IArchivePackagesActionMenu)
67
68+ def initialize(self):
69+ super(ArchivePackagesView, self).initialize()
70+ if self.context.private:
71+ if not check_permission('launchpad.Append', self.context):
72+ raise Unauthorized
73+
74 @property
75 def page_title(self):
76 return smartquote('Packages in "%s"' % self.context.displayname)
77
78=== added file 'lib/lp/soyuz/browser/tests/test_archive_packages.py'
79--- lib/lp/soyuz/browser/tests/test_archive_packages.py 1970-01-01 00:00:00 +0000
80+++ lib/lp/soyuz/browser/tests/test_archive_packages.py 2010-06-24 14:19:31 +0000
81@@ -0,0 +1,101 @@
82+# Copyright 2010 Canonical Ltd. This software is licensed under the
83+# GNU Affero General Public License version 3 (see the file LICENSE).
84+
85+# pylint: disable-msg=F0401
86+
87+"""Unit tests for TestP3APackages."""
88+
89+__metaclass__ = type
90+__all__ = [
91+ 'TestP3APackages',
92+ 'TestPPAPackages',
93+ 'test_suite',
94+ ]
95+
96+import unittest
97+
98+from zope.security.interfaces import Unauthorized
99+
100+from canonical.testing import LaunchpadFunctionalLayer
101+from lp.soyuz.browser.archive import ArchiveNavigationMenu
102+from lp.testing import login, login_person, TestCaseWithFactory
103+from lp.testing.views import create_initialized_view
104+
105+
106+class TestP3APackages(TestCaseWithFactory):
107+ """P3A archive pages are rendered correctly."""
108+
109+ layer = LaunchpadFunctionalLayer
110+
111+ def setUp(self):
112+ super(TestP3APackages, self).setUp()
113+ self.private_ppa = self.factory.makeArchive(description='Foo')
114+ login('admin@canonical.com')
115+ self.private_ppa.buildd_secret = 'blah'
116+ self.private_ppa.private = True
117+ self.joe = self.factory.makePerson(name='joe')
118+ self.fred = self.factory.makePerson(name='fred')
119+ self.mary = self.factory.makePerson(name='mary')
120+ login_person(self.private_ppa.owner)
121+ self.private_ppa.newSubscription(self.joe, self.private_ppa.owner)
122+ self.private_ppa.newComponentUploader(self.mary, 'main')
123+
124+ def test_packages_unauthorized(self):
125+ """A person with no subscription will not be able to view +packages
126+ """
127+ login_person(self.fred)
128+ self.assertRaises(
129+ Unauthorized, create_initialized_view, self.private_ppa,
130+ "+packages")
131+
132+ def test_packages_unauthorized_subscriber(self):
133+ """A person with a subscription will not be able to view +packages
134+ """
135+ login_person(self.joe)
136+ self.assertRaises(
137+ Unauthorized, create_initialized_view, self.private_ppa,
138+ "+packages")
139+
140+ def test_packages_authorized(self):
141+ """A person with launchpad.{Append,Edit} will be able to do so"""
142+ login_person(self.private_ppa.owner)
143+ view = create_initialized_view(self.private_ppa, "+packages")
144+ menu = ArchiveNavigationMenu(view)
145+ self.assertTrue(menu.packages().enabled)
146+
147+ def test_packages_uploader(self):
148+ """A person with launchpad.Append will also be able to do so"""
149+ login_person(self.mary)
150+ view = create_initialized_view(self.private_ppa, "+packages")
151+ menu = ArchiveNavigationMenu(view)
152+ self.assertTrue(menu.packages().enabled)
153+
154+ def test_packages_link_unauthorized(self):
155+ login_person(self.fred)
156+ view = create_initialized_view(self.private_ppa, "+index")
157+ menu = ArchiveNavigationMenu(view)
158+ self.assertFalse(menu.packages().enabled)
159+
160+ def test_packages_link_subscriber(self):
161+ login_person(self.joe)
162+ view = create_initialized_view(self.private_ppa, "+index")
163+ menu = ArchiveNavigationMenu(view)
164+ self.assertFalse(menu.packages().enabled)
165+
166+
167+class TestPPAPackages(TestCaseWithFactory):
168+ layer = LaunchpadFunctionalLayer
169+
170+ def setUp(self):
171+ super(TestPPAPackages, self).setUp()
172+ self.joe = self.factory.makePerson(name='joe')
173+ self.ppa = self.factory.makeArchive()
174+
175+ def test_ppa_packages(self):
176+ login_person(self.joe)
177+ view = create_initialized_view(self.ppa, "+index")
178+ menu = ArchiveNavigationMenu(view)
179+ self.assertTrue(menu.packages().enabled)
180+
181+def test_suite():
182+ return unittest.TestLoader().loadTestsFromName(__name__)
183
184=== added file 'lib/lp/soyuz/tests/test_archive_privacy.py'
185--- lib/lp/soyuz/tests/test_archive_privacy.py 1970-01-01 00:00:00 +0000
186+++ lib/lp/soyuz/tests/test_archive_privacy.py 2010-06-24 14:19:31 +0000
187@@ -0,0 +1,40 @@
188+# Copyright 2010 Canonical Ltd. This software is licensed under the
189+# GNU Affero General Public License version 3 (see the file LICENSE).
190+
191+"""Test Archive privacy features."""
192+
193+from zope.component import getUtility
194+from zope.security.interfaces import Unauthorized
195+from lp.soyuz.interfaces.archive import IArchiveSet
196+
197+from canonical.testing import LaunchpadFunctionalLayer
198+from lp.testing import login, login_person, TestCaseWithFactory
199+
200+
201+class TestArchivePrivacy(TestCaseWithFactory):
202+ layer = LaunchpadFunctionalLayer
203+
204+ def setUp(self):
205+ super(TestArchivePrivacy, self).setUp()
206+ self.private_ppa = self.factory.makeArchive(description='Foo')
207+ login('admin@canonical.com')
208+ self.private_ppa.buildd_secret = 'blah'
209+ self.private_ppa.private = True
210+ self.joe = self.factory.makePerson(name='joe')
211+ self.fred = self.factory.makePerson(name='fred')
212+ login_person(self.private_ppa.owner)
213+ self.private_ppa.newSubscription(self.joe, self.private_ppa.owner)
214+
215+ def _getDescription(self, p3a):
216+ return p3a.description
217+
218+ def test_no_subscription(self):
219+ login_person(self.fred)
220+ p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
221+ self.assertRaises(Unauthorized, self._getDescription, p3a)
222+
223+ def test_subscription(self):
224+ login_person(self.joe)
225+ p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
226+ self.assertEqual(self._getDescription(p3a), "Foo")
227+