launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/open-debug-symbols into lp:launchpad.

Commit message:
Make Archive.*_debug_symbols require edit permissions (archive owner), not commercial admin.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/open-debug-symbols/+merge/271862

Make Archive.*_debug_symbols require edit permissions (archive owner), not commercial admin.  These no longer have any potentially-dangerous effects that need to be restricted.  Fixing this is part of making it possible for untrusted-ci-dev-bot to do a reasonable subset of the work involved in CI Train silo creation for staging builds without needing special privileges.

The one downside of this is that commercial admins can no longer set these flags for PPAs they don't own, only full admins, but that doesn't seem terribly bad.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/open-debug-symbols into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2015-07-08 16:05:11 +0000
+++ lib/lp/soyuz/browser/archive.py	2015-09-21 19:32:54 +0000
@@ -2012,7 +2012,14 @@
 
 class ArchiveEditView(BaseArchiveEditView):
 
-    field_names = ['displayname', 'description', 'enabled', 'publish']
+    field_names = [
+        'displayname',
+        'description',
+        'enabled',
+        'publish',
+        'build_debug_symbols',
+        'publish_debug_symbols',
+        ]
     custom_widget(
         'description', TextAreaWidget, height=10, width=30)
     page_title = 'Change details'
@@ -2051,8 +2058,6 @@
         'private',
         'suppress_subscription_notifications',
         'require_virtualized',
-        'build_debug_symbols',
-        'publish_debug_symbols',
         'permit_obsolete_series_uploads',
         'authorized_size',
         'relative_build_score',

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2015-09-03 15:14:07 +0000
+++ lib/lp/soyuz/configure.zcml	2015-09-21 19:32:54 +0000
@@ -364,7 +364,8 @@
         <require
             permission="launchpad.Edit"
             interface="lp.soyuz.interfaces.archive.IArchiveEdit"
-            set_attributes="description displayname publish status
+            set_attributes="build_debug_symbols description displayname
+                            publish publish_debug_symbols status
                             suppress_subscription_notifications"/>
         <!--
            NOTE: The 'private' permission controls who can turn a public
@@ -375,12 +376,11 @@
         <require
             permission="launchpad.Admin"
             interface="lp.soyuz.interfaces.archive.IArchiveAdmin"
-            set_attributes="authorized_size build_debug_symbols
-                            buildd_secret enabled_restricted_processors
+            set_attributes="authorized_size buildd_secret
+                            enabled_restricted_processors
                             external_dependencies name
                             permit_obsolete_series_uploads
-                            private processors publish_debug_symbols
-                            require_virtualized"/>
+                            private processors require_virtualized"/>
         <require
             permission="launchpad.Moderate"
             set_schema="lp.soyuz.interfaces.archive.IArchiveRestricted"/>

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2015-08-28 16:43:28 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2015-09-21 19:32:54 +0000
@@ -1449,26 +1449,29 @@
         super(TestBuildDebugSymbols, self).setUp()
         self.archive = self.factory.makeArchive()
 
-    def setBuildDebugSymbols(self, archive, build_debug_symbols):
-        """Helper function."""
-        archive.build_debug_symbols = build_debug_symbols
-
     def test_build_debug_symbols_is_public(self):
         # Anyone can see the attribute.
         login(ANONYMOUS)
         self.assertFalse(self.archive.build_debug_symbols)
 
-    def test_owner_cannot_set_build_debug_symbols(self):
-        # The archive owner cannot set it.
+    def test_non_owner_cannot_set_build_debug_symbols(self):
+        # A non-owner cannot set it.
+        login_person(self.factory.makePerson())
+        self.assertRaises(
+            Unauthorized, setattr, self.archive, "build_debug_symbols", True)
+
+    def test_owner_can_set_build_debug_symbols(self):
+        # The archive owner can set it.
         login_person(self.archive.owner)
-        self.assertRaises(
-            Unauthorized, self.setBuildDebugSymbols, self.archive, True)
+        self.archive.build_debug_symbols = True
+        self.assertTrue(self.archive.build_debug_symbols)
 
-    def test_commercial_admin_can_set_build_debug_symbols(self):
-        # A commercial admin can set it.
+    def test_commercial_admin_cannot_set_build_debug_symbols(self):
+        # A commercial admin cannot set it.
         with celebrity_logged_in('commercial_admin'):
-            self.setBuildDebugSymbols(self.archive, True)
-            self.assertTrue(self.archive.build_debug_symbols)
+            self.assertRaises(
+                Unauthorized, setattr,
+                self.archive, "build_debug_symbols", True)
 
 
 class TestAddArchiveDependencies(TestCaseWithFactory):