Merge lp:~rvb/launchpad/change-perm-sync2 into lp:launchpad

[Raphaël Badin]

[r=jcsackett][bug=773261] Fix 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 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.

[j.c.sackett (jcsackett)]

Looks fine, I have a few comments in the diff below.

> === 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_conte...

review: Approve

[Raphaël Badin (rvb)]

> derived_series, _, _ = self._setUpDSD() scans a little weirdly. You might try
> derived_series = self._setUpDSD()[0] or
> (derived_series, ignore1, ignore2) = self._setUpDSD()

Done.

> I think the period was actually the right punctuation here.

Done.

> It looks to me like syncSource isn't necessary; couldn't you just
> pass one source_name to syncSources? Obviously this isn't a problem
> in your branch, but it may be worth filing a bug to capture the issue?

I'll look into that. Thanks a lot for the review.

=== 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 20:54:43 +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 = (self.user is not None and (
+                        archive.hasAnyPermission(self.user) or
+                        check_permission('launchpad.Append', archive)))
+        return (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 18:23:26 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py	2011-05-05 20:54:43 +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,
@@ -176,11 +178,11 @@
                 derived_series,
                 '+index',
                 principal=self.simple_user)
-            html_text = view()
+            html_content = view()
 
         self.assertEqual(
             None, getFeatureFlag('soyuz.derived-series-ui.enabled'))
-        self.assertThat(html_text, 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_text = view()
+            html_content = view()
 
-        self.assertThat(html_text, 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_text = view()
+            html_content = view()
 
-        self.assertThat(html_text, portlet_display)
+        self.assertThat(html_content, portlet_display)
 
     def test_differences_portlet_initialising(self):
         # The difference portlet displays 'The series is initialising.' if
@@ -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_text = view()
+            html_content = view()
 
         self.assertTrue(derived_series.is_initialising)
-        self.assertThat(html_text, portlet_display)
+        self.assertThat(html_content, portlet_display)
 
 
 class TestMilestoneBatchNavigatorAttribute(TestCaseWithFactory):
@@ -404,7 +406,7 @@
 class DistroSeriesDifferenceMixin:
     """A helper class for testing differences pages"""
 
-    def _test_packagesets(self, html_text, packageset_text,
+    def _test_packagesets(self, html_content, packageset_text,
                           packageset_class, msg_text):
         parent_packagesets = soupmatchers.HTMLContains(
             soupmatchers.Tag(
@@ -412,7 +414,7 @@
                 attrs={'class': packageset_class},
                 text=packageset_text))
 
-        self.assertThat(html_text, parent_packagesets)
+        self.assertThat(html_content, parent_packagesets)
 
 
 class TestDistroSeriesLocalDifferences(
@@ -474,11 +476,11 @@
                 ds_diff.derived_series,
                 '+localpackagediffs',
                 principal=self.simple_user)
-            html_text = view()
+            html_content = view()
 
         packageset_text = re.compile('\s*' + ps.name)
         self._test_packagesets(
-            html_text, packageset_text, 'parent-packagesets',
+            html_content, packageset_text, 'parent-packagesets',
             'Parent packagesets')
 
     def test_parent_packagesets_localpackagediffs_sorts(self):
@@ -497,22 +499,25 @@
                 ds_diff.derived_series,
                 '+localpackagediffs',
                 principal=self.simple_user)
-            html_text = view()
+            html = view()
 
         packageset_text = re.compile(
             '\s*' + ', '.join(sorted(unsorted_names)))
         self._test_packagesets(
-            html_text, packageset_text, 'parent-packagesets',
-            'Parent packagesets')
+            html, packageset_text, 'parent-packagesets', 'Parent packagesets')
 
     def test_queries(self):
         # With no DistroSeriesDifferences the query count should be low and
         # 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):
@@ -1007,12 +1012,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(
@@ -1024,33 +1031,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()[0]
+        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()[0]
         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()[0]
+        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,
@@ -1065,47 +1084,87 @@
                     })
             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()[0]
+        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')[0]
+        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')[0]
+        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.
@@ -1114,87 +1173,90 @@
         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.
         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):
@@ -1291,12 +1353,11 @@
                 self.ds_diff.derived_series,
                 '+missingpackages',
                 principal=self.simple_user)
-            html_text = view()
+            html = view()
 
         packageset_text = re.compile('\s*' + ps.name)
         self._test_packagesets(
-            html_text, packageset_text, 'parent-packagesets',
-            'Parent packagesets')
+            html, packageset_text, 'parent-packagesets', 'Parent packagesets')
 
 
 class DistroSerieUniquePackageDiffsTestCase(TestCaseWithFactory):
@@ -1378,8 +1439,8 @@
                 self.ds_diff.derived_series,
                 '+uniquepackages',
                 principal=self.simple_user)
-            html_text = view()
+            html = view()
 
         packageset_text = re.compile('\s*' + ps.name)
         self._test_packagesets(
-            html_text, packageset_text, 'packagesets', 'Packagesets')
+            html, packageset_text, 'packagesets', 'Packagesets')

=== 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 20:54:43 +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 20:54:43 +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 20:54:43 +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("[e-mail]")
 
-    >>> 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("[e-mail]")
-    >>> 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 20:54:43 +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 20:54:43 +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 20:54:43 +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 20:54:43 +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 20:54:43 +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 20:54:43 +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 20:54:43 +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."""