launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23504
[Merge] lp:~cjwatson/launchpad/git-issue-access-tokens into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-issue-access-tokens into lp:launchpad with lp:~cjwatson/launchpad/refactor-code-import-macaroons as a prerequisite.
Commit message:
Add a macaroon issuer for Git access tokens.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1824399 in Launchpad itself: "Add Git HTTPS push tokens for snapcraft experiment"
https://bugs.launchpad.net/launchpad/+bug/1824399
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-issue-access-tokens/+merge/366052
These aren't honoured by the XML-RPC API yet, and there's no way for users to actually request them. Those will come in subsequent branches.
The expiry/persistence policy will likely need to change in future, but the current expiry time is short enough that we should be able to evolve this without too much difficulty.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-issue-access-tokens into lp:launchpad.
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2018-10-16 15:52:00 +0000
+++ lib/lp/code/configure.zcml 2019-04-15 11:35:18 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2019 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -856,6 +856,15 @@
<allow interface="lp.code.interfaces.gitrepository.IGitRepositorySet" />
</securedutility>
+ <!-- GitRepositoryMacaroonIssuer -->
+
+ <securedutility
+ class="lp.code.model.gitrepository.GitRepositoryMacaroonIssuer"
+ provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
+ name="git-repository">
+ <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic" />
+ </securedutility>
+
<!-- GitRepositoryDelta -->
<class class="lp.code.adapters.gitrepository.GitRepositoryDelta">
=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py 2019-04-15 11:35:16 +0000
+++ lib/lp/code/model/codeimportjob.py 2019-04-15 11:35:18 +0000
@@ -431,7 +431,7 @@
raise ValueError("context.code_import.git_repository is None")
return context.id
- def checkVerificationContext(self, context):
+ def checkVerificationContext(self, context, **kwargs):
"""See `MacaroonIssuerBase`.
For verification, the context may be an `ICodeImportJob`, in which
@@ -454,7 +454,7 @@
raise ValueError("%r is not in the RUNNING state." % context)
return context
- def verifyPrimaryCaveat(self, caveat_value, context):
+ def verifyPrimaryCaveat(self, caveat_value, context, **kwargs):
"""See `MacaroonIssuerBase`."""
if context is None:
# We're only verifying that the macaroon could be valid for some
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2019-04-02 08:12:27 +0000
+++ lib/lp/code/model/gitrepository.py 2019-04-15 11:35:18 +0000
@@ -13,7 +13,10 @@
defaultdict,
OrderedDict,
)
-from datetime import datetime
+from datetime import (
+ datetime,
+ timedelta,
+ )
import email
from fnmatch import fnmatch
from functools import partial
@@ -174,6 +177,7 @@
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.enumcol import EnumCol
from lp.services.database.interfaces import IStore
+from lp.services.database.sqlbase import get_transaction_timestamp
from lp.services.database.stormbase import StormBase
from lp.services.database.stormexpr import (
Array,
@@ -182,8 +186,12 @@
BulkUpdate,
Values,
)
+from lp.services.features import getFeatureFlag
+from lp.services.identity.interfaces.account import IAccountSet
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
+from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.model import MacaroonIssuerBase
from lp.services.mail.notificationrecipientset import NotificationRecipientSet
from lp.services.propertycache import (
cachedproperty,
@@ -1764,6 +1772,98 @@
repository.project_id: repository for repository in repositories}
+@implementer(IMacaroonIssuer)
+class GitRepositoryMacaroonIssuer(MacaroonIssuerBase):
+
+ identifier = "git-repository"
+ allow_multiple = {"lp.expires"}
+
+ _timestamp_format = "%Y-%m-%dT%H:%M:%S.%f"
+
+ def __init__(self):
+ super(GitRepositoryMacaroonIssuer, self).__init__()
+ self.checkers = {
+ "lp.openid-identifier": self.verifyOpenIDIdentifier,
+ "lp.expires": self.verifyExpires,
+ }
+
+ def checkIssuingContext(self, context):
+ """See `MacaroonIssuerBase`.
+
+ For issuing, the context is an `IGitRepository`.
+ """
+ if not IGitRepository.providedBy(context):
+ raise ValueError("Cannot handle context %r." % context)
+ return context.id
+
+ def issueMacaroon(self, context):
+ """See `IMacaroonIssuer`."""
+ user = getUtility(ILaunchBag).user
+ if user is None:
+ raise Unauthorized(
+ "git-repository macaroons may only be issued for a logged-in "
+ "user.")
+ macaroon = super(GitRepositoryMacaroonIssuer, self).issueMacaroon(
+ context)
+ macaroon.add_first_party_caveat(
+ "lp.openid-identifier " +
+ user.account.openid_identifiers.any().identifier)
+ store = IStore(GitRepository)
+ # XXX cjwatson 2019-04-09: Expire macaroons after the number of
+ # seconds given in the code.git.access_token_expiry feature flag,
+ # defaulting to a week. This isn't very flexible, but for now it
+ # saves on having to implement macaroon persistence in order that
+ # users can revoke them.
+ expiry_seconds_str = getFeatureFlag("code.git.access_token_expiry")
+ if expiry_seconds_str is None:
+ expiry_seconds = 60 * 60 * 24 * 7
+ else:
+ expiry_seconds = int(expiry_seconds_str)
+ expiry = (
+ get_transaction_timestamp(store) +
+ timedelta(seconds=expiry_seconds))
+ macaroon.add_first_party_caveat(
+ "lp.expires " + expiry.strftime(self._timestamp_format))
+ return macaroon
+
+ def checkVerificationContext(self, context, **kwargs):
+ """See `MacaroonIssuerBase`.
+
+ For verification, the context is an `IGitRepository`.
+ """
+ if not IGitRepository.providedBy(context):
+ raise ValueError("Cannot handle context %r." % context)
+ return context
+
+ def verifyPrimaryCaveat(self, caveat_value, context, **kwargs):
+ """See `MacaroonIssuerBase`."""
+ if context is None:
+ # We're only verifying that the macaroon could be valid for some
+ # context.
+ return True
+ return caveat_value == str(context.id)
+
+ def verifyOpenIDIdentifier(self, caveat_value, context, **kwargs):
+ """Verify an lp.openid-identifier caveat."""
+ user = kwargs.get("user")
+ try:
+ account = getUtility(IAccountSet).getByOpenIDIdentifier(
+ caveat_value)
+ except LookupError:
+ return False
+ return user == IPerson(account)
+
+ def verifyExpires(self, caveat_value, context, **kwargs):
+ """Verify an lp.expires caveat."""
+ try:
+ expires = datetime.strptime(
+ caveat_value, self._timestamp_format).replace(tzinfo=pytz.UTC)
+ except ValueError:
+ return False
+ store = IStore(GitRepository)
+ return get_transaction_timestamp(store) < expires
+
+
def get_git_repository_privacy_filter(user, repository_class=GitRepository):
public_filter = repository_class.information_type.is_in(
PUBLIC_INFORMATION_TYPES)
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2019-02-11 12:31:06 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2019-04-15 11:35:18 +0000
@@ -17,12 +17,15 @@
import json
from bzrlib import urlutils
+from fixtures import MockPatch
from lazr.lifecycle.event import ObjectModifiedEvent
+from pymacaroons import Macaroon
import pytz
from sqlobject import SQLObjectNotFound
from storm.exceptions import LostObjectError
from storm.store import Store
from testtools.matchers import (
+ AnyMatch,
EndsWith,
Equals,
Is,
@@ -34,6 +37,7 @@
)
import transaction
from zope.component import getUtility
+from zope.publisher.xmlrpc import TestRequest
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
@@ -127,14 +131,22 @@
)
from lp.registry.interfaces.personproduct import IPersonProductFactory
from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
+from lp.services.authserver.xmlrpc import AuthServerAPIView
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import get_transaction_timestamp
+from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
from lp.services.job.runner import JobRunner
+from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.testing import (
+ find_caveats_by_name,
+ MacaroonTestMixin,
+ )
from lp.services.mail import stub
+from lp.services.openid.model.openididentifier import OpenIdIdentifier
from lp.services.propertycache import clear_property_cache
from lp.services.utils import seconds_since_epoch
from lp.services.webapp.authorization import check_permission
@@ -163,6 +175,8 @@
HasQueryCount,
)
from lp.testing.pages import webservice_for_person
+from lp.xmlrpc import faults
+from lp.xmlrpc.interfaces import IPrivateApplication
class TestGitRepository(TestCaseWithFactory):
@@ -3894,3 +3908,216 @@
"refs/heads/next": Equals(["push", "force-push"]),
"refs/other": Equals([]),
}))
+
+
+class TestGitRepositoryMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
+ """Test GitRepository macaroon issuing and verification."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestGitRepositoryMacaroonIssuer, self).setUp()
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+
+ def test_issueMacaroon_refuses_branch(self):
+ branch = self.factory.makeAnyBranch()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(branch.owner):
+ self.assertRaises(
+ ValueError, removeSecurityProxy(issuer).issueMacaroon, branch)
+
+ def test_issueMacaroon_good(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ identifier = (
+ repository.owner.account.openid_identifiers.any().identifier)
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ now = get_transaction_timestamp(Store.of(repository))
+ expires = now + timedelta(days=7)
+ self.assertThat(macaroon, MatchesStructure(
+ location=Equals(config.vhost.mainsite.hostname),
+ identifier=Equals("git-repository"),
+ caveats=MatchesListwise([
+ MatchesStructure.byEquality(
+ caveat_id="lp.git-repository %s" % repository.id),
+ MatchesStructure.byEquality(
+ caveat_id="lp.openid-identifier %s" % identifier),
+ MatchesStructure.byEquality(
+ caveat_id="lp.expires %s" % (
+ expires.strftime("%Y-%m-%dT%H:%M:%S.%f"))),
+ ])))
+
+ def test_issueMacaroon_expiry_feature_flag(self):
+ self.useFixture(FeatureFixture(
+ {"code.git.access_token_expiry": "3600"}))
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ now = get_transaction_timestamp(Store.of(repository))
+ expires = now + timedelta(hours=1)
+ self.assertThat(macaroon, MatchesStructure(
+ caveats=AnyMatch(
+ MatchesStructure.byEquality(
+ caveat_id="lp.expires %s" % (
+ expires.strftime("%Y-%m-%dT%H:%M:%S.%f"))))))
+
+ def test_issueMacaroon_no_user(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ self.assertRaises(
+ Unauthorized,
+ removeSecurityProxy(issuer).issueMacaroon, repository)
+
+ def test_issueMacaroon_not_via_authserver(self):
+ repository = self.factory.makeGitRepository()
+ private_root = getUtility(IPrivateApplication)
+ authserver = AuthServerAPIView(private_root.authserver, TestRequest())
+ self.assertEqual(
+ faults.PermissionDenied(),
+ authserver.issueMacaroon("git-repository", repository))
+
+ def test_verifyMacaroon_good(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ self.assertMacaroonVerifies(
+ issuer, macaroon, repository, user=repository.owner)
+
+ def test_verifyMacaroon_wrong_location(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ macaroon = Macaroon(
+ location="another-location",
+ key=removeSecurityProxy(issuer)._root_secret)
+ self.assertMacaroonDoesNotVerify(
+ ["Macaroon has unknown location 'another-location'."],
+ issuer, macaroon, repository, user=repository.owner)
+
+ def test_verifyMacaroon_wrong_key(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname, key="another-secret")
+ self.assertMacaroonDoesNotVerify(
+ ["Signatures do not match."],
+ issuer, macaroon, repository, user=repository.owner)
+
+ def test_verifyMacaroon_wrong_repository(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.git-repository %s' failed." %
+ repository.id],
+ issuer, macaroon, self.factory.makeGitRepository(),
+ user=repository.owner)
+
+ def test_verifyMacaroon_multiple_repository_caveats(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ macaroon.add_first_party_caveat("lp.git-repository another")
+ self.assertMacaroonDoesNotVerify(
+ ["Multiple 'lp.git-repository' caveats are not allowed."],
+ issuer, macaroon, repository, user=repository.owner)
+
+ def test_verifyMacaroon_wrong_user(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ identifier = (
+ repository.owner.account.openid_identifiers.any().identifier)
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.openid-identifier %s' failed." %
+ identifier],
+ issuer, macaroon, repository, user=self.factory.makePerson())
+
+ def test_verifyMacaroon_closed_account(self):
+ # A closed account no longer has an OpenID identifier, so the
+ # corresponding caveat doesn't match.
+ repository = self.factory.makeGitRepository()
+ owner = repository.owner
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(owner):
+ identifier = owner.account.openid_identifiers.any().identifier
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ IStore(OpenIdIdentifier).find(
+ OpenIdIdentifier,
+ OpenIdIdentifier.account_id == owner.account.id).remove()
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for 'lp.openid-identifier %s' failed." %
+ identifier],
+ issuer, macaroon, repository, user=owner)
+
+ def test_verifyMacaroon_multiple_openid_identifier_caveats(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ macaroon.add_first_party_caveat("lp.openid-identifier another")
+ self.assertMacaroonDoesNotVerify(
+ ["Multiple 'lp.openid-identifier' caveats are not allowed."],
+ issuer, macaroon, repository, user=repository.owner)
+
+ def test_verifyMacaroon_expired(self):
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(repository)
+ now = get_transaction_timestamp(Store.of(repository))
+ self.useFixture(MockPatch(
+ "lp.code.model.gitrepository.get_transaction_timestamp",
+ lambda _: now + timedelta(days=8)))
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for '%s' failed." %
+ find_caveats_by_name(macaroon, "lp.expires")[0].caveat_id],
+ issuer, macaroon, repository, user=repository.owner)
+
+ def test_verifyMacaroon_multiple_expires_caveats(self):
+ # If somebody attaches another expires caveat to the macaroon,
+ # that's OK; we just take the strictest.
+ repository = self.factory.makeGitRepository()
+ issuer = getUtility(IMacaroonIssuer, "git-repository")
+ with person_logged_in(repository.owner):
+ macaroon1 = removeSecurityProxy(issuer).issueMacaroon(repository)
+ macaroon2 = removeSecurityProxy(issuer).issueMacaroon(repository)
+ now = get_transaction_timestamp(Store.of(repository))
+ expires1 = now + timedelta(days=1)
+ expires2 = now + timedelta(days=14)
+ macaroon1.add_first_party_caveat(
+ "lp.expires " + expires1.strftime("%Y-%m-%dT%H:%M:%S.%f"))
+ macaroon2.add_first_party_caveat(
+ "lp.expires " + expires2.strftime("%Y-%m-%dT%H:%M:%S.%f"))
+ self.assertMacaroonVerifies(
+ issuer, macaroon1, repository, user=repository.owner)
+ self.assertMacaroonVerifies(
+ issuer, macaroon2, repository, user=repository.owner)
+ with MockPatch(
+ "lp.code.model.gitrepository.get_transaction_timestamp",
+ lambda _: now + timedelta(days=4)):
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for '%s' failed." %
+ find_caveats_by_name(macaroon1, "lp.expires")[1].caveat_id],
+ issuer, macaroon1, repository, user=repository.owner)
+ self.assertMacaroonVerifies(
+ issuer, macaroon2, repository, user=repository.owner)
+ with MockPatch(
+ "lp.code.model.gitrepository.get_transaction_timestamp",
+ lambda _: now + timedelta(days=8)):
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for '%s' failed." %
+ find_caveats_by_name(macaroon1, "lp.expires")[0].caveat_id,
+ "Caveat check for '%s' failed." %
+ find_caveats_by_name(macaroon1, "lp.expires")[1].caveat_id],
+ issuer, macaroon1, repository, user=repository.owner)
+ self.assertMacaroonDoesNotVerify(
+ ["Caveat check for '%s' failed." %
+ find_caveats_by_name(macaroon2, "lp.expires")[0].caveat_id],
+ issuer, macaroon2, repository, user=repository.owner)
=== modified file 'lib/lp/services/authserver/tests/test_authserver.py'
--- lib/lp/services/authserver/tests/test_authserver.py 2019-04-15 11:35:16 +0000
+++ lib/lp/services/authserver/tests/test_authserver.py 2019-04-15 11:35:18 +0000
@@ -88,7 +88,7 @@
raise ValueError("Cannot handle context %r." % context)
return context
- def verifyPrimaryCaveat(self, caveat_value, context):
+ def verifyPrimaryCaveat(self, caveat_value, context, **kwargs):
"""See `MacaroonIssuerBase`."""
return caveat_value == str(context)
=== modified file 'lib/lp/services/macaroons/interfaces.py'
--- lib/lp/services/macaroons/interfaces.py 2019-04-15 11:35:16 +0000
+++ lib/lp/services/macaroons/interfaces.py 2019-04-15 11:35:18 +0000
@@ -20,7 +20,7 @@
issuable_via_authserver = Bool(
"Does this issuer allow issuing macaroons via the authserver?")
- def verifyMacaroon(macaroon, context, require_context=True):
+ def verifyMacaroon(macaroon, context, require_context=True, **kwargs):
"""Verify that `macaroon` is valid for `context`.
:param macaroon: A `Macaroon`.
@@ -30,6 +30,8 @@
verify that the macaroon could be valid for some context. Use
this in the authentication part of an
authentication/authorisation API.
+ :param kwargs: Additional arguments that issuers may require to
+ verify a macaroon.
:return: True if `macaroon` is valid for `context`, otherwise False.
"""
=== modified file 'lib/lp/services/macaroons/model.py'
--- lib/lp/services/macaroons/model.py 2019-04-15 11:35:16 +0000
+++ lib/lp/services/macaroons/model.py 2019-04-15 11:35:18 +0000
@@ -25,6 +25,31 @@
issuable_via_authserver = False
+ # A mapping of caveat names to "checker" callables that verify the
+ # corresponding caveat text. The signature of each checker is
+ # (caveat_value, context, **kwargs) -> bool, where caveat_value is the
+ # text of the caveat with the caveat name removed, context is the
+ # issuer-specific context to check, and kwargs is any other keyword
+ # arguments that were given to verifyMacaroon; it should return True if
+ # the caveat is allowed, otherwise False.
+ #
+ # The context passed in may be None, in which case the checker may
+ # choose to only verify that the caveat could be valid for some context,
+ # or may simply return False if this is unsupported. This is useful for
+ # issuers that support APIs with separate authentication and
+ # authorisation phases.
+ #
+ # The "primary context caveat" added to all macaroons issued by this
+ # base class does not need to be listed here; it is handled by the
+ # verifyContextCaveat method.
+ checkers = {}
+
+ # Caveat names in this set may appear more than once (in which case they
+ # have the usual subtractive semantics, so the union of all the
+ # constraints they express applies). Any other caveats may only appear
+ # once.
+ allow_multiple = set()
+
@property
def identifier(self):
"""An identifying name for this issuer."""
@@ -58,11 +83,7 @@
return context
def issueMacaroon(self, context):
- """See `IMacaroonIssuer`.
-
- Concrete implementations should normally wrap this with some
- additional checks of and/or changes to the context.
- """
+ """See `IMacaroonIssuer`."""
context = self.checkIssuingContext(context)
macaroon = Macaroon(
location=config.vhost.mainsite.hostname,
@@ -71,7 +92,7 @@
"%s %s" % (self.primary_caveat_name, context))
return macaroon
- def checkVerificationContext(self, context):
+ def checkVerificationContext(self, context, **kwargs):
"""Check that the verification context is suitable.
Concrete implementations may implement this method to check that the
@@ -80,22 +101,27 @@
context that was passed in or an adapted one.
:param context: The context to check.
+ :param kwargs: Additional arguments that issuers may require to
+ verify a macaroon.
:raises ValueError: if the context is unsuitable.
:return: The context to pass to individual caveat checkers.
"""
return context
- def verifyPrimaryCaveat(self, caveat_value, context):
+ def verifyPrimaryCaveat(self, caveat_value, context, **kwargs):
"""Verify the primary context caveat on one of this issuer's macaroons.
- :param caveat_value: The text of the caveat, with this issuer's
- prefix removed.
+ :param caveat_value: The text of the caveat with the caveat name
+ removed.
:param context: The context to check.
+ :param kwargs: Additional arguments that issuers may require to
+ verify a macaroon.
:return: True if this caveat is allowed, otherwise False.
"""
raise NotImplementedError
- def verifyMacaroon(self, macaroon, context, require_context=True):
+ def verifyMacaroon(self, macaroon, context, require_context=True,
+ **kwargs):
"""See `IMacaroonIssuer`."""
if macaroon.location != config.vhost.mainsite.hostname:
log.info("Macaroon has unknown location '%s'." % macaroon.location)
@@ -109,6 +135,7 @@
except ValueError as e:
log.info(str(e))
return False
+ seen = set()
# XXX cjwatson 2019-04-11: Once we're on Python 3, we should use
# "nonlocal" instead of this hack.
@@ -124,16 +151,21 @@
log.info("Cannot parse caveat '%s'." % caveat)
state.logged_caveat_error = True
return False
+ if caveat_name not in self.allow_multiple and caveat_name in seen:
+ log.info(
+ "Multiple '%s' caveats are not allowed." % caveat_name)
+ state.logged_caveat_error = True
+ return False
+ seen.add(caveat_name)
if caveat_name == self.primary_caveat_name:
checker = self.verifyPrimaryCaveat
else:
- # XXX cjwatson 2019-04-09: For now we just fail closed if
- # there are any other caveats, which is good enough for
- # internal use.
- log.info("Unhandled caveat name '%s'." % caveat_name)
- state.logged_caveat_error = True
- return False
- if not checker(caveat_value, context):
+ checker = self.checkers.get(caveat_name)
+ if checker is None:
+ log.info("Unhandled caveat name '%s'." % caveat_name)
+ state.logged_caveat_error = True
+ return False
+ if not checker(caveat_value, context, **kwargs):
log.info("Caveat check for '%s' failed." % caveat)
state.logged_caveat_error = True
return False
=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py 2019-04-15 11:35:16 +0000
+++ lib/lp/snappy/model/snapbuild.py 2019-04-15 11:35:18 +0000
@@ -611,13 +611,13 @@
raise ValueError("Refusing to issue macaroon for public build.")
return removeSecurityProxy(context).id
- def checkVerificationContext(self, context):
+ def checkVerificationContext(self, context, **kwargs):
"""See `MacaroonIssuerBase`."""
if not IGitRepository.providedBy(context):
raise ValueError("Cannot handle context %r." % context)
return context
- def verifyPrimaryCaveat(self, caveat_value, context):
+ def verifyPrimaryCaveat(self, caveat_value, context, **kwargs):
"""See `MacaroonIssuerBase`.
For verification, the context is an `IGitRepository`. We check that
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2019-04-15 11:35:16 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2019-04-15 11:35:18 +0000
@@ -1383,13 +1383,13 @@
raise ValueError("Refusing to issue macaroon for public build.")
return removeSecurityProxy(context).id
- def checkVerificationContext(self, context):
+ def checkVerificationContext(self, context, **kwargs):
"""See `MacaroonIssuerBase`."""
if not isinstance(context, int):
raise ValueError("Cannot handle context %r." % context)
return context
- def verifyPrimaryCaveat(self, caveat_value, context):
+ def verifyPrimaryCaveat(self, caveat_value, context, **kwargs):
"""See `MacaroonIssuerBase`.
For verification, the context is a `LibraryFileAlias` ID. We check
Follow ups
