launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03511
[Merge] lp:~rvb/launchpad/change-perm-sync2 into lp:launchpad
Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/change-perm-sync2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #773261 in Launchpad itself: "The permission check for syncing packages on the differences pages (+localpackagediff) should be done on a per-package basis and not by checking lp.Edit on the series."
https://bugs.launchpad.net/launchpad/+bug/773261
For more details, see:
https://code.launchpad.net/~rvb/launchpad/change-perm-sync2/+merge/60077
This branch fixes the permission check used to sync packages. It previously used lp.edit on the series and now checks upload permissions for every synced package.
This is a follow-up branch on the reverted branch https://code.launchpad.net/~rvb/launchpad/change-perm-sync. The added code allow people with lp.Append on the series' main_archive to sync packages.
= Details =
The tricky part is that this perm check cannot be performed by a security adapter because the check needs more information than what is available inside a security adapter.
- Added the permission check inside checkCopy (lib/lp/soyuz/scripts/packagecopier.py). The person requesting the sync had to be passed along the call stack. Also, having lp.Append on the main_archive is enough to sync packages. This is used by the security team in ubuntu to copy any package in the Security pocket.
- Removed the perm check against lp.edit on the series.
- Added new tests and refactored existing ones in lib/lp/registry/browser/tests/test_distroseries.py to avoid code duplication.
- Drive-by documentation fix: added documentation for parameter strict_component of checkUpload (lib/lp/soyuz/interfaces/archive.py).
- Drive-by style fix in lib/lp/registry/browser/tests/test_distroseries.py: renamed a few local variables (that where overriding an import), removed unused variables.
== Pre-implementation notes ==
Spoke with William Grant.
= Tests =
(modified tests)
./bin/test -cvv test_distroseries test_sync_error_nothing_selected
./bin/test -cvv test_distroseries test_sync_notification_on_success
./bin/test -cvv test_distroseries test_sync_error_invalid_selection
(added tests)
./bin/test -cvv test_distroseries test_canPerformSync_anon
./bin/test -cvv test_distroseries test_canPerformSync_non_anon
./bin/test -cvv test_distroseries test_sync_error_no_perm_dest_archive
./bin/test -cvv test_distroseries test_sync_success_perm_component
./bin/test -cvv test_distroseries test_sync_error_no_perm_component
./bin/test -cvv test_distroseries test_sync_success_not_yet_in_derived_series
./bin/test -cvv test_distroseries test_sync_append_main_archive
./bin/test -cvv test_archive test_hasAnyPermission
= QA =
On DF: https://dogfood.launchpad.net/ubuntu/natty/+localpackagediffs
As anon user: the sync button should not be displayed.
As non privileged user: syncing packages should return an error.
As a user with perms on destination component A:
- syncing a package already existing in component B should fail.
- syncing a package already existing in component A should work.
- syncing a package not existing in the destination series should work.
(https://dogfood.launchpad.net/ubuntu/natty/+missingpackages)
As a member of the security team: syncing package should work.
--
https://code.launchpad.net/~rvb/launchpad/change-perm-sync2/+merge/60077
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/change-perm-sync2 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-05-05 00:47:27 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-05-05 15:21:22 +0000
@@ -784,7 +784,8 @@
if self.do_copy(
'selected_differences', sources, self.context.main_archive,
self.context, destination_pocket, include_binaries=False,
- dest_url=series_url, dest_display_name=series_title):
+ dest_url=series_url, dest_display_name=series_title,
+ person=self.user):
# The copy worked so we can redirect back to the page to
# show the results.
self.next_url = self.request.URL
@@ -803,7 +804,11 @@
This method is used as a condition for the above sync action, as
well as directly in the template.
"""
- return (check_permission('launchpad.Edit', self.context) and
+ archive = self.context.main_archive
+ has_perm = (archive.hasAnyPermission(self.user) or
+ check_permission('launchpad.Append', archive))
+ return (self.user is not None and
+ has_perm and
self.cached_differences.batch.total() > 0)
@property
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-05-05 05:22:20 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-05-05 15:21:22 +0000
@@ -27,10 +27,10 @@
from zope.security.proxy import removeSecurityProxy
from canonical.config import config
-from canonical.database.constants import UTC_NOW
from canonical.database.sqlbase import flush_database_caches
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.testing.pages import find_tag_by_id
+from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.batching import BatchNavigator
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.testing.layers import (
@@ -48,8 +48,7 @@
DistroSeriesDifferenceStatus,
DistroSeriesDifferenceType,
)
-from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.interfaces.series import SeriesStatus
+from lp.registry.interfaces.person import IPersonSet
from lp.services.features import (
get_relevant_feature_controller,
getFeatureFlag,
@@ -61,15 +60,18 @@
getFeatureStore,
)
from lp.soyuz.enums import (
+ ArchivePermissionType,
PackagePublishingStatus,
SourcePackageFormat,
)
+from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.distributionjob import (
IInitialiseDistroSeriesJobSource,
)
from lp.soyuz.interfaces.sourcepackageformat import (
ISourcePackageFormatSelectionSet,
)
+from lp.soyuz.model.archivepermission import ArchivePermission
from lp.testing import (
celebrity_logged_in,
feature_flags,
@@ -113,7 +115,7 @@
# Helper function to create a valid DSD.
distroseries = self.factory.makeDistroSeries(
parent_series=self.factory.makeDistroSeries())
- ds_diff = self.factory.makeDistroSeriesDifference(
+ self.factory.makeDistroSeriesDifference(
derived_series=distroseries, difference_type=difference_type)
return create_initialized_view(distroseries, '+index')
@@ -176,11 +178,11 @@
derived_series,
'+index',
principal=self.simple_user)
- html = view()
+ html_content = view()
self.assertEqual(
None, getFeatureFlag('soyuz.derived-series-ui.enabled'))
- self.assertThat(html, Not(portlet_header))
+ self.assertThat(html_content, Not(portlet_header))
def test_differences_portlet_all_differences(self):
# The difference portlet shows the differences with the parent
@@ -212,9 +214,9 @@
# XXX rvb 2011-04-12 bug=758649: LaunchpadTestRequest overrides
# self.features to NullFeatureController.
view.request.features = get_relevant_feature_controller()
- html = view()
+ html_content = view()
- self.assertThat(html, portlet_display)
+ self.assertThat(html_content, portlet_display)
def test_differences_portlet_no_differences(self):
# The difference portlet displays 'No differences' if there is no
@@ -238,9 +240,9 @@
# XXX rvb 2011-04-12 bug=758649: LaunchpadTestRequest overrides
# self.features to NullFeatureController.
view.request.features = get_relevant_feature_controller()
- html = view()
+ html_content = view()
- self.assertThat(html, portlet_display)
+ self.assertThat(html_content, portlet_display)
def test_differences_portlet_initialising(self):
# The difference portlet displays 'The series is initialising.' if
@@ -248,7 +250,7 @@
set_derived_series_ui_feature_flag(self)
derived_series = self._setupDifferences('deri', 'sid', 0, 0, 0)
job_source = getUtility(IInitialiseDistroSeriesJobSource)
- job = job_source.create(derived_series.parent, derived_series)
+ job_source.create(derived_series.parent, derived_series)
portlet_display = soupmatchers.HTMLContains(
soupmatchers.Tag(
'Derived series', 'h2',
@@ -266,10 +268,10 @@
# XXX rvb 2011-04-12 bug=758649: LaunchpadTestRequest overrides
# self.features to NullFeatureController.
view.request.features = get_relevant_feature_controller()
- html = view()
+ html_content = view()
self.assertTrue(derived_series.is_initialising)
- self.assertThat(html, portlet_display)
+ self.assertThat(html_content, portlet_display)
class TestMilestoneBatchNavigatorAttribute(TestCaseWithFactory):
@@ -508,9 +510,13 @@
# fairly static. However, with some DistroSeriesDifferences the query
# count will be higher, but it should remain the same no matter how
# many differences there are.
- login_person(self.simple_user)
derived_series = self.factory.makeDistroSeries(
parent_series=self.factory.makeDistroSeries())
+ ArchivePermission(
+ archive=derived_series.main_archive, person=self.simple_user,
+ component=getUtility(IComponentSet)["main"],
+ permission=ArchivePermissionType.QUEUE_ADMIN)
+ login_person(self.simple_user)
def add_differences(num):
for index in xrange(num):
@@ -1005,12 +1011,14 @@
[resolved_diff], filtered_view.cached_differences.batch)
def _setUpDSD(self, src_name='src-name', versions=None,
- difference_type=None):
+ difference_type=None, distribution=None):
# Helper to create a derived series with fixed names and proper
# source package format selection along with a DSD.
parent_series = self.factory.makeDistroSeries(name='warty')
+ if distribution == None:
+ distribution = self.factory.makeDistribution('deribuntu')
derived_series = self.factory.makeDistroSeries(
- distribution=self.factory.makeDistribution(name='deribuntu'),
+ distribution=distribution,
name='derilucid', parent_series=parent_series)
self._set_source_selection(derived_series)
self.factory.makeDistroSeriesDifference(
@@ -1022,33 +1030,45 @@
set_derived_series_ui_feature_flag(self)
return derived_series, parent_series, sourcepackagename
- def test_canPerformSync_non_editor(self):
- # Non-editors do not see options to sync.
- derived_series = self.factory.makeDistroSeries(
- name='derilucid', parent_series=self.factory.makeDistroSeries(
- name='lucid'))
- self.factory.makeDistroSeriesDifference(
- derived_series=derived_series)
-
- set_derived_series_ui_feature_flag(self)
+ def test_canPerformSync_anon(self):
+ # Anonymous users cannot sync packages.
+ derived_series, _, _ = self._setUpDSD()
+ view = create_initialized_view(
+ derived_series, '+localpackagediffs')
+
+ self.assertFalse(view.canPerformSync())
+
+ def test_canPerformSync_non_anon_no_perm_dest_archive(self):
+ # Logged-in users with no permission on the destination archive
+ # are not presented with options to perform syncs.
+ derived_series, _, _ = self._setUpDSD()
with person_logged_in(self.factory.makePerson()):
view = create_initialized_view(
derived_series, '+localpackagediffs')
- self.assertFalse(view.canPerformSync())
-
- def test_canPerformSync_editor(self):
- # Editors are presented with options to perform syncs.
- derived_series = self.factory.makeDistroSeries(
- name='derilucid', parent_series=self.factory.makeDistroSeries(
- name='lucid'))
- self.factory.makeDistroSeriesDifference(
- derived_series=derived_series)
-
- set_derived_series_ui_feature_flag(self)
- with person_logged_in(derived_series.owner):
+ self.assertFalse(view.canPerformSync())
+
+ def _setUpPersonWithPerm(self, derived_series):
+ # Helper to create a person with an upload permission on the
+ # series' archive.
+ person = self.factory.makePerson()
+ ArchivePermission(
+ archive=derived_series.main_archive, person=person,
+ component=getUtility(IComponentSet)["main"],
+ permission=ArchivePermissionType.QUEUE_ADMIN)
+ return person
+
+ def test_canPerformSync_non_anon(self):
+ # Logged-in users with a permission on the destination archive
+ # are presented with options to perform syncs.
+ # Note that a more fine-grained perm check is done on each
+ # synced package.
+ derived_series, _, _ = self._setUpDSD()
+ person = self._setUpPersonWithPerm(derived_series)
+ with person_logged_in(person):
view = create_initialized_view(
derived_series, '+localpackagediffs')
+
self.assertTrue(view.canPerformSync())
def _syncAndGetView(self, derived_series, person, sync_differences,
@@ -1063,136 +1083,179 @@
})
return view
- def test_sync_notification_on_success(self):
- # Syncing one or more diffs results in a stub notification.
- versions = {
- 'base': '1.0',
- 'derived': '1.0derived1',
- 'parent': '1.0-1',
- }
- parent_series = self.factory.makeDistroSeries(name='warty')
- derived_distro = self.factory.makeDistribution(name='deribuntu')
- derived_series = self.factory.makeDistroSeries(
- distribution=derived_distro, name='derilucid',
- parent_series=parent_series)
- self._set_source_selection(derived_series)
- difference = self.factory.makeDistroSeriesDifference(
- source_package_name_str='my-src-name',
- derived_series=derived_series, versions=versions)
-
- # The inital state is that 1.0-1 is not in the derived series.
- pubs = derived_series.main_archive.getPublishedSources(
- name='my-src-name', version=versions['parent'],
- distroseries=derived_series).any()
- self.assertIs(None, pubs)
-
- # Now, sync the source from the parent using the form.
- set_derived_series_ui_feature_flag(self)
- with person_logged_in(derived_series.owner):
- view = create_initialized_view(
- derived_series, '+localpackagediffs',
- method='POST', form={
- 'field.selected_differences': [
- difference.source_package_name.name,
- ],
- 'field.actions.sync': 'Sync',
- })
-
- # The parent's version should now be in the derived series:
- pub = derived_series.main_archive.getPublishedSources(
- name='my-src-name', version=versions['parent'],
- distroseries=derived_series).one()
+ def test_sync_error_nothing_selected(self):
+ # An error is raised when a sync is requested without any selection.
+ derived_series, _, _ = self._setUpDSD()
+ person = self._setUpPersonWithPerm(derived_series)
+ view = self._syncAndGetView(derived_series, person, [])
+
+ self.assertEqual(1, len(view.errors))
+ self.assertEqual(
+ 'No differences selected.', view.errors[0])
+
+ def test_sync_error_invalid_selection(self):
+ # An error is raised when an invalid difference is selected.
+ derived_series, _, _ = self._setUpDSD('my-src-name')
+ person = self._setUpPersonWithPerm(derived_series)
+ view = self._syncAndGetView(
+ derived_series, person, ['some-other-name'])
+
+ self.assertEqual(2, len(view.errors))
+ self.assertEqual(
+ 'No differences selected.', view.errors[0])
+ self.assertEqual(
+ 'Invalid value', view.errors[1].error_name)
+
+ def test_sync_error_no_perm_dest_archive(self):
+ # A user without upload rights on the destination archive cannot
+ # sync packages.
+ derived_series, _, _ = self._setUpDSD('my-src-name')
+ person = self._setUpPersonWithPerm(derived_series)
+ view = self._syncAndGetView(
+ derived_series, person, ['my-src-name'])
+
+ self.assertEqual(1, len(view.errors))
+ self.assertTrue(
+ "The signer of this package has no upload rights to this "
+ "distribution's primary archive" in view.errors[0])
+
+ def makePersonWithComponentPermission(self, archive, component=None):
+ person = self.factory.makePerson()
+ if component is None:
+ component = self.factory.makeComponent()
+ removeSecurityProxy(archive).newComponentUploader(
+ person, component)
+ return person, component
+
+ def test_sync_success_perm_component(self):
+ # A user with upload rights on the destination component
+ # can sync packages.
+ derived_series, parent_series, sourcepackagename = self._setUpDSD(
+ 'my-src-name')
+ person, _ = self.makePersonWithComponentPermission(
+ derived_series.main_archive,
+ derived_series.getSourcePackage(
+ sourcepackagename).latest_published_component)
+ view = self._syncAndGetView(
+ derived_series, person, ['my-src-name'])
+
+ self.assertEqual(0, len(view.errors))
+
+ def test_sync_error_no_perm_component(self):
+ # A user without upload rights on the destination component
+ # will get an error when he syncs packages to this component.
+ derived_series, parent_series, sourcepackagename = self._setUpDSD(
+ 'my-src-name')
+ person, another_component = self.makePersonWithComponentPermission(
+ derived_series.main_archive)
+ view = self._syncAndGetView(
+ derived_series, person, ['my-src-name'])
+
+ self.assertEqual(1, len(view.errors))
+ self.assertTrue(
+ "Signer is not permitted to upload to the "
+ "component" in view.errors[0])
+
+ def assertPackageCopied(self, series, src_name, version, view):
+ # Helper to check that a package has been copied.
+ # The new version should now be in the destination series:
+ pub = series.main_archive.getPublishedSources(
+ name=src_name, version=version,
+ distroseries=series).one()
self.assertIsNot(None, pub)
- self.assertEqual(versions['parent'], pub.sourcepackagerelease.version)
+ self.assertEqual(version, pub.sourcepackagerelease.version)
# The view should show no errors, and the notification should
- # confirm the sync worked.
+ # confirm the sync worked:
self.assertEqual(0, len(view.errors))
notifications = view.request.response.notifications
self.assertEqual(1, len(notifications))
self.assertEqual(
'<p>Packages copied to '
- '<a href="http://launchpad.dev/deribuntu/derilucid";'
+ '<a href="http://launchpad.dev/%s/%s";'
'>Derilucid</a>:</p>\n<ul>\n<li>my-src-name 1.0-1 in '
- 'derilucid</li>\n</ul>',
+ 'derilucid</li>\n</ul>' % (series.parent.name, series.name),
notifications[0].message)
- # 302 is a redirect back to the same page.
+ # 302 is a redirect back to the same page:
self.assertEqual(302, view.request.response.getStatus())
- def test_sync_error_nothing_selected(self):
- # An error is raised when a sync is requested without any selection.
- derived_series = self.factory.makeDistroSeries(
- name='derilucid', parent_series=self.factory.makeDistroSeries(
- name='lucid'))
- self.factory.makeDistroSeriesDifference(
- source_package_name_str='my-src-name',
- derived_series=derived_series)
-
- set_derived_series_ui_feature_flag(self)
- with person_logged_in(derived_series.owner):
- view = create_initialized_view(
- derived_series, '+localpackagediffs',
- method='POST', form={
- 'field.selected_differences': [],
- 'field.actions.sync': 'Sync',
- })
-
- self.assertEqual(1, len(view.errors))
- self.assertEqual(
- 'No differences selected.', view.errors[0])
-
- def test_sync_error_invalid_selection(self):
- # An error is raised when an invalid difference is selected.
- derived_series = self.factory.makeDistroSeries(
- name='derilucid', parent_series=self.factory.makeDistroSeries(
- name='lucid'))
- self._set_source_selection(derived_series)
- self.factory.makeDistroSeriesDifference(
- source_package_name_str='my-src-name',
- derived_series=derived_series)
-
- set_derived_series_ui_feature_flag(self)
- with person_logged_in(derived_series.owner):
- view = create_initialized_view(
- derived_series, '+localpackagediffs',
- method='POST', form={
- 'field.selected_differences': ['some-other-name'],
- 'field.actions.sync': 'Sync',
- })
-
- self.assertEqual(2, len(view.errors))
- self.assertEqual(
- 'No differences selected.', view.errors[0])
- self.assertEqual(
- 'Invalid value', view.errors[1].error_name)
-
- def test_sync_in_released_series_in_updates(self):
- # If the destination series is released, the sync packages end
- # up in the updates pocket.
+ def test_sync_notification_on_success(self):
+ # A user with upload rights on the destination archive can
+ # sync packages. Notifications about the synced packages are
+ # displayed and the packages are copied inside the destination
+ # series.
versions = {
+ 'base': '1.0',
+ 'derived': '1.0derived1',
'parent': '1.0-1',
}
derived_series, parent_series, sourcepackagename = self._setUpDSD(
'my-src-name', versions=versions)
- # Update destination series status to current and update
- # daterelease.
- with celebrity_logged_in('admin'):
- derived_series.status = SeriesStatus.CURRENT
- derived_series.datereleased = UTC_NOW
-
- self._syncAndGetView(
- derived_series, derived_series.owner, ['my-src-name'])
-
- parent_pub = parent_series.main_archive.getPublishedSources(
- name='my-src-name', version=versions['parent'],
- distroseries=parent_series).one()
- pub = derived_series.main_archive.getPublishedSources(
- name='my-src-name', version=versions['parent'],
- distroseries=derived_series).one()
-
- self.assertEqual(self.factory.getAnyPocket(), parent_pub.pocket)
- self.assertEqual(PackagePublishingPocket.UPDATES, pub.pocket)
+ # Setup a user with upload rights.
+ person = self.factory.makePerson()
+ removeSecurityProxy(derived_series.main_archive).newPackageUploader(
+ person, sourcepackagename)
+
+ # The inital state is that 1.0-1 is not in the derived series.
+ pubs = derived_series.main_archive.getPublishedSources(
+ name='my-src-name', version=versions['parent'],
+ distroseries=derived_series).any()
+ self.assertIs(None, pubs)
+
+ # Now, sync the source from the parent using the form.
+ view = self._syncAndGetView(
+ derived_series, person, ['my-src-name'])
+
+ # The parent's version should now be in the derived series and
+ # the notifications displayed:
+ self.assertPackageCopied(
+ derived_series, 'my-src-name', versions['parent'], view)
+
+ def test_sync_success_not_yet_in_derived_series(self):
+ # If the package to sync does not exist yet in the derived series,
+ # upload right to any component inside the destination series will be
+ # enough to sync the package.
+ versions = {
+ 'parent': '1.0-1',
+ }
+ missing = DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES
+ derived_series, parent_series, sourcepackagename = self._setUpDSD(
+ 'my-src-name', difference_type=missing, versions=versions)
+ person, another_component = self.makePersonWithComponentPermission(
+ derived_series.main_archive)
+ view = self._syncAndGetView(
+ derived_series, person, ['my-src-name'],
+ view_name='+missingpackages')
+
+ self.assertPackageCopied(
+ derived_series, 'my-src-name', versions['parent'], view)
+
+ def test_sync_append_main_archive(self):
+ # A user with lp.Append on the main archive (e.g. members of
+ # ubuntu-security on an ubuntu series) can sync packages.
+ # XXX: rvb 2011-05-05 bug=777911: This check should be refactored
+ # and moved to lib/lp/soyuz/scripts/tests/test_copypackage.py.
+ versions = {
+ 'base': '1.0',
+ 'derived': '1.0derived1',
+ 'parent': '1.0-1',
+ }
+ ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+ derived_series, parent_series, sourcepackagename = self._setUpDSD(
+ 'my-src-name', distribution=ubuntu, versions=versions)
+ ubuntu_security = getUtility(IPersonSet).getByName('ubuntu-security')
+ with person_logged_in(ubuntu_security):
+ self.assertTrue(
+ check_permission(
+ 'launchpad.Append', derived_series.main_archive))
+ view = self._syncAndGetView(
+ derived_series, ubuntu_security, ['my-src-name'])
+
+ self.assertTrue(view.canPerformSync())
+
+ self.assertPackageCopied(
+ derived_series, 'my-src-name', versions['parent'], view)
class TestDistroSeriesNeedsPackagesView(TestCaseWithFactory):
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-05 00:47:27 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-05 15:21:22 +0000
@@ -23,7 +23,8 @@
<div class="top-portlet" metal:fill-slot="main"
tal:define="differences view/cached_differences;
series_name context/displayname;
- parent_name context/parent_series/displayname;">
+ parent_name context/parent_series/displayname;
+ can_perform_sync view/canPerformSync;">
<p><tal:replace replace="structure view/explanation/escapedtext" /></p>
<tal:distroseries_localdiff_search_form
@@ -64,7 +65,7 @@
tal:attributes="class src_name">
<td>
- <input tal:condition="view/canPerformSync"
+ <input tal:condition="can_perform_sync"
name="field.selected_differences" type="checkbox"
tal:attributes="
value src_name;
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/browser/archive.py 2011-05-05 15:21:22 +0000
@@ -1219,7 +1219,8 @@
def do_copy(self, sources_field_name, source_pubs, dest_archive,
dest_series, dest_pocket, include_binaries,
- dest_url=None, dest_display_name=None):
+ dest_url=None, dest_display_name=None, person=None,
+ check_permissions=True):
"""Copy packages and add appropriate feedback to the browser page.
:param sources_field_name: The name of the form field to set errors
@@ -1236,13 +1237,17 @@
:param dest_display_name: The text to use for the dest_url link.
Defaults to the target archive's display name and will be
automatically escaped for inclusion in the output.
+ :param person: The person requesting the copy.
+ :param: check_permissions: boolean indicating whether or not the
+ requester's permissions to copy should be checked.
:return: True if the copying worked, False otherwise.
"""
try:
copies = do_copy(
source_pubs, dest_archive, dest_series,
- dest_pocket, include_binaries)
+ dest_pocket, include_binaries, allow_delayed_copies=True,
+ person=person, check_permissions=check_permissions)
except CannotCopy, error:
messages = []
error_lines = str(error).splitlines()
@@ -1462,7 +1467,8 @@
# setting up on-page notifications.
if self.do_copy(
'selected_sources', selected_sources, destination_archive,
- destination_series, destination_pocket, include_binaries):
+ destination_series, destination_pocket, include_binaries,
+ person=self.user):
# The copy worked so we can redirect back to the page to
# show the result.
self.setNextURL()
=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/doc/archive.txt 2011-05-05 15:21:22 +0000
@@ -2275,7 +2275,8 @@
release pocket, but to do so we must have edit permissions on the archive.
>>> sources = ["package1", "package2"]
- >>> mark.archive.syncSources(sources, cprov.archive, "release")
+ >>> mark.archive.syncSources(
+ ... sources, cprov.archive, "release", person=None)
Traceback (most recent call last):
...
Unauthorized...
@@ -2284,7 +2285,8 @@
>>> login("mark@xxxxxxxxxxx")
- >>> mark.archive.syncSources(sources, cprov.archive, "release")
+ >>> mark.archive.syncSources(
+ ... sources, cprov.archive, "release", person=mark)
>>> mark_one = mark.archive.getPublishedSources(name="package1").one()
>>> print mark_one.sourcepackagerelease.version
@@ -2298,7 +2300,8 @@
Repeating this source copy gives an error:
- >>> mark.archive.syncSources(sources, cprov.archive, "release")
+ >>> mark.archive.syncSources(
+ ... sources, cprov.archive, "release", person=mark)
Traceback (most recent call last):
...
CannotCopy:
@@ -2310,7 +2313,8 @@
Repeating this copy with binaries also gives an error:
>>> mark.archive.syncSources(
- ... sources, cprov.archive, "release", include_binaries=True)
+ ... sources, cprov.archive, "release", include_binaries=True,
+ ... person=mark)
Traceback (most recent call last):
...
CannotCopy:
@@ -2320,18 +2324,21 @@
Specifying non-existent source names, pocket names or distroseries names
all result in a NotFound exception:
- >>> mark.archive.syncSources(["bogus"], cprov.archive, "release")
+ >>> mark.archive.syncSources(["bogus"], cprov.archive, "release",
+ ... person=mark)
Traceback (most recent call last):
...
NoSuchSourcePackageName: No such source package: 'bogus'.
- >>> mark.archive.syncSources(sources, cprov.archive, "badpocket")
+ >>> mark.archive.syncSources(sources, cprov.archive, "badpocket",
+ ... person=mark)
Traceback (most recent call last):
...
PocketNotFound: 'BADPOCKET'
>>> mark.archive.syncSources(
- ... sources, cprov.archive, "release", to_series="badseries")
+ ... sources, cprov.archive, "release", to_series="badseries",
+ ... person=mark)
Traceback (most recent call last):
...
DistroSeriesNotFound: badseries
@@ -2360,7 +2367,8 @@
As with syncSources() you need to have edit permission on the archive.
>>> login(ANONYMOUS)
- >>> mark.archive.syncSource("pack", "1.0", cprov.archive, "release")
+ >>> mark.archive.syncSource("pack", "1.0", cprov.archive, "release",
+ ... person=None)
Traceback (most recent call last):
...
Unauthorized...
@@ -2368,7 +2376,8 @@
Login as mark to continue.
>>> login("mark@xxxxxxxxxxx")
- >>> mark.archive.syncSource("pack", "1.0", cprov.archive, "release")
+ >>> mark.archive.syncSource("pack", "1.0", cprov.archive, "release",
+ ... person=mark)
>>> pack = mark.archive.getPublishedSources(
... name="pack", exact_match=True).one()
>>> print pack.sourcepackagerelease.version
@@ -2376,7 +2385,8 @@
Copy package3 1.0 explicitly:
- >>> mark.archive.syncSource("package3", "1.0", cprov.archive, "release")
+ >>> mark.archive.syncSource("package3", "1.0", cprov.archive,
+ ... "release", person=mark)
>>> mark_three = mark.archive.getPublishedSources(name="package3").one()
>>> print mark_three.sourcepackagerelease.version
1.0
@@ -2403,7 +2413,7 @@
>>> mark.archive.syncSource(
... "built-source", "1.0", cprov.archive, "release",
- ... include_binaries=True)
+ ... include_binaries=True, person=mark)
Now, Mark's PPA has 'built-source' source and it's 2 binaries.
@@ -2415,7 +2425,7 @@
or a CannotCopy exception is thrown.
>>> mark.archive.syncSource(
- ... "package3", "1.2", cprov.archive, "updates")
+ ... "package3", "1.2", cprov.archive, "updates", person=mark)
Traceback (most recent call last):
...
CannotCopy: Destination pocket must be 'release' for a PPA.
@@ -2446,7 +2456,7 @@
>>> mark.archive.syncSource(
... source_name='overridden', version='1.0',
- ... from_archive=source_archive, to_pocket='release')
+ ... from_archive=source_archive, to_pocket='release', person=mark)
>>> copy = mark.archive.getPublishedSources(name="overridden").one()
>>> print copy.section.name
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2011-05-05 15:21:22 +0000
@@ -616,6 +616,9 @@
:param component: The `Component` being uploaded to.
:param pocket: The `PackagePublishingPocket` of 'distroseries' being
uploaded to.
+ :param strict_component: True if access to the specific component for
+ the package is needed to upload to it. If False, then access to
+ any component will do.
:return: The reason for not being able to upload, None otherwise.
"""
@@ -632,7 +635,7 @@
:param distroseries: The upload's target distro series.
:param strict_component: True if access to the specific component for
the package is needed to upload to it. If False, then access to
- any package will do.
+ any component will do.
:return: CannotUploadToArchive if 'person' cannot upload to the
archive,
None otherwise.
@@ -1173,12 +1176,22 @@
@operation_returns_collection_of(Interface)
@export_read_operation()
def getComponentsForQueueAdmin(person):
- """Return `IArchivePermission` for the person's queue admin components
+ """Return `IArchivePermission` for the person's queue admin
+ components.
- :param person: An `IPerson`
+ :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.
+
+ :param person: The `IPerson` for whom the check is performed.
+ :return: A boolean indicating if the person has any permission on this
+ archive at all.
+ """
+
def getPackageDownloadCount(bpr, day, country):
"""Get the `IBinaryPackageDownloadCount` with the given key."""
@@ -1212,6 +1225,7 @@
class IArchiveAppend(Interface):
"""Archive interface for operations restricted by append privilege."""
+ @call_with(person=REQUEST_USER)
@operation_parameters(
source_names=List(
title=_("Source package names"),
@@ -1227,8 +1241,8 @@
@export_write_operation()
# Source_names is a string because exporting a SourcePackageName is
# rather nonsensical as it only has id and name columns.
- def syncSources(source_names, from_archive, to_pocket,
- to_series=None, include_binaries=False):
+ def syncSources(source_names, from_archive, to_pocket, to_series=None,
+ include_binaries=False, person=None):
"""Synchronise (copy) named sources into this archive from another.
It will copy the most recent PUBLISHED versions of the named
@@ -1246,6 +1260,7 @@
:param include_binaries: optional boolean, controls whether or not
the published binaries for each given source should also be
copied along with the source.
+ :param person: the `IPerson` who requests the sync.
:raises NoSuchSourcePackageName: if the source name is invalid
:raises PocketNotFound: if the pocket name is invalid
@@ -1253,6 +1268,7 @@
:raises CannotCopy: if there is a problem copying.
"""
+ @call_with(person=REQUEST_USER)
@operation_parameters(
source_name=TextLine(title=_("Source package name")),
version=TextLine(title=_("Version")),
@@ -1271,7 +1287,7 @@
# we should consider either changing this method or adding a new one
# that takes that object instead.
def syncSource(source_name, version, from_archive, to_pocket,
- to_series=None, include_binaries=False):
+ to_series=None, include_binaries=False, person=None):
"""Synchronise (copy) a single named source into this archive.
Copy a specific version of a named source to the destination
@@ -1285,6 +1301,7 @@
:param include_binaries: optional boolean, controls whether or not
the published binaries for each given source should also be
copied along with the source.
+ :param person: the `IPerson` who requests the sync.
:raises NoSuchSourcePackageName: if the source name is invalid
:raises PocketNotFound: if the pocket name is invalid
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/model/archive.py 2011-05-05 15:21:22 +0000
@@ -28,8 +28,8 @@
Desc,
Or,
Select,
+ SQL,
Sum,
- SQL,
)
from storm.locals import (
Count,
@@ -1001,6 +1001,19 @@
permission_set = getUtility(IArchivePermissionSet)
return permission_set.componentsForQueueAdmin(self, person)
+ def hasAnyPermission(self, person):
+ """See `IArchive`."""
+ # Avoiding circular imports.
+ from lp.soyuz.model.archivepermission import ArchivePermission
+
+ any_perm_on_archive = Store.of(self).find(
+ TeamParticipation,
+ ArchivePermission.archive == self.id,
+ TeamParticipation.person == person.id,
+ TeamParticipation.teamID == ArchivePermission.personID,
+ )
+ return not any_perm_on_archive.is_empty()
+
def getBuildCounters(self, include_needsbuild=True):
"""See `IArchiveSet`."""
@@ -1176,7 +1189,7 @@
strict_component)
def verifyUpload(self, person, sourcepackagename, component,
- distroseries, strict_component=True):
+ distroseries, strict_component=True):
"""See `IArchive`."""
if not self.enabled:
return ArchiveDisabled(self.displayname)
@@ -1435,15 +1448,17 @@
reason)
def syncSources(self, source_names, from_archive, to_pocket,
- to_series=None, include_binaries=False):
+ to_series=None, include_binaries=False, person=None):
"""See `IArchive`."""
# Find and validate the source package names in source_names.
sources = self._collectLatestPublishedSources(
from_archive, source_names)
- self._copySources(sources, to_pocket, to_series, include_binaries)
+ self._copySources(
+ sources, to_pocket, to_series, include_binaries,
+ person=person)
def syncSource(self, source_name, version, from_archive, to_pocket,
- to_series=None, include_binaries=False):
+ to_series=None, include_binaries=False, person=None):
"""See `IArchive`."""
# Check to see if the source package exists, and raise a useful error
# if it doesn't.
@@ -1452,7 +1467,9 @@
source = from_archive.getPublishedSources(
name=source_name, version=version, exact_match=True).first()
- self._copySources([source], to_pocket, to_series, include_binaries)
+ self._copySources(
+ [source], to_pocket, to_series, include_binaries,
+ person=person)
def _collectLatestPublishedSources(self, from_archive, source_names):
"""Private helper to collect the latest published sources for an
@@ -1478,7 +1495,7 @@
return sources
def _copySources(self, sources, to_pocket, to_series=None,
- include_binaries=False):
+ include_binaries=False, person=None):
"""Private helper function to copy sources to this archive.
It takes a list of SourcePackagePublishingHistory but the other args
@@ -1507,7 +1524,8 @@
series = None
# Perform the copy, may raise CannotCopy.
- do_copy(sources, self, series, pocket, include_binaries)
+ do_copy(
+ sources, self, series, pocket, include_binaries, person=person)
def getAuthToken(self, person):
"""See `IArchive`."""
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2011-05-05 15:21:22 +0000
@@ -127,4 +127,4 @@
do_copy(
sources=source_packages, archive=self.target_archive,
series=self.target_distroseries, pocket=self.target_pocket,
- include_binaries=self.include_binaries)
+ include_binaries=self.include_binaries, check_permissions=False)
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2011-05-05 15:21:22 +0000
@@ -24,14 +24,16 @@
from zope.component import getUtility
from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
+from canonical.launchpad.webapp.authorization import check_permission
from canonical.librarian.utils import copy_and_close
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildStatus
from lp.soyuz.adapters.packagelocation import build_package_location
-from lp.soyuz.enums import ArchivePurpose
-from lp.soyuz.interfaces.archive import (
- CannotCopy,
+from lp.soyuz.enums import (
+ ArchivePurpose,
+ SourcePackageFormat,
)
+from lp.soyuz.interfaces.archive import CannotCopy
from lp.soyuz.interfaces.binarypackagebuild import BuildSetStatus
from lp.soyuz.interfaces.publishing import (
active_publishing_status,
@@ -44,7 +46,6 @@
IPackageUploadCustom,
IPackageUploadSet,
)
-from lp.soyuz.enums import SourcePackageFormat
from lp.soyuz.scripts.ftpmasterbase import (
SoyuzScript,
SoyuzScriptError,
@@ -374,7 +375,8 @@
"%s already exists in destination archive with "
"different contents." % lf.libraryfile.filename)
- def checkCopy(self, source, series, pocket):
+ def checkCopy(self, source, series, pocket, person=None,
+ check_permissions=True):
"""Check if the source can be copied to the given location.
Check possible conflicting publications in the destination archive.
@@ -384,13 +386,49 @@
higher than any version of the same source present in the
destination suite (series + pocket).
+ If person is not None, check that this person has upload rights to
+ the destination (archive, component, pocket).
+
:param source: copy candidate, `ISourcePackagePublishingHistory`.
:param series: destination `IDistroSeries`.
:param pocket: destination `PackagePublishingPocket`.
+ :param person: requester `IPerson`.
+ :param check_permissions: boolean indicating whether or not the
+ requester's permissions to copy should be checked.
:raise CannotCopy when a copy is not allowed to be performed
containing the reason of the error.
"""
+ # If there is a requester, check that he has upload permission
+ # into the destination (archive, component, pocket). This check
+ # is done here rather than in the security adapter because it
+ # requires more info than is available in the security adapter.
+ if check_permissions:
+ if person is None:
+ raise CannotCopy(
+ 'Cannot check copy permissions (no requester).')
+ else:
+ sourcepackagerelease = source.sourcepackagerelease
+ sourcepackagename = sourcepackagerelease.sourcepackagename
+ destination_component = series.getSourcePackage(
+ sourcepackagename).latest_published_component
+ # If destination_component is not None, make sure the person
+ # has upload permission for this component. Otherwise, any
+ # upload permission on this archive will do.
+ strict_component = destination_component is not None
+ reason = self.archive.checkUpload(
+ person, series, sourcepackagename, destination_component,
+ pocket, strict_component=strict_component)
+ if reason is not None:
+ # launchpad.Append on the main archive is sufficient
+ # to copy arbitrary packages. This allows for
+ # ubuntu's security team to sync sources into the
+ # primary archive (bypassing the queue and
+ # annoncements).
+ if not check_permission(
+ 'launchpad.Append', series.main_archive):
+ raise CannotCopy(reason)
+
if series not in self.archive.distribution.series:
raise CannotCopy(
"No such distro series %s in distribution %s." %
@@ -459,7 +497,7 @@
def do_copy(sources, archive, series, pocket, include_binaries=False,
- allow_delayed_copies=True):
+ allow_delayed_copies=True, person=None, check_permissions=True):
"""Perform the complete copy of the given sources incrementally.
Verifies if each copy can be performed using `CopyChecker` and
@@ -470,17 +508,20 @@
Wrapper for `do_direct_copy`.
- :param: sources: a list of `ISourcePackagePublishingHistory`.
- :param: archive: the target `IArchive`.
- :param: series: the target `IDistroSeries`, if None is given the same
+ :param sources: a list of `ISourcePackagePublishingHistory`.
+ :param archive: the target `IArchive`.
+ :param series: the target `IDistroSeries`, if None is given the same
current source distroseries will be used as destination.
- :param: pocket: the target `PackagePublishingPocket`.
- :param: include_binaries: optional boolean, controls whether or
+ :param pocket: the target `PackagePublishingPocket`.
+ :param include_binaries: optional boolean, controls whether or
not the published binaries for each given source should be also
copied along with the source.
:param allow_delayed_copies: boolean indicating whether or not private
sources can be copied to public archives using delayed_copies.
Defaults to True, only set as False in the UnembargoPackage context.
+ :param person: the requester `IPerson`.
+ :param check_permissions: boolean indicating whether or not the
+ requester's permissions to copy should be checked.
:raise CannotCopy when one or more copies were not allowed. The error
will contain the reason why each copy was denied.
@@ -500,7 +541,8 @@
else:
destination_series = series
try:
- copy_checker.checkCopy(source, destination_series, pocket)
+ copy_checker.checkCopy(
+ source, destination_series, pocket, person, check_permissions)
except CannotCopy, reason:
errors.append("%s (%s)" % (source.displayname, reason))
continue
@@ -536,12 +578,12 @@
Also copy published binaries for each source if requested to. Again,
only copy binaries that were not yet copied before.
- :param: source: an `ISourcePackagePublishingHistory`.
- :param: archive: the target `IArchive`.
- :param: series: the target `IDistroSeries`, if None is given the same
+ :param source: an `ISourcePackagePublishingHistory`.
+ :param archive: the target `IArchive`.
+ :param series: the target `IDistroSeries`, if None is given the same
current source distroseries will be used as destination.
- :param: pocket: the target `PackagePublishingPocket`.
- :param: include_binaries: optional boolean, controls whether or
+ :param pocket: the target `PackagePublishingPocket`.
+ :param include_binaries: optional boolean, controls whether or
not the published binaries for each given source should be also
copied along with the source.
@@ -608,11 +650,11 @@
Also include published builds for each source if requested to.
- :param: source: an `ISourcePackagePublishingHistory`.
- :param: archive: the target `IArchive`.
- :param: series: the target `IDistroSeries`.
- :param: pocket: the target `PackagePublishingPocket`.
- :param: include_binaries: optional boolean, controls whether or
+ :param source: an `ISourcePackagePublishingHistory`.
+ :param archive: the target `IArchive`.
+ :param series: the target `IDistroSeries`.
+ :param pocket: the target `PackagePublishingPocket`.
+ :param include_binaries: optional boolean, controls whether or
not the published binaries for each given source should be also
copied along with the source.
@@ -778,7 +820,8 @@
copies = do_copy(
sources, self.destination.archive,
self.destination.distroseries, self.destination.pocket,
- self.options.include_binaries, self.allow_delayed_copies)
+ self.options.include_binaries, self.allow_delayed_copies,
+ check_permissions=False)
except CannotCopy, error:
self.logger.error(str(error))
return []
=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py 2011-05-05 15:21:22 +0000
@@ -450,7 +450,9 @@
copy_checker = CopyChecker(self.archive, include_binaries=False)
self.assertIs(
None,
- copy_checker.checkCopy(self.source, self.series, self.pocket))
+ copy_checker.checkCopy(
+ self.source, self.series, self.pocket,
+ check_permissions=False))
checked_copies = list(copy_checker.getCheckedCopies())
self.assertEquals(1, len(checked_copies))
[checked_copy] = checked_copies
@@ -476,7 +478,9 @@
copy_checker = CopyChecker(self.archive, include_binaries=True)
self.assertIs(
None,
- copy_checker.checkCopy(self.source, self.series, self.pocket))
+ copy_checker.checkCopy(
+ self.source, self.series, self.pocket,
+ check_permissions=False))
checked_copies = list(copy_checker.getCheckedCopies())
self.assertEquals(1, len(checked_copies))
[checked_copy] = checked_copies
@@ -485,7 +489,8 @@
BuildSetStatus.FULLYBUILT_PENDING)
self.assertEquals(delayed, checked_copy.delayed)
- def assertCannotCopySourceOnly(self, msg):
+ def assertCannotCopySourceOnly(self, msg, person=None,
+ check_permissions=False):
"""`CopyChecker.checkCopy()` for source-only copy raises CannotCopy.
No `CheckedCopy` is stored.
@@ -493,7 +498,8 @@
copy_checker = CopyChecker(self.archive, include_binaries=False)
self.assertRaisesWithContent(
CannotCopy, msg,
- copy_checker.checkCopy, self.source, self.series, self.pocket)
+ copy_checker.checkCopy, self.source, self.series, self.pocket,
+ person, check_permissions)
checked_copies = list(copy_checker.getCheckedCopies())
self.assertEquals(0, len(checked_copies))
@@ -505,7 +511,8 @@
copy_checker = CopyChecker(self.archive, include_binaries=True)
self.assertRaisesWithContent(
CannotCopy, msg,
- copy_checker.checkCopy, self.source, self.series, self.pocket)
+ copy_checker.checkCopy, self.source, self.series, self.pocket,
+ None, False)
checked_copies = list(copy_checker.getCheckedCopies())
self.assertEquals(0, len(checked_copies))
@@ -514,6 +521,14 @@
self.assertCannotCopyBinaries(
'source has no binaries to be copied')
+ def test_cannot_copy_check_perm_no_person(self):
+ # If check_permissions=True and person=None is passed to
+ # checkCopy, raise an error (cannot check permissions for a
+ # 'None' person).
+ self.assertCannotCopySourceOnly(
+ 'Cannot check copy permissions (no requester).',
+ person=None, check_permissions=True)
+
def test_cannot_copy_binaries_from_FTBFS(self):
[build] = self.source.createMissingBuilds()
build.status = BuildStatus.FAILEDTOBUILD
@@ -699,10 +714,14 @@
# At this point copy is allowed with or without binaries.
copy_checker = CopyChecker(archive, include_binaries=False)
self.assertIs(
- None, copy_checker.checkCopy(source, series, pocket))
+ None,
+ copy_checker.checkCopy(
+ source, series, pocket, check_permissions=False))
copy_checker = CopyChecker(archive, include_binaries=True)
self.assertIs(
- None, copy_checker.checkCopy(source, series, pocket))
+ None,
+ copy_checker.checkCopy(
+ source, series, pocket, check_permissions=False))
# Set the expiration date of one of the testing binary files.
utc = pytz.timezone('UTC')
@@ -713,14 +732,15 @@
# Now source-only copies are allowed.
copy_checker = CopyChecker(archive, include_binaries=False)
self.assertIs(
- None, copy_checker.checkCopy(source, series, pocket))
+ None, copy_checker.checkCopy(
+ source, series, pocket, check_permissions=False))
# Copies with binaries are denied.
copy_checker = CopyChecker(archive, include_binaries=True)
self.assertRaisesWithContent(
CannotCopy,
'source has expired binaries',
- copy_checker.checkCopy, source, series, pocket)
+ copy_checker.checkCopy, source, series, pocket, None, False)
def test_checkCopy_cannot_copy_expired_sources(self):
# checkCopy() raises CannotCopy if the copy requested includes
@@ -745,7 +765,7 @@
self.assertRaisesWithContent(
CannotCopy,
'source contains expired files',
- copy_checker.checkCopy, source, series, pocket)
+ copy_checker.checkCopy, source, series, pocket, None, False)
def test_checkCopy_allows_copies_from_other_distributions(self):
# It is possible to copy packages between distributions,
@@ -768,7 +788,8 @@
# Copy of sources to series in another distribution can be
# performed.
copy_checker = CopyChecker(archive, include_binaries=False)
- copy_checker.checkCopy(source, series, pocket)
+ copy_checker.checkCopy(
+ source, series, pocket, check_permissions=False)
def test_checkCopy_forbids_copies_to_unknown_distroseries(self):
# We currently deny copies to series that are not for the Archive
@@ -794,7 +815,7 @@
self.assertRaisesWithContent(
CannotCopy,
'No such distro series sid in distribution debian.',
- copy_checker.checkCopy, source, sid, pocket)
+ copy_checker.checkCopy, source, sid, pocket, None, False)
def test_checkCopy_respects_sourceformatselection(self):
# A source copy should be denied if the source's dsc_format is
@@ -820,7 +841,8 @@
self.assertRaisesWithContent(
CannotCopy,
"Source format '3.0 (quilt)' not supported by target series "
- "warty.", copy_checker.checkCopy, source, series, pocket)
+ "warty.", copy_checker.checkCopy, source, series, pocket, None,
+ False)
def test_checkCopy_identifies_conflicting_copy_candidates(self):
# checkCopy() is able to identify conflicting candidates within
@@ -850,7 +872,8 @@
self.assertIs(
None,
copy_checker.checkCopy(
- source, source.distroseries, source.pocket))
+ source, source.distroseries, source.pocket,
+ check_permissions=False))
# The second source-only copy, for hoary-test, fails, since it
# conflicts with the just-approved copy.
@@ -859,7 +882,8 @@
'same version already building in the destination archive '
'for Breezy Badger Autotest',
copy_checker.checkCopy,
- copied_source, copied_source.distroseries, copied_source.pocket)
+ copied_source, copied_source.distroseries, copied_source.pocket,
+ None, False)
def test_checkCopy_identifies_delayed_copies_conflicts(self):
# checkCopy() detects copy conflicts in the upload queue for
@@ -882,14 +906,16 @@
# Commit so the just-created files are accessible and perform
# the delayed-copy.
self.layer.txn.commit()
- do_copy([source], archive, series, pocket, include_binaries=False)
+ do_copy(
+ [source], archive, series, pocket, include_binaries=False,
+ check_permissions=False)
# Repeating the copy is denied.
copy_checker = CopyChecker(archive, include_binaries=False)
self.assertRaisesWithContent(
CannotCopy,
'same version already uploaded and waiting in ACCEPTED queue',
- copy_checker.checkCopy, source, series, pocket)
+ copy_checker.checkCopy, source, series, pocket, None, False)
def test_checkCopy_suppressing_delayed_copies(self):
# `CopyChecker` by default will request delayed-copies when it's
@@ -917,7 +943,8 @@
# this operation, since restricted files are being copied to
# public archives.
copy_checker = CopyChecker(archive, include_binaries=False)
- copy_checker.checkCopy(source, series, pocket)
+ copy_checker.checkCopy(
+ source, series, pocket, check_permissions=False)
[checked_copy] = list(copy_checker.getCheckedCopies())
self.assertTrue(checked_copy.delayed)
@@ -925,7 +952,8 @@
# scheduled.
copy_checker = CopyChecker(
archive, include_binaries=False, allow_delayed_copies=False)
- copy_checker.checkCopy(source, series, pocket)
+ copy_checker.checkCopy(
+ source, series, pocket, check_permissions=False)
[checked_copy] = list(copy_checker.getCheckedCopies())
self.assertFalse(checked_copy.delayed)
@@ -2683,7 +2711,7 @@
self.assertIs(
None,
checker.checkCopy(proposed_source, warty,
- PackagePublishingPocket.UPDATES))
+ PackagePublishingPocket.UPDATES, check_permissions=False))
def testCopySourceWithConflictingFilesInPPAs(self):
"""We can copy source if the source files match, both in name and
@@ -2728,7 +2756,7 @@
"test-source_1.0.orig.tar.gz already exists in destination "
"archive with different contents.",
checker.checkCopy, test2_source, warty,
- PackagePublishingPocket.RELEASE)
+ PackagePublishingPocket.RELEASE, None, False)
def testCopySourceWithSameFilenames(self):
"""We can copy source if the source files match, both in name and
@@ -2771,7 +2799,7 @@
self.assertIs(
None,
checker.checkCopy(test2_source, warty,
- PackagePublishingPocket.RELEASE))
+ PackagePublishingPocket.RELEASE, check_permissions=False))
def testCopySourceWithExpiredSourcesInDestination(self):
"""We can also copy sources if the destination archive has expired
@@ -2820,4 +2848,4 @@
self.assertIs(
None,
checker.checkCopy(test2_source, warty,
- PackagePublishingPocket.RELEASE))
+ PackagePublishingPocket.RELEASE, check_permissions=False))
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2011-05-05 15:21:22 +0000
@@ -268,7 +268,8 @@
... no_priv_private_ppa.getPublishedSources(name='test-pkg'),
... no_priv.archive, series=ubuntu['warty'],
... pocket=PackagePublishingPocket.RELEASE,
- ... include_binaries=True, allow_delayed_copies=False)
+ ... include_binaries=True, allow_delayed_copies=False,
+ ... person=no_priv)
>>> from zope.security.proxy import removeSecurityProxy
>>> removeSecurityProxy(dsc_file).restricted = False
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2011-05-05 00:47:27 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2011-05-05 15:21:22 +0000
@@ -37,6 +37,7 @@
from lp.services.propertycache import clear_property_cache
from lp.services.worlddata.interfaces.country import ICountrySet
from lp.soyuz.enums import (
+ ArchivePermissionType,
ArchivePurpose,
ArchiveStatus,
PackagePublishingStatus,
@@ -60,6 +61,7 @@
from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.processor import IProcessorFamilySet
from lp.soyuz.model.archive import Archive
+from lp.soyuz.model.archivepermission import ArchivePermission
from lp.soyuz.model.binarypackagerelease import (
BinaryPackageReleaseDownloadCount,
)
@@ -67,6 +69,7 @@
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
ANONYMOUS,
+ celebrity_logged_in,
login,
login_person,
person_logged_in,
@@ -824,6 +827,22 @@
False,
archive.canUploadSuiteSourcePackage(person, suitesourcepackage))
+ def test_hasAnyPermission(self):
+ # hasAnyPermission returns true if the person is the member of a
+ # team with any kind of permission on the archive.
+ archive = self.factory.makeArchive()
+ person = self.factory.makePerson()
+ team = self.factory.makeTeam()
+ main = getUtility(IComponentSet)["main"]
+ ArchivePermission(
+ archive=archive, person=team, component=main,
+ permission=ArchivePermissionType.UPLOAD)
+
+ self.assertFalse(archive.hasAnyPermission(person))
+ with celebrity_logged_in('admin'):
+ team.addMember(person, team.teamowner)
+ self.assertTrue(archive.hasAnyPermission(person))
+
class TestUpdatePackageDownloadCount(TestCaseWithFactory):
"""Ensure that updatePackageDownloadCount works as expected."""
