launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+439390@xxxxxxxxxxxxxxxxxx>]

Simone Pelosi has proposed merging ~pelpsi/launchpad:enable-registry-admins-to-delete-snap-recipes into launchpad:master.

Commit message:
Enable Registry Admins to delete snap recipes 

Added registry_experts to allowed users and created new test suite to test permissions

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/439390
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:enable-registry-admins-to-delete-snap-recipes into launchpad:master.

diff --git a/lib/lp/snappy/security.py b/lib/lp/snappy/security.py
index 9a61381..1a980a1 100644
--- a/lib/lp/snappy/security.py
+++ b/lib/lp/snappy/security.py
@@ -38,7 +38,10 @@ class EditSnap(AuthorizationBase):
 
     def checkAuthenticated(self, user):
         return (
-            user.isOwner(self.obj) or user.in_commercial_admin or user.in_admin
+            user.isOwner(self.obj)
+            or user.in_commercial_admin
+            or user.in_admin
+            or user.in_registry_experts
         )
 
 
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 1ec33af..ea6e87a 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -119,6 +119,7 @@ from lp.testing import (
     api_url,
     login,
     login_admin,
+    login_celebrity,
     logout,
     person_logged_in,
     record_two_runs,
@@ -154,6 +155,86 @@ class TestSnapFeatureFlag(TestCaseWithFactory):
         )
 
 
+class TestSnapPermissions(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super().setUp()
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+
+    def test_delete_permissions(self):
+        # A snap package can be deleted from registry_experts,
+        # commercial_admin, admin and owner.
+
+        not_owner = login_celebrity("registry_experts")
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        snap = self.factory.makeSnap(
+            registrant=owner,
+            owner=owner,
+            distroseries=distroseries,
+            name="condemned",
+        )
+
+        with person_logged_in(not_owner):
+            snap.destroySelf()
+        flush_database_caches()
+        # The deleted snap is gone.
+        self.assertFalse(getUtility(ISnapSet).exists(owner, "condemned"))
+
+        not_owner = login_celebrity("commercial_admin")
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        snap = self.factory.makeSnap(
+            registrant=owner,
+            owner=owner,
+            distroseries=distroseries,
+            name="condemned",
+        )
+
+        with person_logged_in(not_owner):
+            snap.destroySelf()
+        flush_database_caches()
+        # The deleted snap is gone.
+        self.assertFalse(getUtility(ISnapSet).exists(owner, "condemned"))
+
+        not_owner = login_celebrity("admin")
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        snap = self.factory.makeSnap(
+            registrant=owner,
+            owner=owner,
+            distroseries=distroseries,
+            name="condemned",
+        )
+
+        with person_logged_in(not_owner):
+            snap.destroySelf()
+        flush_database_caches()
+        # The deleted snap is gone.
+        self.assertFalse(getUtility(ISnapSet).exists(owner, "condemned"))
+
+    def test_delete_permissions_unauthorized(self):
+        # A snap package cannot be deleted if unauthorized
+
+        not_owner = self.factory.makePerson()
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        snap = self.factory.makeSnap(
+            registrant=owner,
+            owner=owner,
+            distroseries=distroseries,
+            name="condemned",
+        )
+
+        with person_logged_in(not_owner):
+            self.assertRaises(Unauthorized, getattr, snap, "destroySelf")
+        flush_database_caches()
+        # The snap is not deleted.
+        self.assertTrue(getUtility(ISnapSet).exists(owner, "condemned"))
+
+
 class TestSnap(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer