launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26792
[Merge] ~cjwatson/launchpad:bpb-snapbuild-archive-macaroons into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:bpb-snapbuild-archive-macaroons into launchpad:master.
Commit message:
Allow verifying SnapBuild/BPB macaroons for archives
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400548
As part of replacing `Archive.buildd_secret`, it'll be useful to have tokens that can be used to access an archive but only for the lifetime of a relevant build. Since these tokens won't be owned by users and so don't need to support manual revocation, it should be safe to reuse our existing macaroon infrastructure for this application.
To start with, extend the binary-package-build and snap-build macaroon issuers to support verification using an archive as the context. This currently requires the context archive to be either the build's archive or one of its archive dependencies.
Nothing actually verifies these tokens yet, so there should be no functional change as a result of this commit.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:bpb-snapbuild-archive-macaroons into launchpad:master.
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index 3a9475b..11d5091 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -26,6 +26,7 @@ from storm.locals import (
Desc,
Int,
JSON,
+ Or,
Reference,
Select,
SQL,
@@ -96,8 +97,10 @@ from lp.snappy.model.snapbuildjob import (
SnapBuildJob,
SnapBuildJobType,
)
+from lp.soyuz.interfaces.archive import IArchive
from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.model.archive import Archive
+from lp.soyuz.model.archivedependency import ArchiveDependency
from lp.soyuz.model.distroarchseries import DistroArchSeries
@@ -656,7 +659,8 @@ class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
def checkVerificationContext(self, context, **kwargs):
"""See `MacaroonIssuerBase`."""
- if not IGitRepository.providedBy(context):
+ if (not IGitRepository.providedBy(context) and
+ not IArchive.providedBy(context)):
raise BadMacaroonContext(context)
return context
@@ -664,10 +668,10 @@ class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
**kwargs):
"""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.
+ For verification, the context is an `IGitRepository` or an
+ `IArchive`. We check that the repository or archive 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
@@ -687,9 +691,24 @@ class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
build_id = int(caveat_value)
except ValueError:
return False
- return not IStore(SnapBuild).find(
- SnapBuild,
+ clauses = [
SnapBuild.id == build_id,
- SnapBuild.snap_id == Snap.id,
- Snap.git_repository == context,
- SnapBuild.status == BuildStatus.BUILDING).is_empty()
+ SnapBuild.status == BuildStatus.BUILDING,
+ ]
+ if IGitRepository.providedBy(context):
+ clauses.extend([
+ SnapBuild.snap_id == Snap.id,
+ Snap.git_repository == context,
+ ])
+ elif IArchive.providedBy(context):
+ clauses.append(
+ Or(
+ SnapBuild.archive == context,
+ SnapBuild.archive_id.is_in(Select(
+ Archive.id,
+ where=And(
+ ArchiveDependency.archive == Archive.id,
+ ArchiveDependency.dependency == context)))))
+ else:
+ return False
+ return not IStore(SnapBuild).find(SnapBuild, *clauses).is_empty()
diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
index ee67203..0ada77b 100644
--- a/lib/lp/snappy/tests/test_snapbuild.py
+++ b/lib/lp/snappy/tests/test_snapbuild.py
@@ -40,6 +40,7 @@ from lp.registry.enums import (
PersonVisibility,
TeamMembershipPolicy,
)
+from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.services.authserver.xmlrpc import AuthServerAPIView
from lp.services.config import config
@@ -935,7 +936,7 @@ class TestSnapBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
caveat_id="lp.snap-build %s" % build.id),
])))
- def test_verifyMacaroon_good(self):
+ def test_verifyMacaroon_good_repository(self):
[ref] = self.factory.makeGitRefs(
information_type=InformationType.USERDATA)
build = self.factory.makeSnapBuild(
@@ -946,6 +947,30 @@ class TestSnapBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
macaroon = issuer.issueMacaroon(build)
self.assertMacaroonVerifies(issuer, macaroon, ref.repository)
+ def test_verifyMacaroon_good_direct_archive(self):
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(private=True),
+ archive=self.factory.makeArchive(private=True))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonVerifies(issuer, macaroon, build.archive)
+
+ def test_verifyMacaroon_good_indirect_archive(self):
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(private=True),
+ archive=self.factory.makeArchive(private=True))
+ dependency = self.factory.makeArchive(
+ distribution=build.archive.distribution, private=True)
+ build.archive.addArchiveDependency(
+ dependency, PackagePublishingPocket.RELEASE)
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonVerifies(issuer, macaroon, dependency)
+
def test_verifyMacaroon_good_no_context(self):
[ref] = self.factory.makeGitRefs(
information_type=InformationType.USERDATA)
@@ -1059,3 +1084,17 @@ class TestSnapBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
self.assertMacaroonDoesNotVerify(
["Caveat check for 'lp.snap-build %s' failed." % build.id],
issuer, macaroon, other_repository)
+
+ def test_verifyMacaroon_wrong_archive(self):
+ build = self.factory.makeSnapBuild(
+ snap=self.factory.makeSnap(private=True),
+ archive=self.factory.makeArchive(private=True))
+ other_archive = self.factory.makeArchive(
+ distribution=build.archive.distribution, private=True)
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "snap-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.snap-build %s' failed." % build.id],
+ issuer, macaroon, other_archive)
diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py
index 238221c..b863798 100644
--- a/lib/lp/soyuz/model/binarypackagebuild.py
+++ b/lib/lp/soyuz/model/binarypackagebuild.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -96,6 +96,7 @@ from lp.soyuz.enums import (
PackagePublishingStatus,
)
from lp.soyuz.interfaces.archive import (
+ IArchive,
InvalidExternalDependencies,
validate_external_dependencies,
)
@@ -1423,7 +1424,8 @@ class BinaryPackageBuildMacaroonIssuer(MacaroonIssuerBase):
def checkVerificationContext(self, context, **kwargs):
"""See `MacaroonIssuerBase`."""
- if not ILibraryFileAlias.providedBy(context):
+ if (not ILibraryFileAlias.providedBy(context) and
+ not IArchive.providedBy(context)):
raise BadMacaroonContext(context)
return context
@@ -1431,16 +1433,18 @@ class BinaryPackageBuildMacaroonIssuer(MacaroonIssuerBase):
**kwargs):
"""See `MacaroonIssuerBase`.
- For verification, the context is an `ILibraryFileAlias`. We check
- that the file is one of those required to build the
- `IBinaryPackageBuild` that is the context of the macaroon, and that
- the context build is currently building.
+ For verification, the context is an `ILibraryFileAlias` or an
+ `IArchive`. We check that the file or archive is one of those
+ required to build the `IBinaryPackageBuild` that is the context of
+ the macaroon, and that the context build is currently building.
"""
- # Circular import.
+ # Circular imports.
+ from lp.soyuz.model.archive import Archive
+ from lp.soyuz.model.archivedependency import ArchiveDependency
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
# Binary package builds only support free-floating macaroons for
- # librarian authentication, not ones bound to a user.
+ # librarian or archive authentication, not ones bound to a user.
if user:
return False
verified.user = NO_USER
@@ -1449,12 +1453,28 @@ class BinaryPackageBuildMacaroonIssuer(MacaroonIssuerBase):
build_id = int(caveat_value)
except ValueError:
return False
- return not IStore(BinaryPackageBuild).find(
- BinaryPackageBuild,
+ clauses = [
BinaryPackageBuild.id == build_id,
- BinaryPackageBuild.source_package_release_id ==
- SourcePackageRelease.id,
- SourcePackageReleaseFile.sourcepackagereleaseID ==
- SourcePackageRelease.id,
- SourcePackageReleaseFile.libraryfile == context,
- BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
+ BinaryPackageBuild.status == BuildStatus.BUILDING,
+ ]
+ if ILibraryFileAlias.providedBy(context):
+ clauses.extend([
+ BinaryPackageBuild.source_package_release_id ==
+ SourcePackageRelease.id,
+ SourcePackageReleaseFile.sourcepackagereleaseID ==
+ SourcePackageRelease.id,
+ SourcePackageReleaseFile.libraryfile == context,
+ ])
+ elif IArchive.providedBy(context):
+ clauses.append(
+ Or(
+ BinaryPackageBuild.archive == context,
+ BinaryPackageBuild.archive_id.is_in(Select(
+ Archive.id,
+ where=And(
+ ArchiveDependency.archive == Archive.id,
+ ArchiveDependency.dependency == context)))))
+ else:
+ return False
+ return not IStore(BinaryPackageBuild).find(
+ BinaryPackageBuild, *clauses).is_empty()
diff --git a/lib/lp/soyuz/tests/test_binarypackagebuild.py b/lib/lp/soyuz/tests/test_binarypackagebuild.py
index 0c7b95a..b51f2b7 100644
--- a/lib/lp/soyuz/tests/test_binarypackagebuild.py
+++ b/lib/lp/soyuz/tests/test_binarypackagebuild.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test Build features."""
@@ -975,6 +975,28 @@ class TestBinaryPackageBuildMacaroonIssuer(
macaroon = issuer.issueMacaroon(build)
self.assertMacaroonVerifies(issuer, macaroon, sprf.libraryfile)
+ def test_verifyMacaroon_good_direct_archive(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonVerifies(issuer, macaroon, build.archive)
+
+ def test_verifyMacaroon_good_indirect_archive(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ dependency = self.factory.makeArchive(
+ distribution=build.archive.distribution, private=True)
+ build.archive.addArchiveDependency(
+ dependency, PackagePublishingPocket.RELEASE)
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonVerifies(issuer, macaroon, dependency)
+
def test_verifyMacaroon_wrong_location(self):
build = self.factory.makeBinaryPackageBuild(
archive=self.factory.makeArchive(private=True))
@@ -1046,3 +1068,17 @@ class TestBinaryPackageBuildMacaroonIssuer(
["Caveat check for 'lp.principal.binary-package-build %s' "
"failed." % build.id],
issuer, macaroon, lfa)
+
+ def test_verifyMacaroon_wrong_archive(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ archive = self.factory.makeArchive(
+ distribution=build.archive.distribution, private=True)
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.principal.binary-package-build %s' "
+ "failed." % build.id],
+ issuer, macaroon, archive)
