launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08673
[Merge] lp:~jml/launchpad/validate-ppa-privacy into lp:launchpad
Jonathan Lange has proposed merging lp:~jml/launchpad/validate-ppa-privacy into lp:launchpad with lp:~jml/launchpad/validate-ppa-owner as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jml/launchpad/validate-ppa-privacy/+merge/109598
--
https://code.launchpad.net/~jml/launchpad/validate-ppa-privacy/+merge/109598
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/validate-ppa-privacy into lp:launchpad.
=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py 2012-06-11 10:27:22 +0000
+++ lib/lp/registry/browser/tests/test_team.py 2012-06-11 10:27:22 +0000
@@ -501,8 +501,7 @@
personset = getUtility(IPersonSet)
team = self.factory.makeTeam(
subscription_policy=TeamSubscriptionPolicy.MODERATED)
- product = self.factory.makeProduct(owner=team)
- self.factory.makeCommercialSubscription(product)
+ self.factory.grantCommercialSubscription(team)
with person_logged_in(team.teamowner):
view = create_initialized_view(
personset, name=self.view_name, principal=team.teamowner)
@@ -514,8 +513,7 @@
personset = getUtility(IPersonSet)
team = self.factory.makeTeam(
subscription_policy=TeamSubscriptionPolicy.MODERATED)
- product = self.factory.makeProduct(owner=team)
- self.factory.makeCommercialSubscription(product)
+ self.factory.grantCommercialSubscription(team)
team_name = self.factory.getUniqueString()
form = {
'field.name': team_name,
@@ -550,8 +548,7 @@
team = self.factory.makeTeam(
subscription_policy=TeamSubscriptionPolicy.RESTRICTED,
visibility=PersonVisibility.PRIVATE, owner=owner)
- product = self.factory.makeProduct(owner=owner)
- self.factory.makeCommercialSubscription(product)
+ self.factory.grantCommercialSubscription(team)
with person_logged_in(owner):
url = canonical_url(team)
browser = self.getUserBrowser(url, user=owner)
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-06-11 10:27:22 +0000
+++ lib/lp/registry/model/person.py 2012-06-11 10:27:22 +0000
@@ -2981,6 +2981,9 @@
errors = validate_ppa(self, name, private)
if errors:
raise PPACreationError(errors)
+ # XXX cprov 2009-03-27 bug=188564: We currently only create PPAs
+ # for Ubuntu distribution. PPA creation should be revisited when we
+ # start supporting other distribution (debian, mainly).
ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
return getUtility(IArchiveSet).new(
owner=self, purpose=ArchivePurpose.PPA,
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2012-06-11 10:27:22 +0000
+++ lib/lp/soyuz/browser/archive.py 2012-06-11 10:27:22 +0000
@@ -1988,21 +1988,13 @@
@action(_("Activate"), name="activate")
def save_action(self, action, data):
"""Activate a PPA and moves to its page."""
-
# 'name' field is omitted from the form data for default PPAs and
# it's dealt with by IArchive.new(), which will use the default
# PPA name.
name = data.get('name', None)
-
- # XXX: jml: I think this ought to call createPPA.
- # XXX cprov 2009-03-27 bug=188564: We currently only create PPAs
- # for Ubuntu distribution. PPA creation should be revisited when we
- # start supporting other distribution (debian, mainly).
- ppa = getUtility(IArchiveSet).new(
- owner=self.context, purpose=ArchivePurpose.PPA,
- distribution=self.ubuntu, name=name,
- displayname=data['displayname'], description=data['description'])
-
+ displayname = data['displayname']
+ description = data['description']
+ ppa = self.context.createPPA(name, displayname, description)
self.next_url = canonical_url(ppa)
@property
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2012-06-11 10:27:22 +0000
+++ lib/lp/soyuz/model/archive.py 2012-06-11 10:27:22 +0000
@@ -66,10 +66,7 @@
validate_person,
)
from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.interfaces.role import (
- IHasOwner,
- IPersonRoles,
- )
+from lp.registry.interfaces.role import IHasOwner
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.registry.model.teammembership import TeamParticipation
@@ -2065,14 +2062,16 @@
creator = getUtility(ILaunchBag).user
ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
if private:
- # NOTE: This duplicates the policy in lp/soyuz/configure.zcml
- # which says that one needs 'launchpad.Commercial' permission to
- # set 'private', and the logic in `AdminByCommercialTeamOrAdmins`
- # which determines who is granted launchpad.Commercial
- # permissions.
- role = IPersonRoles(creator)
- if not (role.in_admin or role.in_commercial_admin):
+ # NOTE: This duplicates the policy in lp/soyuz/configure.zcml which
+ # says that one needs 'launchpad.Commercial' permission to set
+ # 'private', and the logic in `AdminByCommercialTeamOrAdmins` which
+ # determines who is granted launchpad.Commercial permissions. The
+ # difference is that here we grant ability to set 'private' to people
+ # with a commercial subscription.
+ if not (owner.private or creator.checkAllowVisibility()):
return '%s is not allowed to make private PPAs' % creator.name
+ elif owner.private:
+ return 'Private teams may not have public archives.'
if owner.is_team and (
owner.subscriptionpolicy in OPEN_TEAM_POLICY):
return "Open teams cannot have PPAs."
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2012-06-11 10:27:22 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2012-06-11 10:27:22 +0000
@@ -26,10 +26,12 @@
from lp.buildmaster.enums import BuildStatus
from lp.registry.interfaces.person import (
IPersonSet,
+ PersonVisibility,
TeamSubscriptionPolicy,
)
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
+from lp.registry.interfaces.teammembership import TeamMembershipStatus
from lp.services.database.sqlbase import sqlvalues
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
@@ -1664,7 +1666,7 @@
'A PPA cannot have the same name as its distribution.',
validate_ppa(ppa_owner, 'ubuntu'))
- def test_private_ppa_non_commercial_admin(self):
+ def test_private_ppa_standard_user(self):
ppa_owner = self.factory.makePerson()
with person_logged_in(ppa_owner):
errors = validate_ppa(
@@ -1673,6 +1675,13 @@
'%s is not allowed to make private PPAs' % (ppa_owner.name,),
errors)
+ def test_private_ppa_commercial_subscription(self):
+ owner = self.factory.makePerson()
+ self.factory.grantCommercialSubscription(owner)
+ with person_logged_in(owner):
+ errors = validate_ppa(owner, 'ppa', private=True)
+ self.assertIsNone(errors)
+
def test_private_ppa_commercial_admin(self):
ppa_owner = self.factory.makePerson()
with celebrity_logged_in('admin'):
@@ -1708,6 +1717,36 @@
ppa_owner = self.factory.makePerson()
self.assertIsNone(validate_ppa(ppa_owner, None))
+ def test_private_team_private_ppa(self):
+ # Folk with launchpad.Edit on a private team can make private PPAs for
+ # that team, regardless of whether they have super-powers.a
+ team_owner = self.factory.makePerson()
+ private_team = self.factory.makeTeam(
+ owner=team_owner, visibility=PersonVisibility.PRIVATE,
+ subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+ team_admin = self.factory.makePerson()
+ with person_logged_in(team_owner):
+ private_team.addMember(
+ team_admin, team_owner, status=TeamMembershipStatus.ADMIN)
+ with person_logged_in(team_admin):
+ result = validate_ppa(private_team, 'ppa', private=True)
+ self.assertIsNone(result)
+
+ def test_private_team_public_ppa(self):
+ # No one can make a public PPA for a private team.
+ team_owner = self.factory.makePerson()
+ private_team = self.factory.makeTeam(
+ owner=team_owner, visibility=PersonVisibility.PRIVATE,
+ subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+ team_admin = self.factory.makePerson()
+ with person_logged_in(team_owner):
+ private_team.addMember(
+ team_admin, team_owner, status=TeamMembershipStatus.ADMIN)
+ with person_logged_in(team_admin):
+ result = validate_ppa(private_team, 'ppa', private=False)
+ self.assertEqual(
+ 'Private teams may not have public archives.', result)
+
class TestGetComponentsForSeries(TestCaseWithFactory):
"""Tests for Archive.getComponentsForSeries."""
=== modified file 'lib/lp/soyuz/tests/test_archive_privacy.py'
--- lib/lp/soyuz/tests/test_archive_privacy.py 2012-05-25 22:14:21 +0000
+++ lib/lp/soyuz/tests/test_archive_privacy.py 2012-06-11 10:27:22 +0000
@@ -8,6 +8,7 @@
from lp.soyuz.interfaces.archive import CannotSwitchPrivacy
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
+ celebrity_logged_in,
person_logged_in,
TestCaseWithFactory,
)
@@ -37,24 +38,46 @@
with person_logged_in(subscriber):
self.assertEqual(ppa.description, "Foo")
-
-class TestArchivePrivacySwitching(TestCaseWithFactory):
+ def test_owner_changing_privacy(self):
+ ppa = self.factory.makeArchive()
+ with person_logged_in(ppa.owner):
+ self.assertRaises(Unauthorized, setattr, ppa, 'private', True)
+
+ def test_owner_with_commercial_subscription_changing_privacy(self):
+ ppa = self.factory.makeArchive()
+ self.factory.grantCommercialSubscription(ppa.owner)
+ with person_logged_in(ppa.owner):
+ # XXX: jml 2012-06-11: We actually want this to be allowed, but I
+ # can't think of any way to grant this without also granting other
+ # attributes that have launchpad.Commercial.
+ self.assertRaises(Unauthorized, setattr, ppa, 'private', True)
+
+ def test_admin_changing_privacy(self):
+ ppa = self.factory.makeArchive()
+ with celebrity_logged_in('admin'):
+ ppa.private = True
+ self.assertEqual(True, ppa.private)
+
+ def test_commercial_admin_changing_privacy(self):
+ ppa = self.factory.makeArchive()
+ with celebrity_logged_in('commercial_admin'):
+ ppa.private = True
+ self.assertEqual(True, ppa.private)
+
+
+class TestPrivacySwitching(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
- def set_ppa_privacy(self, ppa, private):
- """Helper method to privatise a ppa."""
- ppa.private = private
-
def test_switch_privacy_no_pubs_succeeds(self):
# Changing the privacy is fine if there are no publishing
# records.
public_ppa = self.factory.makeArchive()
- self.set_ppa_privacy(public_ppa, private=True)
+ public_ppa.private = True
self.assertTrue(public_ppa.private)
private_ppa = self.factory.makeArchive(private=True)
- self.set_ppa_privacy(private_ppa, private=False)
+ private_ppa.private = False
self.assertFalse(private_ppa.private)
def test_switch_privacy_with_pubs_fails(self):
@@ -69,19 +92,17 @@
publisher.getPubSource(archive=private_ppa)
self.assertRaises(
- CannotSwitchPrivacy,
- self.set_ppa_privacy, public_ppa, private=True)
+ CannotSwitchPrivacy, setattr, public_ppa, 'private', True)
self.assertRaises(
- CannotSwitchPrivacy,
- self.set_ppa_privacy, private_ppa, private=False)
+ CannotSwitchPrivacy, setattr, private_ppa, 'private', False)
def test_buildd_secret_was_generated(self):
public_ppa = self.factory.makeArchive()
- self.set_ppa_privacy(public_ppa, private=True)
+ public_ppa.private = True
self.assertNotEqual(public_ppa.buildd_secret, None)
def test_discard_buildd_was_discarded(self):
private_ppa = self.factory.makeArchive(private=True)
- self.set_ppa_privacy(private_ppa, private=False)
+ private_ppa.private = False
self.assertEqual(private_ppa.buildd_secret, None)
=== modified file 'lib/lp/soyuz/tests/test_person_createppa.py'
--- lib/lp/soyuz/tests/test_person_createppa.py 2012-06-11 10:27:22 +0000
+++ lib/lp/soyuz/tests/test_person_createppa.py 2012-06-11 10:27:22 +0000
@@ -7,6 +7,11 @@
from zope.security.interfaces import Unauthorized
from lp.registry.errors import PPACreationError
+from lp.registry.interfaces.person import (
+ PersonVisibility,
+ TeamSubscriptionPolicy,
+ )
+from lp.registry.interfaces.teammembership import TeamMembershipStatus
from lp.testing import (
celebrity_logged_in,
person_logged_in,
@@ -49,3 +54,16 @@
with person_logged_in(person):
ppa = person.createPPA(suppress_subscription_notifications=True)
self.assertEqual(True, ppa.suppress_subscription_notifications)
+
+ def test_private_team_private_ppa(self):
+ team_owner = self.factory.makePerson()
+ private_team = self.factory.makeTeam(
+ owner=team_owner, visibility=PersonVisibility.PRIVATE,
+ subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+ team_admin = self.factory.makePerson()
+ with person_logged_in(team_owner):
+ private_team.addMember(
+ team_admin, team_owner, status=TeamMembershipStatus.ADMIN)
+ with person_logged_in(team_admin):
+ ppa = private_team.createPPA(private=True)
+ self.assertEqual(True, ppa.private)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-06-08 06:01:50 +0000
+++ lib/lp/testing/factory.py 2012-06-11 10:27:22 +0000
@@ -4338,6 +4338,12 @@
sales_system_id='new',
whiteboard='')
+ def grantCommercialSubscription(self, person, months=12):
+ """Give 'person' a commercial subscription."""
+ product = self.makeProduct(owner=person)
+ product.redeemSubscriptionVoucher(
+ self.getUniqueString(), person, person, months)
+
# Some factory methods return simple Python types. We don't add
# security wrappers for them, as well as for objects created by
