launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10495
[Merge] lp:~cjwatson/launchpad/pocket-queue-admin into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/pocket-queue-admin into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #648611 in Launchpad itself: "ubuntu-sru either have too much or too little permission as queue admins"
https://bugs.launchpad.net/launchpad/+bug/648611
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/pocket-queue-admin/+merge/117630
== Summary ==
Bug 648611 reports that we don't have sufficient ability to delegate queue administration tasks for specific pockets (which are often managed in quite different ways by quite different sets of people). As a result, everything ends up going through the overworked ~ubuntu-archive team, and we often end up granting overly-broad privileges to people who don't need it in order to try to manage that overloading. Instead, we'd like to be able to grant explicit queue admin permissions to particular pockets.
== Proposed fix ==
Add the usual large cluster of Archive and ArchivePermission methods to support per-pocket queue admin permissions, and extend EditPackageUploadQueue and Archive.canAdministerQueue to check these.
I made the necessary schema changes some time ago and they've already been deployed.
== Pre-implementation notes ==
Discussion with Scott Kitterman revealed that at least some per-pocket queue admin permissions will need to be per-distroseries as well: for example, ~ubuntu-sru should only have queue admin on PROPOSED and UPDATES in stable releases, while ~ubuntu-release should only have queue admin on RELEASE and PROPOSED in the development release. I've implemented this. See also:
https://code.launchpad.net/~cjwatson/launchpad/db-pocket-queue-admin/+merge/115734
== LOC Rationale ==
Archive permission handling generally seems to wind up being fairly verbose, and this has come out at +485. I have 3911 lines of credit at the moment so I think this should be tolerated; and this should let us get rid of delayed copies and the ubuntu-security celebrity, which will comfortably offset this.
== Tests ==
bin/test -vvct soyuz.browser.tests.test_queue -t archivepermission -t lib/lp/soyuz/stories/webservice/xx-archive.txt
== Demo and Q/A ==
On dogfood, create a pocket queue admin permission using Archive.newPocketQueueAdmin for a user who doesn't otherwise have queue admin permission, and try to accept/reject a package from an appropriate queue. Do likewise with a series-specific permission.
== Lint ==
A false positive due to pocketlint not understanding property setters, and doctest line length cruft which I don't think is worth major contortions to avoid (it's basically due to testing long URLs).
./lib/lp/soyuz/model/archive.py
346: redefinition of function 'private' from line 342
./lib/lp/soyuz/stories/webservice/xx-archive.txt
43: want exceeds 78 characters.
47: want exceeds 78 characters.
173: want exceeds 78 characters.
190: want exceeds 78 characters.
207: want exceeds 78 characters.
224: want exceeds 78 characters.
371: want exceeds 78 characters.
432: want exceeds 78 characters.
563: want exceeds 78 characters.
625: want exceeds 78 characters.
705: want exceeds 78 characters.
725: want exceeds 78 characters.
--
https://code.launchpad.net/~cjwatson/launchpad/pocket-queue-admin/+merge/117630
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/pocket-queue-admin into lp:launchpad.
=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py 2012-07-25 04:35:35 +0000
+++ lib/lp/_schema_circular_imports.py 2012-08-01 12:05:24 +0000
@@ -412,6 +412,10 @@
patch_collection_return_type(
IArchive, 'getComponentsForQueueAdmin', IArchivePermission)
patch_collection_return_type(
+ IArchive, 'getQueueAdminsForPocket', IArchivePermission)
+patch_collection_return_type(
+ IArchive, 'getPocketsForQueueAdmin', IArchivePermission)
+patch_collection_return_type(
IArchive, 'getPocketsForUploader', IArchivePermission)
patch_collection_return_type(
IArchive, 'getUploadersForPocket', IArchivePermission)
@@ -420,6 +424,7 @@
patch_entry_return_type(IArchive, 'newComponentUploader', IArchivePermission)
patch_entry_return_type(IArchive, 'newPocketUploader', IArchivePermission)
patch_entry_return_type(IArchive, 'newQueueAdmin', IArchivePermission)
+patch_entry_return_type(IArchive, 'newPocketQueueAdmin', IArchivePermission)
patch_plain_parameter_type(IArchive, 'syncSources', 'from_archive', IArchive)
patch_plain_parameter_type(IArchive, 'syncSource', 'from_archive', IArchive)
patch_plain_parameter_type(IArchive, 'copyPackage', 'from_archive', IArchive)
@@ -455,9 +460,21 @@
patch_choice_parameter_type(
IArchive, 'getUploadersForPocket', 'pocket', PackagePublishingPocket)
patch_choice_parameter_type(
+ IArchive, 'getQueueAdminsForPocket', 'pocket', PackagePublishingPocket)
+patch_choice_parameter_type(
+ IArchive, 'getQueueAdminsForPocket', 'distroseries', IDistroSeries)
+patch_choice_parameter_type(
IArchive, 'newPocketUploader', 'pocket', PackagePublishingPocket)
patch_choice_parameter_type(
+ IArchive, 'newPocketQueueAdmin', 'pocket', PackagePublishingPocket)
+patch_choice_parameter_type(
+ IArchive, 'newPocketQueueAdmin', 'distroseries', IDistroSeries)
+patch_choice_parameter_type(
IArchive, 'deletePocketUploader', 'pocket', PackagePublishingPocket)
+patch_choice_parameter_type(
+ IArchive, 'deletePocketQueueAdmin', 'pocket', PackagePublishingPocket)
+patch_choice_parameter_type(
+ IArchive, 'deletePocketQueueAdmin', 'distroseries', IDistroSeries)
patch_plain_parameter_type(
IArchive, 'newPackagesetUploader', 'packageset', IPackageset)
patch_plain_parameter_type(
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2012-07-24 19:50:54 +0000
+++ lib/lp/security.py 2012-08-01 12:05:24 +0000
@@ -1659,10 +1659,18 @@
return True
permission_set = getUtility(IArchivePermissionSet)
- permissions = permission_set.componentsForQueueAdmin(
- self.obj.distroseries.distribution.all_distro_archives,
- user.person)
- return not permissions.is_empty()
+ component_permissions = permission_set.componentsForQueueAdmin(
+ self.obj.distroseries.distribution.all_distro_archives,
+ user.person)
+ if not component_permissions.is_empty():
+ return True
+ pocket_permissions = permission_set.pocketsForQueueAdmin(
+ self.obj.distroseries.distribution.all_distro_archives,
+ user.person)
+ for permission in pocket_permissions:
+ if permission.distroseries in (None, self.obj.distroseries):
+ return True
+ return False
class EditPlainPackageCopyJob(AuthorizationBase):
@@ -1731,7 +1739,8 @@
return archive_append.checkAuthenticated(user)
return self.obj.archive.canAdministerQueue(
- user.person, self.obj.components)
+ user.person, self.obj.components, self.obj.pocket,
+ self.obj.distroseries)
class AdminByBuilddAdmin(AuthorizationBase):
=== modified file 'lib/lp/soyuz/browser/archivepermission.py'
--- lib/lp/soyuz/browser/archivepermission.py 2012-06-13 11:11:22 +0000
+++ lib/lp/soyuz/browser/archivepermission.py 2012-08-01 12:05:24 +0000
@@ -50,6 +50,9 @@
self.context.distro_series_name))
elif self.context.pocket is not None:
item = "type=pocket&item=%s" % self.context.pocket.name
+ # Queue admin permissions for pockets may be granted by series.
+ if self.context.distroseries is not None:
+ item += "&series=%s" % self.context.distroseries.name
else:
raise AssertionError(
"One of component, sourcepackagename or package set should "
=== modified file 'lib/lp/soyuz/browser/queue.py'
--- lib/lp/soyuz/browser/queue.py 2012-07-09 12:32:23 +0000
+++ lib/lp/soyuz/browser/queue.py 2012-08-01 12:05:24 +0000
@@ -335,10 +335,12 @@
# Get a list of components for which the user has rights to
# override to or from.
permission_set = getUtility(IArchivePermissionSet)
- permissions = permission_set.componentsForQueueAdmin(
+ component_permissions = permission_set.componentsForQueueAdmin(
self.context.main_archive, self.user)
allowed_components = set(
- permission.component for permission in permissions)
+ permission.component for permission in component_permissions)
+ pocket_permissions = permission_set.pocketsForQueueAdmin(
+ self.context.main_archive, self.user)
try:
if section_override:
@@ -385,15 +387,23 @@
# Sources and binaries are mutually exclusive when it comes to
# overriding, so only one of these will be set.
try:
+ for permission in pocket_permissions:
+ if (permission.pocket == queue_item.pocket and
+ permission.distroseries in (
+ None, queue_item.distroseries)):
+ item_allowed_components = (
+ queue_item.distroseries.upload_components)
+ else:
+ item_allowed_components = allowed_components
source_overridden = queue_item.overrideSource(
- new_component, new_section, allowed_components)
+ new_component, new_section, item_allowed_components)
binary_changes = [{
"component": new_component,
"section": new_section,
"priority": new_priority,
}]
binary_overridden = queue_item.overrideBinaries(
- binary_changes, allowed_components)
+ binary_changes, item_allowed_components)
except (QueueAdminUnauthorizedError,
QueueInconsistentStateError) as info:
failure.append("FAILED: %s (%s)" %
=== modified file 'lib/lp/soyuz/browser/tests/test_queue.py'
--- lib/lp/soyuz/browser/tests/test_queue.py 2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/browser/tests/test_queue.py 2012-08-01 12:05:24 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for QueueItemsView."""
@@ -15,6 +15,7 @@
)
from lp.archiveuploader.tests import datadir
+from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.soyuz.browser.queue import CompletePackageUpload
from lp.soyuz.enums import PackageUploadStatus
@@ -50,6 +51,9 @@
self.test_publisher = SoyuzTestPublisher()
self.test_publisher.prepareBreezyAutotest()
distribution = self.test_publisher.distroseries.distribution
+ self.second_series = self.factory.makeDistroSeries(
+ distribution=distribution)
+ self.factory.makeComponentSelection(self.second_series, 'main')
self.main_archive = distribution.getArchiveByComponent('main')
self.partner_archive = distribution.getArchiveByComponent('partner')
@@ -68,6 +72,17 @@
sourcename='main-upload', spr_only=True,
component='main', changes_file_content=changes_file_content)
self.main_spr.package_upload.setNew()
+ self.proposed_spr = self.test_publisher.getPubSource(
+ sourcename='proposed-upload', spr_only=True,
+ component='main', changes_file_content=changes_file_content,
+ pocket=PackagePublishingPocket.PROPOSED)
+ self.proposed_spr.package_upload.setNew()
+ self.proposed_series_spr = self.test_publisher.getPubSource(
+ sourcename='proposed-series-upload', spr_only=True,
+ component='main', changes_file_content=changes_file_content,
+ pocket=PackagePublishingPocket.PROPOSED,
+ distroseries=self.second_series)
+ self.proposed_series_spr.package_upload.setNew()
# Define the form that will be used to post to the view.
self.form = {
@@ -89,15 +104,28 @@
distribution.getArchiveByComponent('partner'),
self.partner_queue_admin, self.partner_spr.component)
+ # Create users with various pocket queue admin rights.
+ self.proposed_queue_admin = self.factory.makePerson(
+ email='proposed-queue@xxxxxxxxxxx')
+ getUtility(IArchivePermissionSet).newPocketQueueAdmin(
+ self.main_archive, self.proposed_queue_admin,
+ PackagePublishingPocket.PROPOSED)
+ self.proposed_series_queue_admin = self.factory.makePerson(
+ email='proposed-series-queue@xxxxxxxxxxx')
+ getUtility(IArchivePermissionSet).newPocketQueueAdmin(
+ self.main_archive, self.proposed_series_queue_admin,
+ PackagePublishingPocket.PROPOSED, distroseries=self.second_series)
+
# We need to commit to ensure the changes file exists in the
# librarian.
transaction.commit()
logout()
- def setupQueueView(self, request):
+ def setupQueueView(self, request, series=None):
"""A helper to create and setup the view for testing."""
- view = queryMultiAdapter(
- (self.test_publisher.distroseries, request), name="+queue")
+ if series is None:
+ series = self.test_publisher.distroseries
+ view = queryMultiAdapter((series, request), name="+queue")
view.setupQueueList()
view.performQueueAction()
return view
@@ -197,6 +225,97 @@
'NEW',
getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+ def test_proposed_admin_can_accept_proposed_upload(self):
+ # A person with queue admin access for proposed can accept uploads
+ # to the proposed pocket for any series.
+ login('proposed-queue@xxxxxxxxxxx')
+ self.assertTrue(
+ self.main_archive.canAdministerQueue(
+ self.proposed_queue_admin,
+ pocket=PackagePublishingPocket.PROPOSED))
+ for distroseries in self.test_publisher.distroseries.distribution:
+ self.assertTrue(
+ self.main_archive.canAdministerQueue(
+ self.proposed_queue_admin,
+ pocket=PackagePublishingPocket.PROPOSED,
+ distroseries=distroseries))
+ package_upload_set = getUtility(IPackageUploadSet)
+
+ for spr in (self.proposed_spr, self.proposed_series_spr):
+ package_upload_id = spr.package_upload.id
+ self.form['QUEUE_ID'] = [package_upload_id]
+ request = LaunchpadTestRequest(form=self.form)
+ request.method = 'POST'
+ self.setupQueueView(request, series=spr.upload_distroseries)
+
+ self.assertEqual(
+ 'DONE', package_upload_set.get(package_upload_id).status.name)
+
+ def test_proposed_admin_cannot_accept_release_upload(self):
+ # A person with queue admin access for proposed cannot necessarly
+ # accept uploads to the release pocket.
+ login('proposed-queue@xxxxxxxxxxx')
+ self.assertFalse(
+ self.main_archive.canAdministerQueue(
+ self.proposed_queue_admin,
+ pocket=PackagePublishingPocket.RELEASE))
+
+ package_upload_id = self.main_spr.package_upload.id
+ self.form['QUEUE_ID'] = [package_upload_id]
+ request = LaunchpadTestRequest(form=self.form)
+ request.method = 'POST'
+ view = self.setupQueueView(request)
+
+ self.assertEqual(
+ "FAILED: main-upload (You have no rights to accept "
+ "component(s) 'main')",
+ view.request.response.notifications[0].message)
+ self.assertEqual(
+ 'NEW',
+ getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+
+ def test_proposed_series_admin_can_accept_that_series_upload(self):
+ # A person with queue admin access for proposed for one series can
+ # accept uploads to that series.
+ login('proposed-series-queue@xxxxxxxxxxx')
+ self.assertTrue(
+ self.main_archive.canAdministerQueue(
+ self.proposed_series_queue_admin,
+ pocket=PackagePublishingPocket.PROPOSED,
+ distroseries=self.second_series))
+
+ package_upload_id = self.proposed_series_spr.package_upload.id
+ self.form['QUEUE_ID'] = [package_upload_id]
+ request = LaunchpadTestRequest(form=self.form)
+ request.method = 'POST'
+ self.setupQueueView(request, series=self.second_series)
+
+ self.assertEqual(
+ 'DONE',
+ getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+
+ def test_proposed_series_admin_cannot_accept_other_series_upload(self):
+ # A person with queue admin access for proposed for one series
+ # cannot necessarily accept uploads to other series.
+ login('proposed-series-queue@xxxxxxxxxxx')
+ self.assertFalse(
+ self.main_archive.canAdministerQueue(
+ self.proposed_series_queue_admin,
+ pocket=PackagePublishingPocket.PROPOSED,
+ distroseries=self.test_publisher.distroseries))
+
+ package_upload_id = self.proposed_spr.package_upload.id
+ self.form['QUEUE_ID'] = [package_upload_id]
+ request = LaunchpadTestRequest(form=self.form)
+ request.method = 'POST'
+ view = self.setupQueueView(request)
+
+ self.assertEqual(
+ "You do not have permission to act on queue items.", view.error)
+ self.assertEqual(
+ 'NEW',
+ getUtility(IPackageUploadSet).get(package_upload_id).status.name)
+
class TestQueueItemsView(TestCaseWithFactory):
"""Unit tests for `QueueItemsView`."""
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2012-07-30 16:48:37 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2012-08-01 12:05:24 +0000
@@ -680,15 +680,20 @@
None otherwise.
"""
- def canAdministerQueue(person, components):
+ def canAdministerQueue(person, components=None, pocket=None,
+ distroseries=None):
"""Check to see if person is allowed to administer queue items.
:param person: An `IPerson` who should be checked for authentication.
:param components: The context `IComponent`(s) for the check.
+ :param pocket: The context `PackagePublishingPocket` for the check.
+ :param distroseries: The context `IDistroSeries` for the check.
:return: True if 'person' is allowed to administer the package upload
- queue for all given 'components'. If 'components' is empty or None,
- check if 'person' has any queue admin permissions for this archive.
+ queue for all given 'components', or for the given 'pocket'
+ (optionally restricted to a single 'distroseries'). If 'components'
+ is empty or None and 'pocket' is None, check if 'person' has any
+ queue admin permissions for this archive.
"""
def getFileByName(filename):
@@ -1236,6 +1241,41 @@
:return: A list of `IArchivePermission` records.
"""
+ @operation_parameters(
+ pocket=Choice(
+ title=_("Pocket"),
+ # Really PackagePublishingPocket, circular import fixed below.
+ vocabulary=DBEnumeratedType,
+ required=True),
+ distroseries=Reference(
+ # Really IDistroSeries, avoiding a circular import here.
+ Interface,
+ title=_("Distro series"), required=False),
+ )
+ # Really IArchivePermission, set below to avoid circular import.
+ @operation_returns_collection_of(Interface)
+ @export_read_operation()
+ @operation_for_version("devel")
+ def getQueueAdminsForPocket(pocket, distroseries=None):
+ """Return `IArchivePermission` records for authorised queue admins.
+
+ :param pocket: A `PackagePublishingPocket`.
+ :param distroseries: An optional `IDistroSeries`.
+ :return: A list of `IArchivePermission` records.
+ """
+
+ @operation_parameters(person=Reference(schema=IPerson))
+ # Really IArchivePermission, set below to avoid circular import.
+ @operation_returns_collection_of(Interface)
+ @export_read_operation()
+ @operation_for_version("devel")
+ def getPocketsForQueueAdmin(person):
+ """Return `IArchivePermission` for the person's queue admin pockets.
+
+ :param person: An `IPerson`.
+ :return: A list of `IArchivePermission` records.
+ """
+
def hasAnyPermission(person):
"""Whether or not this person has any permission at all on this
archive.
@@ -1607,7 +1647,7 @@
"""Add permission for a person to upload to a pocket.
:param person: An `IPerson` whom should be given permission.
- :param component: A `PackagePublishingPocket`.
+ :param pocket: A `PackagePublishingPocket`.
:return: An `IArchivePermission` which is the newly-created
permission.
:raises InvalidPocketForPartnerArchive: if this archive is a partner
@@ -1636,6 +1676,34 @@
@operation_parameters(
person=Reference(schema=IPerson),
+ pocket=Choice(
+ title=_("Pocket"),
+ # Really PackagePublishingPocket, circular import fixed below.
+ vocabulary=DBEnumeratedType,
+ required=True),
+ distroseries=Reference(
+ # Really IDistroSeries, avoiding a circular import here.
+ Interface,
+ title=_("Distro series"), required=True),
+ )
+ # Really IArchivePermission, set below to avoid circular import.
+ @export_factory_operation(Interface, [])
+ @operation_for_version("devel")
+ def newPocketQueueAdmin(person, pocket, distroseries=None):
+ """Add permission for a person to administer a distroseries queue.
+
+ The supplied person will gain permission to administer the
+ distroseries queue for packages in the supplied series and pocket.
+
+ :param person: An `IPerson` whom should be given permission.
+ :param pocket: A `PackagePublishingPocket`.
+ :param distroseries: An optional `IDistroSeries`.
+ :return: An `IArchivePermission` which is the newly-created
+ permission.
+ """
+
+ @operation_parameters(
+ person=Reference(schema=IPerson),
# Really IPackageset, corrected in _schema_circular_imports to avoid
# circular import.
packageset=Reference(
@@ -1696,6 +1764,7 @@
"""Revoke permission for the person to upload to the pocket.
:param person: An `IPerson` whose permission should be revoked.
+ :param distroseries: An `IDistroSeries`.
:param pocket: A `PackagePublishingPocket`.
"""
@@ -1716,6 +1785,31 @@
@operation_parameters(
person=Reference(schema=IPerson),
+ pocket=Choice(
+ title=_("Pocket"),
+ # Really PackagePublishingPocket, circular import fixed below.
+ vocabulary=DBEnumeratedType,
+ required=True),
+ distroseries=Reference(
+ # Really IDistroSeries, avoiding a circular import here.
+ Interface,
+ title=_("Distro series"), required=True),
+ )
+ @export_write_operation()
+ @operation_for_version("devel")
+ def deletePocketQueueAdmin(person, pocket, distroseries=None):
+ """Revoke permission for the person to administer distroseries queues.
+
+ The supplied person will lose permission to administer the
+ distroseries queue for packages in the supplied series and pocket.
+
+ :param person: An `IPerson` whose permission should be revoked.
+ :param pocket: A `PackagePublishingPocket`.
+ :param distroseries: An optional `IDistroSeries`.
+ """
+
+ @operation_parameters(
+ person=Reference(schema=IPerson),
# Really IPackageset, corrected in _schema_circular_imports to avoid
# circular import.
packageset=Reference(
=== modified file 'lib/lp/soyuz/interfaces/archivepermission.py'
--- lib/lp/soyuz/interfaces/archivepermission.py 2012-06-07 10:28:07 +0000
+++ lib/lp/soyuz/interfaces/archivepermission.py 2012-08-01 12:05:24 +0000
@@ -31,6 +31,7 @@
)
from lp import _
+from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.sourcepackagename import ISourcePackageName
from lp.services.fields import PublicPersonChoice
@@ -125,6 +126,15 @@
vocabulary=PackagePublishingPocket,
required=True))
+ distroseries = exported(
+ Reference(
+ IDistroSeries,
+ title=_("Distro series"),
+ description=_(
+ "The distro series that this permission is for (only for "
+ "pocket permissions)."),
+ required=False))
+
class IArchiveUploader(IArchivePermission):
"""Marker interface for URL traversal of uploader permissions."""
@@ -357,6 +367,29 @@
'person' is allowed to administer the queue for.
"""
+ def queueAdminsForPocket(archive, pocket, distroseries=None):
+ """The `ArchivePermission` records for authorised pocket queue admins.
+
+ :param archive: The context `IArchive` for the permission check.
+ :param pocket: A `PackagePublishingPocket`.
+ :param distroseries: An optional `IDistroSeries`.
+
+ :return: `ArchivePermission` records for all the persons who are
+ allowed to administer the pocket upload queue.
+ """
+
+ def pocketsForQueueAdmin(archive, person):
+ """Return `ArchivePermission` for the person's queue admin pockets.
+
+ :param archive: The context `IArchive` for the permission check, or
+ an iterable of `IArchive`s.
+ :param person: An `IPerson` for whom you want to find out which
+ pockets he has access to.
+
+ :return: `ArchivePermission` records for all the pockets that
+ 'person' is allowed to administer the queue for.
+ """
+
def newPackageUploader(archive, person, sourcepackagename):
"""Create and return a new `ArchivePermission` for an uploader.
@@ -421,6 +454,18 @@
already exists.
"""
+ def newPocketQueueAdmin(archive, person, pocket, distroseries=None):
+ """Create and return a new `ArchivePermission` for a queue admin.
+
+ :param archive: The context `IArchive` for the permission check.
+ :param person: An `IPerson` for whom you want to add permission.
+ :param pocket: A `PackagePublishingPocket`.
+ :param distroseries: An optional `IDistroSeries`.
+
+ :return: The new `ArchivePermission`, or the existing one if it
+ already exists.
+ """
+
def deletePackageUploader(archive, person, sourcepackagename):
"""Revoke upload permissions for a person.
@@ -466,3 +511,12 @@
:param person: An `IPerson` for whom you want to revoke permission.
:param component: An `IComponent` or a string package name.
"""
+
+ def deletePocketQueueAdmin(archive, person, pocket, distroseries=None):
+ """Revoke queue admin permissions for a person.
+
+ :param archive: The context `IArchive` for the permission check.
+ :param person: An `IPerson` for whom you want to revoke permission.
+ :param pocket: A `PackagePublishingPocket`.
+ :param distroseries: An optional `IDistroSeries`.
+ """
=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py 2012-07-09 12:32:23 +0000
+++ lib/lp/soyuz/interfaces/queue.py 2012-08-01 12:05:24 +0000
@@ -105,7 +105,7 @@
class IPackageUploadQueue(Interface):
"""Used to establish permission to a group of package uploads.
- Recieves an IDistroSeries and a PackageUploadStatus dbschema
+ Receives an IDistroSeries and a PackageUploadStatus dbschema
on initialization.
No attributes exposed via interface, only used to check permissions.
"""
@@ -811,4 +811,4 @@
"""An Object that has queue items"""
def getPackageUploadQueue(state):
- """Return an IPackageUploadeQueue occording the given state."""
+ """Return an IPackageUploadQueue according to the given state."""
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2012-07-30 16:48:37 +0000
+++ lib/lp/soyuz/model/archive.py 2012-08-01 12:05:24 +0000
@@ -1052,10 +1052,11 @@
raise ComponentNotFound(e)
return self.addArchiveDependency(dependency, pocket, component)
- def getPermissions(self, user, item, perm_type):
+ def getPermissions(self, user, item, perm_type, distroseries=None):
"""See `IArchive`."""
permission_set = getUtility(IArchivePermissionSet)
- return permission_set.checkAuthenticated(user, self, perm_type, item)
+ return permission_set.checkAuthenticated(
+ user, self, perm_type, item, distroseries=distroseries)
def getPermissionsForPerson(self, person):
"""See `IArchive`."""
@@ -1087,6 +1088,17 @@
permission_set = getUtility(IArchivePermissionSet)
return permission_set.componentsForQueueAdmin(self, person)
+ def getQueueAdminsForPocket(self, pocket, distroseries=None):
+ """See `IArchive`."""
+ permission_set = getUtility(IArchivePermissionSet)
+ return permission_set.queueAdminsForPocket(
+ self, pocket, distroseries=distroseries)
+
+ def getPocketsForQueueAdmin(self, person):
+ """See `IArchive`."""
+ permission_set = getUtility(IArchivePermissionSet)
+ return permission_set.pocketsForQueueAdmin(self, person)
+
def hasAnyPermission(self, person):
"""See `IArchive`."""
# Avoiding circular imports.
@@ -1347,24 +1359,33 @@
return None
- def canAdministerQueue(self, user, components):
+ def canAdministerQueue(self, user, components=None, pocket=None,
+ distroseries=None):
"""See `IArchive`."""
if components is None:
components = []
elif IComponent.providedBy(components):
components = [components]
- permissions = self.getComponentsForQueueAdmin(user)
- if permissions.count() == 0:
- return False
- allowed_components = set(
- permission.component for permission in permissions)
- # The intersection of allowed_components and components must be
- # equal to components to allow the operation to go ahead.
- return allowed_components.intersection(components) == set(components)
+ component_permissions = self.getComponentsForQueueAdmin(user)
+ if not component_permissions.is_empty():
+ allowed_components = set(
+ permission.component for permission in component_permissions)
+ # The intersection of allowed_components and components must be
+ # equal to components to allow the operation to go ahead.
+ if allowed_components.intersection(components) == set(components):
+ return True
+ if pocket is not None:
+ pocket_permissions = self.getPocketsForQueueAdmin(user)
+ for permission in pocket_permissions:
+ if (permission.pocket == pocket and
+ permission.distroseries in (None, distroseries)):
+ return True
+ return False
- def _authenticate(self, user, item, permission):
+ def _authenticate(self, user, item, permission, distroseries=None):
"""Private helper method to check permissions."""
- permissions = self.getPermissions(user, item, permission)
+ permissions = self.getPermissions(
+ user, item, permission, distroseries=distroseries)
return bool(permissions)
def newPackageUploader(self, person, source_package_name):
@@ -1416,6 +1437,12 @@
permission_set = getUtility(IArchivePermissionSet)
return permission_set.newQueueAdmin(self, person, component_name)
+ def newPocketQueueAdmin(self, person, pocket, distroseries=None):
+ """See `IArchive`."""
+ permission_set = getUtility(IArchivePermissionSet)
+ return permission_set.newPocketQueueAdmin(
+ self, person, pocket, distroseries=distroseries)
+
def deletePackageUploader(self, person, source_package_name):
"""See `IArchive`."""
permission_set = getUtility(IArchivePermissionSet)
@@ -1438,6 +1465,12 @@
permission_set = getUtility(IArchivePermissionSet)
return permission_set.deleteQueueAdmin(self, person, component_name)
+ def deletePocketQueueAdmin(self, person, pocket, distroseries=None):
+ """See `IArchive`."""
+ permission_set = getUtility(IArchivePermissionSet)
+ return permission_set.deletePocketQueueAdmin(
+ self, person, pocket, distroseries=distroseries)
+
def getUploadersForPackageset(self, packageset, direct_permissions=True):
"""See `IArchive`."""
permission_set = getUtility(IArchivePermissionSet)
=== modified file 'lib/lp/soyuz/model/archivepermission.py'
--- lib/lp/soyuz/model/archivepermission.py 2012-06-07 10:28:07 +0000
+++ lib/lp/soyuz/model/archivepermission.py 2012-08-01 12:05:24 +0000
@@ -106,6 +106,9 @@
pocket = EnumCol(dbName="pocket", schema=PackagePublishingPocket)
+ distroseries = ForeignKey(
+ foreignKey='DistroSeries', dbName='distroseries', notNull=False)
+
def _init(self, *args, **kw):
"""Provide the right interface for URL traversal."""
SQLBase._init(self, *args, **kw)
@@ -150,6 +153,8 @@
"""See `IArchivePermission`"""
if self.packageset:
return self.packageset.distroseries.name
+ elif self.distroseries:
+ return self.distroseries.name
else:
return None
@@ -158,7 +163,8 @@
"""See `IArchivePermissionSet`."""
implements(IArchivePermissionSet)
- def checkAuthenticated(self, person, archive, permission, item):
+ def checkAuthenticated(self, person, archive, permission, item,
+ distroseries=None):
"""See `IArchivePermissionSet`."""
clauses = ["""
ArchivePermission.archive = %s AND
@@ -184,6 +190,12 @@
elif (zope_isinstance(item, DBItem) and
item.enum.name == "PackagePublishingPocket"):
clauses.append("ArchivePermission.pocket = %s" % sqlvalues(item))
+ if distroseries is not None:
+ clauses.append(
+ "ArchivePermission.distroseries IS NULL OR "
+ "ArchivePermission.distroseries = %s" %
+ sqlvalues(distroseries))
+ prejoins.append("distroseries")
else:
raise AssertionError(
"'item' %r is not an IComponent, IPackageset, "
@@ -248,7 +260,7 @@
archive, person, ArchivePermissionType.UPLOAD)
def uploadersForComponent(self, archive, component=None):
- "See `IArchivePermissionSet`."""
+ """See `IArchivePermissionSet`."""
clauses = ["""
ArchivePermission.archive = %s AND
ArchivePermission.permission = %s
@@ -278,33 +290,42 @@
prejoins=["sourcepackagename"])
def uploadersForPackage(self, archive, sourcepackagename):
- "See `IArchivePermissionSet`."""
+ """See `IArchivePermissionSet`."""
sourcepackagename = self._nameToSourcePackageName(sourcepackagename)
results = ArchivePermission.selectBy(
archive=archive, permission=ArchivePermissionType.UPLOAD,
sourcepackagename=sourcepackagename)
return results.prejoin(["sourcepackagename"])
+ def _pocketsFor(self, archives, person, permission_type):
+ """Helper function to get ArchivePermission objects."""
+ if IArchive.providedBy(archives):
+ archive_ids = [archives.id]
+ else:
+ archive_ids = [archive.id for archive in archives]
+
+ return ArchivePermission.select("""
+ ArchivePermission.archive IN %s AND
+ ArchivePermission.permission = %s AND
+ ArchivePermission.pocket IS NOT NULL AND
+ EXISTS (SELECT TeamParticipation.person
+ FROM TeamParticipation
+ WHERE TeamParticipation.person = %s AND
+ TeamParticipation.team = ArchivePermission.person)
+ """ % sqlvalues(archive_ids, permission_type, person))
+
def pocketsForUploader(self, archive, person):
"""See `IArchivePermissionSet`."""
- return ArchivePermission.select("""
- ArchivePermission.archive = %s AND
- ArchivePermission.permission = %s AND
- ArchivePermission.pocket IS NOT NULL AND
- EXISTS (SELECT TeamParticipation.person
- FROM TeamParticipation
- WHERE TeamParticipation.person = %s AND
- TeamParticipation.team = ArchivePermission.person)
- """ % sqlvalues(archive, ArchivePermissionType.UPLOAD, person))
+ return self._pocketsFor(archive, person, ArchivePermissionType.UPLOAD)
def uploadersForPocket(self, archive, pocket):
- "See `IArchivePermissionSet`."""
+ """See `IArchivePermissionSet`."""
return ArchivePermission.selectBy(
archive=archive, permission=ArchivePermissionType.UPLOAD,
pocket=pocket)
def queueAdminsForComponent(self, archive, component):
- "See `IArchivePermissionSet`."""
+ """See `IArchivePermissionSet`."""
component = self._nameToComponent(component)
results = ArchivePermission.selectBy(
archive=archive, permission=ArchivePermissionType.QUEUE_ADMIN,
@@ -316,6 +337,20 @@
return self._componentsFor(
archive, person, ArchivePermissionType.QUEUE_ADMIN)
+ def queueAdminsForPocket(self, archive, pocket, distroseries=None):
+ """See `IArchivePermissionSet`."""
+ kwargs = {}
+ if distroseries is not None:
+ kwargs["distroseries"] = distroseries
+ return ArchivePermission.selectBy(
+ archive=archive, permission=ArchivePermissionType.QUEUE_ADMIN,
+ pocket=pocket, **kwargs)
+
+ def pocketsForQueueAdmin(self, archive, person):
+ """See `IArchivePermissionSet`."""
+ return self._pocketsFor(
+ archive, person, ArchivePermissionType.QUEUE_ADMIN)
+
def newPackageUploader(self, archive, person, sourcepackagename):
"""See `IArchivePermissionSet`."""
sourcepackagename = self._nameToSourcePackageName(sourcepackagename)
@@ -364,6 +399,19 @@
archive=archive, person=person, component=component,
permission=ArchivePermissionType.QUEUE_ADMIN)
+ def newPocketQueueAdmin(self, archive, person, pocket, distroseries=None):
+ """See `IArchivePermissionSet`."""
+ existing = self.checkAuthenticated(
+ person, archive, ArchivePermissionType.QUEUE_ADMIN, pocket,
+ distroseries=distroseries)
+ try:
+ return existing[0]
+ except IndexError:
+ return ArchivePermission(
+ archive=archive, person=person, pocket=pocket,
+ distroseries=distroseries,
+ permission=ArchivePermissionType.QUEUE_ADMIN)
+
@staticmethod
def _remove_permission(permission):
if permission is None:
@@ -404,6 +452,17 @@
permission=ArchivePermissionType.QUEUE_ADMIN)
self._remove_permission(permission)
+ def deletePocketQueueAdmin(self, archive, person, pocket,
+ distroseries=None):
+ """See `IArchivePermissionSet`."""
+ kwargs = {}
+ if distroseries is not None:
+ kwargs["distroseries"] = distroseries
+ permission = ArchivePermission.selectOneBy(
+ archive=archive, person=person, pocket=pocket,
+ permission=ArchivePermissionType.QUEUE_ADMIN, **kwargs)
+ self._remove_permission(permission)
+
def _nameToPackageset(self, packageset):
"""Helper to convert a possible string name to IPackageset."""
if isinstance(packageset, basestring):
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2012-07-30 20:14:38 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2012-08-01 12:05:24 +0000
@@ -226,7 +226,8 @@
strict_component=strict_component, pocket=pocket)
if reason is not None:
# Queue admins are allowed to copy even if they can't upload.
- if not archive.canAdministerQueue(person, destination_component):
+ if not archive.canAdministerQueue(
+ person, destination_component, pocket, dest_series):
raise CannotCopy(reason)
=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive.txt 2012-06-14 03:22:43 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive.txt 2012-08-01 12:05:24 +0000
@@ -253,10 +253,11 @@
... print entry['component_name']
... print entry['source_package_name']
... print entry['pocket']
+ ... print entry['distroseries_link']
>>> show_permission_entries(permissions)
- Archive Upload Rights ...~ubuntu-team main None None
- Archive Upload Rights ...~ubuntu-team universe None None
+ Archive Upload Rights ...~ubuntu-team main None None None
+ Archive Upload Rights ...~ubuntu-team universe None None None
`getUploadersForPackage` returns all the permissions where someone can
upload a particular package.
@@ -268,7 +269,7 @@
... show_permission_entries(permissions)
>>> show_mozilla_permissions()
- Archive Upload Rights ...~carlos None mozilla-firefox None
+ Archive Upload Rights ...~carlos None mozilla-firefox None None
Passing a bad package name results in an error:
@@ -294,7 +295,7 @@
Let's also make a new Person to own the Ubuntu distro.
- >>> ubuntu_owner = factory.makePerson()
+ >>> ubuntu_owner = factory.makePerson(name='ubuntu-owner')
>>> ubuntu_db.owner = ubuntu_owner
>>> logout()
@@ -370,8 +371,8 @@
http://.../ubuntu/+archive/primary/+upload/name12?type=packagename&item=mozilla-firefox
>>> show_mozilla_permissions()
- Archive Upload Rights ...~carlos None mozilla-firefox None
- Archive Upload Rights ...~name12 None mozilla-firefox None
+ Archive Upload Rights ...~carlos None mozilla-firefox None None
+ Archive Upload Rights ...~name12 None mozilla-firefox None None
deletePackageUploader() removes that permission:
@@ -385,7 +386,7 @@
And we can see that it's gone:
>>> show_mozilla_permissions()
- Archive Upload Rights ...~carlos None mozilla-firefox None
+ Archive Upload Rights ...~carlos None mozilla-firefox None None
getUploadersForComponent returns all the permissions where someone can
upload to a particular component:
@@ -397,7 +398,7 @@
... show_permission_entries(permissions)
>>> show_component_permissions("main")
- Archive Upload Rights ...~ubuntu-team main None None
+ Archive Upload Rights ...~ubuntu-team main None None None
Passing a bad component name results in an error:
@@ -411,8 +412,8 @@
all components.
>>> show_component_permissions()
- Archive Upload Rights ...~ubuntu-team main None None
- Archive Upload Rights ...~ubuntu-team universe None None
+ Archive Upload Rights ...~ubuntu-team main None None None
+ Archive Upload Rights ...~ubuntu-team universe None None None
newComponentUploader adds a new permission for a person to upload to a
component.
@@ -431,10 +432,10 @@
http://.../ubuntu/+archive/primary/+upload/name12?type=component&item=restricted
>>> show_component_permissions()
- Archive Upload Rights ...~name12 restricted None None
- Archive Upload Rights ...~ubuntu-team main None None
- Archive Upload Rights ...~ubuntu-team restricted None None
- Archive Upload Rights ...~ubuntu-team universe None None
+ Archive Upload Rights ...~name12 restricted None None None
+ Archive Upload Rights ...~ubuntu-team main None None None
+ Archive Upload Rights ...~ubuntu-team restricted None None None
+ Archive Upload Rights ...~ubuntu-team universe None None None
We can use ``checkUpload`` to verify that a person can upload a
sourcepackage.
@@ -461,9 +462,9 @@
And we can see that it's gone:
>>> show_component_permissions()
- Archive Upload Rights ...~ubuntu-team main None None
- Archive Upload Rights ...~ubuntu-team restricted None None
- Archive Upload Rights ...~ubuntu-team universe None None
+ Archive Upload Rights ...~ubuntu-team main None None None
+ Archive Upload Rights ...~ubuntu-team restricted None None None
+ Archive Upload Rights ...~ubuntu-team universe None None None
And ``checkUpload`` now also no longer passes:
@@ -527,8 +528,8 @@
... show_permission_entries(permissions)
>>> show_admins_for_component("main")
- Queue Administration Rights ...~name12 main None None
- Queue Administration Rights ...~ubuntu-team main None None
+ Queue Administration Rights ...~name12 main None None None
+ Queue Administration Rights ...~ubuntu-team main None None None
getComponentsForQueueAdmin returns all the permissions relating to components
where the user is able to administer distroseries queues.
@@ -540,10 +541,10 @@
... show_permission_entries(permissions)
>>> show_components_for_admin(name12)
- Queue Administration Rights ...~name12 main None None
- Queue Administration Rights ...~name12 multiverse None None
- Queue Administration Rights ...~name12 restricted None None
- Queue Administration Rights ...~name12 universe None None
+ Queue Administration Rights ...~name12 main None None None
+ Queue Administration Rights ...~name12 multiverse None None None
+ Queue Administration Rights ...~name12 restricted None None None
+ Queue Administration Rights ...~name12 universe None None None
newQueueAdmin adds a new permission for a person to administer distroseries
queues in a particular component.
@@ -562,11 +563,11 @@
http://.../ubuntu/+archive/primary/+queue-admin/name12?type=component&item=partner
>>> show_components_for_admin(name12)
- Queue Administration Rights ...~name12 main None None
- Queue Administration Rights ...~name12 multiverse None None
- Queue Administration Rights ...~name12 partner None None
- Queue Administration Rights ...~name12 restricted None None
- Queue Administration Rights ...~name12 universe None None
+ Queue Administration Rights ...~name12 main None None None
+ Queue Administration Rights ...~name12 multiverse None None None
+ Queue Administration Rights ...~name12 partner None None None
+ Queue Administration Rights ...~name12 restricted None None None
+ Queue Administration Rights ...~name12 universe None None None
deleteQueueAdmin removes that permission.
@@ -580,10 +581,10 @@
And we can see that it's gone:
>>> show_components_for_admin(name12)
- Queue Administration Rights ...~name12 main None None
- Queue Administration Rights ...~name12 multiverse None None
- Queue Administration Rights ...~name12 restricted None None
- Queue Administration Rights ...~name12 universe None None
+ Queue Administration Rights ...~name12 main None None None
+ Queue Administration Rights ...~name12 multiverse None None None
+ Queue Administration Rights ...~name12 restricted None None None
+ Queue Administration Rights ...~name12 universe None None None
getUploadersForPocket returns all the permissions where someone can upload
to a particular pocket:
@@ -624,7 +625,7 @@
http://.../ubuntu/+archive/primary/+upload/name12?type=pocket&item=PROPOSED
>>> show_pocket_permissions('Proposed')
- Archive Upload Rights ...~name12 None None Proposed
+ Archive Upload Rights ...~name12 None None Proposed None
The person named in the permission can upload a package to this pocket.
@@ -661,6 +662,91 @@
The signer of this package has no upload rights to
this distribution's primary archive. Did you mean to upload to a PPA?
+getQueueAdminsForPocket returns all the permissions where someone can
+administer distroseries queues in a particular pocket.
+
+ >>> def show_admins_for_pocket(pocket, distroseries=None):
+ ... kwargs = {}
+ ... if distroseries is not None:
+ ... kwargs['distroseries'] = distroseries
+ ... permissions = webservice.named_get(
+ ... ubuntu_devel['main_archive_link'], 'getQueueAdminsForPocket',
+ ... api_version='devel', pocket=pocket, **kwargs).jsonBody()
+ ... show_permission_entries(permissions)
+
+ >>> show_admins_for_pocket('Security')
+ >>> show_admins_for_pocket('Security', distroseries=grumpy['self_link'])
+
+getPocketsForQueueAdmin returns all the permissions relating to pockets
+where the user is able to administer distroseries queues.
+
+ >>> def show_pockets_for_admin(person):
+ ... permissions = webservice.named_get(
+ ... ubuntu_devel['main_archive_link'], 'getPocketsForQueueAdmin',
+ ... api_version='devel', person=person['self_link']).jsonBody()
+ ... show_permission_entries(permissions)
+
+ >>> show_pockets_for_admin(name12)
+
+newPocketQueueAdmin adds a new permission for a person to administer
+distroseries queues in a particular pocket.
+
+ >>> response = ubuntu_owner_webservice.named_post(
+ ... ubuntu_devel['main_archive_link'], 'newPocketQueueAdmin', {},
+ ... api_version='devel', person=name12['self_link'],
+ ... pocket='Security')
+ >>> print response
+ HTTP/1.1 201 Created
+ ...
+
+ >>> new_permission = ubuntu_owner_webservice.get(
+ ... response.getHeader('Location')).jsonBody()
+ >>> print new_permission['self_link']
+ http://.../ubuntu/+archive/primary/+queue-admin/name12?type=pocket&item=SECURITY
+
+ >>> show_pockets_for_admin(name12)
+ Queue Administration Rights ...~name12 None None Security None
+
+It can also grant series-specific pocket queue admin permissions.
+
+ >>> ubuntu_owner_ws = ubuntu_owner_webservice.get(
+ ... "/~ubuntu-owner").jsonBody()
+ >>> response = ubuntu_owner_webservice.named_post(
+ ... ubuntu_devel['main_archive_link'], 'newPocketQueueAdmin', {},
+ ... api_version='devel', person=ubuntu_owner_ws['self_link'],
+ ... pocket='Security', distroseries=grumpy['self_link'])
+ >>> print response
+ HTTP/1.1 201 Created
+ ...
+
+ >>> new_permission = ubuntu_owner_webservice.get(
+ ... response.getHeader('Location')).jsonBody()
+ >>> print new_permission['self_link']
+ http://.../ubuntu/+archive/primary/+queue-admin/ubuntu-owner?type=pocket&item=SECURITY&series=grumpy
+
+ >>> show_pockets_for_admin(ubuntu_owner_ws)
+ Queue Administration Rights ...~ubuntu-owner None None Security .../grumpy
+
+deletePocketQueueAdmin removes these permissions.
+
+ >>> print ubuntu_owner_webservice.named_post(
+ ... ubuntu_devel['main_archive_link'], 'deletePocketQueueAdmin', {},
+ ... api_version='devel', person=name12['self_link'],
+ ... pocket='Security')
+ HTTP/1.1 200 Ok
+ ...
+ >>> print ubuntu_owner_webservice.named_post(
+ ... ubuntu_devel['main_archive_link'], 'deletePocketQueueAdmin', {},
+ ... api_version='devel', person=ubuntu_owner_ws['self_link'],
+ ... pocket='Security', distroseries=grumpy['self_link'])
+ HTTP/1.1 200 Ok
+ ...
+
+And we can see that they're gone:
+
+ >>> show_pockets_for_admin(name12)
+ >>> show_pockets_for_admin(ubuntu_owner_ws)
+
Malformed archive permission URLs
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Follow ups
