Merge ~pappacena/launchpad:xmlrpc-git-confirmRepoCreation into launchpad:master

[Thiago F. Pappacena]

New Git XML-RPC methods: confirmRepoCreation/abortRepoCreation

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.

bae4995... by Thiago F. Pappacena on 2020-06-08

Adding abortRepoCreation method

[Colin Watson (cjwatson)]

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.

review: Needs Fixing

[Colin Watson (cjwatson)]

Also, could you make the MP commit message follow the recommendations in https://chris.beams.io/posts/git-commit/, so that the end result looks nicer in our history?

3e4eb7e... by Thiago F. Pappacena on 2020-06-12

Merge branch 'gitrepo-status' into xmlrpc-git-confirmRepoCreation

a93ffa7... by Thiago F. Pappacena on 2020-06-12

Automatically reclaim space if repo is AVAILABLE

5f1d7f1... by Thiago F. Pappacena on 2020-06-12

Adding more tests for operations authorization check

cfae7b8... by Thiago F. Pappacena on 2020-06-15

Using repository's path instead of id on abort/confirmRepoCreation calls

[Thiago F. Pappacena (pappacena)]

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)]

Nicely done!
Apart from one small comment inline, it looks good.

review: Needs Information

44fb2a8... by Thiago F. Pappacena on 2020-06-22

Merge branch 'master' into xmlrpc-git-confirmRepoCreation

367bfb3... by Thiago F. Pappacena on 2020-06-22

Break references when aborting repository creation

[Thiago F. Pappacena (pappacena)]

Thanks for the comment, ilasc! I haven't noticed that. Pushing the changes now.

[Ioana Lasc (ilasc)]

review: Approve

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes.

diff --git a/lib/lp/code/interfaces/gitapi.py b/lib/lp/code/interfaces/gitapi.py
index c289dee..e4a91db 100644
--- a/lib/lp/code/interfaces/gitapi.py
+++ b/lib/lp/code/interfaces/gitapi.py
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for internal Git APIs."""
@@ -92,3 +92,26 @@ class IGitAPI(Interface):
             if no repository can be found for 'translated_path',
             or an `Unauthorized` fault for unauthorized push attempts.
         """
+
+    def confirmRepoCreation(repository_id):
+        """Confirm that repository creation.
+
+        When code hosting finishes creating the repository locally,
+        it should call back this method to confirm that the repository was
+        created, and Launchpad should make the repository available for end
+        users.
+
+        :param repository_id: The database ID of the repository, provided by
+                    translatePath call when repo creation is necessary.
+        """
+
+    def abortRepoCreation(repository_id):
+        """Abort the creation of a repository, removing it from database.
+
+        When code hosting fails to create a repository locally, it should
+        call back this method to indicate that the operation failed and the
+        repository should be removed from Launchpad's database.
+
+        :param repository_id: The database ID of the repository, provided by
+                    translatePath call when repo creation is necessary.
+        """
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index bd1f3e7..c05b43b 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1639,8 +1639,9 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
         Store.of(self).remove(self)
         # And now create a job to remove the repository from storage when
         # it's done.
-        getUtility(IReclaimGitRepositorySpaceJobSource).create(
-            repository_name, repository_path)
+        if self.status == GitRepositoryStatus.AVAILABLE:
+            getUtility(IReclaimGitRepositorySpaceJobSource).create(
+                repository_name, repository_path)
 
 
 class DeletionOperation:
diff --git a/lib/lp/code/xmlrpc/git.py b/lib/lp/code/xmlrpc/git.py
index b34307a..1e00b78 100644
--- a/lib/lp/code/xmlrpc/git.py
+++ b/lib/lp/code/xmlrpc/git.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Implementations of the XML-RPC APIs for Git."""
@@ -31,6 +31,7 @@ from lp.app.validators import LaunchpadValidationError
 from lp.code.enums import (
     GitGranteeType,
     GitPermissionType,
+    GitRepositoryStatus,
     GitRepositoryType,
     )
 from lp.code.errors import (
@@ -584,3 +585,83 @@ class GitAPI(LaunchpadXMLRPCView):
                 [(ref_path.data, permissions)
                  for ref_path, permissions in result])
         return result
+
+    def _validateRequesterCanManageRepoCreation(
+            self, requester, repository, auth_params):
+        """Makes sure the requester has permission to change repository
+        creation status."""
+        naked_repo = removeSecurityProxy(repository)
+        if requester == LAUNCHPAD_ANONYMOUS:
+            requester = None
+
+        verified = self._verifyAuthParams(requester, repository, auth_params)
+        if verified is not None and verified.user is NO_USER:
+            # For internal-services authentication, we check if its using a
+            # suitable macaroon that specifically grants access to this
+            # repository.  This is only permitted for macaroons not bound to
+            # a user.
+            if not _can_internal_issuer_write(verified):
+                raise faults.Unauthorized()
+        else:
+            # This checks `requester` against `repo.registrant` because the
+            # requester should be the only user able to confirm/abort
+            # repository creation while it's being created.
+            if requester != naked_repo.registrant:
+                raise faults.Unauthorized()
+
+        if naked_repo.status != GitRepositoryStatus.CREATING:
+            raise faults.Unauthorized()
+
+    def _confirmRepoCreation(self, requester, translated_path, auth_params):
+        naked_repo = removeSecurityProxy(
+            getUtility(IGitLookup).getByHostingPath(translated_path))
+        if naked_repo is None:
+            raise faults.GitRepositoryNotFound(translated_path)
+        self._validateRequesterCanManageRepoCreation(
+            requester, naked_repo, auth_params)
+        naked_repo.status = GitRepositoryStatus.AVAILABLE
+
+    def confirmRepoCreation(self, translated_path, auth_params):
+        """See `IGitAPI`."""
+        logger = self._getLogger(auth_params.get("request-id"))
+        requester_id = _get_requester_id(auth_params)
+        logger.info(
+            "Request received: confirmRepoCreation('%s')", translated_path)
+        try:
+            result = run_with_login(
+                requester_id, self._confirmRepoCreation,
+                translated_path, auth_params)
+        except Exception as e:
+            result = e
+        if isinstance(result, xmlrpc_client.Fault):
+            logger.error("confirmRepoCreation failed: %r", result)
+        else:
+            logger.info("confirmRepoCreation succeeded: %s" % result)
+        return result
+
+    def _abortRepoCreation(self, requester, translated_path, auth_params):
+        naked_repo = removeSecurityProxy(
+            getUtility(IGitLookup).getByHostingPath(translated_path))
+        if naked_repo is None:
+            raise faults.GitRepositoryNotFound(translated_path)
+        self._validateRequesterCanManageRepoCreation(
+            requester, naked_repo, auth_params)
+        naked_repo.destroySelf(break_references=True)
+
+    def abortRepoCreation(self, translated_path, auth_params):
+        """See `IGitAPI`."""
+        logger = self._getLogger(auth_params.get("request-id"))
+        requester_id = _get_requester_id(auth_params)
+        logger.info(
+            "Request received: abortRepoCreation('%s')", translated_path)
+        try:
+            result = run_with_login(
+                requester_id, self._abortRepoCreation,
+                translated_path, auth_params)
+        except Exception as e:
+            result = e
+        if isinstance(result, xmlrpc_client.Fault):
+            logger.error("abortRepoCreation failed: %r", result)
+        else:
+            logger.info("abortRepoCreation succeeded: %s" % result)
+        return result
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index 6f12f1f..e2983a3 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -12,6 +12,7 @@ from pymacaroons import Macaroon
 import six
 from six.moves import xmlrpc_client
 from six.moves.urllib.parse import quote
+from storm.store import Store
 from testtools.matchers import (
     Equals,
     IsInstance,
@@ -29,6 +30,7 @@ from lp.app.enums import InformationType
 from lp.buildmaster.enums import BuildStatus
 from lp.code.enums import (
     GitGranteeType,
+    GitRepositoryStatus,
     GitRepositoryType,
     TargetRevisionControlSystems,
     )
@@ -37,6 +39,7 @@ from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES
 from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
 from lp.code.interfaces.gitcollection import IAllGitRepositories
 from lp.code.interfaces.gitjob import IGitRefScanJobSource
+from lp.code.interfaces.gitlookup import IGitLookup
 from lp.code.interfaces.gitrepository import (
     GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE,
     IGitRepository,
@@ -280,6 +283,88 @@ class TestGitAPIMixin:
              "writable": writable, "trailing": trailing, "private": private},
             translation)
 
+    def assertConfirmsRepoCreation(self, requester, git_repository,
+                                   can_authenticate=True, macaroon_raw=None):
+        translated_path = git_repository.getInternalPath()
+        auth_params = _make_auth_params(
+            requester, can_authenticate=can_authenticate,
+            macaroon_raw=macaroon_raw)
+        request_id = auth_params["request-id"]
+        result = self.assertDoesNotFault(
+            request_id, "confirmRepoCreation", translated_path, auth_params)
+        login(ANONYMOUS)
+        self.assertIsNone(result)
+        Store.of(git_repository).invalidate(git_repository)
+        self.assertEqual(GitRepositoryStatus.AVAILABLE, git_repository.status)
+
+    def assertConfirmRepoCreationFails(
+            self, failure, requester, git_repository, can_authenticate=True,
+            macaroon_raw=None):
+        translated_path = git_repository.getInternalPath()
+        auth_params = _make_auth_params(
+            requester, can_authenticate=can_authenticate,
+            macaroon_raw=macaroon_raw)
+        request_id = auth_params["request-id"]
+        original_status = git_repository.status
+        self.assertFault(
+            failure, request_id, "confirmRepoCreation", translated_path,
+            auth_params)
+        store = Store.of(git_repository)
+        if store:
+            store.invalidate(git_repository)
+        self.assertEqual(original_status, git_repository.status)
+
+    def assertConfirmRepoCreationUnauthorized(
+            self, requester, git_repository, can_authenticate=True,
+            macaroon_raw=None):
+        failure = faults.Unauthorized
+        self.assertConfirmRepoCreationFails(
+            failure, requester, git_repository, can_authenticate,
+            macaroon_raw)
+
+    def assertAbortsRepoCreation(self, requester, git_repository,
+                                   can_authenticate=True, macaroon_raw=None):
+        translated_path = git_repository.getInternalPath()
+        auth_params = _make_auth_params(
+            requester, can_authenticate=can_authenticate,
+            macaroon_raw=macaroon_raw)
+        request_id = auth_params["request-id"]
+        result = self.assertDoesNotFault(
+            request_id, "abortRepoCreation", translated_path, auth_params)
+        login(ANONYMOUS)
+        self.assertIsNone(result)
+        self.assertIsNone(
+            getUtility(IGitLookup).getByHostingPath(translated_path))
+
+    def assertAbortRepoCreationFails(
+            self, failure, requester, git_repository, can_authenticate=True,
+            macaroon_raw=None):
+        translated_path = git_repository.getInternalPath()
+        auth_params = _make_auth_params(
+            requester, can_authenticate=can_authenticate,
+            macaroon_raw=macaroon_raw)
+        request_id = auth_params["request-id"]
+        original_status = git_repository.status
+        self.assertFault(
+            failure, request_id, "abortRepoCreation", translated_path,
+            auth_params)
+
+        # If it's not expected to fail because the repo isn't there,
+        # make sure the repository was not changed in any way.
+        if not isinstance(failure, faults.GitRepositoryNotFound):
+            repo = removeSecurityProxy(
+                getUtility(IGitLookup).getByHostingPath(translated_path))
+            self.assertEqual(GitRepositoryStatus.CREATING, repo.status)
+            self.assertEqual(original_status, git_repository.status)
+
+    def assertAbortRepoCreationUnauthorized(
+            self, requester, git_repository, can_authenticate=True,
+            macaroon_raw=None):
+        failure = faults.Unauthorized
+        self.assertAbortRepoCreationFails(
+            failure, requester, git_repository, can_authenticate,
+            macaroon_raw)
+
     def assertCreates(self, requester, path, can_authenticate=False,
                       private=False):
         auth_params = _make_auth_params(
@@ -660,6 +745,367 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
 
+    def test_confirm_git_repository_creation(self):
+        owner = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
+        repo.status = GitRepositoryStatus.CREATING
+        self.assertConfirmsRepoCreation(owner, repo)
+
+    def test_only_requester_can_confirm_git_repository_creation(self):
+        requester = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository())
+        repo.status = GitRepositoryStatus.CREATING
+
+        self.assertConfirmRepoCreationUnauthorized(requester, repo)
+
+    def test_confirm_git_repository_creation_of_non_existing_repository(self):
+        owner = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
+        repo.status = GitRepositoryStatus.CREATING
+        repo.destroySelf()
+
+        expected_failure = faults.GitRepositoryNotFound(str(repo.id))
+        self.assertConfirmRepoCreationFails(expected_failure, owner, repo)
+
+    def test_confirm_git_repository_creation_public_code_import(self):
+        # A code import worker with a suitable macaroon can write to a
+        # repository associated with a running code import job.
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret")
+        machine = self.factory.makeCodeImportMachine(set_online=True)
+        code_imports = [
+            self.factory.makeCodeImport(
+                target_rcs_type=TargetRevisionControlSystems.GIT)
+            for _ in range(2)]
+        with celebrity_logged_in("vcs_imports"):
+            jobs = [
+                self.factory.makeCodeImportJob(code_import=code_import)
+                for code_import in code_imports]
+        issuer = getUtility(IMacaroonIssuer, "code-import-job")
+        macaroons = [
+            removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs]
+
+        repo = removeSecurityProxy(code_imports[0].git_repository)
+        repo.status = GitRepositoryStatus.CREATING
+
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+        with celebrity_logged_in("vcs_imports"):
+            getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine)
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[1].serialize())
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=Macaroon(
+                location=config.vhost.mainsite.hostname,
+                identifier="another",
+                key="another-secret").serialize())
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo, macaroon_raw="nonsense")
+        self.assertConfirmRepoCreationUnauthorized(
+            code_imports[0].registrant, repo,
+            macaroon_raw=macaroons[0].serialize())
+        self.assertConfirmsRepoCreation(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+
+    def test_confirm_git_repository_creation_private_code_import(self):
+        # A code import worker with a suitable macaroon can write to a
+        # repository associated with a running private code import job.
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret")
+        machine = self.factory.makeCodeImportMachine(set_online=True)
+        code_imports = [
+            self.factory.makeCodeImport(
+                target_rcs_type=TargetRevisionControlSystems.GIT)
+            for _ in range(2)]
+        private_repository = code_imports[0].git_repository
+        removeSecurityProxy(
+            private_repository).transitionToInformationType(
+            InformationType.PRIVATESECURITY, private_repository.owner)
+        with celebrity_logged_in("vcs_imports"):
+            jobs = [
+                self.factory.makeCodeImportJob(code_import=code_import)
+                for code_import in code_imports]
+        issuer = getUtility(IMacaroonIssuer, "code-import-job")
+        macaroons = [
+            removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs]
+
+        repo = removeSecurityProxy(code_imports[0].git_repository)
+        repo.status = GitRepositoryStatus.CREATING
+
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+        with celebrity_logged_in("vcs_imports"):
+            getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine)
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[1].serialize())
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=Macaroon(
+                location=config.vhost.mainsite.hostname,
+                identifier="another",
+                key="another-secret").serialize())
+        self.assertConfirmRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw="nonsense")
+        self.assertConfirmRepoCreationUnauthorized(
+            code_imports[0].registrant, repo,
+            macaroon_raw=macaroons[0].serialize())
+        self.assertConfirmsRepoCreation(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+
+    def test_confirm_git_repository_creation_user_macaroon(self):
+        # A user with a suitable macaroon can write to the corresponding
+        # repository, but not others, even if they own them.
+        self.pushConfig("codehosting",
+                        git_macaroon_secret_key="some-secret")
+        requester = self.factory.makePerson()
+        repositories = [
+            self.factory.makeGitRepository(owner=requester) for _ in
+            range(2)]
+        repositories.append(self.factory.makeGitRepository(
+            owner=requester,
+            information_type=InformationType.PRIVATESECURITY))
+        issuer = getUtility(IMacaroonIssuer, "git-repository")
+        with person_logged_in(requester):
+            macaroons = [
+                removeSecurityProxy(issuer).issueMacaroon(
+                    repository, user=requester)
+                for repository in repositories]
+        for i, repository in enumerate(repositories):
+            login(ANONYMOUS)
+            repository = removeSecurityProxy(repository)
+            repository.status = GitRepositoryStatus.CREATING
+
+            correct_macaroon = macaroons[i]
+            wrong_macaroon = macaroons[(i + 1) % len(macaroons)]
+
+            self.assertConfirmRepoCreationUnauthorized(
+                requester, repository, macaroon_raw=wrong_macaroon.serialize())
+            self.assertConfirmRepoCreationUnauthorized(
+                requester, repository,
+                macaroon_raw=Macaroon(
+                    location=config.vhost.mainsite.hostname,
+                    identifier="another",
+                    key="another-secret").serialize())
+            self.assertConfirmRepoCreationUnauthorized(
+                requester, repository, macaroon_raw="nonsense")
+            self.assertConfirmsRepoCreation(
+                requester, repository,
+                macaroon_raw=correct_macaroon.serialize())
+
+    def test_confirm_git_repository_creation_user_mismatch(self):
+        # confirmRepoCreation refuses macaroons in the case where the user
+        # doesn't match what the issuer claims was verified.
+        issuer = DummyMacaroonIssuer()
+
+        self.useFixture(ZopeUtilityFixture(
+            issuer, IMacaroonIssuer, name="test"))
+        repository = self.factory.makeGitRepository()
+
+        macaroon = issuer.issueMacaroon(repository)
+        requesters = [self.factory.makePerson() for _ in range(2)]
+        for verified_user, unauthorized in (
+                (NO_USER, requesters + [None]),
+                (requesters[0], [LAUNCHPAD_SERVICES, requesters[1], None]),
+                (None, [LAUNCHPAD_SERVICES] + requesters + [None]),
+        ):
+            repository = removeSecurityProxy(repository)
+            repository.status = GitRepositoryStatus.CREATING
+            issuer._verified_user = verified_user
+            for requester in unauthorized:
+                login(ANONYMOUS)
+                self.assertConfirmRepoCreationUnauthorized(
+                    requester, repository,
+                    macaroon_raw=macaroon.serialize())
+
+    def test_abort_repo_creation(self):
+        requester = self.factory.makePerson()
+        repo = self.factory.makeGitRepository(owner=requester)
+        repo = removeSecurityProxy(repo)
+        repo.status = GitRepositoryStatus.CREATING
+        self.assertAbortsRepoCreation(requester, repo)
+
+    def test_only_requester_can_abort_git_repository_creation(self):
+        requester = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository())
+        repo.status = GitRepositoryStatus.CREATING
+
+        self.assertAbortRepoCreationUnauthorized(requester, repo)
+
+    def test_abort_git_repository_creation_public_code_import(self):
+        # A code import worker with a suitable macaroon can write to a
+        # repository associated with a running code import job.
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret")
+        machine = self.factory.makeCodeImportMachine(set_online=True)
+        code_imports = [
+            self.factory.makeCodeImport(
+                target_rcs_type=TargetRevisionControlSystems.GIT)
+            for _ in range(2)]
+        with celebrity_logged_in("vcs_imports"):
+            jobs = [
+                self.factory.makeCodeImportJob(code_import=code_import)
+                for code_import in code_imports]
+        issuer = getUtility(IMacaroonIssuer, "code-import-job")
+        macaroons = [
+            removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs]
+
+        repo = removeSecurityProxy(code_imports[0].git_repository)
+        repo.status = GitRepositoryStatus.CREATING
+
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+        with celebrity_logged_in("vcs_imports"):
+            getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine)
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[1].serialize())
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=Macaroon(
+                location=config.vhost.mainsite.hostname,
+                identifier="another",
+                key="another-secret").serialize())
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo, macaroon_raw="nonsense")
+        self.assertAbortRepoCreationUnauthorized(
+            code_imports[0].registrant, repo,
+            macaroon_raw=macaroons[0].serialize())
+        self.assertConfirmsRepoCreation(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+
+    def test_abort_git_repository_creation_private_code_import(self):
+        # A code import worker with a suitable macaroon can write to a
+        # repository associated with a running private code import job.
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret")
+        machine = self.factory.makeCodeImportMachine(set_online=True)
+        code_imports = [
+            self.factory.makeCodeImport(
+                target_rcs_type=TargetRevisionControlSystems.GIT)
+            for _ in range(2)]
+        private_repository = code_imports[0].git_repository
+        removeSecurityProxy(
+            private_repository).transitionToInformationType(
+            InformationType.PRIVATESECURITY, private_repository.owner)
+        with celebrity_logged_in("vcs_imports"):
+            jobs = [
+                self.factory.makeCodeImportJob(code_import=code_import)
+                for code_import in code_imports]
+        issuer = getUtility(IMacaroonIssuer, "code-import-job")
+        macaroons = [
+            removeSecurityProxy(issuer).issueMacaroon(job) for job in jobs]
+
+        repo = removeSecurityProxy(code_imports[0].git_repository)
+        repo.status = GitRepositoryStatus.CREATING
+
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+        with celebrity_logged_in("vcs_imports"):
+            getUtility(ICodeImportJobWorkflow).startJob(jobs[0], machine)
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[1].serialize())
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=Macaroon(
+                location=config.vhost.mainsite.hostname,
+                identifier="another",
+                key="another-secret").serialize())
+        self.assertAbortRepoCreationUnauthorized(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw="nonsense")
+        self.assertAbortRepoCreationUnauthorized(
+            code_imports[0].registrant, repo,
+            macaroon_raw=macaroons[0].serialize())
+        self.assertAbortsRepoCreation(
+            LAUNCHPAD_SERVICES, repo,
+            macaroon_raw=macaroons[0].serialize())
+
+    def test_abort_git_repository_creation_user_macaroon(self):
+        # A user with a suitable macaroon can write to the corresponding
+        # repository, but not others, even if they own them.
+        self.pushConfig("codehosting",
+                        git_macaroon_secret_key="some-secret")
+        requester = self.factory.makePerson()
+        repositories = [
+            self.factory.makeGitRepository(owner=requester) for _ in
+            range(2)]
+        repositories.append(self.factory.makeGitRepository(
+            owner=requester,
+            information_type=InformationType.PRIVATESECURITY))
+        issuer = getUtility(IMacaroonIssuer, "git-repository")
+        with person_logged_in(requester):
+            macaroons = [
+                removeSecurityProxy(issuer).issueMacaroon(
+                    repository, user=requester)
+                for repository in repositories]
+        for i, repository in enumerate(repositories):
+            login(ANONYMOUS)
+            repository = removeSecurityProxy(repository)
+            repository.status = GitRepositoryStatus.CREATING
+
+            correct_macaroon = macaroons[i]
+            wrong_macaroon = macaroons[(i + 1) % len(macaroons)]
+
+            self.assertAbortRepoCreationUnauthorized(
+                requester, repository, macaroon_raw=wrong_macaroon.serialize())
+            self.assertAbortRepoCreationUnauthorized(
+                requester, repository,
+                macaroon_raw=Macaroon(
+                    location=config.vhost.mainsite.hostname,
+                    identifier="another",
+                    key="another-secret").serialize())
+            self.assertAbortRepoCreationUnauthorized(
+                requester, repository, macaroon_raw="nonsense")
+            self.assertAbortsRepoCreation(
+                requester, repository,
+                macaroon_raw=correct_macaroon.serialize())
+
+    def test_abort_git_repository_creation_user_mismatch(self):
+        # confirmRepoCreation refuses macaroons in the case where the user
+        # doesn't match what the issuer claims was verified.
+        issuer = DummyMacaroonIssuer()
+
+        self.useFixture(ZopeUtilityFixture(
+            issuer, IMacaroonIssuer, name="test"))
+        repository = self.factory.makeGitRepository()
+
+        macaroon = issuer.issueMacaroon(repository)
+        requesters = [self.factory.makePerson() for _ in range(2)]
+        for verified_user, unauthorized in (
+                (NO_USER, requesters + [None]),
+                (requesters[0], [LAUNCHPAD_SERVICES, requesters[1], None]),
+                (None, [LAUNCHPAD_SERVICES] + requesters + [None]),
+        ):
+            repository = removeSecurityProxy(repository)
+            repository.status = GitRepositoryStatus.CREATING
+            issuer._verified_user = verified_user
+            for requester in unauthorized:
+                login(ANONYMOUS)
+                self.assertAbortRepoCreationUnauthorized(
+                    requester, repository,
+                    macaroon_raw=macaroon.serialize())
+
+    def test_abort_git_repository_creation_of_non_existing_repository(self):
+        owner = self.factory.makePerson()
+        repo = removeSecurityProxy(self.factory.makeGitRepository(owner=owner))
+        repo.status = GitRepositoryStatus.CREATING
+        repo.destroySelf()
+
+        expected_failure = faults.GitRepositoryNotFound(str(repo.id))
+        self.assertAbortRepoCreationFails(expected_failure, owner, repo)
+
     def test_translatePath_cannot_translate(self):
         # Sometimes translatePath will not know how to translate a path.
         # When this happens, it returns a Fault saying so, including the
@@ -1270,7 +1716,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
                 (requesters[0], [requesters[0]],
                  [LAUNCHPAD_SERVICES, requesters[1], None]),
                 (None, [], [LAUNCHPAD_SERVICES] + requesters + [None]),
-                ):
+        ):
             issuer._verified_user = verified_user
             for requester in authorized:
                 login(ANONYMOUS)