launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23406
[Merge] lp:~cjwatson/launchpad/snap-build-macaroon into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-build-macaroon into lp:launchpad with lp:~cjwatson/launchpad/librarian-accept-macaroon as a prerequisite.
Commit message:
Add a snap-build macaroon issuer.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1639975 in Launchpad itself: "support for building private snaps"
https://bugs.launchpad.net/launchpad/+bug/1639975
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-build-macaroon/+merge/364333
I factored out some common code into a base class, since there are now three rather similar implementations.
Nothing uses this yet, but I'm planning for SnapBuildBehaviour to do so via an extension to the authserver.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-build-macaroon into lp:launchpad.
=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py 2019-03-12 17:19:57 +0000
+++ lib/lp/code/model/codeimportjob.py 2019-03-12 17:19:58 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database classes for the CodeImportJob table."""
@@ -12,10 +12,6 @@
import datetime
-from pymacaroons import (
- Macaroon,
- Verifier,
- )
from sqlobject import (
ForeignKey,
IntCol,
@@ -59,6 +55,7 @@
sqlvalues,
)
from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.model import MacaroonIssuerBase
@implementer(ICodeImportJob)
@@ -409,7 +406,9 @@
@implementer(IMacaroonIssuer)
-class CodeImportJobMacaroonIssuer:
+class CodeImportJobMacaroonIssuer(MacaroonIssuerBase):
+
+ identifier = "code-import-job"
@property
def _root_secret(self):
@@ -426,35 +425,17 @@
def issueMacaroon(self, context):
"""See `IMacaroonIssuer`."""
assert context.code_import.git_repository is not None
- macaroon = Macaroon(
- location=config.vhost.mainsite.hostname,
- identifier="code-import-job", key=self._root_secret)
- macaroon.add_first_party_caveat("lp.code-import-job %s" % context.id)
- return macaroon
+ return super(CodeImportJobMacaroonIssuer, self).issueMacaroon(
+ context.id)
- def checkMacaroonIssuer(self, macaroon):
- """See `IMacaroonIssuer`."""
- if macaroon.location != config.vhost.mainsite.hostname:
- return False
- try:
- verifier = Verifier()
- verifier.satisfy_general(
- lambda caveat: caveat.startswith("lp.code-import-job "))
- return verifier.verify(macaroon, self._root_secret)
- except Exception:
- return False
+ def verifyCaveat(self, caveat_text, context):
+ """See `MacaroonIssuerBase`."""
+ return caveat_text == str(context.id)
def verifyMacaroon(self, macaroon, context):
"""See `IMacaroonIssuer`."""
- if not ICodeImportJob.providedBy(context):
- return False
- if not self.checkMacaroonIssuer(macaroon):
- return False
- try:
- verifier = Verifier()
- verifier.satisfy_exact("lp.code-import-job %s" % context.id)
- return (
- verifier.verify(macaroon, self._root_secret) and
- context.state == CodeImportJobState.RUNNING)
- except Exception:
- return False
+ if (not ICodeImportJob.providedBy(context) or
+ context.state != CodeImportJobState.RUNNING):
+ return False
+ return super(CodeImportJobMacaroonIssuer, self).verifyMacaroon(
+ macaroon, context)
=== added file 'lib/lp/services/macaroons/model.py'
--- lib/lp/services/macaroons/model.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/macaroons/model.py 2019-03-12 17:19:58 +0000
@@ -0,0 +1,100 @@
+# Copyright 2016-2019 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Policies for issuing and verifying macaroons."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+ "MacaroonIssuerBase",
+ ]
+
+from pymacaroons import (
+ Macaroon,
+ Verifier,
+ )
+
+from lp.services.config import config
+
+
+class MacaroonIssuerBase:
+ """See `IMacaroonIssuer`."""
+
+ @property
+ def identifier(self):
+ """An identifying name for this issuer."""
+ raise NotImplementedError
+
+ @property
+ def prefix(self):
+ """Prefix for caveats issued by this issuer."""
+ return "lp.%s " % self.identifier
+
+ @property
+ def _root_secret(self):
+ secret = config.launchpad.internal_macaroon_secret_key
+ if not secret:
+ raise RuntimeError(
+ "launchpad.internal_macaroon_secret_key not configured.")
+ return secret
+
+ def issueMacaroon(self, context):
+ """See `IMacaroonIssuer`.
+
+ Concrete implementations should normally wrap this with some
+ additional checks of and/or changes to the context.
+ """
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname,
+ identifier=self.identifier, key=self._root_secret)
+ macaroon.add_first_party_caveat(self.prefix + str(context))
+ return macaroon
+
+ def checkMacaroonIssuer(self, macaroon):
+ """See `IMacaroonIssuer`."""
+ if macaroon.location != config.vhost.mainsite.hostname:
+ return False
+ try:
+ verifier = Verifier()
+ verifier.satisfy_general(
+ lambda caveat: caveat.startswith(self.prefix))
+ return verifier.verify(macaroon, self._root_secret)
+ except Exception:
+ return False
+
+ def verifyCaveat(self, caveat_text, context):
+ """Verify the sole caveat on macaroons issued by this issuer.
+
+ :param caveat_text: The text of the caveat, with this issuer's
+ prefix removed.
+ :param context: The context to check.
+ :return: True if this caveat is allowed, otherwise False.
+ """
+ # We will need to change this interface if we ever support macaroons
+ # with more than one caveat or macaroons that are issued to users,
+ # but this is good enough for internal use. Any unrecognised
+ # caveats will fail closed.
+ raise NotImplementedError
+
+ def verifyMacaroon(self, macaroon, context):
+ """See `IMacaroonIssuer`.
+
+ Concrete implementations should normally wrap this with some
+ additional checks of the context, and must implement `verifyCaveat`.
+ """
+ if not self.checkMacaroonIssuer(macaroon):
+ return False
+
+ def verify(caveat):
+ prefix = self.prefix
+ if not caveat.startswith(prefix):
+ return False
+ return self.verifyCaveat(caveat[len(prefix):], context)
+
+ try:
+ verifier = Verifier()
+ verifier.satisfy_general(verify)
+ return verifier.verify(macaroon, self._root_secret)
+ except Exception:
+ return False
=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml 2019-02-11 13:23:34 +0000
+++ lib/lp/snappy/configure.zcml 2019-03-12 17:19:58 +0000
@@ -83,6 +83,14 @@
<allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource" />
</securedutility>
+ <!-- SnapBuildMacaroonIssuer -->
+ <securedutility
+ class="lp.snappy.model.snapbuild.SnapBuildMacaroonIssuer"
+ provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
+ name="snap-build">
+ <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic"/>
+ </securedutility>
+
<!-- SnapBuildBehaviour -->
<adapter
for="lp.snappy.interfaces.snapbuild.ISnapBuild"
=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py 2018-12-18 18:14:37 +0000
+++ lib/lp/snappy/model/snapbuild.py 2019-03-12 17:19:58 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -35,6 +35,7 @@
from zope.component.interfaces import ObjectEvent
from zope.event import notify
from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import (
@@ -45,6 +46,7 @@
from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
from lp.buildmaster.model.buildfarmjob import SpecificBuildFarmJobSourceMixin
from lp.buildmaster.model.packagebuild import PackageBuildMixin
+from lp.code.interfaces.gitrepository import IGitRepository
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.model.distribution import Distribution
from lp.registry.model.distroseries import DistroSeries
@@ -65,6 +67,8 @@
LibraryFileAlias,
LibraryFileContent,
)
+from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.model import MacaroonIssuerBase
from lp.services.propertycache import (
cachedproperty,
get_property_cache,
@@ -584,3 +588,54 @@
SnapBuild, SnapBuild.build_farm_job_id.is_in(
bfj.id for bfj in build_farm_jobs))
return DecoratedResultSet(rows, pre_iter_hook=self.preloadBuildsData)
+
+
+@implementer(IMacaroonIssuer)
+class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
+
+ identifier = "snap-build"
+
+ def issueMacaroon(self, context):
+ """See `IMacaroonIssuer`.
+
+ For issuing, the context is an `ISnapBuild` or its ID.
+ """
+ if ISnapBuild.providedBy(context):
+ pass
+ elif isinstance(context, int):
+ context = getUtility(ISnapBuildSet).getByID(context)
+ else:
+ raise ValueError("Cannot handle context %r." % context)
+ if not removeSecurityProxy(context).is_private:
+ raise ValueError("Refusing to issue macaroon for public build.")
+ return super(SnapBuildMacaroonIssuer, self).issueMacaroon(
+ removeSecurityProxy(context).id)
+
+ def verifyCaveat(self, caveat_text, context):
+ """See `MacaroonIssuerBase`.
+
+ For verification, the context is an `IGitRepository`. We check that
+ the repository is needed to build the `ISnapBuild` that is the
+ context of the macaroon, and that the context build is currently
+ building.
+ """
+ # Circular import.
+ from lp.snappy.model.snap import Snap
+
+ try:
+ build_id = int(caveat_text)
+ except ValueError:
+ return False
+ return not IStore(SnapBuild).find(
+ SnapBuild,
+ SnapBuild.id == build_id,
+ SnapBuild.snap_id == Snap.id,
+ Snap.git_repository == context,
+ SnapBuild.status == BuildStatus.BUILDING).is_empty()
+
+ def verifyMacaroon(self, macaroon, context):
+ """See `IMacaroonIssuer`."""
+ if not IGitRepository.providedBy(context):
+ return False
+ return super(SnapBuildMacaroonIssuer, self).verifyMacaroon(
+ macaroon, context)
=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py 2018-12-18 18:23:52 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py 2019-03-12 17:19:58 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test snap package build features."""
@@ -22,11 +22,13 @@
Equals,
Is,
MatchesDict,
+ MatchesListwise,
MatchesStructure,
)
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from lp.app.enums import InformationType
from lp.app.errors import NotFoundError
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.buildmaster.enums import BuildStatus
@@ -38,6 +40,7 @@
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.librarian.browser import ProxiedLibraryFileAlias
+from lp.services.macaroons.interfaces import IMacaroonIssuer
from lp.services.propertycache import clear_property_cache
from lp.services.webapp.interfaces import OAuthPermission
from lp.services.webapp.publisher import canonical_url
@@ -774,3 +777,124 @@
browser = self.getNonRedirectingBrowser(user=self.person)
for file_url in file_urls:
self.assertCanOpenRedirectedUrl(browser, file_url)
+
+
+class TestSnapBuildMacaroonIssuer(TestCaseWithFactory):
+ """Test SnapBuild macaroon issuing and verification."""
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestSnapBuildMacaroonIssuer, self).setUp()
+ self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+
+ def test_issueMacaroon_refuses_public_snap(self):
+ build = self.factory.makeSnapBuild()
+ issuer = getUtility(IMacaroonIssuer, "snap-build")
+ self.assertRaises(
+ ValueError, removeSecurityProxy(issuer).issueMacaroon, build)
+
+ def test_issueMacaroon_good(self):
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(private=True))
+ issuer = getUtility(IMacaroonIssuer, "snap-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.assertEqual("launchpad.dev", macaroon.location)
+ self.assertEqual("snap-build", macaroon.identifier)
+ self.assertThat(macaroon.caveats, MatchesListwise([
+ MatchesStructure.byEquality(
+ caveat_id="lp.snap-build %s" % build.id),
+ ]))
+
+ def test_checkMacaroonIssuer_good(self):
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(private=True))
+ issuer = getUtility(IMacaroonIssuer, "snap-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.assertTrue(issuer.checkMacaroonIssuer(macaroon))
+
+ def test_checkMacaroonIssuer_wrong_location(self):
+ issuer = getUtility(IMacaroonIssuer, "snap-build")
+ macaroon = Macaroon(
+ location="another-location",
+ key=removeSecurityProxy(issuer)._root_secret)
+ self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
+
+ def test_checkMacaroonIssuer_wrong_key(self):
+ issuer = getUtility(IMacaroonIssuer, "snap-build")
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname, key="another-secret")
+ self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
+
+ def test_verifyMacaroon_good(self):
+ [ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(git_ref=ref, private=True))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertTrue(issuer.verifyMacaroon(macaroon, ref.repository))
+
+ def test_verifyMacaroon_wrong_location(self):
+ [ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(git_ref=ref, private=True))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = Macaroon(
+ location="another-location", key=issuer._root_secret)
+ self.assertFalse(issuer.verifyMacaroon(macaroon, ref.repository))
+
+ def test_verifyMacaroon_wrong_key(self):
+ [ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(git_ref=ref, private=True))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname, key="another-secret")
+ self.assertFalse(issuer.verifyMacaroon(macaroon, ref.repository))
+
+ def test_verifyMacaroon_not_building(self):
+ [ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(git_ref=ref, private=True))
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertFalse(issuer.verifyMacaroon(macaroon, ref.repository))
+
+ def test_verifyMacaroon_wrong_build(self):
+ [ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(git_ref=ref, private=True))
+ build.updateStatus(BuildStatus.BUILDING)
+ other_build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(private=True))
+ other_build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(other_build)
+ self.assertFalse(issuer.verifyMacaroon(macaroon, ref.repository))
+
+ def test_verifyMacaroon_wrong_repository(self):
+ [ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(git_ref=ref, private=True))
+ other_repository = self.factory.makeGitRepository()
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertFalse(issuer.verifyMacaroon(macaroon, other_repository))
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2019-03-12 17:19:57 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2019-03-12 17:19:58 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -21,10 +21,6 @@
import apt_pkg
from debian.deb822 import PkgRelation
-from pymacaroons import (
- Macaroon,
- Verifier,
- )
import pytz
from sqlobject import SQLObjectNotFound
from storm.expr import (
@@ -86,6 +82,7 @@
LibraryFileContent,
)
from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.model import MacaroonIssuerBase
from lp.soyuz.adapters.buildarch import determine_architectures_to_build
from lp.soyuz.enums import (
ArchivePurpose,
@@ -1373,15 +1370,9 @@
@implementer(IMacaroonIssuer)
-class BinaryPackageBuildMacaroonIssuer:
+class BinaryPackageBuildMacaroonIssuer(MacaroonIssuerBase):
- @property
- def _root_secret(self):
- secret = config.launchpad.internal_macaroon_secret_key
- if not secret:
- raise RuntimeError(
- "launchpad.internal_macaroon_secret_key not configured.")
- return secret
+ identifier = "binary-package-build"
def issueMacaroon(self, context):
"""See `IMacaroonIssuer`.
@@ -1390,27 +1381,11 @@
"""
if not removeSecurityProxy(context).archive.private:
raise ValueError("Refusing to issue macaroon for public build.")
- macaroon = Macaroon(
- location=config.vhost.mainsite.hostname,
- identifier="binary-package-build", key=self._root_secret)
- macaroon.add_first_party_caveat(
- "lp.binary-package-build %s" % removeSecurityProxy(context).id)
- return macaroon
-
- def checkMacaroonIssuer(self, macaroon):
- """See `IMacaroonIssuer`."""
- if macaroon.location != config.vhost.mainsite.hostname:
- return False
- try:
- verifier = Verifier()
- verifier.satisfy_general(
- lambda caveat: caveat.startswith("lp.binary-package-build "))
- return verifier.verify(macaroon, self._root_secret)
- except Exception:
- return False
-
- def verifyMacaroon(self, macaroon, context):
- """See `IMacaroonIssuer`.
+ return super(BinaryPackageBuildMacaroonIssuer, self).issueMacaroon(
+ removeSecurityProxy(context).id)
+
+ def verifyCaveat(self, caveat_text, context):
+ """See `MacaroonIssuerBase`.
For verification, the context is a `LibraryFileAlias` ID. We check
that the file is one of those required to build the
@@ -1420,32 +1395,23 @@
# Circular import.
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+ try:
+ build_id = int(caveat_text)
+ except ValueError:
+ return False
+ return not IStore(BinaryPackageBuild).find(
+ BinaryPackageBuild,
+ BinaryPackageBuild.id == build_id,
+ BinaryPackageBuild.source_package_release_id ==
+ SourcePackageRelease.id,
+ SourcePackageReleaseFile.sourcepackagereleaseID ==
+ SourcePackageRelease.id,
+ SourcePackageReleaseFile.libraryfileID == context,
+ BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
+
+ def verifyMacaroon(self, macaroon, context):
+ """See `IMacaroonIssuer`."""
if not isinstance(context, int):
return False
- if not self.checkMacaroonIssuer(macaroon):
- return False
-
- def verify_build(caveat):
- prefix = "lp.binary-package-build "
- if not caveat.startswith(prefix):
- return False
- try:
- build_id = int(caveat[len(prefix):])
- except ValueError:
- return False
- return not IStore(BinaryPackageBuild).find(
- BinaryPackageBuild,
- BinaryPackageBuild.id == build_id,
- BinaryPackageBuild.source_package_release_id ==
- SourcePackageRelease.id,
- SourcePackageReleaseFile.sourcepackagereleaseID ==
- SourcePackageRelease.id,
- SourcePackageReleaseFile.libraryfileID == context,
- BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
-
- try:
- verifier = Verifier()
- verifier.satisfy_general(verify_build)
- return verifier.verify(macaroon, self._root_secret)
- except Exception:
- return False
+ return super(BinaryPackageBuildMacaroonIssuer, self).verifyMacaroon(
+ macaroon, context)
Follow ups
