diff --git a/tests/unit/macaroons/test_caveats.py b/tests/unit/macaroons/test_caveats.py index 052a6d9ede2a..51f9c44f04d6 100644 --- a/tests/unit/macaroons/test_caveats.py +++ b/tests/unit/macaroons/test_caveats.py @@ -11,13 +11,16 @@ # limitations under the License. import json +import os +import time import pretend +import pymacaroons import pytest from pymacaroons.exceptions import MacaroonInvalidSignatureException -from warehouse.macaroons.caveats import Caveat, InvalidMacaroonError, V1Caveat, Verifier +from warehouse.macaroons.caveats import Caveat, ExpiryCaveat, V1Caveat, Verifier from ...common.db.packaging import ProjectFactory @@ -28,10 +31,8 @@ def test_creation(self): caveat = Caveat(verifier) assert caveat.verifier is verifier - with pytest.raises(InvalidMacaroonError): - caveat.verify(pretend.stub()) - with pytest.raises(InvalidMacaroonError): - caveat(pretend.stub()) + assert caveat.verify(pretend.stub()) is False + assert caveat(pretend.stub()) is False class TestV1Caveat: @@ -47,8 +48,7 @@ def test_verify_invalid_predicates(self, predicate, result): verifier = pretend.stub() caveat = V1Caveat(verifier) - with pytest.raises(InvalidMacaroonError): - caveat(predicate) + assert caveat(predicate) is False def test_verify_valid_predicate(self): verifier = pretend.stub() @@ -62,8 +62,8 @@ def test_verify_project_invalid_context(self): caveat = V1Caveat(verifier) predicate = {"version": 1, "permissions": {"projects": ["notfoobar"]}} - with pytest.raises(InvalidMacaroonError): - caveat(json.dumps(predicate)) + + assert caveat(json.dumps(predicate)) is False def test_verify_project_invalid_project_name(self, db_request): project = ProjectFactory.create(name="foobar") @@ -71,8 +71,8 @@ def test_verify_project_invalid_project_name(self, db_request): caveat = V1Caveat(verifier) predicate = {"version": 1, "permissions": {"projects": ["notfoobar"]}} - with pytest.raises(InvalidMacaroonError): - caveat(json.dumps(predicate)) + + assert caveat(json.dumps(predicate)) is False def test_verify_project_no_projects_object(self, db_request): project = ProjectFactory.create(name="foobar") @@ -83,8 +83,8 @@ def test_verify_project_no_projects_object(self, db_request): "version": 1, "permissions": {"somethingthatisntprojects": ["blah"]}, } - with pytest.raises(InvalidMacaroonError): - caveat(json.dumps(predicate)) + + assert caveat(json.dumps(predicate)) is False def test_verify_project(self, db_request): project = ProjectFactory.create(name="foobar") @@ -95,6 +95,56 @@ def test_verify_project(self, db_request): assert caveat(json.dumps(predicate)) is True +class TestExpiryCaveat: + @pytest.mark.parametrize( + "predicate", + [ + # invalid JSON + "invalid json", + # missing nbf and exp + '{"missing": "values"}', + # nbf and exp present, but null + '{"nbf": null, "exp": null}', + # nbf and exp present, but empty + '{"nbf": "", "exp": ""}', + # valid JSON, but wrong type + "[]", + ], + ) + def test_verify_invalid_predicates(self, predicate): + verifier = pretend.stub() + caveat = ExpiryCaveat(verifier) + + assert caveat(predicate) is False + + def test_verify_not_before(self): + verifier = pretend.stub() + caveat = ExpiryCaveat(verifier) + + not_before = int(time.time()) + 60 + expiry = not_before + 60 + predicate = json.dumps({"exp": expiry, "nbf": not_before}) + assert caveat(predicate) is False + + def test_verify_already_expired(self): + verifier = pretend.stub() + caveat = ExpiryCaveat(verifier) + + not_before = int(time.time()) - 10 + expiry = not_before - 5 + predicate = json.dumps({"exp": expiry, "nbf": not_before}) + assert caveat(predicate) is False + + def test_verify_ok(self): + verifier = pretend.stub() + caveat = ExpiryCaveat(verifier) + + not_before = int(time.time()) - 10 + expiry = int(time.time()) + 60 + predicate = json.dumps({"exp": expiry, "nbf": not_before}) + assert caveat(predicate) + + class TestVerifier: def test_creation(self): macaroon = pretend.stub() @@ -108,7 +158,7 @@ def test_creation(self): assert verifier.principals is principals assert verifier.permission is permission - def test_verify(self, monkeypatch): + def test_verify_invalid_signature(self, monkeypatch): verify = pretend.call_recorder( pretend.raiser(MacaroonInvalidSignatureException) ) @@ -120,6 +170,53 @@ def test_verify(self, monkeypatch): verifier = Verifier(macaroon, context, principals, permission) monkeypatch.setattr(verifier.verifier, "verify", verify) - with pytest.raises(InvalidMacaroonError): - verifier.verify(key) + assert verifier.verify(key) is False assert verify.calls == [pretend.call(macaroon, key)] + + @pytest.mark.parametrize( + ["caveats", "valid"], + [ + # Both V1 and expiry present and valid. + ( + [ + {"permissions": "user", "version": 1}, + {"exp": int(time.time()) + 3600, "nbf": int(time.time()) - 1}, + ], + True, + ), + # V1 only present and valid. + ([{"permissions": "user", "version": 1}], True), + # V1 and expiry present but V1 invalid. + ([{"permissions": "bad", "version": 1}], False), + # V1 and expiry present but expiry invalid. + ( + [ + {"permissions": "user", "version": 1}, + {"exp": int(time.time()) + 1, "nbf": int(time.time()) + 3600}, + ], + False, + ), + ], + ) + def test_verify(self, monkeypatch, caveats, valid): + key = os.urandom(32) + m = pymacaroons.Macaroon( + location="fakelocation", + identifier="fakeid", + key=key, + version=pymacaroons.MACAROON_V2, + ) + + for caveat in caveats: + m.add_first_party_caveat(json.dumps(caveat)) + + # Round-trip through serialization to ensure we're not clinging to any state. + serialized_macaroon = m.serialize() + deserialized_macaroon = pymacaroons.Macaroon.deserialize(serialized_macaroon) + + context = pretend.stub() + principals = pretend.stub() + permission = pretend.stub() + + verifier = Verifier(deserialized_macaroon, context, principals, permission) + assert verifier.verify(key) is valid diff --git a/warehouse/macaroons/caveats.py b/warehouse/macaroons/caveats.py index accb743b8a3d..98efa0ccde24 100644 --- a/warehouse/macaroons/caveats.py +++ b/warehouse/macaroons/caveats.py @@ -11,6 +11,7 @@ # limitations under the License. import json +import time import pymacaroons @@ -24,43 +25,51 @@ class InvalidMacaroonError(Exception): class Caveat: def __init__(self, verifier): self.verifier = verifier + # TODO: Surface this failure reason to the user. + # See: https://github.com/pypa/warehouse/issues/9018 + self.failure_reason = None - def verify(self, predicate): - raise InvalidMacaroonError + def verify(self, predicate) -> bool: + return False def __call__(self, predicate): return self.verify(predicate) class V1Caveat(Caveat): - def verify_projects(self, projects): + def verify_projects(self, projects) -> bool: # First, ensure that we're actually operating in # the context of a package. if not isinstance(self.verifier.context, Project): - raise InvalidMacaroonError( + self.failure_reason = ( "project-scoped token used outside of a project context" ) + return False project = self.verifier.context if project.normalized_name in projects: return True - raise InvalidMacaroonError( + self.failure_reason = ( f"project-scoped token is not valid for project '{project.name}'" ) + return False - def verify(self, predicate): + def verify(self, predicate) -> bool: try: data = json.loads(predicate) except ValueError: - raise InvalidMacaroonError("malformatted predicate") + self.failure_reason = "malformatted predicate" + return False if data.get("version") != 1: - raise InvalidMacaroonError("invalidate version in predicate") + self.failure_reason = "invalid version in predicate" + return False permissions = data.get("permissions") if permissions is None: - raise InvalidMacaroonError("invalid permissions in predicate") + self.failure_reason = "invalid permissions in predicate" + return False if permissions == "user": # User-scoped tokens behave exactly like a user's normal credentials. @@ -68,11 +77,34 @@ def verify(self, predicate): projects = permissions.get("projects") if projects is None: - raise InvalidMacaroonError("invalid projects in predicate") + self.failure_reason = "invalid projects in predicate" + return False return self.verify_projects(projects) +class ExpiryCaveat(Caveat): + def verify(self, predicate): + try: + data = json.loads(predicate) + expiry = data["exp"] + not_before = data["nbf"] + except (KeyError, ValueError, TypeError): + self.failure_reason = "malformatted predicate" + return False + + if not expiry or not not_before: + self.failure_reason = "missing fields" + return False + + now = int(time.time()) + if now < not_before or now >= expiry: + self.failure_reason = "token is expired" + return False + + return True + + class Verifier: def __init__(self, macaroon, context, principals, permission): self.macaroon = macaroon @@ -83,6 +115,7 @@ def __init__(self, macaroon, context, principals, permission): def verify(self, key): self.verifier.satisfy_general(V1Caveat(self)) + self.verifier.satisfy_general(ExpiryCaveat(self)) try: return self.verifier.verify(self.macaroon, key) @@ -90,4 +123,4 @@ def verify(self, key): pymacaroons.exceptions.MacaroonInvalidSignatureException, Exception, # https://github.com/ecordell/pymacaroons/issues/50 ): - raise InvalidMacaroonError("invalid macaroon signature") + return False