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 @@ | |||
7 | 1 | # Copyright 2015 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
8 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
9 | 3 | 3 | ||
10 | 4 | """Interfaces for internal Git APIs.""" | 4 | """Interfaces for internal Git APIs.""" |
11 | @@ -92,3 +92,26 @@ class IGitAPI(Interface): | |||
12 | 92 | if no repository can be found for 'translated_path', | 92 | if no repository can be found for 'translated_path', |
13 | 93 | or an `Unauthorized` fault for unauthorized push attempts. | 93 | or an `Unauthorized` fault for unauthorized push attempts. |
14 | 94 | """ | 94 | """ |
15 | 95 | |||
16 | 96 | def confirmRepoCreation(repository_id): | ||
17 | 97 | """Confirm that repository creation. | ||
18 | 98 | |||
19 | 99 | When code hosting finishes creating the repository locally, | ||
20 | 100 | it should call back this method to confirm that the repository was | ||
21 | 101 | created, and Launchpad should make the repository available for end | ||
22 | 102 | users. | ||
23 | 103 | |||
24 | 104 | :param repository_id: The database ID of the repository, provided by | ||
25 | 105 | translatePath call when repo creation is necessary. | ||
26 | 106 | """ | ||
27 | 107 | |||
28 | 108 | def abortRepoCreation(repository_id): | ||
29 | 109 | """Abort the creation of a repository, removing it from database. | ||
30 | 110 | |||
31 | 111 | When code hosting fails to create a repository locally, it should | ||
32 | 112 | call back this method to indicate that the operation failed and the | ||
33 | 113 | repository should be removed from Launchpad's database. | ||
34 | 114 | |||
35 | 115 | :param repository_id: The database ID of the repository, provided by | ||
36 | 116 | translatePath call when repo creation is necessary. | ||
37 | 117 | """ | ||
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 | 1639 | Store.of(self).remove(self) | 1639 | Store.of(self).remove(self) |
44 | 1640 | # And now create a job to remove the repository from storage when | 1640 | # And now create a job to remove the repository from storage when |
45 | 1641 | # it's done. | 1641 | # it's done. |
48 | 1642 | getUtility(IReclaimGitRepositorySpaceJobSource).create( | 1642 | if self.status == GitRepositoryStatus.AVAILABLE: |
49 | 1643 | repository_name, repository_path) | 1643 | getUtility(IReclaimGitRepositorySpaceJobSource).create( |
50 | 1644 | repository_name, repository_path) | ||
51 | 1644 | 1645 | ||
52 | 1645 | 1646 | ||
53 | 1646 | class DeletionOperation: | 1647 | 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 @@ | |||
60 | 1 | # Copyright 2015-2019 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
61 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
62 | 3 | 3 | ||
63 | 4 | """Implementations of the XML-RPC APIs for Git.""" | 4 | """Implementations of the XML-RPC APIs for Git.""" |
64 | @@ -31,6 +31,7 @@ from lp.app.validators import LaunchpadValidationError | |||
65 | 31 | from lp.code.enums import ( | 31 | from lp.code.enums import ( |
66 | 32 | GitGranteeType, | 32 | GitGranteeType, |
67 | 33 | GitPermissionType, | 33 | GitPermissionType, |
68 | 34 | GitRepositoryStatus, | ||
69 | 34 | GitRepositoryType, | 35 | GitRepositoryType, |
70 | 35 | ) | 36 | ) |
71 | 36 | from lp.code.errors import ( | 37 | from lp.code.errors import ( |
72 | @@ -584,3 +585,83 @@ class GitAPI(LaunchpadXMLRPCView): | |||
73 | 584 | [(ref_path.data, permissions) | 585 | [(ref_path.data, permissions) |
74 | 585 | for ref_path, permissions in result]) | 586 | for ref_path, permissions in result]) |
75 | 586 | return result | 587 | return result |
76 | 588 | |||
77 | 589 | def _validateRequesterCanManageRepoCreation( | ||
78 | 590 | self, requester, repository, auth_params): | ||
79 | 591 | """Makes sure the requester has permission to change repository | ||
80 | 592 | creation status.""" | ||
81 | 593 | naked_repo = removeSecurityProxy(repository) | ||
82 | 594 | if requester == LAUNCHPAD_ANONYMOUS: | ||
83 | 595 | requester = None | ||
84 | 596 | |||
85 | 597 | verified = self._verifyAuthParams(requester, repository, auth_params) | ||
86 | 598 | if verified is not None and verified.user is NO_USER: | ||
87 | 599 | # For internal-services authentication, we check if its using a | ||
88 | 600 | # suitable macaroon that specifically grants access to this | ||
89 | 601 | # repository. This is only permitted for macaroons not bound to | ||
90 | 602 | # a user. | ||
91 | 603 | if not _can_internal_issuer_write(verified): | ||
92 | 604 | raise faults.Unauthorized() | ||
93 | 605 | else: | ||
94 | 606 | # This checks `requester` against `repo.registrant` because the | ||
95 | 607 | # requester should be the only user able to confirm/abort | ||
96 | 608 | # repository creation while it's being created. | ||
97 | 609 | if requester != naked_repo.registrant: | ||
98 | 610 | raise faults.Unauthorized() | ||
99 | 611 | |||
100 | 612 | if naked_repo.status != GitRepositoryStatus.CREATING: | ||
101 | 613 | raise faults.Unauthorized() | ||
102 | 614 | |||
103 | 615 | def _confirmRepoCreation(self, requester, translated_path, auth_params): | ||
104 | 616 | naked_repo = removeSecurityProxy( | ||
105 | 617 | getUtility(IGitLookup).getByHostingPath(translated_path)) | ||
106 | 618 | if naked_repo is None: | ||
107 | 619 | raise faults.GitRepositoryNotFound(translated_path) | ||
108 | 620 | self._validateRequesterCanManageRepoCreation( | ||
109 | 621 | requester, naked_repo, auth_params) | ||
110 | 622 | naked_repo.status = GitRepositoryStatus.AVAILABLE | ||
111 | 623 | |||
112 | 624 | def confirmRepoCreation(self, translated_path, auth_params): | ||
113 | 625 | """See `IGitAPI`.""" | ||
114 | 626 | logger = self._getLogger(auth_params.get("request-id")) | ||
115 | 627 | requester_id = _get_requester_id(auth_params) | ||
116 | 628 | logger.info( | ||
117 | 629 | "Request received: confirmRepoCreation('%s')", translated_path) | ||
118 | 630 | try: | ||
119 | 631 | result = run_with_login( | ||
120 | 632 | requester_id, self._confirmRepoCreation, | ||
121 | 633 | translated_path, auth_params) | ||
122 | 634 | except Exception as e: | ||
123 | 635 | result = e | ||
124 | 636 | if isinstance(result, xmlrpc_client.Fault): | ||
125 | 637 | logger.error("confirmRepoCreation failed: %r", result) | ||
126 | 638 | else: | ||
127 | 639 | logger.info("confirmRepoCreation succeeded: %s" % result) | ||
128 | 640 | return result | ||
129 | 641 | |||
130 | 642 | def _abortRepoCreation(self, requester, translated_path, auth_params): | ||
131 | 643 | naked_repo = removeSecurityProxy( | ||
132 | 644 | getUtility(IGitLookup).getByHostingPath(translated_path)) | ||
133 | 645 | if naked_repo is None: | ||
134 | 646 | raise faults.GitRepositoryNotFound(translated_path) | ||
135 | 647 | self._validateRequesterCanManageRepoCreation( | ||
136 | 648 | requester, naked_repo, auth_params) | ||
137 | 649 | naked_repo.destroySelf(break_references=True) | ||
138 | 650 | |||
139 | 651 | def abortRepoCreation(self, translated_path, auth_params): | ||
140 | 652 | """See `IGitAPI`.""" | ||
141 | 653 | logger = self._getLogger(auth_params.get("request-id")) | ||
142 | 654 | requester_id = _get_requester_id(auth_params) | ||
143 | 655 | logger.info( | ||
144 | 656 | "Request received: abortRepoCreation('%s')", translated_path) | ||
145 | 657 | try: | ||
146 | 658 | result = run_with_login( | ||
147 | 659 | requester_id, self._abortRepoCreation, | ||
148 | 660 | translated_path, auth_params) | ||
149 | 661 | except Exception as e: | ||
150 | 662 | result = e | ||
151 | 663 | if isinstance(result, xmlrpc_client.Fault): | ||
152 | 664 | logger.error("abortRepoCreation failed: %r", result) | ||
153 | 665 | else: | ||
154 | 666 | logger.info("abortRepoCreation succeeded: %s" % result) | ||
155 | 667 | 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 | 12 | import six | 12 | import six |
162 | 13 | from six.moves import xmlrpc_client | 13 | from six.moves import xmlrpc_client |
163 | 14 | from six.moves.urllib.parse import quote | 14 | from six.moves.urllib.parse import quote |
164 | 15 | from storm.store import Store | ||
165 | 15 | from testtools.matchers import ( | 16 | from testtools.matchers import ( |
166 | 16 | Equals, | 17 | Equals, |
167 | 17 | IsInstance, | 18 | IsInstance, |
168 | @@ -29,6 +30,7 @@ from lp.app.enums import InformationType | |||
169 | 29 | from lp.buildmaster.enums import BuildStatus | 30 | from lp.buildmaster.enums import BuildStatus |
170 | 30 | from lp.code.enums import ( | 31 | from lp.code.enums import ( |
171 | 31 | GitGranteeType, | 32 | GitGranteeType, |
172 | 33 | GitRepositoryStatus, | ||
173 | 32 | GitRepositoryType, | 34 | GitRepositoryType, |
174 | 33 | TargetRevisionControlSystems, | 35 | TargetRevisionControlSystems, |
175 | 34 | ) | 36 | ) |
176 | @@ -37,6 +39,7 @@ from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES | |||
177 | 37 | from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow | 39 | from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow |
178 | 38 | from lp.code.interfaces.gitcollection import IAllGitRepositories | 40 | from lp.code.interfaces.gitcollection import IAllGitRepositories |
179 | 39 | from lp.code.interfaces.gitjob import IGitRefScanJobSource | 41 | from lp.code.interfaces.gitjob import IGitRefScanJobSource |
180 | 42 | from lp.code.interfaces.gitlookup import IGitLookup | ||
181 | 40 | from lp.code.interfaces.gitrepository import ( | 43 | from lp.code.interfaces.gitrepository import ( |
182 | 41 | GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE, | 44 | GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE, |
183 | 42 | IGitRepository, | 45 | IGitRepository, |
184 | @@ -280,6 +283,88 @@ class TestGitAPIMixin: | |||
185 | 280 | "writable": writable, "trailing": trailing, "private": private}, | 283 | "writable": writable, "trailing": trailing, "private": private}, |
186 | 281 | translation) | 284 | translation) |
187 | 282 | 285 | ||
188 | 286 | def assertConfirmsRepoCreation(self, requester, git_repository, | ||
189 | 287 | can_authenticate=True, macaroon_raw=None): | ||
190 | 288 | translated_path = git_repository.getInternalPath() | ||
191 | 289 | auth_params = _make_auth_params( | ||
192 | 290 | requester, can_authenticate=can_authenticate, | ||
193 | 291 | macaroon_raw=macaroon_raw) | ||
194 | 292 | request_id = auth_params["request-id"] | ||
195 | 293 | result = self.assertDoesNotFault( | ||
196 | 294 | request_id, "confirmRepoCreation", translated_path, auth_params) | ||
197 | 295 | login(ANONYMOUS) | ||
198 | 296 | self.assertIsNone(result) | ||
199 | 297 | Store.of(git_repository).invalidate(git_repository) | ||
200 | 298 | self.assertEqual(GitRepositoryStatus.AVAILABLE, git_repository.status) | ||
201 | 299 | |||
202 | 300 | def assertConfirmRepoCreationFails( | ||
203 | 301 | self, failure, requester, git_repository, can_authenticate=True, | ||
204 | 302 | macaroon_raw=None): | ||
205 | 303 | translated_path = git_repository.getInternalPath() | ||
206 | 304 | auth_params = _make_auth_params( | ||
207 | 305 | requester, can_authenticate=can_authenticate, | ||
208 | 306 | macaroon_raw=macaroon_raw) | ||
209 | 307 | request_id = auth_params["request-id"] | ||
210 | 308 | original_status = git_repository.status | ||
211 | 309 | self.assertFault( | ||
212 | 310 | failure, request_id, "confirmRepoCreation", translated_path, | ||
213 | 311 | auth_params) | ||
214 | 312 | store = Store.of(git_repository) | ||
215 | 313 | if store: | ||
216 | 314 | store.invalidate(git_repository) | ||
217 | 315 | self.assertEqual(original_status, git_repository.status) | ||
218 | 316 | |||
219 | 317 | def assertConfirmRepoCreationUnauthorized( | ||
220 | 318 | self, requester, git_repository, can_authenticate=True, | ||
221 | 319 | macaroon_raw=None): | ||
222 | 320 | failure = faults.Unauthorized | ||
223 | 321 | self.assertConfirmRepoCreationFails( | ||
224 | 322 | failure, requester, git_repository, can_authenticate, | ||
225 | 323 | macaroon_raw) | ||
226 | 324 | |||
227 | 325 | def assertAbortsRepoCreation(self, requester, git_repository, | ||
228 | 326 | can_authenticate=True, macaroon_raw=None): | ||
229 | 327 | translated_path = git_repository.getInternalPath() | ||
230 | 328 | auth_params = _make_auth_params( | ||
231 | 329 | requester, can_authenticate=can_authenticate, | ||
232 | 330 | macaroon_raw=macaroon_raw) | ||
233 | 331 | request_id = auth_params["request-id"] | ||
234 | 332 | result = self.assertDoesNotFault( | ||
235 | 333 | request_id, "abortRepoCreation", translated_path, auth_params) | ||
236 | 334 | login(ANONYMOUS) | ||
237 | 335 | self.assertIsNone(result) | ||
238 | 336 | self.assertIsNone( | ||
239 | 337 | getUtility(IGitLookup).getByHostingPath(translated_path)) | ||
240 | 338 | |||
241 | 339 | def assertAbortRepoCreationFails( | ||
242 | 340 | self, failure, requester, git_repository, can_authenticate=True, | ||
243 | 341 | macaroon_raw=None): | ||
244 | 342 | translated_path = git_repository.getInternalPath() | ||
245 | 343 | auth_params = _make_auth_params( | ||
246 | 344 | requester, can_authenticate=can_authenticate, | ||
247 | 345 | macaroon_raw=macaroon_raw) | ||
248 | 346 | request_id = auth_params["request-id"] | ||
249 | 347 | original_status = git_repository.status | ||
250 | 348 | self.assertFault( | ||
251 | 349 | failure, request_id, "abortRepoCreation", translated_path, | ||
252 | 350 | auth_params) | ||
253 | 351 | |||
254 | 352 | # If it's not expected to fail because the repo isn't there, | ||
255 | 353 | # make sure the repository was not changed in any way. | ||
256 | 354 | if not isinstance(failure, faults.GitRepositoryNotFound): | ||
257 | 355 | repo = removeSecurityProxy( | ||
258 | 356 | getUtility(IGitLookup).getByHostingPath(translated_path)) | ||
259 | 357 | self.assertEqual(GitRepositoryStatus.CREATING, repo.status) | ||
260 | 358 | self.assertEqual(original_status, git_repository.status) | ||
261 | 359 | |||
262 | 360 | def assertAbortRepoCreationUnauthorized( | ||
263 | 361 | self, requester, git_repository, can_authenticate=True, | ||
264 | 362 | macaroon_raw=None): | ||
265 | 363 | failure = faults.Unauthorized | ||
266 | 364 | self.assertAbortRepoCreationFails( | ||
267 | 365 | failure, requester, git_repository, can_authenticate, | ||
268 | 366 | macaroon_raw) | ||
269 | 367 | |||
270 | 283 | def assertCreates(self, requester, path, can_authenticate=False, | 368 | def assertCreates(self, requester, path, can_authenticate=False, |
271 | 284 | private=False): | 369 | private=False): |
272 | 285 | auth_params = _make_auth_params( | 370 | auth_params = _make_auth_params( |
273 | @@ -660,6 +745,367 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): | |||
274 | 660 | 745 | ||
275 | 661 | layer = LaunchpadFunctionalLayer | 746 | layer = LaunchpadFunctionalLayer |
276 | 662 | 747 | ||
277 | 748 | def test_confirm_git_repository_creation(self): | ||
278 | 749 | owner = self.factory.makePerson() | ||
279 | 750 | repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) | ||
280 | 751 | repo.status = GitRepositoryStatus.CREATING | ||
281 | 752 | self.assertConfirmsRepoCreation(owner, repo) | ||
282 | 753 | |||
283 | 754 | def test_only_requester_can_confirm_git_repository_creation(self): | ||
284 | 755 | requester = self.factory.makePerson() | ||
285 | 756 | repo = removeSecurityProxy(self.factory.makeGitRepository()) | ||
286 | 757 | repo.status = GitRepositoryStatus.CREATING | ||
287 | 758 | |||
288 | 759 | self.assertConfirmRepoCreationUnauthorized(requester, repo) | ||
289 | 760 | |||
290 | 761 | def test_confirm_git_repository_creation_of_non_existing_repository(self): | ||
291 | 762 | owner = self.factory.makePerson() | ||
292 | 763 | repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) | ||
293 | 764 | repo.status = GitRepositoryStatus.CREATING | ||
294 | 765 | repo.destroySelf() | ||
295 | 766 | |||
296 | 767 | expected_failure = faults.GitRepositoryNotFound(str(repo.id)) | ||
297 | 768 | self.assertConfirmRepoCreationFails(expected_failure, owner, repo) | ||
298 | 769 | |||
299 | 770 | def test_confirm_git_repository_creation_public_code_import(self): | ||
300 | 771 | # A code import worker with a suitable macaroon can write to a | ||
301 | 772 | # repository associated with a running code import job. | ||
302 | 773 | self.pushConfig( | ||
303 | 774 | "launchpad", internal_macaroon_secret_key="some-secret") | ||
304 | 775 | machine = self.factory.makeCodeImportMachine(set_online=True) | ||
305 | 776 | code_imports = [ | ||
306 | 777 | self.factory.makeCodeImport( | ||
307 | 778 | target_rcs_type=TargetRevisionControlSystems.GIT) | ||
308 | 779 | for _ in range(2)] | ||
309 | 780 | with celebrity_logged_in("vcs_imports"): | ||
310 | 781 | jobs = [ | ||
311 | 782 | self.factory.makeCodeImportJob(code_import=code_import) | ||
312 | 783 | for code_import in code_imports] | ||
313 | 784 | issuer = getUtility(IMacaroonIssuer, "code-import-job") | ||
314 | 785 | macaroons = [ | ||
315 | 786 | removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] | ||
316 | 787 | |||
317 | 788 | repo = removeSecurityProxy(code_imports[0].git_repository) | ||
318 | 789 | repo.status = GitRepositoryStatus.CREATING | ||
319 | 790 | |||
320 | 791 | self.assertConfirmRepoCreationUnauthorized( | ||
321 | 792 | LAUNCHPAD_SERVICES, repo, | ||
322 | 793 | macaroon_raw=macaroons[0].serialize()) | ||
323 | 794 | with celebrity_logged_in("vcs_imports"): | ||
324 | 795 | getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) | ||
325 | 796 | self.assertConfirmRepoCreationUnauthorized( | ||
326 | 797 | LAUNCHPAD_SERVICES, repo, | ||
327 | 798 | macaroon_raw=macaroons[1].serialize()) | ||
328 | 799 | self.assertConfirmRepoCreationUnauthorized( | ||
329 | 800 | LAUNCHPAD_SERVICES, repo, | ||
330 | 801 | macaroon_raw=Macaroon( | ||
331 | 802 | location=config.vhost.mainsite.hostname, | ||
332 | 803 | identifier="another", | ||
333 | 804 | key="another-secret").serialize()) | ||
334 | 805 | self.assertConfirmRepoCreationUnauthorized( | ||
335 | 806 | LAUNCHPAD_SERVICES, repo, macaroon_raw="nonsense") | ||
336 | 807 | self.assertConfirmRepoCreationUnauthorized( | ||
337 | 808 | code_imports[0].registrant, repo, | ||
338 | 809 | macaroon_raw=macaroons[0].serialize()) | ||
339 | 810 | self.assertConfirmsRepoCreation( | ||
340 | 811 | LAUNCHPAD_SERVICES, repo, | ||
341 | 812 | macaroon_raw=macaroons[0].serialize()) | ||
342 | 813 | |||
343 | 814 | def test_confirm_git_repository_creation_private_code_import(self): | ||
344 | 815 | # A code import worker with a suitable macaroon can write to a | ||
345 | 816 | # repository associated with a running private code import job. | ||
346 | 817 | self.pushConfig( | ||
347 | 818 | "launchpad", internal_macaroon_secret_key="some-secret") | ||
348 | 819 | machine = self.factory.makeCodeImportMachine(set_online=True) | ||
349 | 820 | code_imports = [ | ||
350 | 821 | self.factory.makeCodeImport( | ||
351 | 822 | target_rcs_type=TargetRevisionControlSystems.GIT) | ||
352 | 823 | for _ in range(2)] | ||
353 | 824 | private_repository = code_imports[0].git_repository | ||
354 | 825 | removeSecurityProxy( | ||
355 | 826 | private_repository).transitionToInformationType( | ||
356 | 827 | InformationType.PRIVATESECURITY, private_repository.owner) | ||
357 | 828 | with celebrity_logged_in("vcs_imports"): | ||
358 | 829 | jobs = [ | ||
359 | 830 | self.factory.makeCodeImportJob(code_import=code_import) | ||
360 | 831 | for code_import in code_imports] | ||
361 | 832 | issuer = getUtility(IMacaroonIssuer, "code-import-job") | ||
362 | 833 | macaroons = [ | ||
363 | 834 | removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] | ||
364 | 835 | |||
365 | 836 | repo = removeSecurityProxy(code_imports[0].git_repository) | ||
366 | 837 | repo.status = GitRepositoryStatus.CREATING | ||
367 | 838 | |||
368 | 839 | self.assertConfirmRepoCreationUnauthorized( | ||
369 | 840 | LAUNCHPAD_SERVICES, repo, | ||
370 | 841 | macaroon_raw=macaroons[0].serialize()) | ||
371 | 842 | with celebrity_logged_in("vcs_imports"): | ||
372 | 843 | getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) | ||
373 | 844 | self.assertConfirmRepoCreationUnauthorized( | ||
374 | 845 | LAUNCHPAD_SERVICES, repo, | ||
375 | 846 | macaroon_raw=macaroons[1].serialize()) | ||
376 | 847 | self.assertConfirmRepoCreationUnauthorized( | ||
377 | 848 | LAUNCHPAD_SERVICES, repo, | ||
378 | 849 | macaroon_raw=Macaroon( | ||
379 | 850 | location=config.vhost.mainsite.hostname, | ||
380 | 851 | identifier="another", | ||
381 | 852 | key="another-secret").serialize()) | ||
382 | 853 | self.assertConfirmRepoCreationUnauthorized( | ||
383 | 854 | LAUNCHPAD_SERVICES, repo, | ||
384 | 855 | macaroon_raw="nonsense") | ||
385 | 856 | self.assertConfirmRepoCreationUnauthorized( | ||
386 | 857 | code_imports[0].registrant, repo, | ||
387 | 858 | macaroon_raw=macaroons[0].serialize()) | ||
388 | 859 | self.assertConfirmsRepoCreation( | ||
389 | 860 | LAUNCHPAD_SERVICES, repo, | ||
390 | 861 | macaroon_raw=macaroons[0].serialize()) | ||
391 | 862 | |||
392 | 863 | def test_confirm_git_repository_creation_user_macaroon(self): | ||
393 | 864 | # A user with a suitable macaroon can write to the corresponding | ||
394 | 865 | # repository, but not others, even if they own them. | ||
395 | 866 | self.pushConfig("codehosting", | ||
396 | 867 | git_macaroon_secret_key="some-secret") | ||
397 | 868 | requester = self.factory.makePerson() | ||
398 | 869 | repositories = [ | ||
399 | 870 | self.factory.makeGitRepository(owner=requester) for _ in | ||
400 | 871 | range(2)] | ||
401 | 872 | repositories.append(self.factory.makeGitRepository( | ||
402 | 873 | owner=requester, | ||
403 | 874 | information_type=InformationType.PRIVATESECURITY)) | ||
404 | 875 | issuer = getUtility(IMacaroonIssuer, "git-repository") | ||
405 | 876 | with person_logged_in(requester): | ||
406 | 877 | macaroons = [ | ||
407 | 878 | removeSecurityProxy(issuer).issueMacaroon( | ||
408 | 879 | repository, user=requester) | ||
409 | 880 | for repository in repositories] | ||
410 | 881 | for i, repository in enumerate(repositories): | ||
411 | 882 | login(ANONYMOUS) | ||
412 | 883 | repository = removeSecurityProxy(repository) | ||
413 | 884 | repository.status = GitRepositoryStatus.CREATING | ||
414 | 885 | |||
415 | 886 | correct_macaroon = macaroons[i] | ||
416 | 887 | wrong_macaroon = macaroons[(i + 1) % len(macaroons)] | ||
417 | 888 | |||
418 | 889 | self.assertConfirmRepoCreationUnauthorized( | ||
419 | 890 | requester, repository, macaroon_raw=wrong_macaroon.serialize()) | ||
420 | 891 | self.assertConfirmRepoCreationUnauthorized( | ||
421 | 892 | requester, repository, | ||
422 | 893 | macaroon_raw=Macaroon( | ||
423 | 894 | location=config.vhost.mainsite.hostname, | ||
424 | 895 | identifier="another", | ||
425 | 896 | key="another-secret").serialize()) | ||
426 | 897 | self.assertConfirmRepoCreationUnauthorized( | ||
427 | 898 | requester, repository, macaroon_raw="nonsense") | ||
428 | 899 | self.assertConfirmsRepoCreation( | ||
429 | 900 | requester, repository, | ||
430 | 901 | macaroon_raw=correct_macaroon.serialize()) | ||
431 | 902 | |||
432 | 903 | def test_confirm_git_repository_creation_user_mismatch(self): | ||
433 | 904 | # confirmRepoCreation refuses macaroons in the case where the user | ||
434 | 905 | # doesn't match what the issuer claims was verified. | ||
435 | 906 | issuer = DummyMacaroonIssuer() | ||
436 | 907 | |||
437 | 908 | self.useFixture(ZopeUtilityFixture( | ||
438 | 909 | issuer, IMacaroonIssuer, name="test")) | ||
439 | 910 | repository = self.factory.makeGitRepository() | ||
440 | 911 | |||
441 | 912 | macaroon = issuer.issueMacaroon(repository) | ||
442 | 913 | requesters = [self.factory.makePerson() for _ in range(2)] | ||
443 | 914 | for verified_user, unauthorized in ( | ||
444 | 915 | (NO_USER, requesters + [None]), | ||
445 | 916 | (requesters[0], [LAUNCHPAD_SERVICES, requesters[1], None]), | ||
446 | 917 | (None, [LAUNCHPAD_SERVICES] + requesters + [None]), | ||
447 | 918 | ): | ||
448 | 919 | repository = removeSecurityProxy(repository) | ||
449 | 920 | repository.status = GitRepositoryStatus.CREATING | ||
450 | 921 | issuer._verified_user = verified_user | ||
451 | 922 | for requester in unauthorized: | ||
452 | 923 | login(ANONYMOUS) | ||
453 | 924 | self.assertConfirmRepoCreationUnauthorized( | ||
454 | 925 | requester, repository, | ||
455 | 926 | macaroon_raw=macaroon.serialize()) | ||
456 | 927 | |||
457 | 928 | def test_abort_repo_creation(self): | ||
458 | 929 | requester = self.factory.makePerson() | ||
459 | 930 | repo = self.factory.makeGitRepository(owner=requester) | ||
460 | 931 | repo = removeSecurityProxy(repo) | ||
461 | 932 | repo.status = GitRepositoryStatus.CREATING | ||
462 | 933 | self.assertAbortsRepoCreation(requester, repo) | ||
463 | 934 | |||
464 | 935 | def test_only_requester_can_abort_git_repository_creation(self): | ||
465 | 936 | requester = self.factory.makePerson() | ||
466 | 937 | repo = removeSecurityProxy(self.factory.makeGitRepository()) | ||
467 | 938 | repo.status = GitRepositoryStatus.CREATING | ||
468 | 939 | |||
469 | 940 | self.assertAbortRepoCreationUnauthorized(requester, repo) | ||
470 | 941 | |||
471 | 942 | def test_abort_git_repository_creation_public_code_import(self): | ||
472 | 943 | # A code import worker with a suitable macaroon can write to a | ||
473 | 944 | # repository associated with a running code import job. | ||
474 | 945 | self.pushConfig( | ||
475 | 946 | "launchpad", internal_macaroon_secret_key="some-secret") | ||
476 | 947 | machine = self.factory.makeCodeImportMachine(set_online=True) | ||
477 | 948 | code_imports = [ | ||
478 | 949 | self.factory.makeCodeImport( | ||
479 | 950 | target_rcs_type=TargetRevisionControlSystems.GIT) | ||
480 | 951 | for _ in range(2)] | ||
481 | 952 | with celebrity_logged_in("vcs_imports"): | ||
482 | 953 | jobs = [ | ||
483 | 954 | self.factory.makeCodeImportJob(code_import=code_import) | ||
484 | 955 | for code_import in code_imports] | ||
485 | 956 | issuer = getUtility(IMacaroonIssuer, "code-import-job") | ||
486 | 957 | macaroons = [ | ||
487 | 958 | removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] | ||
488 | 959 | |||
489 | 960 | repo = removeSecurityProxy(code_imports[0].git_repository) | ||
490 | 961 | repo.status = GitRepositoryStatus.CREATING | ||
491 | 962 | |||
492 | 963 | self.assertAbortRepoCreationUnauthorized( | ||
493 | 964 | LAUNCHPAD_SERVICES, repo, | ||
494 | 965 | macaroon_raw=macaroons[0].serialize()) | ||
495 | 966 | with celebrity_logged_in("vcs_imports"): | ||
496 | 967 | getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) | ||
497 | 968 | self.assertAbortRepoCreationUnauthorized( | ||
498 | 969 | LAUNCHPAD_SERVICES, repo, | ||
499 | 970 | macaroon_raw=macaroons[1].serialize()) | ||
500 | 971 | self.assertAbortRepoCreationUnauthorized( | ||
501 | 972 | LAUNCHPAD_SERVICES, repo, | ||
502 | 973 | macaroon_raw=Macaroon( | ||
503 | 974 | location=config.vhost.mainsite.hostname, | ||
504 | 975 | identifier="another", | ||
505 | 976 | key="another-secret").serialize()) | ||
506 | 977 | self.assertAbortRepoCreationUnauthorized( | ||
507 | 978 | LAUNCHPAD_SERVICES, repo, macaroon_raw="nonsense") | ||
508 | 979 | self.assertAbortRepoCreationUnauthorized( | ||
509 | 980 | code_imports[0].registrant, repo, | ||
510 | 981 | macaroon_raw=macaroons[0].serialize()) | ||
511 | 982 | self.assertConfirmsRepoCreation( | ||
512 | 983 | LAUNCHPAD_SERVICES, repo, | ||
513 | 984 | macaroon_raw=macaroons[0].serialize()) | ||
514 | 985 | |||
515 | 986 | def test_abort_git_repository_creation_private_code_import(self): | ||
516 | 987 | # A code import worker with a suitable macaroon can write to a | ||
517 | 988 | # repository associated with a running private code import job. | ||
518 | 989 | self.pushConfig( | ||
519 | 990 | "launchpad", internal_macaroon_secret_key="some-secret") | ||
520 | 991 | machine = self.factory.makeCodeImportMachine(set_online=True) | ||
521 | 992 | code_imports = [ | ||
522 | 993 | self.factory.makeCodeImport( | ||
523 | 994 | target_rcs_type=TargetRevisionControlSystems.GIT) | ||
524 | 995 | for _ in range(2)] | ||
525 | 996 | private_repository = code_imports[0].git_repository | ||
526 | 997 | removeSecurityProxy( | ||
527 | 998 | private_repository).transitionToInformationType( | ||
528 | 999 | InformationType.PRIVATESECURITY, private_repository.owner) | ||
529 | 1000 | with celebrity_logged_in("vcs_imports"): | ||
530 | 1001 | jobs = [ | ||
531 | 1002 | self.factory.makeCodeImportJob(code_import=code_import) | ||
532 | 1003 | for code_import in code_imports] | ||
533 | 1004 | issuer = getUtility(IMacaroonIssuer, "code-import-job") | ||
534 | 1005 | macaroons = [ | ||
535 | 1006 | removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs] | ||
536 | 1007 | |||
537 | 1008 | repo = removeSecurityProxy(code_imports[0].git_repository) | ||
538 | 1009 | repo.status = GitRepositoryStatus.CREATING | ||
539 | 1010 | |||
540 | 1011 | self.assertAbortRepoCreationUnauthorized( | ||
541 | 1012 | LAUNCHPAD_SERVICES, repo, | ||
542 | 1013 | macaroon_raw=macaroons[0].serialize()) | ||
543 | 1014 | with celebrity_logged_in("vcs_imports"): | ||
544 | 1015 | getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine) | ||
545 | 1016 | self.assertAbortRepoCreationUnauthorized( | ||
546 | 1017 | LAUNCHPAD_SERVICES, repo, | ||
547 | 1018 | macaroon_raw=macaroons[1].serialize()) | ||
548 | 1019 | self.assertAbortRepoCreationUnauthorized( | ||
549 | 1020 | LAUNCHPAD_SERVICES, repo, | ||
550 | 1021 | macaroon_raw=Macaroon( | ||
551 | 1022 | location=config.vhost.mainsite.hostname, | ||
552 | 1023 | identifier="another", | ||
553 | 1024 | key="another-secret").serialize()) | ||
554 | 1025 | self.assertAbortRepoCreationUnauthorized( | ||
555 | 1026 | LAUNCHPAD_SERVICES, repo, | ||
556 | 1027 | macaroon_raw="nonsense") | ||
557 | 1028 | self.assertAbortRepoCreationUnauthorized( | ||
558 | 1029 | code_imports[0].registrant, repo, | ||
559 | 1030 | macaroon_raw=macaroons[0].serialize()) | ||
560 | 1031 | self.assertAbortsRepoCreation( | ||
561 | 1032 | LAUNCHPAD_SERVICES, repo, | ||
562 | 1033 | macaroon_raw=macaroons[0].serialize()) | ||
563 | 1034 | |||
564 | 1035 | def test_abort_git_repository_creation_user_macaroon(self): | ||
565 | 1036 | # A user with a suitable macaroon can write to the corresponding | ||
566 | 1037 | # repository, but not others, even if they own them. | ||
567 | 1038 | self.pushConfig("codehosting", | ||
568 | 1039 | git_macaroon_secret_key="some-secret") | ||
569 | 1040 | requester = self.factory.makePerson() | ||
570 | 1041 | repositories = [ | ||
571 | 1042 | self.factory.makeGitRepository(owner=requester) for _ in | ||
572 | 1043 | range(2)] | ||
573 | 1044 | repositories.append(self.factory.makeGitRepository( | ||
574 | 1045 | owner=requester, | ||
575 | 1046 | information_type=InformationType.PRIVATESECURITY)) | ||
576 | 1047 | issuer = getUtility(IMacaroonIssuer, "git-repository") | ||
577 | 1048 | with person_logged_in(requester): | ||
578 | 1049 | macaroons = [ | ||
579 | 1050 | removeSecurityProxy(issuer).issueMacaroon( | ||
580 | 1051 | repository, user=requester) | ||
581 | 1052 | for repository in repositories] | ||
582 | 1053 | for i, repository in enumerate(repositories): | ||
583 | 1054 | login(ANONYMOUS) | ||
584 | 1055 | repository = removeSecurityProxy(repository) | ||
585 | 1056 | repository.status = GitRepositoryStatus.CREATING | ||
586 | 1057 | |||
587 | 1058 | correct_macaroon = macaroons[i] | ||
588 | 1059 | wrong_macaroon = macaroons[(i + 1) % len(macaroons)] | ||
589 | 1060 | |||
590 | 1061 | self.assertAbortRepoCreationUnauthorized( | ||
591 | 1062 | requester, repository, macaroon_raw=wrong_macaroon.serialize()) | ||
592 | 1063 | self.assertAbortRepoCreationUnauthorized( | ||
593 | 1064 | requester, repository, | ||
594 | 1065 | macaroon_raw=Macaroon( | ||
595 | 1066 | location=config.vhost.mainsite.hostname, | ||
596 | 1067 | identifier="another", | ||
597 | 1068 | key="another-secret").serialize()) | ||
598 | 1069 | self.assertAbortRepoCreationUnauthorized( | ||
599 | 1070 | requester, repository, macaroon_raw="nonsense") | ||
600 | 1071 | self.assertAbortsRepoCreation( | ||
601 | 1072 | requester, repository, | ||
602 | 1073 | macaroon_raw=correct_macaroon.serialize()) | ||
603 | 1074 | |||
604 | 1075 | def test_abort_git_repository_creation_user_mismatch(self): | ||
605 | 1076 | # confirmRepoCreation refuses macaroons in the case where the user | ||
606 | 1077 | # doesn't match what the issuer claims was verified. | ||
607 | 1078 | issuer = DummyMacaroonIssuer() | ||
608 | 1079 | |||
609 | 1080 | self.useFixture(ZopeUtilityFixture( | ||
610 | 1081 | issuer, IMacaroonIssuer, name="test")) | ||
611 | 1082 | repository = self.factory.makeGitRepository() | ||
612 | 1083 | |||
613 | 1084 | macaroon = issuer.issueMacaroon(repository) | ||
614 | 1085 | requesters = [self.factory.makePerson() for _ in range(2)] | ||
615 | 1086 | for verified_user, unauthorized in ( | ||
616 | 1087 | (NO_USER, requesters + [None]), | ||
617 | 1088 | (requesters[0], [LAUNCHPAD_SERVICES, requesters[1], None]), | ||
618 | 1089 | (None, [LAUNCHPAD_SERVICES] + requesters + [None]), | ||
619 | 1090 | ): | ||
620 | 1091 | repository = removeSecurityProxy(repository) | ||
621 | 1092 | repository.status = GitRepositoryStatus.CREATING | ||
622 | 1093 | issuer._verified_user = verified_user | ||
623 | 1094 | for requester in unauthorized: | ||
624 | 1095 | login(ANONYMOUS) | ||
625 | 1096 | self.assertAbortRepoCreationUnauthorized( | ||
626 | 1097 | requester, repository, | ||
627 | 1098 | macaroon_raw=macaroon.serialize()) | ||
628 | 1099 | |||
629 | 1100 | def test_abort_git_repository_creation_of_non_existing_repository(self): | ||
630 | 1101 | owner = self.factory.makePerson() | ||
631 | 1102 | repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner)) | ||
632 | 1103 | repo.status = GitRepositoryStatus.CREATING | ||
633 | 1104 | repo.destroySelf() | ||
634 | 1105 | |||
635 | 1106 | expected_failure = faults.GitRepositoryNotFound(str(repo.id)) | ||
636 | 1107 | self.assertAbortRepoCreationFails(expected_failure, owner, repo) | ||
637 | 1108 | |||
638 | 663 | def test_translatePath_cannot_translate(self): | 1109 | def test_translatePath_cannot_translate(self): |
639 | 664 | # Sometimes translatePath will not know how to translate a path. | 1110 | # Sometimes translatePath will not know how to translate a path. |
640 | 665 | # When this happens, it returns a Fault saying so, including the | 1111 | # When this happens, it returns a Fault saying so, including the |
641 | @@ -1270,7 +1716,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory): | |||
642 | 1270 | (requesters[0], [requesters[0]], | 1716 | (requesters[0], [requesters[0]], |
643 | 1271 | [LAUNCHPAD_SERVICES, requesters[1], None]), | 1717 | [LAUNCHPAD_SERVICES, requesters[1], None]), |
644 | 1272 | (None, [], [LAUNCHPAD_SERVICES] + requesters + [None]), | 1718 | (None, [], [LAUNCHPAD_SERVICES] + requesters + [None]), |
646 | 1273 | ): | 1719 | ): |
647 | 1274 | issuer._verified_user = verified_user | 1720 | issuer._verified_user = verified_user |
648 | 1275 | for requester in authorized: | 1721 | for requester in authorized: |
649 | 1276 | login(ANONYMOUS) | 1722 | 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.