From 4c87599a479a84f8e8da078db1a1b7fe40b7ad48 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 14 Mar 2021 16:49:35 +0100 Subject: [PATCH 1/3] Build docs in CI How come that wasn't the case before ? --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ee4f3f..3e644d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,8 @@ jobs: include: - tox-env: lint python-version: 3.x + - tox-env: docs + python-version: 3.x - python-version: 3.6 tox-env: py36-tests - python-version: 3.7 From f0e5fd00e3344ec6b09e4b9182693883e05fa471 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 14 Mar 2021 17:35:44 +0100 Subject: [PATCH 2/3] Create a context object, remove MissingContextError Make exceptions more consistent --- docs/reference.rst | 3 - pypitoken/__init__.py | 8 +-- pypitoken/exceptions.py | 12 ---- pypitoken/token.py | 98 ++++++++++++++++++-------------- tests/test_token.py | 120 +++++++++++++++++++--------------------- 5 files changed, 117 insertions(+), 124 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 0c63c23..fda454e 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -32,6 +32,3 @@ Exceptions .. autoclass:: pypitoken.ValidationError :show-inheritance: - -.. autoclass:: pypitoken.MissingContextError - :show-inheritance: diff --git a/pypitoken/__init__.py b/pypitoken/__init__.py index 6c882fb..3ace7aa 100644 --- a/pypitoken/__init__.py +++ b/pypitoken/__init__.py @@ -1,9 +1,4 @@ -from .exceptions import ( - LoadError, - MissingContextError, - PyPITokenException, - ValidationError, -) +from .exceptions import LoadError, PyPITokenException, ValidationError from .token import NoopRestriction, ProjectsRestriction, Token __all__ = [ @@ -12,6 +7,5 @@ "ProjectsRestriction", "PyPITokenException", "LoadError", - "MissingContextError", "ValidationError", ] diff --git a/pypitoken/exceptions.py b/pypitoken/exceptions.py index 279c66d..6cf7d0f 100644 --- a/pypitoken/exceptions.py +++ b/pypitoken/exceptions.py @@ -18,15 +18,3 @@ class ValidationError(PyPITokenException): Exception encoutered while calling `Token.check`, the token should be considered invalid. """ - - -class MissingContextError(ValidationError, NotImplementedError): - """ - Token check failed du to missing information in the passed context. - - The token should be considered invalid, but this is likely a problem in the - implementing code: we most likely forgot a context element. - - Due to PyMacaroons & this lib implementation details, this exception is currently - not fired by `Token.check`. - """ diff --git a/pypitoken/token.py b/pypitoken/token.py index 6cab2a1..4191ba1 100644 --- a/pypitoken/token.py +++ b/pypitoken/token.py @@ -1,7 +1,7 @@ import dataclasses import functools import json -from typing import Dict, List, Optional, Type, TypeVar +from typing import Any, Dict, List, Optional, Type, TypeVar import jsonschema import pymacaroons @@ -10,6 +10,23 @@ PREFIX = "pypi" + +@dataclasses.dataclass +class Context: + """ + Describe the circumstances sourrounding how the bearer is attempting to use + the token. Will be checked against restriction to determine whether the bearer + can or cannot use this token for the operation. + + Parameters + ---------- + project : + Normalized name of the project the bearer is attempting to upload to + """ + + project: str + + T = TypeVar("T", bound="Restriction") @@ -28,20 +45,39 @@ def get_schema() -> Dict: raise NotImplementedError @classmethod - def validate_value(cls, value: Dict) -> None: + def load_from_value(cls: Type[T], value: Any) -> T: try: jsonschema.validate( instance=value, schema=cls.get_schema(), ) except jsonschema.ValidationError as exc: - raise exceptions.ValidationError() from exc + raise exceptions.LoadError() from exc + + return cls(**cls.extract_kwargs(value=value)) # type: ignore @classmethod - def load_from_value(cls: Type[T], value: Dict) -> T: + def extract_kwargs(cls, value: Dict) -> Dict: + """ + Receive the parsed JSON value of a caveat for which the schema has been + validated. Returns the instantiation kwargs (``__init__`` parameters). + """ raise NotImplementedError - def check(self, context) -> None: + def check(self, context: Context) -> None: + """ + Receive the context of a check + + Parameters + ---------- + context : + Describes how the bearer is attempting to use the token. + + Raises + ------ + `ValidationError` + Restriction was checked and appeared unmet. + """ raise NotImplementedError def dump_value(self) -> Dict: @@ -67,10 +103,10 @@ def get_schema() -> Dict: } @classmethod - def load_from_value(cls, value: Dict) -> "NoopRestriction": - return cls() + def extract_kwargs(cls, value: Dict) -> Dict: + return {} - def check(self, context: Dict) -> None: + def check(self, context: Context) -> None: # Always passes return @@ -111,20 +147,14 @@ def get_schema() -> Dict: } @classmethod - def load_from_value(cls, value: Dict) -> "ProjectsRestriction": - projects = value["permissions"]["projects"] - return cls(projects=projects) + def extract_kwargs(cls, value: Dict) -> Dict: + return {"projects": value["permissions"]["projects"]} def dump_value(self) -> Dict: return {"version": 1, "permissions": {"projects": self.projects}} - def check(self, context: Dict) -> None: - try: - project = context["project"] - except KeyError: - raise exceptions.MissingContextError( - "Missing key 'project' from validation context" - ) + def check(self, context: Context) -> None: + project = context.project if project not in self.projects: raise exceptions.ValidationError( @@ -138,17 +168,12 @@ def check(self, context: Dict) -> None: RESTRICTION_CLASSES: List[Type[Restriction]] = [NoopRestriction, ProjectsRestriction] -def json_load_caveat(caveat: str) -> Dict: +def json_load_caveat(caveat: str) -> Any: try: value = json.loads(caveat) except Exception as exc: raise exceptions.LoadError(f"Error while loading caveat: {exc}") from exc - if not isinstance(value, dict): - raise exceptions.LoadError( - f"Caveat is a well-formed JSON string but not a dict: {value}" - ) - return value @@ -171,15 +196,14 @@ def load_restriction( value = json_load_caveat(caveat=caveat) for subclass in classes: try: - subclass.validate_value(value) - except exceptions.ValidationError: + return subclass.load_from_value(value=value) + except exceptions.LoadError: continue - return subclass.load_from_value(value) raise exceptions.LoadError(f"Could not find matching Restriction for {value}") -def check_caveat(caveat: str, context: Dict) -> bool: +def check_caveat(caveat: str, context: Context) -> bool: """ This function follows the pymacaroon Verifier.satisfy_general API, except that it takes a context parameter. It's expected to be used with `functools.partial` @@ -207,7 +231,7 @@ def check_caveat(caveat: str, context: Dict) -> bool: # Actually, the pymacaroons API tells us to return False when a validation fail, # which keeps us from raising a more expressive error. # Hopefully, this will be adressed in pymacaroons in the future. - except exceptions.PyPITokenException: + except exceptions.ValidationError: return False return True @@ -291,6 +315,7 @@ def load(cls, raw: str) -> "Token": raise exceptions.LoadError("Token is missing a prefix") try: macaroon = pymacaroons.Macaroon.deserialize(raw_macaroon) + # https://github.com/ecordell/pymacaroons/issues/50 except Exception as exc: raise exceptions.LoadError(f"Deserialization error: {exc}") from exc return cls( @@ -407,7 +432,7 @@ def dump(self) -> str: def check( self, key: str, - project: Optional[str] = None, + project: str, ) -> None: """ Raises pypitoken.ValidationError if the token is invalid. @@ -423,14 +448,7 @@ def check( key : str Key of the macaroon, stored in PyPI database project : Optional[str], optional - Name of the project the bearer is attempting to interact with, by default - None - filename : Optional[str], optional - Name of the file the bearer is attempting to upload, by default None - hash : Optional[str], optional - [description], by default None - timestamp : Optional[datetime.datetime], optional - [description], by default None + Normalized name of the project the bearer is attempting to upload to. Raises ------ @@ -442,13 +460,13 @@ def check( """ verifier = pymacaroons.Verifier() - context = {"project": project} + context = Context(project=project) verifier.satisfy_general(functools.partial(check_caveat, context=context)) try: verifier.verify(self._macaroon, key) + # https://github.com/ecordell/pymacaroons/issues/51 except Exception as exc: - # https://github.com/ecordell/pymacaroons/issues/51 raise exceptions.ValidationError( f"Error while validating token: {exc}" ) from exc diff --git a/tests/test_token.py b/tests/test_token.py index a4394de..4c01630 100644 --- a/tests/test_token.py +++ b/tests/test_token.py @@ -1,3 +1,5 @@ +import dataclasses + import pymacaroons import pytest @@ -12,8 +14,11 @@ def dump_value(self): assert MyRestriction().dump() == '{"a": ["b"]}' -def test__Restriction__validate_value__pass(): +def test__Restriction__load_from_value__pass(): + @dataclasses.dataclass class MyRestriction(token.Restriction): + version: int + @staticmethod def get_schema(): return { @@ -24,7 +29,11 @@ def get_schema(): "required": ["version"], } - assert MyRestriction().validate_value({"version": 42}) is None + @classmethod + def extract_kwargs(cls, value): + return {"version": value["version"]} + + assert MyRestriction.load_from_value(value={"version": 42}).version == 42 @pytest.mark.parametrize( @@ -37,7 +46,7 @@ def get_schema(): {"version": 17}, ], ) -def test__Restriction__validate_value__fail(value): +def test__Restriction__load_from_value__fail(value): class MyRestriction(token.Restriction): @staticmethod def get_schema(): @@ -49,17 +58,15 @@ def get_schema(): "required": ["version"], } - with pytest.raises(exceptions.ValidationError): - MyRestriction().validate_value(value) + with pytest.raises(exceptions.LoadError): + MyRestriction.load_from_value(value=value) -def test__NoopRestriction__validate_value__pass(): - assert ( - token.NoopRestriction.validate_value( - value={"version": 1, "permissions": "user"} - ) - is None +def test__NoopRestriction__load_from_value__pass(): + tok = token.NoopRestriction.load_from_value( + value={"version": 1, "permissions": "user"} ) + assert tok == token.NoopRestriction() @pytest.mark.parametrize( @@ -73,19 +80,19 @@ def test__NoopRestriction__validate_value__pass(): {"version": 1, "permissions": "user", "additional": "key"}, ], ) -def test__NoopRestriction__validate_value__fail(value): - with pytest.raises(exceptions.ValidationError): - token.NoopRestriction.validate_value(value=value) +def test__NoopRestriction__load_from_value__fail(value): + with pytest.raises(exceptions.LoadError): + token.NoopRestriction.load_from_value(value=value) -def test__NoopRestriction__load_from_value(): - noop = token.NoopRestriction.load_from_value(value={}) - assert noop == token.NoopRestriction() +def test__NoopRestriction__extract_kwargs(): + noop = token.NoopRestriction.extract_kwargs(value={"any": "content"}) + assert noop == {} def test__NoopRestriction__check(): noop = token.NoopRestriction() - assert noop.check(context={}) is None + assert noop.check(context=token.Context(project="foo")) is None def test__NoopRestriction__dump_value(): @@ -94,15 +101,24 @@ def test__NoopRestriction__dump_value(): @pytest.mark.parametrize( - "value", + "value, restriction", [ - {"version": 1, "permissions": {"projects": []}}, - {"version": 1, "permissions": {"projects": ["a"]}}, - {"version": 1, "permissions": {"projects": ["a", "b"]}}, + ( + {"version": 1, "permissions": {"projects": []}}, + token.ProjectsRestriction(projects=[]), + ), + ( + {"version": 1, "permissions": {"projects": ["a"]}}, + token.ProjectsRestriction(projects=["a"]), + ), + ( + {"version": 1, "permissions": {"projects": ["a", "b"]}}, + token.ProjectsRestriction(projects=["a", "b"]), + ), ], ) -def test__ProjectsRestriction__validate_value__pass(value): - assert token.ProjectsRestriction.validate_value(value=value) is None +def test__ProjectsRestriction__load_from_value__pass(value, restriction): + assert token.ProjectsRestriction.load_from_value(value=value) == restriction @pytest.mark.parametrize( @@ -120,32 +136,26 @@ def test__ProjectsRestriction__validate_value__pass(value): {"version": 1, "permissions": {"projects": ["a"], "additional": "key"}}, ], ) -def test__ProjectsRestriction__validate_value__fail(value): - with pytest.raises(exceptions.ValidationError): - token.ProjectsRestriction.validate_value(value=value) +def test__ProjectsRestriction__load_from_value__fail(value): + with pytest.raises(exceptions.LoadError): + token.ProjectsRestriction.load_from_value(value=value) -def test__ProjectsRestriction__load_from_value(): +def test__ProjectsRestriction__extract_kwargs(): value = {"version": 1, "permissions": {"projects": ["a", "b"]}} - restriction = token.ProjectsRestriction.load_from_value(value=value) - assert restriction == token.ProjectsRestriction(projects=["a", "b"]) + kwargs = token.ProjectsRestriction.extract_kwargs(value=value) + assert kwargs == {"projects": ["a", "b"]} def test__ProjectsRestriction__check__pass(): restriction = token.ProjectsRestriction(projects=["a", "b"]) - assert restriction.check(context={"project": "a"}) is None + assert restriction.check(context=token.Context(project="a")) is None -def test__ProjectsRestriction__check__fail_project(): +def test__ProjectsRestriction__check__fail(): restriction = token.ProjectsRestriction(projects=["a", "b"]) with pytest.raises(exceptions.ValidationError): - restriction.check(context={"project": "c"}) - - -def test__ProjectsRestriction__check__fail_context(): - restriction = token.ProjectsRestriction(projects=["a", "b"]) - with pytest.raises(exceptions.MissingContextError): - restriction.check(context={}) + restriction.check(context=token.Context(project="c")) def test__ProjectsRestriction__dump_value(): @@ -170,20 +180,13 @@ def test__json_load_caveat__pass(): assert token.json_load_caveat('{"a": "b"}') == {"a": "b"} -@pytest.mark.parametrize( - "caveat, error", - [ - ( - '{"a": "b"', - "Error while loading caveat: Expecting ',' delimiter: line 1 column 10 (char 9)", - ), - ("[1, 2, 3]", "Caveat is a well-formed JSON string but not a dict: [1, 2, 3]"), - ], -) -def test__json_load_caveat__fail(caveat, error): +def test__json_load_caveat__fail(): with pytest.raises(exceptions.LoadError) as exc_info: - token.json_load_caveat(caveat=caveat) - assert str(exc_info.value) == error + token.json_load_caveat(caveat='{"a": "b"') + assert ( + str(exc_info.value) == "Error while loading caveat: " + "Expecting ',' delimiter: line 1 column 10 (char 9)" + ) @pytest.mark.parametrize( @@ -225,27 +228,20 @@ def test__load_restriction__fail(caveat, error): def test__check_caveat__pass(): value = token.check_caveat( caveat='{"version": 1, "permissions": {"projects": ["a", "b"]}}', - context={"project": "a"}, + context=token.Context(project="a"), ) assert value is True def test__check_caveat__fail_load(): - value = token.check_caveat("{", context={}) + value = token.check_caveat("{", context=token.Context(project="a")) assert value is False def test__check_caveat__fail_check(): value = token.check_caveat( '{"version": 1, "permissions": {"projects": ["a", "b"]}}', - context={"project": "c"}, - ) - assert value is False - - -def test__check_caveat__fail_context(): - value = token.check_caveat( - '{"version": 1, "permissions": {"projects": ["a", "b"]}}', context={} + context=token.Context(project="c"), ) assert value is False From ef4691843288c5d7ed0472fe6c1fc9f3c4132522 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 14 Mar 2021 17:37:30 +0100 Subject: [PATCH 3/3] Add precision in doc --- docs/discussions.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/discussions.rst b/docs/discussions.rst index 66a91e1..365abae 100644 --- a/docs/discussions.rst +++ b/docs/discussions.rst @@ -164,6 +164,17 @@ Having multiple restrictions in a single json payload makes it harder to check w it's valid or not (at least, while the general format of the json payloads is not unified). No definitive answer is given at this time. +What does "normalized name" mean? +--------------------------------- + +Throughout the doc, the term "normalized name" for a project is regularily used. +This is because some characters are synonymous in a project name, so in order to match +a project name, we need to put it to canonical form first. + +See `PEP 503`__ for all the details. + +.. __: https://www.python.org/dev/peps/pep-0503/#normalized-names + All this talking about Macaroons, I'm hungry now! -------------------------------------------------