launchpad-reviewers team mailing list archive

[Colin Watson <mp+414740@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:lock-edit-bug-task-permissions into launchpad:master.

Commit message:
Restrict edit permissions for tasks on locked bugs

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

The recent work to allow locking metadata changes to bugs has the effect of restricting edit permissions on locked bugs.  However, we also need to restrict edit permissions on the corresponding bug tasks, as that's where most of the interesting metadata lives.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:lock-edit-bug-task-permissions into launchpad:master.

diff --git a/lib/lp/bugs/security.py b/lib/lp/bugs/security.py
index beba8af..9ec2b05 100644
--- a/lib/lp/bugs/security.py
+++ b/lib/lp/bugs/security.py
@@ -38,19 +38,16 @@ class EditBugNominationStatus(AuthorizationBase):
         return self.obj.canApprove(user.person)
 
 
-class EditBugTask(AuthorizationBase):
+class EditBugTask(DelegatedAuthorization):
     """Permission checker for editing objects linked to a bug.
 
-    Allow any logged-in user to edit objects linked to public
-    bugs. Allow only explicit subscribers to edit objects linked to
-    private bugs.
+    Allow people who can edit a bug to edit the tasks linked to it.
     """
     permission = 'launchpad.Edit'
     usedfor = IHasBug
 
-    def checkAuthenticated(self, user):
-        # Delegated entirely to the bug.
-        return self.obj.bug.userCanView(user)
+    def __init__(self, obj):
+        super().__init__(obj, obj.bug)
 
 
 class DeleteBugTask(AuthorizationBase):
diff --git a/lib/lp/bugs/tests/test_bug.py b/lib/lp/bugs/tests/test_bug.py
index ebe1e3b..ae35bb0 100644
--- a/lib/lp/bugs/tests/test_bug.py
+++ b/lib/lp/bugs/tests/test_bug.py
@@ -549,29 +549,40 @@ class TestBugLocking(TestCaseWithFactory):
         # A user without the relevant role cannot edit a locked bug.
         with person_logged_in(another_person):
             self.assertFalse(checkPermission('launchpad.Edit', bug))
+            self.assertFalse(
+                checkPermission('launchpad.Edit', bug.default_bugtask))
 
         # The bug reporter cannot edit a locked bug.
         with person_logged_in(self.person):
             self.assertFalse(checkPermission('launchpad.Edit', bug))
+            self.assertFalse(
+                checkPermission('launchpad.Edit', bug.default_bugtask))
 
         # Target driver can edit a locked bug.
         new_person = self.factory.makePerson()
         removeSecurityProxy(bug.default_bugtask.target).driver = new_person
         with person_logged_in(new_person):
             self.assertTrue(checkPermission('launchpad.Edit', bug))
+            self.assertTrue(
+                checkPermission('launchpad.Edit', bug.default_bugtask))
 
         # Admins can edit a locked bug.
         with admin_logged_in():
             self.assertTrue(checkPermission('launchpad.Edit', bug))
+            self.assertTrue(
+                checkPermission('launchpad.Edit', bug.default_bugtask))
 
         # Commercial admins can edit a locked bug.
         with celebrity_logged_in('commercial_admin'):
             self.assertTrue(checkPermission('launchpad.Edit', bug))
+            self.assertTrue(
+                checkPermission('launchpad.Edit', bug.default_bugtask))
 
         # Registry experts can edit a locked bug.
         with celebrity_logged_in('registry_experts'):
             self.assertTrue(checkPermission('launchpad.Edit', bug))
-
+            self.assertTrue(
+                checkPermission('launchpad.Edit', bug.default_bugtask))
 
     def test_only_those_with_moderate_permission_can_lock_unlock_a_bug(self):
         bug = self.factory.makeBug(owner=self.person, target=self.target)