Merge ~cjwatson/launchpad:cibuild-macaroons into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 163146dfdb0fde5d0a9496a5729c0e73ab08c020
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:cibuild-macaroons
Merge into: launchpad:master
Diff against target: 466 lines (+315/-5)
6 files modified
lib/lp/code/configure.zcml (+8/-0)
lib/lp/code/model/cibuild.py (+63/-0)
lib/lp/code/model/tests/test_cibuild.py (+149/-0)
lib/lp/code/xmlrpc/tests/test_git.py (+84/-0)
lib/lp/services/authserver/interfaces.py (+4/-3)
lib/lp/services/authserver/xmlrpc.py (+7/-2)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+419976@code.launchpad.net

Commit message

Add CIBuild macaroons

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Looks good from comparing with the other build types (not that I fully understand every bit) :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
2index 882d3da..99a5bf0 100644
3--- a/lib/lp/code/configure.zcml
4+++ b/lib/lp/code/configure.zcml
5@@ -1307,6 +1307,14 @@
6 <allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource" />
7 </securedutility>
8
9+ <!-- CIBuildMacaroonIssuer -->
10+ <securedutility
11+ class="lp.code.model.cibuild.CIBuildMacaroonIssuer"
12+ provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
13+ name="ci-build">
14+ <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic" />
15+ </securedutility>
16+
17 <!-- CIBuildBehaviour -->
18 <adapter
19 for="lp.code.interfaces.cibuild.ICIBuild"
20diff --git a/lib/lp/code/model/cibuild.py b/lib/lp/code/model/cibuild.py
21index 912d31f..dd2390f 100644
22--- a/lib/lp/code/model/cibuild.py
23+++ b/lib/lp/code/model/cibuild.py
24@@ -25,6 +25,7 @@ from storm.store import EmptyResultSet
25 from zope.component import getUtility
26 from zope.event import notify
27 from zope.interface import implementer
28+from zope.security.proxy import removeSecurityProxy
29
30 from lp.app.errors import NotFoundError
31 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
32@@ -51,6 +52,7 @@ from lp.code.interfaces.cibuild import (
33 MissingConfiguration,
34 )
35 from lp.code.interfaces.githosting import IGitHostingClient
36+from lp.code.interfaces.gitrepository import IGitRepository
37 from lp.code.interfaces.revisionstatus import IRevisionStatusReportSet
38 from lp.code.model.gitref import GitRef
39 from lp.code.model.lpcraft import load_configuration
40@@ -72,6 +74,12 @@ from lp.services.librarian.model import (
41 LibraryFileAlias,
42 LibraryFileContent,
43 )
44+from lp.services.macaroons.interfaces import (
45+ BadMacaroonContext,
46+ IMacaroonIssuer,
47+ NO_USER,
48+ )
49+from lp.services.macaroons.model import MacaroonIssuerBase
50 from lp.services.propertycache import cachedproperty
51 from lp.soyuz.model.distroarchseries import DistroArchSeries
52
53@@ -596,3 +604,58 @@ class CIBuildSet(SpecificBuildFarmJobSourceMixin):
54 def deleteByGitRepository(self, git_repository):
55 """See `ICIBuildSet`."""
56 self.findByGitRepository(git_repository).remove()
57+
58+
59+@implementer(IMacaroonIssuer)
60+class CIBuildMacaroonIssuer(MacaroonIssuerBase):
61+
62+ identifier = "ci-build"
63+ issuable_via_authserver = True
64+
65+ def checkIssuingContext(self, context, **kwargs):
66+ """See `MacaroonIssuerBase`.
67+
68+ For issuing, the context is an `ICIBuild`.
69+ """
70+ if not ICIBuild.providedBy(context):
71+ raise BadMacaroonContext(context)
72+ return removeSecurityProxy(context).id
73+
74+ def checkVerificationContext(self, context, **kwargs):
75+ """See `MacaroonIssuerBase`."""
76+ if not IGitRepository.providedBy(context):
77+ raise BadMacaroonContext(context)
78+ return context
79+
80+ def verifyPrimaryCaveat(self, verified, caveat_value, context, user=None,
81+ **kwargs):
82+ """See `MacaroonIssuerBase`.
83+
84+ For verification, the context is an `IGitRepository`. We check that
85+ the repository or archive is needed to build the `ICIBuild` that is
86+ the context of the macaroon, and that the context build is currently
87+ building.
88+ """
89+ # CI builds only support free-floating macaroons for Git
90+ # authentication, not ones bound to a user.
91+ if user:
92+ return False
93+ verified.user = NO_USER
94+
95+ if context is None:
96+ # We're only verifying that the macaroon could be valid for some
97+ # context.
98+ return True
99+ if not IGitRepository.providedBy(context):
100+ return False
101+
102+ try:
103+ build_id = int(caveat_value)
104+ except ValueError:
105+ return False
106+ clauses = [
107+ CIBuild.id == build_id,
108+ CIBuild.status == BuildStatus.BUILDING,
109+ CIBuild.git_repository == context,
110+ ]
111+ return not IStore(CIBuild).find(CIBuild, *clauses).is_empty()
112diff --git a/lib/lp/code/model/tests/test_cibuild.py b/lib/lp/code/model/tests/test_cibuild.py
113index d4cc4ec..3f9682e 100644
114--- a/lib/lp/code/model/tests/test_cibuild.py
115+++ b/lib/lp/code/model/tests/test_cibuild.py
116@@ -12,14 +12,17 @@ from textwrap import dedent
117 from unittest.mock import Mock
118
119 from fixtures import MockPatchObject
120+from pymacaroons import Macaroon
121 import pytz
122 from storm.locals import Store
123 from testtools.matchers import (
124 Equals,
125+ MatchesListwise,
126 MatchesSetwise,
127 MatchesStructure,
128 )
129 from zope.component import getUtility
130+from zope.publisher.xmlrpc import TestRequest
131 from zope.security.proxy import removeSecurityProxy
132
133 from lp.app.enums import InformationType
134@@ -53,7 +56,11 @@ from lp.code.model.cibuild import (
135 from lp.code.model.lpcraft import load_configuration
136 from lp.code.tests.helpers import GitHostingFixture
137 from lp.registry.interfaces.series import SeriesStatus
138+from lp.services.authserver.xmlrpc import AuthServerAPIView
139+from lp.services.config import config
140 from lp.services.log.logger import BufferLogger
141+from lp.services.macaroons.interfaces import IMacaroonIssuer
142+from lp.services.macaroons.testing import MacaroonTestMixin
143 from lp.services.propertycache import clear_property_cache
144 from lp.testing import (
145 person_logged_in,
146@@ -62,6 +69,7 @@ from lp.testing import (
147 )
148 from lp.testing.layers import LaunchpadZopelessLayer
149 from lp.testing.matchers import HasQueryCount
150+from lp.xmlrpc.interfaces import IPrivateApplication
151
152
153 class TestGetAllCommitsForPaths(TestCaseWithFactory):
154@@ -983,3 +991,144 @@ class TestDetermineDASesToBuild(TestCaseWithFactory):
155 "name in Ubuntu %s\n" % distro_series.name,
156 logger.getLogBuffer()
157 )
158+
159+
160+class TestCIBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
161+ """Test CIBuild macaroon issuing and verification."""
162+
163+ layer = LaunchpadZopelessLayer
164+
165+ def setUp(self):
166+ super().setUp()
167+ self.pushConfig(
168+ "launchpad", internal_macaroon_secret_key="some-secret")
169+
170+ def test_issueMacaroon_good(self):
171+ build = self.factory.makeCIBuild(
172+ git_repository=self.factory.makeGitRepository(
173+ information_type=InformationType.USERDATA))
174+ issuer = getUtility(IMacaroonIssuer, "ci-build")
175+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
176+ self.assertThat(macaroon, MatchesStructure(
177+ location=Equals("launchpad.test"),
178+ identifier=Equals("ci-build"),
179+ caveats=MatchesListwise([
180+ MatchesStructure.byEquality(
181+ caveat_id="lp.ci-build %s" % build.id),
182+ ])))
183+
184+ def test_issueMacaroon_via_authserver(self):
185+ build = self.factory.makeCIBuild(
186+ git_repository=self.factory.makeGitRepository(
187+ information_type=InformationType.USERDATA))
188+ private_root = getUtility(IPrivateApplication)
189+ authserver = AuthServerAPIView(private_root.authserver, TestRequest())
190+ macaroon = Macaroon.deserialize(
191+ authserver.issueMacaroon("ci-build", "CIBuild", build.id))
192+ self.assertThat(macaroon, MatchesStructure(
193+ location=Equals("launchpad.test"),
194+ identifier=Equals("ci-build"),
195+ caveats=MatchesListwise([
196+ MatchesStructure.byEquality(
197+ caveat_id="lp.ci-build %s" % build.id),
198+ ])))
199+
200+ def test_verifyMacaroon_good_repository(self):
201+ build = self.factory.makeCIBuild(
202+ git_repository=self.factory.makeGitRepository(
203+ information_type=InformationType.USERDATA))
204+ build.updateStatus(BuildStatus.BUILDING)
205+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
206+ macaroon = issuer.issueMacaroon(build)
207+ self.assertMacaroonVerifies(issuer, macaroon, build.git_repository)
208+
209+ def test_verifyMacaroon_good_no_context(self):
210+ build = self.factory.makeCIBuild(
211+ git_repository=self.factory.makeGitRepository(
212+ information_type=InformationType.USERDATA))
213+ build.updateStatus(BuildStatus.BUILDING)
214+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
215+ macaroon = issuer.issueMacaroon(build)
216+ self.assertMacaroonVerifies(
217+ issuer, macaroon, None, require_context=False)
218+ self.assertMacaroonVerifies(
219+ issuer, macaroon, build.git_repository, require_context=False)
220+
221+ def test_verifyMacaroon_no_context_but_require_context(self):
222+ build = self.factory.makeCIBuild(
223+ git_repository=self.factory.makeGitRepository(
224+ information_type=InformationType.USERDATA))
225+ build.updateStatus(BuildStatus.BUILDING)
226+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
227+ macaroon = issuer.issueMacaroon(build)
228+ self.assertMacaroonDoesNotVerify(
229+ ["Expected macaroon verification context but got None."],
230+ issuer, macaroon, None)
231+
232+ def test_verifyMacaroon_wrong_location(self):
233+ build = self.factory.makeCIBuild(
234+ git_repository=self.factory.makeGitRepository(
235+ information_type=InformationType.USERDATA))
236+ build.updateStatus(BuildStatus.BUILDING)
237+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
238+ macaroon = Macaroon(
239+ location="another-location", key=issuer._root_secret)
240+ self.assertMacaroonDoesNotVerify(
241+ ["Macaroon has unknown location 'another-location'."],
242+ issuer, macaroon, build.git_repository)
243+ self.assertMacaroonDoesNotVerify(
244+ ["Macaroon has unknown location 'another-location'."],
245+ issuer, macaroon, build.git_repository, require_context=False)
246+
247+ def test_verifyMacaroon_wrong_key(self):
248+ build = self.factory.makeCIBuild(
249+ git_repository=self.factory.makeGitRepository(
250+ information_type=InformationType.USERDATA))
251+ build.updateStatus(BuildStatus.BUILDING)
252+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
253+ macaroon = Macaroon(
254+ location=config.vhost.mainsite.hostname, key="another-secret")
255+ self.assertMacaroonDoesNotVerify(
256+ ["Signatures do not match"],
257+ issuer, macaroon, build.git_repository)
258+ self.assertMacaroonDoesNotVerify(
259+ ["Signatures do not match"],
260+ issuer, macaroon, build.git_repository, require_context=False)
261+
262+ def test_verifyMacaroon_not_building(self):
263+ build = self.factory.makeCIBuild(
264+ git_repository=self.factory.makeGitRepository(
265+ information_type=InformationType.USERDATA))
266+ issuer = removeSecurityProxy(
267+ getUtility(IMacaroonIssuer, "ci-build"))
268+ macaroon = issuer.issueMacaroon(build)
269+ self.assertMacaroonDoesNotVerify(
270+ ["Caveat check for 'lp.ci-build %s' failed." % build.id],
271+ issuer, macaroon, build.git_repository)
272+
273+ def test_verifyMacaroon_wrong_build(self):
274+ build = self.factory.makeCIBuild(
275+ git_repository=self.factory.makeGitRepository(
276+ information_type=InformationType.USERDATA))
277+ build.updateStatus(BuildStatus.BUILDING)
278+ other_build = self.factory.makeCIBuild(
279+ git_repository=self.factory.makeGitRepository(
280+ information_type=InformationType.USERDATA))
281+ other_build.updateStatus(BuildStatus.BUILDING)
282+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
283+ macaroon = issuer.issueMacaroon(other_build)
284+ self.assertMacaroonDoesNotVerify(
285+ ["Caveat check for 'lp.ci-build %s' failed." % other_build.id],
286+ issuer, macaroon, build.git_repository)
287+
288+ def test_verifyMacaroon_wrong_repository(self):
289+ build = self.factory.makeCIBuild(
290+ git_repository=self.factory.makeGitRepository(
291+ information_type=InformationType.USERDATA))
292+ other_repository = self.factory.makeGitRepository()
293+ build.updateStatus(BuildStatus.BUILDING)
294+ issuer = removeSecurityProxy(getUtility(IMacaroonIssuer, "ci-build"))
295+ macaroon = issuer.issueMacaroon(build)
296+ self.assertMacaroonDoesNotVerify(
297+ ["Caveat check for 'lp.ci-build %s' failed." % build.id],
298+ issuer, macaroon, other_repository)
299diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
300index 9dd57c1..f87abb7 100644
301--- a/lib/lp/code/xmlrpc/tests/test_git.py
302+++ b/lib/lp/code/xmlrpc/tests/test_git.py
303@@ -1803,6 +1803,47 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
304 repository.registrant, path, permission="read",
305 macaroon_raw=macaroons[0].serialize())
306
307+ def test_translatePath_private_ci_build(self):
308+ # A builder with a suitable macaroon can read from a repository
309+ # associated with a running private CI build.
310+ self.pushConfig(
311+ "launchpad", internal_macaroon_secret_key="some-secret")
312+ with person_logged_in(self.factory.makePerson()) as owner:
313+ repositories = [
314+ self.factory.makeGitRepository(
315+ owner=owner, information_type=InformationType.USERDATA)
316+ for _ in range(2)]
317+ builds = [
318+ self.factory.makeCIBuild(git_repository=repository)
319+ for repository in repositories]
320+ issuer = getUtility(IMacaroonIssuer, "ci-build")
321+ macaroons = [
322+ removeSecurityProxy(issuer).issueMacaroon(build)
323+ for build in builds]
324+ repository = repositories[0]
325+ path = "/%s" % repository.unique_name
326+ self.assertUnauthorized(
327+ LAUNCHPAD_SERVICES, path, permission="write",
328+ macaroon_raw=macaroons[0].serialize())
329+ removeSecurityProxy(builds[0]).updateStatus(BuildStatus.BUILDING)
330+ self.assertTranslates(
331+ LAUNCHPAD_SERVICES, path, repository, False, permission="read",
332+ macaroon_raw=macaroons[0].serialize(), private=True)
333+ self.assertUnauthorized(
334+ LAUNCHPAD_SERVICES, path, permission="read",
335+ macaroon_raw=macaroons[1].serialize())
336+ self.assertUnauthorized(
337+ LAUNCHPAD_SERVICES, path, permission="read",
338+ macaroon_raw=Macaroon(
339+ location=config.vhost.mainsite.hostname, identifier="another",
340+ key="another-secret").serialize())
341+ self.assertUnauthorized(
342+ LAUNCHPAD_SERVICES, path, permission="read",
343+ macaroon_raw="nonsense")
344+ self.assertUnauthorized(
345+ removeSecurityProxy(repository).registrant, path,
346+ permission="read", macaroon_raw=macaroons[0].serialize())
347+
348 def test_translatePath_user_macaroon(self):
349 # A user with a suitable macaroon can write to the corresponding
350 # repository, but not others, even if they own them.
351@@ -2345,6 +2386,32 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
352 faults.Unauthorized, None,
353 "authenticateWithPassword", username, "nonsense")
354
355+ def test_authenticateWithPassword_private_ci_build(self):
356+ self.pushConfig(
357+ "launchpad", internal_macaroon_secret_key="some-secret")
358+ with person_logged_in(self.factory.makePerson()) as owner:
359+ repository = self.factory.makeGitRepository(
360+ owner=owner, information_type=InformationType.USERDATA)
361+ build = self.factory.makeCIBuild(git_repository=repository)
362+ issuer = getUtility(IMacaroonIssuer, "ci-build")
363+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
364+ for username in ("", "+launchpad-services"):
365+ self.assertEqual(
366+ {"macaroon": macaroon.serialize(),
367+ "user": "+launchpad-services"},
368+ self.assertDoesNotFault(
369+ None, "authenticateWithPassword",
370+ username, macaroon.serialize()))
371+ other_macaroon = Macaroon(
372+ identifier="another", key="another-secret")
373+ self.assertFault(
374+ faults.Unauthorized, None,
375+ "authenticateWithPassword",
376+ username, other_macaroon.serialize())
377+ self.assertFault(
378+ faults.Unauthorized, None,
379+ "authenticateWithPassword", username, "nonsense")
380+
381 def test_authenticateWithPassword_user_macaroon(self):
382 # A user with a suitable macaroon can authenticate using it, in
383 # which case we return both the macaroon and the uid for use by
384@@ -2524,6 +2591,23 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
385 LAUNCHPAD_SERVICES, ref.repository, [path], {path: []},
386 macaroon_raw=macaroon.serialize())
387
388+ def test_checkRefPermissions_private_ci_build(self):
389+ # A builder with a suitable macaroon cannot write to a repository,
390+ # even if it is associated with a running private CI build.
391+ self.pushConfig(
392+ "launchpad", internal_macaroon_secret_key="some-secret")
393+ with person_logged_in(self.factory.makePerson()) as owner:
394+ [ref] = self.factory.makeGitRefs(
395+ owner=owner, information_type=InformationType.USERDATA)
396+ build = self.factory.makeCIBuild(git_repository=ref.repository)
397+ issuer = getUtility(IMacaroonIssuer, "ci-build")
398+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
399+ build.updateStatus(BuildStatus.BUILDING)
400+ path = ref.path.encode("UTF-8")
401+ self.assertHasRefPermissions(
402+ LAUNCHPAD_SERVICES, ref.repository, [path], {path: []},
403+ macaroon_raw=macaroon.serialize())
404+
405 def test_checkRefPermissions_user_macaroon(self):
406 # A user with a suitable macaroon has their ordinary privileges on
407 # the corresponding repository, but not others, even if they own
408diff --git a/lib/lp/services/authserver/interfaces.py b/lib/lp/services/authserver/interfaces.py
409index a1ec353..d94755f 100644
410--- a/lib/lp/services/authserver/interfaces.py
411+++ b/lib/lp/services/authserver/interfaces.py
412@@ -33,8 +33,8 @@ class IAuthServer(Interface):
413 `issuable_via_authserver` is True are permitted.
414 :param context_type: A string identifying the type of context for
415 which to issue the macaroon. Currently only 'LibraryFileAlias',
416- 'BinaryPackageBuild', 'LiveFSBuild', 'SnapBuild', and
417- 'OCIRecipeBuild' are supported.
418+ 'BinaryPackageBuild', 'LiveFSBuild', 'SnapBuild',
419+ 'OCIRecipeBuild', and 'CIBuild' are supported.
420 :param context: The context for which to issue the macaroon. Note
421 that this is passed over XML-RPC, so it should be plain data
422 (e.g. an ID) rather than a database object.
423@@ -47,7 +47,8 @@ class IAuthServer(Interface):
424 :param macaroon_raw: A serialised macaroon.
425 :param context_type: A string identifying the type of context to
426 check. Currently only 'LibraryFileAlias', 'BinaryPackageBuild',
427- 'LiveFSBuild', 'SnapBuild', and 'OCIRecipeBuild' are supported.
428+ 'LiveFSBuild', 'SnapBuild', 'OCIRecipeBuild', and 'CIBuild' are
429+ supported.
430 :param context: The context to check. Note that this is passed over
431 XML-RPC, so it should be plain data (e.g. an ID) rather than a
432 database object.
433diff --git a/lib/lp/services/authserver/xmlrpc.py b/lib/lp/services/authserver/xmlrpc.py
434index dbee3e9..1c91bdf 100644
435--- a/lib/lp/services/authserver/xmlrpc.py
436+++ b/lib/lp/services/authserver/xmlrpc.py
437@@ -15,6 +15,7 @@ from zope.interface import implementer
438 from zope.interface.interfaces import ComponentLookupError
439 from zope.security.proxy import removeSecurityProxy
440
441+from lp.code.interfaces.cibuild import ICIBuildSet
442 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
443 from lp.registry.interfaces.person import IPersonSet
444 from lp.services.authserver.interfaces import (
445@@ -58,7 +59,8 @@ class AuthServerAPIView(LaunchpadXMLRPCView):
446
447 :param context_type: A string identifying the type of context.
448 Currently only 'LibraryFileAlias', 'BinaryPackageBuild',
449- 'LiveFSBuild', 'SnapBuild', and 'OCIRecipeBuild' are supported.
450+ 'LiveFSBuild', 'SnapBuild', 'OCIRecipeBuild', and 'CIBuild' are
451+ supported.
452 :param context: The context as plain data (e.g. an ID).
453 :return: The resolved context, or None.
454 """
455@@ -78,8 +80,11 @@ class AuthServerAPIView(LaunchpadXMLRPCView):
456 # The context is a `SnapBuild` ID.
457 return getUtility(ISnapBuildSet).getByID(context)
458 elif context_type == 'OCIRecipeBuild':
459- # The context is an OCIRecipe ID.
460+ # The context is an `OCIRecipeBuild` ID.
461 return getUtility(IOCIRecipeBuildSet).getByID(context)
462+ elif context_type == 'CIBuild':
463+ # The context is a `CIBuild` ID.
464+ return getUtility(ICIBuildSet).getByID(context)
465 else:
466 return None
467

Subscribers

People subscribed via source and target branches

to status/vote changes: