Diff comments:

> 
> === modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
> --- lib/lp/soyuz/model/binarypackagebuild.py  2017-03-29 09:28:09 +0000
> +++ lib/lp/soyuz/model/binarypackagebuild.py  2019-03-12 19:17:31 +0000
> @@ -1362,3 +1370,82 @@
>                      % (build.title, build.id, build.archive.displayname,
>                      build_queue.lastscore))
>          return new_builds
> +
> +
> +@implementer(IMacaroonIssuer)
> +class BinaryPackageBuildMacaroonIssuer:
> +
> +    @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`.
> +
> +        For issuing, the context is an `IBinaryPackageBuild`.
> +        """
> +        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)

The ambiguity is that it sounds like it's accessing the named BPB, but actually 
it's accessing all resources that should be accessible to a job acting on 
behalf of the BPB. I don't know how to solve it, and it's not a blocker, but 
the inclarity is not ideal.

> +        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`.
> +
> +        For verification, the context is a `LibraryFileAlias` ID.  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.
> +        """

We can't send an LFA over XML-RPC, but we need to indicate that the value is an 
LFA ID by method name, parameter name, value prefix, or some equivalent 
mechanism.  Having the generic authserver verifyMacaroon method take an 
unmarked LFA ID is just asking for trouble when we start adding more object 
types, and it's going to be slightly annoying to fix later.

> +        # Circular import.
> +        from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
> +
> +        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


-- 
https://code.launchpad.net/~cjwatson/launchpad/librarian-accept-macaroon/+merge/345079
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to