launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:bpb-macaroons-via-authserver into launchpad:master with ~cjwatson/launchpad:bpb-snapbuild-archive-macaroons as a prerequisite.

Commit message:
Allow issuing BPB macaroons via authserver

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400646

This will be needed to allow buildd-manager to issue macaroons allowing binary package builds to access private archives, since buildd-manager doesn't have access to the appropriate secret key and needs to use the authserver to do the work.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:bpb-macaroons-via-authserver into launchpad:master.

diff --git a/lib/lp/services/authserver/interfaces.py b/lib/lp/services/authserver/interfaces.py
index 3eea530..69c8d58 100644
--- a/lib/lp/services/authserver/interfaces.py
+++ b/lib/lp/services/authserver/interfaces.py
@@ -33,8 +33,9 @@ class IAuthServer(Interface):
         :param issuer_name: An `IMacaroonIssuer` name.  Only issuers where
             `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'
-            and 'SnapBuild' are supported.
+            which to issue the macaroon.  Currently only 'LibraryFileAlias',
+            'BinaryPackageBuild', 'SnapBuild', and 'OCIRecipeBuild' 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.
@@ -46,8 +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' and 'SnapBuild' are
-            supported.
+            check.  Currently only 'LibraryFileAlias', 'BinaryPackageBuild',
+            'SnapBuild', and 'OCIRecipeBuild' 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 4154424..2f8594d 100644
--- a/lib/lp/services/authserver/xmlrpc.py
+++ b/lib/lp/services/authserver/xmlrpc.py
@@ -34,6 +34,7 @@ from lp.services.macaroons.interfaces import (
     )
 from lp.services.webapp import LaunchpadXMLRPCView
 from lp.snappy.interfaces.snapbuild import ISnapBuildSet
+from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
 from lp.xmlrpc import faults
 
 
@@ -59,7 +60,8 @@ class AuthServerAPIView(LaunchpadXMLRPCView):
         """Resolve a serialised context.
 
         :param context_type: A string identifying the type of context.
-            Currently only 'LibraryFileAlias' and 'SnapBuild' are supported.
+            Currently only 'LibraryFileAlias', 'BinaryPackageBuild',
+            'SnapBuild', and 'OCIRecipeBuild' are supported.
         :param context: The context as plain data (e.g. an ID).
         :return: The resolved context, or None.
         """
@@ -69,6 +71,9 @@ class AuthServerAPIView(LaunchpadXMLRPCView):
                 return getUtility(ILibraryFileAliasSet)[context]
             except SQLObjectNotFound:
                 return None
+        elif context_type == 'BinaryPackageBuild':
+            # The context is a `BinaryPackageBuild` ID.
+            return getUtility(IBinaryPackageBuildSet).getByID(context)
         elif context_type == 'SnapBuild':
             # The context is a `SnapBuild` ID.
             return getUtility(ISnapBuildSet).getByID(context)
diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py
index b863798..322ff6c 100644
--- a/lib/lp/soyuz/model/binarypackagebuild.py
+++ b/lib/lp/soyuz/model/binarypackagebuild.py
@@ -1402,6 +1402,7 @@ class BinaryPackageBuildSet(SpecificBuildFarmJobSourceMixin):
 class BinaryPackageBuildMacaroonIssuer(MacaroonIssuerBase):
 
     identifier = "binary-package-build"
+    issuable_via_authserver = True
 
     @property
     def _primary_caveat_name(self):
diff --git a/lib/lp/soyuz/tests/test_binarypackagebuild.py b/lib/lp/soyuz/tests/test_binarypackagebuild.py
index b51f2b7..694ed7d 100644
--- a/lib/lp/soyuz/tests/test_binarypackagebuild.py
+++ b/lib/lp/soyuz/tests/test_binarypackagebuild.py
@@ -14,6 +14,7 @@ from pymacaroons import Macaroon
 import pytz
 from simplejson import dumps
 from testtools.matchers import (
+    Equals,
     MatchesListwise,
     MatchesStructure,
     )
@@ -70,7 +71,6 @@ from lp.testing.layers import (
     LaunchpadZopelessLayer,
     )
 from lp.testing.pages import webservice_for_person
-from lp.xmlrpc import faults
 from lp.xmlrpc.interfaces import IPrivateApplication
 
 
@@ -954,17 +954,24 @@ class TestBinaryPackageBuildMacaroonIssuer(
                 caveat_id="lp.principal.binary-package-build %s" % build.id),
             ]))
 
-    def test_issueMacaroon_not_via_authserver(self):
+    def test_issueMacaroon_via_authserver(self):
         build = self.factory.makeBinaryPackageBuild(
             archive=self.factory.makeArchive(private=True))
         private_root = getUtility(IPrivateApplication)
         authserver = AuthServerAPIView(private_root.authserver, TestRequest())
-        self.assertEqual(
-            faults.PermissionDenied(),
+        macaroon = Macaroon.deserialize(
             authserver.issueMacaroon(
-                "binary-package-build", "BinaryPackageBuild", build))
-
-    def test_verifyMacaroon_good(self):
+                "binary-package-build", "BinaryPackageBuild", build.id))
+        self.assertThat(macaroon, MatchesStructure(
+            location=Equals("launchpad.test"),
+            identifier=Equals("binary-package-build"),
+            caveats=MatchesListwise([
+                MatchesStructure.byEquality(
+                    caveat_id=(
+                        "lp.principal.binary-package-build %s" % build.id)),
+                ])))
+
+    def test_verifyMacaroon_good_file(self):
         build = self.factory.makeBinaryPackageBuild(
             archive=self.factory.makeArchive(private=True))
         sprf = self.factory.makeSourcePackageReleaseFile(