Merge ~ilasc/launchpad:bug-1933971 into launchpad:master

[Ioana Lasc]

Widen GitRepository rescan permissions

[Colin Watson (cjwatson)]

review: Approve

[Ioana Lasc (ilasc)]

Thanks Colin! Indeed worth a set of tests at the repo browser level.

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/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
index 05c7043..9b8966c 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -685,6 +685,54 @@ class TestGitRepositoryView(BrowserTestCase):
         ])
 
 
+class TestGitRepositoryRescanView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_owner_can_see_rescan(self):
+        repository = self.factory.makeGitRepository()
+        job = GitRefScanJob.create(repository)
+        job.job._status = JobStatus.FAILED
+        url = canonical_url(
+            repository, view_name='+rescan', rootsite='code')
+        browser = self.getUserBrowser(url, user=repository.owner)
+        browser.open(url)
+        self.assertIn('schedule a rescan', browser.contents)
+
+    def test_product_owner_can_see_rescan(self):
+        project_owner = self.factory.makePerson()
+        product = self.factory.makeProduct(owner=project_owner)
+        repository = self.factory.makeGitRepository(target=product)
+        job = GitRefScanJob.create(repository)
+        job.job._status = JobStatus.FAILED
+        url = canonical_url(
+            repository, view_name='+rescan', rootsite='code')
+        browser = self.getUserBrowser(url, user=project_owner)
+        browser.open(url)
+        self.assertIn('schedule a rescan', browser.contents)
+
+    def test_admin_can_see_rescan(self):
+        repository = self.factory.makeGitRepository()
+        job = GitRefScanJob.create(repository)
+        job.job._status = JobStatus.FAILED
+        url = canonical_url(
+            repository, view_name='+rescan', rootsite='code')
+        browser = self.getUserBrowser(
+            url,
+            user=self.factory.makeCommercialAdmin())
+        browser.open(url)
+        self.assertIn('schedule a rescan', browser.contents)
+
+    def test_other_user_can_not_see_rescan(self):
+        repository = self.factory.makeGitRepository()
+        job = GitRefScanJob.create(repository)
+        job.job._status = JobStatus.FAILED
+        url = canonical_url(
+            repository, view_name='+rescan', rootsite='code')
+        self.assertRaises(
+            Unauthorized, self.getUserBrowser, url)
+
+
 class TestGitRepositoryViewPrivateArtifacts(BrowserTestCase):
     """Tests that Git repositories with private team artifacts can be viewed.
 
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):