launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/ppa-admins-private into lp:launchpad.

Commit message:
Allow PPA admins to set Archive.private again.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1531168 in Launchpad itself: "PPA admins can no longer set Archive.private"
  https://bugs.launchpad.net/launchpad/+bug/1531168

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ppa-admins-private/+merge/281647

Allow PPA admins to set Archive.private again.

This is conceptually a bit odd, but it matches behaviour from before https://code.launchpad.net/~cjwatson/launchpad/ppa-admins/+merge/278720, and ~lexbuilder makes use of this; it probably makes more sense to continue to let launchpad-ppa-self-admins set Archive.private for their own archives (and to newly allow launchpad-ppa-admins to do so for all archives) than to add lexbuilder to commercial-admins.

Note that this doesn't involve giving launchpad-ppa-admins access to any more private resources than before; it just lets them set Archive.private and Archive.buildd_secret.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ppa-admins-private into lp:launchpad.

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2015-11-26 15:46:38 +0000
+++ lib/lp/soyuz/configure.zcml	2016-01-05 15:22:29 +0000
@@ -370,19 +370,17 @@
         <!--
            NOTE: The 'private' permission controls who can turn a public
            archive into a private one, and vice versa. The logic that
-           says this requires launchpad.Commercial permissions is duplicated
+           says this requires launchpad.Admin permissions is duplicated
            in validate_ppa.
           -->
         <require
-            permission="launchpad.Commercial"
-            set_attributes="buildd_secret private"/>
-        <require
             permission="launchpad.Admin"
             interface="lp.soyuz.interfaces.archive.IArchiveAdmin"
-            set_attributes="authorized_size enabled_restricted_processors
+            set_attributes="authorized_size buildd_secret
+                            enabled_restricted_processors
                             external_dependencies name
                             permit_obsolete_series_uploads
-                            require_virtualized"/>
+                            private require_virtualized"/>
         <require
             permission="launchpad.Moderate"
             set_schema="lp.soyuz.interfaces.archive.IArchiveRestricted"/>

=== modified file 'lib/lp/soyuz/stories/webservice/xx-archive.txt'
--- lib/lp/soyuz/stories/webservice/xx-archive.txt	2015-11-26 15:46:38 +0000
+++ lib/lp/soyuz/stories/webservice/xx-archive.txt	2016-01-05 15:22:29 +0000
@@ -1330,14 +1330,19 @@
 ~~~~~~~~~~~~~~~~~
 
 Modifying the privacy flag through the API is not allowed except for
-admins and commercial admins.  Mere PPA admins can't do it.
+admins, commercial admins, and PPA admins.
 
-    >>> mark_archive = webservice.get("/~mark/+archive/ubuntu/ppa").jsonBody()
-    >>> mark_archive['private'] = True
-    >>> print modify_archive(user_webservice, mark_archive)
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> pubpriv_archive_db = factory.makeArchive(
+    ...     owner=cprov, distribution=ubuntu_db, name="pubpriv")
+    >>> logout()
+    >>> pubpriv_archive = webservice.get(
+    ...     "/~cprov/+archive/ubuntu/pubpriv").jsonBody()
+    >>> pubpriv_archive['private'] = True
+    >>> print modify_archive(user_webservice, pubpriv_archive)
     HTTP/1.1 401 Unauthorized
     ...
-    (<Archive at ...>, 'private', 'launchpad.Commercial')
+    (<Archive at ...>, 'private', 'launchpad.Admin')
 
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> ppa_admin = factory.makePerson(member_of=[
@@ -1345,10 +1350,12 @@
     >>> logout()
     >>> ppa_admin_webservice = webservice_for_person(
     ...     ppa_admin, permission=OAuthPermission.WRITE_PRIVATE)
-    >>> print modify_archive(ppa_admin_webservice, mark_archive)
-    HTTP/1.1 401 Unauthorized
+    >>> print modify_archive(ppa_admin_webservice, pubpriv_archive)
+    HTTP/1.1 209 Content Returned
     ...
-    (<Archive at ...>, 'private', 'launchpad.Commercial')
+    >>> webservice.get(
+    ...     "/~cprov/+archive/ubuntu/pubpriv").jsonBody()['private']
+    True
 
 
 Copying private file to public archives

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2015-11-26 15:46:38 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2016-01-05 15:22:29 +0000
@@ -1211,7 +1211,7 @@
         login(ANONYMOUS)
         e = self.assertRaises(
             Unauthorized, setattr, self.archive, "buildd_secret", "boing")
-        self.assertEqual("launchpad.Commercial", e.args[2])
+        self.assertEqual("launchpad.Admin", e.args[2])
 
     def test_commercial_admin_can_set_buildd_secret(self):
         with celebrity_logged_in("commercial_admin"):