launchpad-reviewers team mailing list archive

[Colin Watson <mp+418937@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad/+git/security:tighten-edit-dsp-permissions into launchpad:master.

Commit message:
Tighten edit permissions on DistributionSourcePackage

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/security/+merge/418937

People with upload permission to a component to which the package in question isn't published shouldn't gain edit permission on the package as a result.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad/+git/security:tighten-edit-dsp-permissions into launchpad:master.

diff --git a/lib/lp/registry/tests/test_distributionsourcepackage.py b/lib/lp/registry/tests/test_distributionsourcepackage.py
index c9972dc..95a11b0 100644
--- a/lib/lp/registry/tests/test_distributionsourcepackage.py
+++ b/lib/lp/registry/tests/test_distributionsourcepackage.py
@@ -20,9 +20,11 @@ from lp.registry.model.distributionsourcepackage import (
 from lp.registry.model.karma import KarmaTotalCache
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import flush_database_updates
+from lp.services.webapp.authorization import check_permission
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import (
+    person_logged_in,
     StormStatementRecorder,
     TestCaseWithFactory,
     )
@@ -181,6 +183,43 @@ class TestDistributionSourcePackage(TestCaseWithFactory):
             sourcepackagerelease=spph.sourcepackagerelease))
         self.assertIsNone(dsp.getVersion("0.07-4"))
 
+    def test_non_uploader_cannot_edit(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        with person_logged_in(self.factory.makePerson()):
+            self.assertFalse(check_permission("launchpad.Edit", dsp))
+
+    def test_distribution_owner_can_edit(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        with person_logged_in(dsp.distribution.owner):
+            self.assertTrue(check_permission("launchpad.Edit", dsp))
+
+    def test_correct_component_uploader_can_edit(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        self.factory.makeSourcePackagePublishingHistory(
+            archive=dsp.distribution.main_archive,
+            distroseries=dsp.distribution.currentseries,
+            sourcepackagename=dsp.sourcepackagename,
+            component="main", status=PackagePublishingStatus.PUBLISHED)
+        person = self.factory.makePerson()
+        with person_logged_in(dsp.distribution.main_archive.owner):
+            dsp.distribution.main_archive.newComponentUploader(person, "main")
+        with person_logged_in(person):
+            self.assertTrue(check_permission("launchpad.Edit", dsp))
+
+    def test_incorrect_component_uploader_cannot_edit(self):
+        dsp = self.factory.makeDistributionSourcePackage()
+        self.factory.makeSourcePackagePublishingHistory(
+            archive=dsp.distribution.main_archive,
+            distroseries=dsp.distribution.currentseries,
+            sourcepackagename=dsp.sourcepackagename,
+            component="main", status=PackagePublishingStatus.PUBLISHED)
+        person = self.factory.makePerson()
+        with person_logged_in(dsp.distribution.main_archive.owner):
+            dsp.distribution.main_archive.newComponentUploader(
+                person, "universe")
+        with person_logged_in(person):
+            self.assertFalse(check_permission("launchpad.Edit", dsp))
+
 
 class TestDistributionSourcePackageFindRelatedArchives(TestCaseWithFactory):
 
diff --git a/lib/lp/security.py b/lib/lp/security.py
index fbada1b..7e227e9 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -1335,9 +1335,12 @@ class EditDistributionSourcePackage(AuthorizationBase):
         # None if they are allowed.
         if distroseries is None:
             return False
+        sourcepackage = distroseries.getSourcePackage(
+            self.obj.sourcepackagename)
         reason = archive.verifyUpload(
             user.person, sourcepackagename=self.obj.sourcepackagename,
-            component=None, distroseries=distroseries, strict_component=False)
+            component=sourcepackage.latest_published_component,
+            distroseries=distroseries)
         return reason is None
 
     def checkAuthenticated(self, user):