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

Proposed by Ioana Lasc
Status: Merged
Approved by: Ioana Lasc
Approved revision: 563ac8b1b9a54729ea007f3c529eaf7086ef54b9
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ilasc/launchpad:bug-1933971
Merge into: launchpad:master
Diff against target: 133 lines (+74/-9)
4 files modified
lib/lp/code/browser/configure.zcml (+1/-1)
lib/lp/code/browser/tests/test_gitrepository.py (+48/-0)
lib/lp/code/interfaces/gitrepository.py (+8/-8)
lib/lp/code/model/tests/test_gitrepository.py (+17/-0)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+416388@code.launchpad.net

Commit message

Widen GitRepository rescan permissions

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Ioana Lasc (ilasc) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/browser/configure.zcml b/lib/lp/code/browser/configure.zcml
2index 5a92aa8..5520698 100644
3--- a/lib/lp/code/browser/configure.zcml
4+++ b/lib/lp/code/browser/configure.zcml
5@@ -883,7 +883,7 @@
6 <browser:page
7 for="lp.code.interfaces.gitrepository.IGitRepository"
8 class="lp.code.browser.gitrepository.GitRepositoryRescanView"
9- permission="launchpad.Edit"
10+ permission="launchpad.Moderate"
11 name="+rescan"
12 template="../templates/gitrepository-rescan.pt"/>
13 <browser:page
14diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
15index 05c7043..9b8966c 100644
16--- a/lib/lp/code/browser/tests/test_gitrepository.py
17+++ b/lib/lp/code/browser/tests/test_gitrepository.py
18@@ -685,6 +685,54 @@ class TestGitRepositoryView(BrowserTestCase):
19 ])
20
21
22+class TestGitRepositoryRescanView(BrowserTestCase):
23+
24+ layer = DatabaseFunctionalLayer
25+
26+ def test_owner_can_see_rescan(self):
27+ repository = self.factory.makeGitRepository()
28+ job = GitRefScanJob.create(repository)
29+ job.job._status = JobStatus.FAILED
30+ url = canonical_url(
31+ repository, view_name='+rescan', rootsite='code')
32+ browser = self.getUserBrowser(url, user=repository.owner)
33+ browser.open(url)
34+ self.assertIn('schedule a rescan', browser.contents)
35+
36+ def test_product_owner_can_see_rescan(self):
37+ project_owner = self.factory.makePerson()
38+ product = self.factory.makeProduct(owner=project_owner)
39+ repository = self.factory.makeGitRepository(target=product)
40+ job = GitRefScanJob.create(repository)
41+ job.job._status = JobStatus.FAILED
42+ url = canonical_url(
43+ repository, view_name='+rescan', rootsite='code')
44+ browser = self.getUserBrowser(url, user=project_owner)
45+ browser.open(url)
46+ self.assertIn('schedule a rescan', browser.contents)
47+
48+ def test_admin_can_see_rescan(self):
49+ repository = self.factory.makeGitRepository()
50+ job = GitRefScanJob.create(repository)
51+ job.job._status = JobStatus.FAILED
52+ url = canonical_url(
53+ repository, view_name='+rescan', rootsite='code')
54+ browser = self.getUserBrowser(
55+ url,
56+ user=self.factory.makeCommercialAdmin())
57+ browser.open(url)
58+ self.assertIn('schedule a rescan', browser.contents)
59+
60+ def test_other_user_can_not_see_rescan(self):
61+ repository = self.factory.makeGitRepository()
62+ job = GitRefScanJob.create(repository)
63+ job.job._status = JobStatus.FAILED
64+ url = canonical_url(
65+ repository, view_name='+rescan', rootsite='code')
66+ self.assertRaises(
67+ Unauthorized, self.getUserBrowser, url)
68+
69+
70 class TestGitRepositoryViewPrivateArtifacts(BrowserTestCase):
71 """Tests that Git repositories with private team artifacts can be viewed.
72
73diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
74index 4396096..a541026 100644
75--- a/lib/lp/code/interfaces/gitrepository.py
76+++ b/lib/lp/code/interfaces/gitrepository.py
77@@ -781,6 +781,14 @@ class IGitRepositoryModerate(Interface):
78 with the `IGitNamespacePolicy`.
79 """
80
81+ @export_write_operation()
82+ @operation_for_version("devel")
83+ def rescan():
84+ """Force a rescan of this repository as a celery task.
85+
86+ This may be helpful in cases where a previous scan crashed.
87+ """
88+
89
90 class IGitRepositoryEditableAttributes(Interface):
91 """IGitRepository attributes that can be edited.
92@@ -859,14 +867,6 @@ class IGitRepositoryEdit(IWebhookTarget, IAccessTokenTarget):
93 :return: A tuple with (upserted_refs, deleted_refs).
94 """
95
96- @export_write_operation()
97- @operation_for_version("devel")
98- def rescan():
99- """Force a rescan of this repository as a celery task.
100-
101- This may be helpful in cases where a previous scan crashed.
102- """
103-
104 def addRule(ref_pattern, creator, position=None):
105 """Add an access rule to this repository.
106
107diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
108index 3ad7c8e..3272b3b 100644
109--- a/lib/lp/code/model/tests/test_gitrepository.py
110+++ b/lib/lp/code/model/tests/test_gitrepository.py
111@@ -2843,6 +2843,23 @@ class TestGitRepositoryRescan(TestCaseWithFactory):
112 self.assertTrue(result)
113 self.assertIsNone(result.job.date_finished)
114
115+ def test_security(self):
116+ repository = self.factory.makeGitRepository()
117+
118+ # Random users can't rescan a branch.
119+ with person_logged_in(self.factory.makePerson()):
120+ self.assertRaises(Unauthorized, getattr, repository, 'rescan')
121+
122+ # But the owner can.
123+ with person_logged_in(repository.owner):
124+ repository.rescan()
125+
126+ # And so can commercial-admins (and maybe registry too,
127+ # eventually).
128+ with person_logged_in(
129+ getUtility(ILaunchpadCelebrities).commercial_admin):
130+ repository.rescan()
131+
132
133 class TestGitRepositoryUpdateMergeCommitIDs(TestCaseWithFactory):
134

Subscribers

People subscribed via source and target branches

to status/vote changes: