launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28358
[Merge] ~cjwatson/launchpad:cibuild-macaroons into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:cibuild-macaroons into launchpad:master.
Commit message:
Add CIBuild macaroons
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/419976
This implements the same macaroon-based authorization protocol used by several other build types, allowing builders to clone private repositories associated with the CI build they're running.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:cibuild-macaroons into launchpad:master.
diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
index 882d3da..99a5bf0 100644
--- a/lib/lp/code/configure.zcml
+++ b/lib/lp/code/configure.zcml
@@ -1307,6 +1307,14 @@
<allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource" />
</securedutility>
+ <!-- CIBuildMacaroonIssuer -->
+ <securedutility
+ class="lp.code.model.cibuild.CIBuildMacaroonIssuer"
+ provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
+ name="ci-build">
+ <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic" />
+ </securedutility>
+
<!-- CIBuildBehaviour -->
<adapter
for="lp.code.interfaces.cibuild.ICIBuild"
diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
index 912d31f..dd2390f 100644
--- a/lib/lp/code/model/cibuild.py
+++ b/lib/lp/code/model/cibuild.py
@@ -25,6 +25,7 @@ from storm.store import EmptyResultSet
from zope.component import getUtility
from zope.event import notify
from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
from lp.app.errors import NotFoundError
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -51,6 +52,7 @@ from lp.code.interfaces.cibuild import (
MissingConfiguration,
)
from lp.code.interfaces.githosting import IGitHostingClient
+from lp.code.interfaces.gitrepository import IGitRepository
from lp.code.interfaces.revisionstatus import IRevisionStatusReportSet
from lp.code.model.gitref import GitRef
from lp.code.model.lpcraft import load_configuration
@@ -72,6 +74,12 @@ from lp.services.librarian.model import (
LibraryFileAlias,
LibraryFileContent,
)
+from lp.services.macaroons.interfaces import (
+ BadMacaroonContext,
+ IMacaroonIssuer,
+ NO_USER,
+ )
+from lp.services.macaroons.model import MacaroonIssuerBase
from lp.services.propertycache import cachedproperty
from lp.soyuz.model.distroarchseries import DistroArchSeries
@@ -596,3 +604,58 @@ class CIBuildSet(SpecificBuildFarmJobSourceMixin):
def deleteByGitRepository(self, git_repository):
"""See `ICIBuildSet`."""
self.findByGitRepository(git_repository).remove()
+
+
+@implementer(IMacaroonIssuer)
+class CIBuildMacaroonIssuer(MacaroonIssuerBase):
+
+ identifier = "ci-build"
+ issuable_via_authserver = True
+
+ def checkIssuingContext(self, context, **kwargs):
+ """See `MacaroonIssuerBase`.
+
+ For issuing, the context is an `ICIBuild`.
+ """
+ if not ICIBuild.providedBy(context):
+ raise BadMacaroonContext(context)
+ return removeSecurityProxy(context).id
+
+ def checkVerificationContext(self, context, **kwargs):
+ """See `MacaroonIssuerBase`."""
+ if not IGitRepository.providedBy(context):
+ raise BadMacaroonContext(context)
+ return context
+
+ def verifyPrimaryCaveat(self, verified, caveat_value, context, user=None,
+ **kwargs):
+ """See `MacaroonIssuerBase`.
+
+ For verification, the context is an `IGitRepository`. We check that
+ the repository or archive is needed to build the `ICIBuild` that is
+ the context of the macaroon, and that the context build is currently
+ building.
+ """
+ # CI builds only support free-floating macaroons for Git
+ # authentication, not ones bound to a user.
+ if user:
+ return False
+ verified.user = NO_USER
+
+ if context is None:
+ # We're only verifying that the macaroon could be valid for some
+ # context.
+ return True
+ if not IGitRepository.providedBy(context):
+ return False
+
+ try:
+ build_id = int(caveat_value)
+ except ValueError:
+ return False
+ clauses = [
+ CIBuild.id == build_id,
+ CIBuild.status == BuildStatus.BUILDING,
+ CIBuild.git_repository == context,
+ ]
+ return not IStore(CIBuild).find(CIBuild, *clauses).is_empty()
diff --git a/lib/lp/code/model/tests/test_cibuild.py b/lib/lp/code/model/tests/test_cibuild.py
index d4cc4ec..3f9682e 100644
--- a/lib/lp/code/model/tests/test_cibuild.py
+++ b/lib/lp/code/model/tests/test_cibuild.py
@@ -12,14 +12,17 @@ from textwrap import dedent
from unittest.mock import Mock
from fixtures import MockPatchObject
+from pymacaroons import Macaroon
import pytz
from storm.locals import Store
from testtools.matchers import (
Equals,
+ MatchesListwise,
MatchesSetwise,
MatchesStructure,
)
from zope.component import getUtility
+from zope.publisher.xmlrpc import TestRequest
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
@@ -53,7 +56,11 @@ from lp.code.model.cibuild import (
from lp.code.model.lpcraft import load_configuration
from lp.code.tests.helpers import GitHostingFixture
from lp.registry.interfaces.series import SeriesStatus
+from lp.services.authserver.xmlrpc import AuthServerAPIView
+from lp.services.config import config
from lp.services.log.logger import BufferLogger
+from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.testing import MacaroonTestMixin
from lp.services.propertycache import clear_property_cache
from lp.testing import (
person_logged_in,
@@ -62,6 +69,7 @@ from lp.testing import (
)
from lp.testing.layers import LaunchpadZopelessLayer
from lp.testing.matchers import HasQueryCount
+from lp.xmlrpc.interfaces import IPrivateApplication
class TestGetAllCommitsForPaths(TestCaseWithFactory):
@@ -983,3 +991,144 @@ class TestDetermineDASesToBuild(TestCaseWithFactory):
"name in Ubuntu %s\n" % distro_series.name,
logger.getLogBuffer()
)
+
+
+class TestCIBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
+ """Test CIBuild macaroon issuing and verification."""
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super().setUp()
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+
+ def test_issueMacaroon_good(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ issuer = getUtility(IMacaroonIssuer, "ci-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.assertThat(macaroon, MatchesStructure(
+ location=Equals("launchpad.test"),
+ identifier=Equals("ci-build"),
+ caveats=MatchesListwise([
+ MatchesStructure.byEquality(
+ caveat_id="lp.ci-build %s" % build.id),
+ ])))
+
+ def test_issueMacaroon_via_authserver(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ private_root = getUtility(IPrivateApplication)
+ authserver = AuthServerAPIView(private_root.authserver, TestRequest())
+ macaroon = Macaroon.deserialize(
+ authserver.issueMacaroon("ci-build", "CIBuild", build.id))
+ self.assertThat(macaroon, MatchesStructure(
+ location=Equals("launchpad.test"),
+ identifier=Equals("ci-build"),
+ caveats=MatchesListwise([
+ MatchesStructure.byEquality(
+ caveat_id="lp.ci-build %s" % build.id),
+ ])))
+
+ def test_verifyMacaroon_good_repository(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonVerifies(issuer, macaroon, build.git_repository)
+
+ def test_verifyMacaroon_good_no_context(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonVerifies(
+ issuer, macaroon, None, require_context=False)
+ self.assertMacaroonVerifies(
+ issuer, macaroon, build.git_repository, require_context=False)
+
+ def test_verifyMacaroon_no_context_but_require_context(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonDoesNotVerify(
+ ["Expected macaroon verification context but got None."],
+ issuer, macaroon, None)
+
+ def test_verifyMacaroon_wrong_location(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = Macaroon(
+ location="another-location", key=issuer._root_secret)
+ self.assertMacaroonDoesNotVerify(
+ ["Macaroon has unknown location 'another-location'."],
+ issuer, macaroon, build.git_repository)
+ self.assertMacaroonDoesNotVerify(
+ ["Macaroon has unknown location 'another-location'."],
+ issuer, macaroon, build.git_repository, require_context=False)
+
+ def test_verifyMacaroon_wrong_key(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname, key="another-secret")
+ self.assertMacaroonDoesNotVerify(
+ ["Signatures do not match"],
+ issuer, macaroon, build.git_repository)
+ self.assertMacaroonDoesNotVerify(
+ ["Signatures do not match"],
+ issuer, macaroon, build.git_repository, require_context=False)
+
+ def test_verifyMacaroon_not_building(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.ci-build %s' failed." % build.id],
+ issuer, macaroon, build.git_repository)
+
+ def test_verifyMacaroon_wrong_build(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ build.updateStatus(BuildStatus.BUILDING)
+ other_build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ other_build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = issuer.issueMacaroon(other_build)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.ci-build %s' failed." % other_build.id],
+ issuer, macaroon, build.git_repository)
+
+ def test_verifyMacaroon_wrong_repository(self):
+ build = self.factory.makeCIBuild(
+ git_repository=self.factory.makeGitRepository(
+ information_type=InformationType.USERDATA))
+ other_repository = self.factory.makeGitRepository()
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
+ macaroon = issuer.issueMacaroon(build)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.ci-build %s' failed." % build.id],
+ issuer, macaroon, other_repository)
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index 9dd57c1..f87abb7 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -1803,6 +1803,47 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
repository.registrant, path, permission="read",
macaroon_raw=macaroons[0].serialize())
+ def test_translatePath_private_ci_build(self):
+ # A builder with a suitable macaroon can read from a repository
+ # associated with a running private CI build.
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+ with person_logged_in(self.factory.makePerson()) as owner:
+ repositories = [
+ self.factory.makeGitRepository(
+ owner=owner, information_type=InformationType.USERDATA)
+ for _ in range(2)]
+ builds = [
+ self.factory.makeCIBuild(git_repository=repository)
+ for repository in repositories]
+ issuer = getUtility(IMacaroonIssuer, "ci-build")
+ macaroons = [
+ removeSecurityProxy(issuer).issueMacaroon(build)
+ for build in builds]
+ repository = repositories[0]
+ path = "/%s" % repository.unique_name
+ self.assertUnauthorized(
+ LAUNCHPAD_SERVICES, path, permission="write",
+ macaroon_raw=macaroons[0].serialize())
+ removeSecurityProxy(builds[0]).updateStatus(BuildStatus.BUILDING)
+ self.assertTranslates(
+ LAUNCHPAD_SERVICES, path, repository, False, permission="read",
+ macaroon_raw=macaroons[0].serialize(), private=True)
+ self.assertUnauthorized(
+ LAUNCHPAD_SERVICES, path, permission="read",
+ macaroon_raw=macaroons[1].serialize())
+ self.assertUnauthorized(
+ LAUNCHPAD_SERVICES, path, permission="read",
+ macaroon_raw=Macaroon(
+ location=config.vhost.mainsite.hostname, identifier="another",
+ key="another-secret").serialize())
+ self.assertUnauthorized(
+ LAUNCHPAD_SERVICES, path, permission="read",
+ macaroon_raw="nonsense")
+ self.assertUnauthorized(
+ removeSecurityProxy(repository).registrant, path,
+ permission="read", macaroon_raw=macaroons[0].serialize())
+
def test_translatePath_user_macaroon(self):
# A user with a suitable macaroon can write to the corresponding
# repository, but not others, even if they own them.
@@ -2345,6 +2386,32 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
faults.Unauthorized, None,
"authenticateWithPassword", username, "nonsense")
+ def test_authenticateWithPassword_private_ci_build(self):
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+ with person_logged_in(self.factory.makePerson()) as owner:
+ repository = self.factory.makeGitRepository(
+ owner=owner, information_type=InformationType.USERDATA)
+ build = self.factory.makeCIBuild(git_repository=repository)
+ issuer = getUtility(IMacaroonIssuer, "ci-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ for username in ("", "+launchpad-services"):
+ self.assertEqual(
+ {"macaroon": macaroon.serialize(),
+ "user": "+launchpad-services"},
+ self.assertDoesNotFault(
+ None, "authenticateWithPassword",
+ username, macaroon.serialize()))
+ other_macaroon = Macaroon(
+ identifier="another", key="another-secret")
+ self.assertFault(
+ faults.Unauthorized, None,
+ "authenticateWithPassword",
+ username, other_macaroon.serialize())
+ self.assertFault(
+ faults.Unauthorized, None,
+ "authenticateWithPassword", username, "nonsense")
+
def test_authenticateWithPassword_user_macaroon(self):
# A user with a suitable macaroon can authenticate using it, in
# which case we return both the macaroon and the uid for use by
@@ -2524,6 +2591,23 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
LAUNCHPAD_SERVICES, ref.repository, [path], {path: []},
macaroon_raw=macaroon.serialize())
+ def test_checkRefPermissions_private_ci_build(self):
+ # A builder with a suitable macaroon cannot write to a repository,
+ # even if it is associated with a running private CI build.
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+ with person_logged_in(self.factory.makePerson()) as owner:
+ [ref] = self.factory.makeGitRefs(
+ owner=owner, information_type=InformationType.USERDATA)
+ build = self.factory.makeCIBuild(git_repository=ref.repository)
+ issuer = getUtility(IMacaroonIssuer, "ci-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ build.updateStatus(BuildStatus.BUILDING)
+ path = ref.path.encode("UTF-8")
+ self.assertHasRefPermissions(
+ LAUNCHPAD_SERVICES, ref.repository, [path], {path: []},
+ macaroon_raw=macaroon.serialize())
+
def test_checkRefPermissions_user_macaroon(self):
# A user with a suitable macaroon has their ordinary privileges on
# the corresponding repository, but not others, even if they own
diff --git a/lib/lp/services/authserver/interfaces.py b/lib/lp/services/authserver/interfaces.py
index a1ec353..d94755f 100644
--- a/lib/lp/services/authserver/interfaces.py
+++ b/lib/lp/services/authserver/interfaces.py
@@ -33,8 +33,8 @@ class IAuthServer(Interface):
`issuable_via_authserver` is True are permitted.
:param context_type: A string identifying the type of context for
which to issue the macaroon. Currently only 'LibraryFileAlias',
- 'BinaryPackageBuild', 'LiveFSBuild', 'SnapBuild', and
- 'OCIRecipeBuild' are supported.
+ 'BinaryPackageBuild', 'LiveFSBuild', 'SnapBuild',
+ 'OCIRecipeBuild', and 'CIBuild' are supported.
:param context: The context for which to issue the macaroon. Note
that this is passed over XML-RPC, so it should be plain data
(e.g. an ID) rather than a database object.
@@ -47,7 +47,8 @@ class IAuthServer(Interface):
:param macaroon_raw: A serialised macaroon.
:param context_type: A string identifying the type of context to
check. Currently only 'LibraryFileAlias', 'BinaryPackageBuild',
- 'LiveFSBuild', 'SnapBuild', and 'OCIRecipeBuild' are supported.
+ 'LiveFSBuild', 'SnapBuild', 'OCIRecipeBuild', and 'CIBuild' are
+ supported.
:param context: The context to check. Note that this is passed over
XML-RPC, so it should be plain data (e.g. an ID) rather than a
database object.
diff --git a/lib/lp/services/authserver/xmlrpc.py b/lib/lp/services/authserver/xmlrpc.py
index dbee3e9..1c91bdf 100644
--- a/lib/lp/services/authserver/xmlrpc.py
+++ b/lib/lp/services/authserver/xmlrpc.py
@@ -15,6 +15,7 @@ from zope.interface import implementer
from zope.interface.interfaces import ComponentLookupError
from zope.security.proxy import removeSecurityProxy
+from lp.code.interfaces.cibuild import ICIBuildSet
from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
from lp.registry.interfaces.person import IPersonSet
from lp.services.authserver.interfaces import (
@@ -58,7 +59,8 @@ class AuthServerAPIView(LaunchpadXMLRPCView):
:param context_type: A string identifying the type of context.
Currently only 'LibraryFileAlias', 'BinaryPackageBuild',
- 'LiveFSBuild', 'SnapBuild', and 'OCIRecipeBuild' are supported.
+ 'LiveFSBuild', 'SnapBuild', 'OCIRecipeBuild', and 'CIBuild' are
+ supported.
:param context: The context as plain data (e.g. an ID).
:return: The resolved context, or None.
"""
@@ -78,8 +80,11 @@ class AuthServerAPIView(LaunchpadXMLRPCView):
# The context is a `SnapBuild` ID.
return getUtility(ISnapBuildSet).getByID(context)
elif context_type == 'OCIRecipeBuild':
- # The context is an OCIRecipe ID.
+ # The context is an `OCIRecipeBuild` ID.
return getUtility(IOCIRecipeBuildSet).getByID(context)
+ elif context_type == 'CIBuild':
+ # The context is a `CIBuild` ID.
+ return getUtility(ICIBuildSet).getByID(context)
else:
return None
