launchpad-reviewers team mailing list archive

[Ioana Lasc <mp+416388@xxxxxxxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:bug-1933971 into launchpad:master.

Commit message:
Widen GitRepository rescan permissions

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1933971 in Launchpad itself: "Widen GitRepository rescan permissions slightly"
  https://bugs.launchpad.net/launchpad/+bug/1933971

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/416388
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:bug-1933971 into launchpad:master.

diff --git a/lib/lp/code/browser/configure.zcml b/lib/lp/code/browser/configure.zcml
index 5a92aa8..5520698 100644
--- a/lib/lp/code/browser/configure.zcml
+++ b/lib/lp/code/browser/configure.zcml
@@ -883,7 +883,7 @@
     <browser:page
         for="lp.code.interfaces.gitrepository.IGitRepository"
         class="lp.code.browser.gitrepository.GitRepositoryRescanView"
-        permission="launchpad.Edit"
+        permission="launchpad.Moderate"
         name="+rescan"
         template="../templates/gitrepository-rescan.pt"/>
     <browser:page
diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index 4396096..a541026 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -781,6 +781,14 @@ class IGitRepositoryModerate(Interface):
             with the `IGitNamespacePolicy`.
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
+    def rescan():
+        """Force a rescan of this repository as a celery task.
+
+        This may be helpful in cases where a previous scan crashed.
+        """
+
 
 class IGitRepositoryEditableAttributes(Interface):
     """IGitRepository attributes that can be edited.
@@ -859,14 +867,6 @@ class IGitRepositoryEdit(IWebhookTarget, IAccessTokenTarget):
         :return: A tuple with (upserted_refs, deleted_refs).
         """
 
-    @export_write_operation()
-    @operation_for_version("devel")
-    def rescan():
-        """Force a rescan of this repository as a celery task.
-
-        This may be helpful in cases where a previous scan crashed.
-        """
-
     def addRule(ref_pattern, creator, position=None):
         """Add an access rule to this repository.
 
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 3ad7c8e..3272b3b 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -2843,6 +2843,23 @@ class TestGitRepositoryRescan(TestCaseWithFactory):
         self.assertTrue(result)
         self.assertIsNone(result.job.date_finished)
 
+    def test_security(self):
+        repository = self.factory.makeGitRepository()
+
+        # Random users can't rescan a branch.
+        with person_logged_in(self.factory.makePerson()):
+            self.assertRaises(Unauthorized, getattr, repository, 'rescan')
+
+        # But the owner can.
+        with person_logged_in(repository.owner):
+            repository.rescan()
+
+        # And so can commercial-admins (and maybe registry too,
+        # eventually).
+        with person_logged_in(
+                getUtility(ILaunchpadCelebrities).commercial_admin):
+            repository.rescan()
+
 
 class TestGitRepositoryUpdateMergeCommitIDs(TestCaseWithFactory):