Merge ~pappacena/launchpad:xmlrpc-git-confirmRepoCreation into launchpad:master
- Git
- lp:~pappacena/launchpad
- xmlrpc-git-confirmRepoCreation
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Thiago F. Pappacena | ||||
Approved revision: | 367bfb34f2edaa7a7402d21724b4e2acb35d9014 | ||||
Merge reported by: | Otto Co-Pilot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~pappacena/launchpad:xmlrpc-git-confirmRepoCreation | ||||
Merge into: | launchpad:master | ||||
Prerequisite: | ~pappacena/launchpad:gitrepo-status | ||||
Diff against target: |
649 lines (+556/-5) 4 files modified
lib/lp/code/interfaces/gitapi.py (+24/-1) lib/lp/code/model/gitrepository.py (+3/-2) lib/lp/code/xmlrpc/git.py (+82/-1) lib/lp/code/xmlrpc/tests/test_git.py (+447/-1) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Ioana Lasc (community) | Approve | ||
Review via email: mp+385301@code.launchpad.net |
Commit message
New Git XML-RPC methods: confirmRepoCrea
The new flow on Turnip for repository creation will start on Turnip side itself, after Launchpad's translatePath call returning that a repository does not exist. After the local repository creation, Turnip will either confirm or abort the repository creation on LP using those new XML-RPC methods.
Description of the change
- bae4995... by Thiago F. Pappacena
-
Adding abortRepoCreation method
Colin Watson (cjwatson) wrote : | # |
Also, could you make the MP commit message follow the recommendations in https:/
- 3e4eb7e... by Thiago F. Pappacena
-
Merge branch 'gitrepo-status' into xmlrpc-
git-confirmRepo Creation - a93ffa7... by Thiago F. Pappacena
-
Automatically reclaim space if repo is AVAILABLE
- 5f1d7f1... by Thiago F. Pappacena
-
Adding more tests for operations authorization check
- cfae7b8... by Thiago F. Pappacena
-
Using repository's path instead of id on abort/confirmRe
poCreation calls
Thiago F. Pappacena (pappacena) wrote : | # |
I went through all the comments and fixed them.
Also, while writing some new tests I realized that we are not deleting code imports when we delete git repositories (and it breaks some "abort" scenarios). I've added the CodeImport removal.
Ioana Lasc (ilasc) wrote : | # |
Nicely done!
Apart from one small comment inline, it looks good.
- 44fb2a8... by Thiago F. Pappacena
-
Merge branch 'master' into xmlrpc-
git-confirmRepo Creation - 367bfb3... by Thiago F. Pappacena
-
Break references when aborting repository creation
Thiago F. Pappacena (pappacena) wrote : | # |
Thanks for the comment, ilasc! I haven't noticed that. Pushing the changes now.
Ioana Lasc (ilasc) : | # |
Colin Watson (cjwatson) : | # |
Thiago F. Pappacena (pappacena) wrote : | # |
Pushed the requested changes.
Preview Diff
1 | diff --git a/lib/lp/code/interfaces/gitapi.py b/lib/lp/code/interfaces/gitapi.py |
2 | index c289dee..e4a91db 100644 |
3 | --- a/lib/lp/code/interfaces/gitapi.py |
4 | +++ b/lib/lp/code/interfaces/gitapi.py |
5 | @@ -1,4 +1,4 @@ |
6 | -# Copyright 2015 Canonical Ltd. This software is licensed under the |
7 | +# Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
8 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | |
10 | """Interfaces for internal Git APIs.""" |
11 | @@ -92,3 +92,26 @@ class IGitAPI(Interface): |
12 | if no repository can be found for 'translated_path', |
13 | or an `Unauthorized` fault for unauthorized push attempts. |
14 | """ |
15 | + |
16 | + def confirmRepoCreation(repository_id): |
17 | + """Confirm that repository creation. |
18 | + |
19 | + When code hosting finishes creating the repository locally, |
20 | + it should call back this method to confirm that the repository was |
21 | + created, and Launchpad should make the repository available for end |
22 | + users. |
23 | + |
24 | + :param repository_id: The database ID of the repository, provided by |
25 | + translatePath call when repo creation is necessary. |
26 | + """ |
27 | + |
28 | + def abortRepoCreation(repository_id): |
29 | + """Abort the creation of a repository, removing it from database. |
30 | + |
31 | + When code hosting fails to create a repository locally, it should |
32 | + call back this method to indicate that the operation failed and the |
33 | + repository should be removed from Launchpad's database. |
34 | + |
35 | + :param repository_id: The database ID of the repository, provided by |
36 | + translatePath call when repo creation is necessary. |
37 | + """ |
38 | diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py |
39 | index bd1f3e7..c05b43b 100644 |
40 | --- a/lib/lp/code/model/gitrepository.py |
41 | +++ b/lib/lp/code/model/gitrepository.py |
42 | @@ -1639,8 +1639,9 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin): |
43 | Store.of(self).remove(self) |
44 | # And now create a job to remove the repository from storage when |
45 | # it's done. |
46 | - getUtility(IReclaimGitRepositorySpaceJobSource).create( |
47 | - repository_name, repository_path) |
48 | + if self.status == GitRepositoryStatus.AVAILABLE: |
49 | + getUtility(IReclaimGitRepositorySpaceJobSource).create( |
50 | + repository_name, repository_path) |
51 | |
52 | |
53 | class DeletionOperation: |
54 | diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py |
55 | index b34307a..1e00b78 100644 |
56 | --- a/lib/lp/code/xmlrpc/git.py |
57 | +++ b/lib/lp/code/xmlrpc/git.py |
58 | @@ -1,4 +1,4 @@ |
59 | -# Copyright 2015-2019 Canonical Ltd. This software is licensed under the |
60 | +# Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
61 | # GNU Affero General Public License version 3 (see the file LICENSE). |
62 | |
63 | """Implementations of the XML-RPC APIs for Git.""" |
64 | @@ -31,6 +31,7 @@ from lp.app.validators import LaunchpadValidationError |
65 | from lp.code.enums import ( |
66 | GitGranteeType, |
67 | GitPermissionType, |
68 | + GitRepositoryStatus, |
69 | GitRepositoryType, |
70 | ) |
71 | from lp.code.errors import ( |
72 | @@ -584,3 +585,83 @@ class GitAPI(LaunchpadXMLRPCView): |
73 | [(ref_path.data, permissions) |
74 | for ref_path, permissions in result]) |
75 | return result |
76 | + |
77 | + def _validateRequesterCanManageRepoCreation( |
78 | + self, requester, repository, auth_params): |
79 | + """Makes sure the requester has permission to change repository |
80 | + creation status.""" |
81 | + naked_repo = removeSecurityProxy(repository) |
82 | + if requester == LAUNCHPAD_ANONYMOUS: |
83 | + requester = None |
84 | + |
85 | + verified = self._verifyAuthParams(requester, repository, auth_params) |
86 | + if verified is not None and verified.user is NO_USER: |
87 | + # For internal-services authentication, we check if its using a |
88 | + # suitable macaroon that specifically grants access to this |
89 | + # repository. This is only permitted for macaroons not bound to |
90 | + # a user. |
91 | + if not _can_internal_issuer_write(verified): |
92 | + raise faults.Unauthorized() |
93 | + else: |
94 | + # This checks `requester` against `repo.registrant` because the |
95 | + # requester should be the only user able to confirm/abort |
96 | + # repository creation while it's being created. |
97 | + if requester != naked_repo.registrant: |
98 | + raise faults.Unauthorized() |
99 | + |
100 | + if naked_repo.status != GitRepositoryStatus.CREATING: |
101 | + raise faults.Unauthorized() |
102 | + |
103 | + def _confirmRepoCreation(self, requester, translated_path, auth_params): |
104 | + naked_repo = removeSecurityProxy( |
105 | + getUtility(IGitLookup).getByHostingPath(translated_path)) |
106 | + if naked_repo is None: |
107 | + raise faults.GitRepositoryNotFound(translated_path) |
108 | + self._validateRequesterCanManageRepoCreation( |
109 | + requester, naked_repo, auth_params) |
110 | + naked_repo.status = GitRepositoryStatus.AVAILABLE |
111 | + |
112 | + def confirmRepoCreation(self, translated_path, auth_params): |
113 | + """See `IGitAPI`.""" |
114 | + logger = self._getLogger(auth_params.get("request-id")) |
115 | + requester_id = _get_requester_id(auth_params) |
116 | + logger.info( |
117 | + "Request received: confirmRepoCreation('%s')", translated_path) |
118 | + try: |
119 | + result = run_with_login( |
120 | + requester_id, self._confirmRepoCreation, |
121 | + translated_path, auth_params) |
122 | + except Exception as e: |
123 | + result = e |
124 | + if isinstance(result, xmlrpc_client.Fault): |
125 | + logger.error("confirmRepoCreation failed: %r", result) |
126 | + else: |
127 | + logger.info("confirmRepoCreation succeeded: %s" % result) |
128 | + return result |
129 | + |
130 | + def _abortRepoCreation(self, requester, translated_path, auth_params): |
131 | + naked_repo = removeSecurityProxy( |
132 | + getUtility(IGitLookup).getByHostingPath(translated_path)) |
133 | + if naked_repo is None: |
134 | + raise faults.GitRepositoryNotFound(translated_path) |
135 | + self._validateRequesterCanManageRepoCreation( |
136 | + requester, naked_repo, auth_params) |
137 | + naked_repo.destroySelf(break_references=True) |
138 | + |
139 | + def abortRepoCreation(self, translated_path, auth_params): |
140 | + """See `IGitAPI`.""" |
141 | + logger = self._getLogger(auth_params.get("request-id")) |
142 | + requester_id = _get_requester_id(auth_params) |
143 | + logger.info( |
144 | + "Request received: abortRepoCreation('%s')", translated_path) |
145 | + try: |
146 | + result = run_with_login( |
147 | + requester_id, self._abortRepoCreation, |
148 | + translated_path, auth_params) |
149 | + except Exception as e: |
150 | + result = e |
151 | + if isinstance(result, xmlrpc_client.Fault): |
152 | + logger.error("abortRepoCreation failed: %r", result) |
153 | + else: |
154 | + logger.info("abortRepoCreation succeeded: %s" % result) |
155 | + return result |
156 | diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py |
157 | index 6f12f1f..e2983a3 100644 |
158 | --- a/lib/lp/code/xmlrpc/tests/test_git.py |
159 | +++ b/lib/lp/code/xmlrpc/tests/test_git.py |
160 | @@ -12,6 +12,7 @@ from pymacaroons import Macaroon |
161 | import six |
162 | from six.moves import xmlrpc_client |
163 | from six.moves.urllib.parse import quote |
164 | +from storm.store import Store |
165 | from testtools.matchers import ( |
166 | Equals, |
167 | IsInstance, |
168 | @@ -29,6 +30,7 @@ from lp.app.enums import InformationType |
169 | from lp.buildmaster.enums import BuildStatus |
170 | from lp.code.enums import ( |
171 | GitGranteeType, |
172 | + GitRepositoryStatus, |
173 | GitRepositoryType, |
174 | TargetRevisionControlSystems, |
175 | ) |
176 | @@ -37,6 +39,7 @@ from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES |
177 | from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow |
178 | from lp.code.interfaces.gitcollection import IAllGitRepositories |
179 | from lp.code.interfaces.gitjob import IGitRefScanJobSource |
180 | +from lp.code.interfaces.gitlookup import IGitLookup |
181 | from lp.code.interfaces.gitrepository import ( |
182 | GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE, |
183 | IGitRepository, |
184 | @@ -280,6 +283,88 @@ class TestGitAPIMixin: |
185 | "writable": writable, "trailing": trailing, "private": private}, |
186 | translation) |
187 | |
188 | + def assertConfirmsRepoCreation(self, requester, git_repository, |
189 | + can_authenticate=True, macaroon_raw=None): |
190 | + translated_path = git_repository.getInternalPath() |
191 | + auth_params = _make_auth_params( |
192 | + requester, can_authenticate=can_authenticate, |
193 | + macaroon_raw=macaroon_raw) |
194 | + request_id = auth_params["request-id"] |
195 | + result = self.assertDoesNotFault( |
196 | + request_id, "confirmRepoCreation", translated_path, auth_params) |
197 | + login(ANONYMOUS) |
198 | + self.assertIsNone(result) |
199 | + Store.of(git_repository).invalidate(git_repository) |
200 | + self.assertEqual(GitRepositoryStatus.AVAILABLE, git_repository.status) |
201 | + |
202 | + def assertConfirmRepoCreationFails( |
203 | + self, failure, requester, git_repository, can_authenticate=True, |
204 | + macaroon_raw=None): |
205 | + translated_path = git_repository.getInternalPath() |
206 | + auth_params = _make_auth_params( |
207 | + requester, can_authenticate=can_authenticate, |
208 | + macaroon_raw=macaroon_raw) |
209 | + request_id = auth_params["request-id"] |
210 | + original_status = git_repository.status |
211 | + self.assertFault( |
212 | + failure, request_id, "confirmRepoCreation", translated_path, |
213 | + auth_params) |
214 | + store = Store.of(git_repository) |
215 | + if store: |
216 | + store.invalidate(git_repository) |
217 | + self.assertEqual(original_status, git_repository.status) |
218 | + |
219 | + def assertConfirmRepoCreationUnauthorized( |
220 | + self, requester, git_repository, can_authenticate=True, |
221 | + macaroon_raw=None): |
222 | + failure = faults.Unauthorized |
223 | + self.assertConfirmRepoCreationFails( |
224 | + failure, requester, git_repository, can_authenticate, |
225 | + macaroon_raw) |
226 | + |
227 | + def assertAbortsRepoCreation(self, requester, git_repository, |
228 | + can_authenticate=True, macaroon_raw=None): |
229 | + translated_path = git_repository.getInternalPath() |
230 | + auth_params = _make_auth_params( |
231 | + requester, can_authenticate=can_authenticate, |
232 | + macaroon_raw=macaroon_raw) |
233 | + request_id = auth_params["request-id"] |
234 | + result = self.assertDoesNotFault( |
235 | + request_id, "abortRepoCreation", translated_path, auth_params) |
236 | + login(ANONYMOUS) |
237 | + self.assertIsNone(result) |
238 | + self.assertIsNone( |
239 | + getUtility(IGitLookup).getByHostingPath(translated_path)) |
240 | + |
241 | + def assertAbortRepoCreationFails( |
242 | + self, failure, requester, git_repository, can_authenticate=True, |
243 | + macaroon_raw=None): |
244 | + translated_path = git_repository.getInternalPath() |
245 | + auth_params = _make_auth_params( |
246 | + requester, can_authenticate=can_authenticate, |
247 | + macaroon_raw=macaroon_raw) |
248 | + request_id = auth_params["request-id"] |
249 | + original_status = git_repository.status |
250 | + self.assertFault( |
251 | + failure, request_id, "abortRepoCreation", translated_path, |
252 | + auth_params) |
253 | + |
254 | + # If it's not expected to fail because the repo isn't there, |
255 | + # make sure the repository was not changed in any way. |
256 | + if not isinstance(failure, faults.GitRepositoryNotFound): |
257 | + repo = removeSecurityProxy( |
258 | + getUtility(IGitLookup).getByHostingPath(translated_path)) |
259 | + self.assertEqual(GitRepositoryStatus.CREATING, repo.status) |
260 | + self.assertEqual(original_status, git_repository.status) |
261 | + |
262 | + def assertAbortRepoCreationUnauthorized( |
263 | + self, requester, git_repository, can_authenticate=True, |
264 | + macaroon_raw=None): |
265 | + failure = faults.Unauthorized |
266 | + self.assertAbortRepoCreationFails( |
267 | + failure, requester, git_repository, can_authenticate, |
268 | + macaroon_raw) |
269 | + |
270 | def assertCreates(self, requester, path, can_authenticate=False, |
271 | private=False): |
272 | auth_params = _make_auth_params( |
273 | @@ -660,6 +745,367 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): |
274 | |
275 | layer = LaunchpadFunctionalLayer |
276 | |
277 | + def test_confirm_git_repository_creation(self): |
278 | + owner = self.factory.makePerson() |
279 | + repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) |
280 | + repo.status = GitRepositoryStatus.CREATING |
281 | + self.assertConfirmsRepoCreation(owner, repo) |
282 | + |
283 | + def test_only_requester_can_confirm_git_repository_creation(self): |
284 | + requester = self.factory.makePerson() |
285 | + repo = removeSecurityProxy(self.factory.makeGitRepository()) |
286 | + repo.status = GitRepositoryStatus.CREATING |
287 | + |
288 | + self.assertConfirmRepoCreationUnauthorized(requester, repo) |
289 | + |
290 | + def test_confirm_git_repository_creation_of_non_existing_repository(self): |
291 | + owner = self.factory.makePerson() |
292 | + repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) |
293 | + repo.status = GitRepositoryStatus.CREATING |
294 | + repo.destroySelf() |
295 | + |
296 | + expected_failure = faults.GitRepositoryNotFound(str(repo.id)) |
297 | + self.assertConfirmRepoCreationFails(expected_failure, owner, repo) |
298 | + |
299 | + def test_confirm_git_repository_creation_public_code_import(self): |
300 | + # A code import worker with a suitable macaroon can write to a |
301 | + # repository associated with a running code import job. |
302 | + self.pushConfig( |
303 | + "launchpad", internal_macaroon_secret_key="some-secret") |
304 | + machine = self.factory.makeCodeImportMachine(set_online=True) |
305 | + code_imports = [ |
306 | + self.factory.makeCodeImport( |
307 | + target_rcs_type=TargetRevisionControlSystems.GIT) |
308 | + for _ in range(2)] |
309 | + with celebrity_logged_in("vcs_imports"): |
310 | + jobs = [ |
311 | + self.factory.makeCodeImportJob(code_import=code_import) |
312 | + for code_import in code_imports] |
313 | + issuer = getUtility(IMacaroonIssuer, "code-import-job") |
314 | + macaroons = [ |
315 | + removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] |
316 | + |
317 | + repo = removeSecurityProxy(code_imports[0].git_repository) |
318 | + repo.status = GitRepositoryStatus.CREATING |
319 | + |
320 | + self.assertConfirmRepoCreationUnauthorized( |
321 | + LAUNCHPAD_SERVICES, repo, |
322 | + macaroon_raw=macaroons[0].serialize()) |
323 | + with celebrity_logged_in("vcs_imports"): |
324 | + getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) |
325 | + self.assertConfirmRepoCreationUnauthorized( |
326 | + LAUNCHPAD_SERVICES, repo, |
327 | + macaroon_raw=macaroons[1].serialize()) |
328 | + self.assertConfirmRepoCreationUnauthorized( |
329 | + LAUNCHPAD_SERVICES, repo, |
330 | + macaroon_raw=Macaroon( |
331 | + location=config.vhost.mainsite.hostname, |
332 | + identifier="another", |
333 | + key="another-secret").serialize()) |
334 | + self.assertConfirmRepoCreationUnauthorized( |
335 | + LAUNCHPAD_SERVICES, repo, macaroon_raw="nonsense") |
336 | + self.assertConfirmRepoCreationUnauthorized( |
337 | + code_imports[0].registrant, repo, |
338 | + macaroon_raw=macaroons[0].serialize()) |
339 | + self.assertConfirmsRepoCreation( |
340 | + LAUNCHPAD_SERVICES, repo, |
341 | + macaroon_raw=macaroons[0].serialize()) |
342 | + |
343 | + def test_confirm_git_repository_creation_private_code_import(self): |
344 | + # A code import worker with a suitable macaroon can write to a |
345 | + # repository associated with a running private code import job. |
346 | + self.pushConfig( |
347 | + "launchpad", internal_macaroon_secret_key="some-secret") |
348 | + machine = self.factory.makeCodeImportMachine(set_online=True) |
349 | + code_imports = [ |
350 | + self.factory.makeCodeImport( |
351 | + target_rcs_type=TargetRevisionControlSystems.GIT) |
352 | + for _ in range(2)] |
353 | + private_repository = code_imports[0].git_repository |
354 | + removeSecurityProxy( |
355 | + private_repository).transitionToInformationType( |
356 | + InformationType.PRIVATESECURITY, private_repository.owner) |
357 | + with celebrity_logged_in("vcs_imports"): |
358 | + jobs = [ |
359 | + self.factory.makeCodeImportJob(code_import=code_import) |
360 | + for code_import in code_imports] |
361 | + issuer = getUtility(IMacaroonIssuer, "code-import-job") |
362 | + macaroons = [ |
363 | + removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] |
364 | + |
365 | + repo = removeSecurityProxy(code_imports[0].git_repository) |
366 | + repo.status = GitRepositoryStatus.CREATING |
367 | + |
368 | + self.assertConfirmRepoCreationUnauthorized( |
369 | + LAUNCHPAD_SERVICES, repo, |
370 | + macaroon_raw=macaroons[0].serialize()) |
371 | + with celebrity_logged_in("vcs_imports"): |
372 | + getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) |
373 | + self.assertConfirmRepoCreationUnauthorized( |
374 | + LAUNCHPAD_SERVICES, repo, |
375 | + macaroon_raw=macaroons[1].serialize()) |
376 | + self.assertConfirmRepoCreationUnauthorized( |
377 | + LAUNCHPAD_SERVICES, repo, |
378 | + macaroon_raw=Macaroon( |
379 | + location=config.vhost.mainsite.hostname, |
380 | + identifier="another", |
381 | + key="another-secret").serialize()) |
382 | + self.assertConfirmRepoCreationUnauthorized( |
383 | + LAUNCHPAD_SERVICES, repo, |
384 | + macaroon_raw="nonsense") |
385 | + self.assertConfirmRepoCreationUnauthorized( |
386 | + code_imports[0].registrant, repo, |
387 | + macaroon_raw=macaroons[0].serialize()) |
388 | + self.assertConfirmsRepoCreation( |
389 | + LAUNCHPAD_SERVICES, repo, |
390 | + macaroon_raw=macaroons[0].serialize()) |
391 | + |
392 | + def test_confirm_git_repository_creation_user_macaroon(self): |
393 | + # A user with a suitable macaroon can write to the corresponding |
394 | + # repository, but not others, even if they own them. |
395 | + self.pushConfig("codehosting", |
396 | + git_macaroon_secret_key="some-secret") |
397 | + requester = self.factory.makePerson() |
398 | + repositories = [ |
399 | + self.factory.makeGitRepository(owner=requester) for _ in |
400 | + range(2)] |
401 | + repositories.append(self.factory.makeGitRepository( |
402 | + owner=requester, |
403 | + information_type=InformationType.PRIVATESECURITY)) |
404 | + issuer = getUtility(IMacaroonIssuer, "git-repository") |
405 | + with person_logged_in(requester): |
406 | + macaroons = [ |
407 | + removeSecurityProxy(issuer).issueMacaroon( |
408 | + repository, user=requester) |
409 | + for repository in repositories] |
410 | + for i, repository in enumerate(repositories): |
411 | + login(ANONYMOUS) |
412 | + repository = removeSecurityProxy(repository) |
413 | + repository.status = GitRepositoryStatus.CREATING |
414 | + |
415 | + correct_macaroon = macaroons[i] |
416 | + wrong_macaroon = macaroons[(i + 1) % len(macaroons)] |
417 | + |
418 | + self.assertConfirmRepoCreationUnauthorized( |
419 | + requester, repository, macaroon_raw=wrong_macaroon.serialize()) |
420 | + self.assertConfirmRepoCreationUnauthorized( |
421 | + requester, repository, |
422 | + macaroon_raw=Macaroon( |
423 | + location=config.vhost.mainsite.hostname, |
424 | + identifier="another", |
425 | + key="another-secret").serialize()) |
426 | + self.assertConfirmRepoCreationUnauthorized( |
427 | + requester, repository, macaroon_raw="nonsense") |
428 | + self.assertConfirmsRepoCreation( |
429 | + requester, repository, |
430 | + macaroon_raw=correct_macaroon.serialize()) |
431 | + |
432 | + def test_confirm_git_repository_creation_user_mismatch(self): |
433 | + # confirmRepoCreation refuses macaroons in the case where the user |
434 | + # doesn't match what the issuer claims was verified. |
435 | + issuer = DummyMacaroonIssuer() |
436 | + |
437 | + self.useFixture(ZopeUtilityFixture( |
438 | + issuer, IMacaroonIssuer, name="test")) |
439 | + repository = self.factory.makeGitRepository() |
440 | + |
441 | + macaroon = issuer.issueMacaroon(repository) |
442 | + requesters = [self.factory.makePerson() for _ in range(2)] |
443 | + for verified_user, unauthorized in ( |
444 | + (NO_USER, requesters + [None]), |
445 | + (requesters[0], [LAUNCHPAD_SERVICES, requesters[1], None]), |
446 | + (None, [LAUNCHPAD_SERVICES] + requesters + [None]), |
447 | + ): |
448 | + repository = removeSecurityProxy(repository) |
449 | + repository.status = GitRepositoryStatus.CREATING |
450 | + issuer._verified_user = verified_user |
451 | + for requester in unauthorized: |
452 | + login(ANONYMOUS) |
453 | + self.assertConfirmRepoCreationUnauthorized( |
454 | + requester, repository, |
455 | + macaroon_raw=macaroon.serialize()) |
456 | + |
457 | + def test_abort_repo_creation(self): |
458 | + requester = self.factory.makePerson() |
459 | + repo = self.factory.makeGitRepository(owner=requester) |
460 | + repo = removeSecurityProxy(repo) |
461 | + repo.status = GitRepositoryStatus.CREATING |
462 | + self.assertAbortsRepoCreation(requester, repo) |
463 | + |
464 | + def test_only_requester_can_abort_git_repository_creation(self): |
465 | + requester = self.factory.makePerson() |
466 | + repo = removeSecurityProxy(self.factory.makeGitRepository()) |
467 | + repo.status = GitRepositoryStatus.CREATING |
468 | + |
469 | + self.assertAbortRepoCreationUnauthorized(requester, repo) |
470 | + |
471 | + def test_abort_git_repository_creation_public_code_import(self): |
472 | + # A code import worker with a suitable macaroon can write to a |
473 | + # repository associated with a running code import job. |
474 | + self.pushConfig( |
475 | + "launchpad", internal_macaroon_secret_key="some-secret") |
476 | + machine = self.factory.makeCodeImportMachine(set_online=True) |
477 | + code_imports = [ |
478 | + self.factory.makeCodeImport( |
479 | + target_rcs_type=TargetRevisionControlSystems.GIT) |
480 | + for _ in range(2)] |
481 | + with celebrity_logged_in("vcs_imports"): |
482 | + jobs = [ |
483 | + self.factory.makeCodeImportJob(code_import=code_import) |
484 | + for code_import in code_imports] |
485 | + issuer = getUtility(IMacaroonIssuer, "code-import-job") |
486 | + macaroons = [ |
487 | + removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] |
488 | + |
489 | + repo = removeSecurityProxy(code_imports[0].git_repository) |
490 | + repo.status = GitRepositoryStatus.CREATING |
491 | + |
492 | + self.assertAbortRepoCreationUnauthorized( |
493 | + LAUNCHPAD_SERVICES, repo, |
494 | + macaroon_raw=macaroons[0].serialize()) |
495 | + with celebrity_logged_in("vcs_imports"): |
496 | + getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) |
497 | + self.assertAbortRepoCreationUnauthorized( |
498 | + LAUNCHPAD_SERVICES, repo, |
499 | + macaroon_raw=macaroons[1].serialize()) |
500 | + self.assertAbortRepoCreationUnauthorized( |
501 | + LAUNCHPAD_SERVICES, repo, |
502 | + macaroon_raw=Macaroon( |
503 | + location=config.vhost.mainsite.hostname, |
504 | + identifier="another", |
505 | + key="another-secret").serialize()) |
506 | + self.assertAbortRepoCreationUnauthorized( |
507 | + LAUNCHPAD_SERVICES, repo, macaroon_raw="nonsense") |
508 | + self.assertAbortRepoCreationUnauthorized( |
509 | + code_imports[0].registrant, repo, |
510 | + macaroon_raw=macaroons[0].serialize()) |
511 | + self.assertConfirmsRepoCreation( |
512 | + LAUNCHPAD_SERVICES, repo, |
513 | + macaroon_raw=macaroons[0].serialize()) |
514 | + |
515 | + def test_abort_git_repository_creation_private_code_import(self): |
516 | + # A code import worker with a suitable macaroon can write to a |
517 | + # repository associated with a running private code import job. |
518 | + self.pushConfig( |
519 | + "launchpad", internal_macaroon_secret_key="some-secret") |
520 | + machine = self.factory.makeCodeImportMachine(set_online=True) |
521 | + code_imports = [ |
522 | + self.factory.makeCodeImport( |
523 | + target_rcs_type=TargetRevisionControlSystems.GIT) |
524 | + for _ in range(2)] |
525 | + private_repository = code_imports[0].git_repository |
526 | + removeSecurityProxy( |
527 | + private_repository).transitionToInformationType( |
528 | + InformationType.PRIVATESECURITY, private_repository.owner) |
529 | + with celebrity_logged_in("vcs_imports"): |
530 | + jobs = [ |
531 | + self.factory.makeCodeImportJob(code_import=code_import) |
532 | + for code_import in code_imports] |
533 | + issuer = getUtility(IMacaroonIssuer, "code-import-job") |
534 | + macaroons = [ |
535 | + removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] |
536 | + |
537 | + repo = removeSecurityProxy(code_imports[0].git_repository) |
538 | + repo.status = GitRepositoryStatus.CREATING |
539 | + |
540 | + self.assertAbortRepoCreationUnauthorized( |
541 | + LAUNCHPAD_SERVICES, repo, |
542 | + macaroon_raw=macaroons[0].serialize()) |
543 | + with celebrity_logged_in("vcs_imports"): |
544 | + getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) |
545 | + self.assertAbortRepoCreationUnauthorized( |
546 | + LAUNCHPAD_SERVICES, repo, |
547 | + macaroon_raw=macaroons[1].serialize()) |
548 | + self.assertAbortRepoCreationUnauthorized( |
549 | + LAUNCHPAD_SERVICES, repo, |
550 | + macaroon_raw=Macaroon( |
551 | + location=config.vhost.mainsite.hostname, |
552 | + identifier="another", |
553 | + key="another-secret").serialize()) |
554 | + self.assertAbortRepoCreationUnauthorized( |
555 | + LAUNCHPAD_SERVICES, repo, |
556 | + macaroon_raw="nonsense") |
557 | + self.assertAbortRepoCreationUnauthorized( |
558 | + code_imports[0].registrant, repo, |
559 | + macaroon_raw=macaroons[0].serialize()) |
560 | + self.assertAbortsRepoCreation( |
561 | + LAUNCHPAD_SERVICES, repo, |
562 | + macaroon_raw=macaroons[0].serialize()) |
563 | + |
564 | + def test_abort_git_repository_creation_user_macaroon(self): |
565 | + # A user with a suitable macaroon can write to the corresponding |
566 | + # repository, but not others, even if they own them. |
567 | + self.pushConfig("codehosting", |
568 | + git_macaroon_secret_key="some-secret") |
569 | + requester = self.factory.makePerson() |
570 | + repositories = [ |
571 | + self.factory.makeGitRepository(owner=requester) for _ in |
572 | + range(2)] |
573 | + repositories.append(self.factory.makeGitRepository( |
574 | + owner=requester, |
575 | + information_type=InformationType.PRIVATESECURITY)) |
576 | + issuer = getUtility(IMacaroonIssuer, "git-repository") |
577 | + with person_logged_in(requester): |
578 | + macaroons = [ |
579 | + removeSecurityProxy(issuer).issueMacaroon( |
580 | + repository, user=requester) |
581 | + for repository in repositories] |
582 | + for i, repository in enumerate(repositories): |
583 | + login(ANONYMOUS) |
584 | + repository = removeSecurityProxy(repository) |
585 | + repository.status = GitRepositoryStatus.CREATING |
586 | + |
587 | + correct_macaroon = macaroons[i] |
588 | + wrong_macaroon = macaroons[(i + 1) % len(macaroons)] |
589 | + |
590 | + self.assertAbortRepoCreationUnauthorized( |
591 | + requester, repository, macaroon_raw=wrong_macaroon.serialize()) |
592 | + self.assertAbortRepoCreationUnauthorized( |
593 | + requester, repository, |
594 | + macaroon_raw=Macaroon( |
595 | + location=config.vhost.mainsite.hostname, |
596 | + identifier="another", |
597 | + key="another-secret").serialize()) |
598 | + self.assertAbortRepoCreationUnauthorized( |
599 | + requester, repository, macaroon_raw="nonsense") |
600 | + self.assertAbortsRepoCreation( |
601 | + requester, repository, |
602 | + macaroon_raw=correct_macaroon.serialize()) |
603 | + |
604 | + def test_abort_git_repository_creation_user_mismatch(self): |
605 | + # confirmRepoCreation refuses macaroons in the case where the user |
606 | + # doesn't match what the issuer claims was verified. |
607 | + issuer = DummyMacaroonIssuer() |
608 | + |
609 | + self.useFixture(ZopeUtilityFixture( |
610 | + issuer, IMacaroonIssuer, name="test")) |
611 | + repository = self.factory.makeGitRepository() |
612 | + |
613 | + macaroon = issuer.issueMacaroon(repository) |
614 | + requesters = [self.factory.makePerson() for _ in range(2)] |
615 | + for verified_user, unauthorized in ( |
616 | + (NO_USER, requesters + [None]), |
617 | + (requesters[0], [LAUNCHPAD_SERVICES, requesters[1], None]), |
618 | + (None, [LAUNCHPAD_SERVICES] + requesters + [None]), |
619 | + ): |
620 | + repository = removeSecurityProxy(repository) |
621 | + repository.status = GitRepositoryStatus.CREATING |
622 | + issuer._verified_user = verified_user |
623 | + for requester in unauthorized: |
624 | + login(ANONYMOUS) |
625 | + self.assertAbortRepoCreationUnauthorized( |
626 | + requester, repository, |
627 | + macaroon_raw=macaroon.serialize()) |
628 | + |
629 | + def test_abort_git_repository_creation_of_non_existing_repository(self): |
630 | + owner = self.factory.makePerson() |
631 | + repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) |
632 | + repo.status = GitRepositoryStatus.CREATING |
633 | + repo.destroySelf() |
634 | + |
635 | + expected_failure = faults.GitRepositoryNotFound(str(repo.id)) |
636 | + self.assertAbortRepoCreationFails(expected_failure, owner, repo) |
637 | + |
638 | def test_translatePath_cannot_translate(self): |
639 | # Sometimes translatePath will not know how to translate a path. |
640 | # When this happens, it returns a Fault saying so, including the |
641 | @@ -1270,7 +1716,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): |
642 | (requesters[0], [requesters[0]], |
643 | [LAUNCHPAD_SERVICES, requesters[1], None]), |
644 | (None, [], [LAUNCHPAD_SERVICES] + requesters + [None]), |
645 | - ): |
646 | + ): |
647 | issuer._verified_user = verified_user |
648 | for requester in authorized: |
649 | login(ANONYMOUS) |
This looks like a really good start - thanks! There are some things that need sorting out related to the exact shape of the API and details of authorisation.