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
=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2010-06-16 18:49:47 +0000
+++ lib/canonical/launchpad/security.py 2010-06-24 14:19:31 +0000
@@ -19,7 +19,7 @@
19 IArchivePermissionSet)19 IArchivePermissionSet)
20from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthToken20from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthToken
21from lp.soyuz.interfaces.archivesubscriber import (21from lp.soyuz.interfaces.archivesubscriber import (
22 IArchiveSubscriber, IPersonalArchiveSubscription)22 IArchiveSubscriber, IArchiveSubscriberSet, IPersonalArchiveSubscription)
23from lp.code.interfaces.branch import (23from lp.code.interfaces.branch import (
24 IBranch, user_has_special_branch_access)24 IBranch, user_has_special_branch_access)
25from lp.code.interfaces.branchmergeproposal import (25from lp.code.interfaces.branchmergeproposal import (
@@ -2040,6 +2040,13 @@
2040 if self.obj.is_ppa and self.obj.checkArchivePermission(user.person):2040 if self.obj.is_ppa and self.obj.checkArchivePermission(user.person):
2041 return True2041 return True
20422042
2043 # Subscribers can view private PPAs.
2044 if self.obj.is_ppa and self.obj.private:
2045 archive_subs = getUtility(IArchiveSubscriberSet).getBySubscriber(
2046 user.person, self.obj).any()
2047 if archive_subs:
2048 return True
2049
2043 return False2050 return False
20442051
2045 def checkUnauthenticated(self):2052 def checkUnauthenticated(self):
20462053
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2010-06-14 09:31:26 +0000
+++ lib/lp/soyuz/browser/archive.py 2010-06-24 14:19:31 +0000
@@ -35,6 +35,7 @@
35from zope.component import getUtility35from zope.component import getUtility
36from zope.formlib import form36from zope.formlib import form
37from zope.interface import implements, Interface37from zope.interface import implements, Interface
38from zope.security.interfaces import Unauthorized
38from zope.security.proxy import removeSecurityProxy39from zope.security.proxy import removeSecurityProxy
39from zope.schema import Choice, List, TextLine40from zope.schema import Choice, List, TextLine
40from zope.schema.interfaces import IContextSourceBinder41from zope.schema.interfaces import IContextSourceBinder
@@ -426,7 +427,12 @@
426427
427 def packages(self):428 def packages(self):
428 text = 'View package details'429 text = 'View package details'
429 return Link('+packages', text, icon='info')430 link = Link('+packages', text, icon='info')
431 # Disable the link for P3As if they don't have upload rights.
432 if self.context.private:
433 if not check_permission('launchpad.Append', self.context):
434 link.enabled = False
435 return link
430436
431 @enabled_with_permission('launchpad.Edit')437 @enabled_with_permission('launchpad.Edit')
432 def delete(self):438 def delete(self):
@@ -500,6 +506,10 @@
500 """Common features for Archive view classes."""506 """Common features for Archive view classes."""
501507
502 @cachedproperty508 @cachedproperty
509 def private(self):
510 return self.context.private
511
512 @cachedproperty
503 def has_sources(self):513 def has_sources(self):
504 """Whether or not this PPA has any sources for the view.514 """Whether or not this PPA has any sources for the view.
505515
@@ -960,6 +970,12 @@
960 """Detailed packages view for an archive."""970 """Detailed packages view for an archive."""
961 implements(IArchivePackagesActionMenu)971 implements(IArchivePackagesActionMenu)
962972
973 def initialize(self):
974 super(ArchivePackagesView, self).initialize()
975 if self.context.private:
976 if not check_permission('launchpad.Append', self.context):
977 raise Unauthorized
978
963 @property979 @property
964 def page_title(self):980 def page_title(self):
965 return smartquote('Packages in "%s"' % self.context.displayname)981 return smartquote('Packages in "%s"' % self.context.displayname)
966982
=== added file 'lib/lp/soyuz/browser/tests/test_archive_packages.py'
--- lib/lp/soyuz/browser/tests/test_archive_packages.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_packages.py 2010-06-24 14:19:31 +0000
@@ -0,0 +1,101 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4# pylint: disable-msg=F0401
5
6"""Unit tests for TestP3APackages."""
7
8__metaclass__ = type
9__all__ = [
10 'TestP3APackages',
11 'TestPPAPackages',
12 'test_suite',
13 ]
14
15import unittest
16
17from zope.security.interfaces import Unauthorized
18
19from canonical.testing import LaunchpadFunctionalLayer
20from lp.soyuz.browser.archive import ArchiveNavigationMenu
21from lp.testing import login, login_person, TestCaseWithFactory
22from lp.testing.views import create_initialized_view
23
24
25class TestP3APackages(TestCaseWithFactory):
26 """P3A archive pages are rendered correctly."""
27
28 layer = LaunchpadFunctionalLayer
29
30 def setUp(self):
31 super(TestP3APackages, self).setUp()
32 self.private_ppa = self.factory.makeArchive(description='Foo')
33 login('admin@canonical.com')
34 self.private_ppa.buildd_secret = 'blah'
35 self.private_ppa.private = True
36 self.joe = self.factory.makePerson(name='joe')
37 self.fred = self.factory.makePerson(name='fred')
38 self.mary = self.factory.makePerson(name='mary')
39 login_person(self.private_ppa.owner)
40 self.private_ppa.newSubscription(self.joe, self.private_ppa.owner)
41 self.private_ppa.newComponentUploader(self.mary, 'main')
42
43 def test_packages_unauthorized(self):
44 """A person with no subscription will not be able to view +packages
45 """
46 login_person(self.fred)
47 self.assertRaises(
48 Unauthorized, create_initialized_view, self.private_ppa,
49 "+packages")
50
51 def test_packages_unauthorized_subscriber(self):
52 """A person with a subscription will not be able to view +packages
53 """
54 login_person(self.joe)
55 self.assertRaises(
56 Unauthorized, create_initialized_view, self.private_ppa,
57 "+packages")
58
59 def test_packages_authorized(self):
60 """A person with launchpad.{Append,Edit} will be able to do so"""
61 login_person(self.private_ppa.owner)
62 view = create_initialized_view(self.private_ppa, "+packages")
63 menu = ArchiveNavigationMenu(view)
64 self.assertTrue(menu.packages().enabled)
65
66 def test_packages_uploader(self):
67 """A person with launchpad.Append will also be able to do so"""
68 login_person(self.mary)
69 view = create_initialized_view(self.private_ppa, "+packages")
70 menu = ArchiveNavigationMenu(view)
71 self.assertTrue(menu.packages().enabled)
72
73 def test_packages_link_unauthorized(self):
74 login_person(self.fred)
75 view = create_initialized_view(self.private_ppa, "+index")
76 menu = ArchiveNavigationMenu(view)
77 self.assertFalse(menu.packages().enabled)
78
79 def test_packages_link_subscriber(self):
80 login_person(self.joe)
81 view = create_initialized_view(self.private_ppa, "+index")
82 menu = ArchiveNavigationMenu(view)
83 self.assertFalse(menu.packages().enabled)
84
85
86class TestPPAPackages(TestCaseWithFactory):
87 layer = LaunchpadFunctionalLayer
88
89 def setUp(self):
90 super(TestPPAPackages, self).setUp()
91 self.joe = self.factory.makePerson(name='joe')
92 self.ppa = self.factory.makeArchive()
93
94 def test_ppa_packages(self):
95 login_person(self.joe)
96 view = create_initialized_view(self.ppa, "+index")
97 menu = ArchiveNavigationMenu(view)
98 self.assertTrue(menu.packages().enabled)
99
100def test_suite():
101 return unittest.TestLoader().loadTestsFromName(__name__)
0102
=== added file 'lib/lp/soyuz/tests/test_archive_privacy.py'
--- lib/lp/soyuz/tests/test_archive_privacy.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/tests/test_archive_privacy.py 2010-06-24 14:19:31 +0000
@@ -0,0 +1,40 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test Archive privacy features."""
5
6from zope.component import getUtility
7from zope.security.interfaces import Unauthorized
8from lp.soyuz.interfaces.archive import IArchiveSet
9
10from canonical.testing import LaunchpadFunctionalLayer
11from lp.testing import login, login_person, TestCaseWithFactory
12
13
14class TestArchivePrivacy(TestCaseWithFactory):
15 layer = LaunchpadFunctionalLayer
16
17 def setUp(self):
18 super(TestArchivePrivacy, self).setUp()
19 self.private_ppa = self.factory.makeArchive(description='Foo')
20 login('admin@canonical.com')
21 self.private_ppa.buildd_secret = 'blah'
22 self.private_ppa.private = True
23 self.joe = self.factory.makePerson(name='joe')
24 self.fred = self.factory.makePerson(name='fred')
25 login_person(self.private_ppa.owner)
26 self.private_ppa.newSubscription(self.joe, self.private_ppa.owner)
27
28 def _getDescription(self, p3a):
29 return p3a.description
30
31 def test_no_subscription(self):
32 login_person(self.fred)
33 p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
34 self.assertRaises(Unauthorized, self._getDescription, p3a)
35
36 def test_subscription(self):
37 login_person(self.joe)
38 p3a = getUtility(IArchiveSet).get(self.private_ppa.id)
39 self.assertEqual(self._getDescription(p3a), "Foo")
40