launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/forbid-private-maintainers into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #611617 in Launchpad itself: "Email does not explain that private teams cannot be package maintainers"
  https://bugs.launchpad.net/launchpad/+bug/611617

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/forbid-private-maintainers/+merge/87567

Forbid private teams to be private teams.

In effect, this is already implemented, the Storm validators on SourcePackageRelease only allow public teams or persons to be the creator or maintainer of an SPR, so this is just giving it a better rejection message.
-- 
https://code.launchpad.net/~stevenk/launchpad/forbid-private-maintainers/+merge/87567
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/forbid-private-maintainers into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py	2012-01-03 05:05:39 +0000
+++ lib/lp/archiveuploader/dscfile.py	2012-01-05 04:11:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """ DSCFile and related.
@@ -199,6 +199,14 @@
             raise UploadError(str(error))
 
         person = getUtility(IPersonSet).getByEmail(email)
+        if person and person.private:
+            # Private teams can not be maintainers.
+            error = "Invalid Maintainer."
+            if self.changes.changed_by and self.changes.changed_by['person']:
+                if self.changes.changed_by['person'].inTeam(person):
+                    error = "Maintainer can not be set to a private team."
+            raise UploadError(error)
+
         if person is None and self.policy.create_people:
             package = self._dict['Source']
             version = self._dict['Version']

=== added file 'lib/lp/archiveuploader/tests/test_private_maintainers.py'
--- lib/lp/archiveuploader/tests/test_private_maintainers.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archiveuploader/tests/test_private_maintainers.py	2012-01-05 04:11:17 +0000
@@ -0,0 +1,48 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from zope.security.proxy import removeSecurityProxy
+
+from lp.archiveuploader.dscfile import SignableTagFile
+from lp.archiveuploader.nascentuploadfile import UploadError
+from lp.registry.interfaces.person import PersonVisibility
+from lp.testing import (
+    celebrity_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestPrivateMaintainers(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_private_team_non_member(self):
+        # Maintainers can not be private teams. If the uploader isn't set,
+        # or isn't a member, the rejection message is delibrately vague.
+        with celebrity_logged_in('admin'):
+            team = self.factory.makeTeam(
+                email="foo@xxxxxxx", visibility=PersonVisibility.PRIVATE)
+        sigfile = SignableTagFile()
+        sigfile.changes = SignableTagFile()
+        sigfile.changes.changed_by = {}
+        self.assertRaisesWithContent(
+            UploadError, 'Invalid Maintainer.', sigfile.parseAddress,
+            "foo@xxxxxxx")
+
+    def test_private_team_member(self):
+        # Maintainers can not be private teams. If the uploader is a member
+        # of the team, the rejection message can be clear.
+        uploader = self.factory.makePerson()
+        with celebrity_logged_in('admin'):
+            team = self.factory.makeTeam(
+                email="foo@xxxxxxx", visibility=PersonVisibility.PRIVATE,
+                members=[uploader])
+        sigfile = SignableTagFile()
+        sigfile.changes = SignableTagFile()
+        sigfile.changes.changed_by = {'person': uploader}
+        self.assertRaisesWithContent(
+            UploadError, 'Maintainer can not be set to a private team.',
+            sigfile.parseAddress, "foo@xxxxxxx")