From cf8ef63d3e6bc958eb8fc4a1b6af357248d76e1d Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 29 May 2022 02:11:33 +0200 Subject: [PATCH 1/4] Delegate constructing Restriction in .restrict to Restrictions subclasses --- pypitoken/token.py | 57 ++++++++++++++++++++++++++++++--------------- tests/test_token.py | 30 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/pypitoken/token.py b/pypitoken/token.py index 1dc5146..7b96be5 100644 --- a/pypitoken/token.py +++ b/pypitoken/token.py @@ -1,7 +1,7 @@ import dataclasses import functools import json -from typing import Any, Dict, List, Optional, Type, TypeVar, Union +from typing import Any, Dict, Iterable, List, Optional, Type, TypeVar, Union import jsonschema import pymacaroons @@ -139,6 +139,24 @@ def _extract_kwargs(cls, value: Dict) -> Dict: """ raise NotImplementedError + @classmethod + def from_parameters(cls: Type[T], **kwargs) -> Optional[T]: + """ + Contructs an instance from the parameters passed to `Token.restrict` + """ + raise NotImplementedError + + @classmethod + def restrictions_from_parameters(cls, **kwargs) -> Iterable["Restriction"]: + """ + Contructs an iterable of Restriction subclass instances from the parameters + passed to `Token.restrict` + """ + for subclass in cls._get_subclasses(): + restriction = subclass.from_parameters(**kwargs) + if restriction: + yield restriction + def check(self, context: Context) -> None: """ Receive the context of a check @@ -197,6 +215,12 @@ def check(self, context: Context) -> None: def dump(self) -> Dict: return {"version": 1, "permissions": "user"} + @classmethod + def from_parameters(cls, **kwargs) -> Optional["NoopRestriction"]: + if not kwargs: + return cls() + return None + @dataclasses.dataclass class ProjectsRestriction(Restriction): @@ -246,6 +270,16 @@ def check(self, context: Context) -> None: f"{', '.join(self.projects)}. Received: {project}" ) + @classmethod + def from_parameters( + cls, + projects: Optional[List[str]] = None, + **kwargs, + ) -> Optional["ProjectsRestriction"]: + if projects is not None: + return cls(projects=projects) + return None + class Token: """ @@ -382,10 +416,7 @@ def create( token = cls(prefix=prefix, macaroon=macaroon) return token - def restrict( - self, - projects: Optional[List[str]] = None, - ) -> "Token": + def restrict(self, **kwargs) -> "Token": """ Modifies the token in-place to add restrictions to it. This can be called by PyPI as well as by anyone, adding restrictions to new or existing tokens. @@ -412,20 +443,8 @@ def restrict( `Token` The modified Token, to ease chaining calls. """ - caveats: List[str] = [] - if projects is not None: - caveats.append(ProjectsRestriction(projects).dump_json()) - - # Add other restrictions here - - if not self._macaroon.caveats and not caveats: - # It's actually not really useful to add a noop restriction, but - # it's done that way in the original implementation, and has been kept so - # far - caveats = [NoopRestriction().dump_json()] - - for caveat in caveats: - self._macaroon.add_first_party_caveat(caveat) + for restriction in Restriction.restrictions_from_parameters(**kwargs): + self._macaroon.add_first_party_caveat(restriction.dump_json()) return self diff --git a/tests/test_token.py b/tests/test_token.py index ba2dc91..2410da7 100644 --- a/tests/test_token.py +++ b/tests/test_token.py @@ -98,6 +98,14 @@ def test__NoopRestriction__dump(): assert noop.dump() == {"version": 1, "permissions": "user"} +def test__NoopRestriction__from_parameters__empty(): + assert token.NoopRestriction.from_parameters() == token.NoopRestriction() + + +def test__NoopRestriction__from_parameters__not_empty(): + assert token.NoopRestriction.from_parameters(a=1) is None + + @pytest.mark.parametrize( "value, restriction", [ @@ -164,6 +172,16 @@ def test__ProjectsRestriction__dump(): } +def test__ProjectsRestriction__from_parameters__empty(): + assert token.ProjectsRestriction.from_parameters() is None + + +def test__ProjectsRestriction__from_parameters__not_empty(): + assert token.ProjectsRestriction.from_parameters( + projects=["a", "b"] + ) == token.ProjectsRestriction(projects=["a", "b"]) + + def test__Restriction__get_subclasses(): # This test ensures we didn't forget to add new restriction classes to # the set. @@ -220,6 +238,18 @@ def test__Restriction__load_json(): assert restriction == token.NoopRestriction() +def test__Restriction__restrictions_from_parameters(): + restrictions = list( + token.Restriction.restrictions_from_parameters( + projects=["a", "b"], not_before=1, not_after=5 + ) + ) + assert restrictions == [ + token.ProjectsRestriction(projects=["a", "b"]), + token.DateRestriction(not_before=1, not_after=5), + ] + + def test__Token__check_caveat__pass(): errors = [] value = token.Token._check_caveat( From 5efab5cb7c72dadf1dc8b6a76b98e5303427708d Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 29 May 2022 02:13:48 +0200 Subject: [PATCH 2/4] Add DateRestriction --- pypitoken/__init__.py | 17 +++++- pypitoken/exceptions.py | 7 +++ pypitoken/token.py | 109 ++++++++++++++++++++++++++++++++++-- tests/test_token.py | 119 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 244 insertions(+), 8 deletions(-) diff --git a/pypitoken/__init__.py b/pypitoken/__init__.py index 1f5a35e..78637c9 100644 --- a/pypitoken/__init__.py +++ b/pypitoken/__init__.py @@ -1,15 +1,28 @@ -from .exceptions import LoaderError, PyPITokenException, ValidationError -from .token import NoopRestriction, ProjectsRestriction, Restriction, Token +from .exceptions import ( + InvalidRestriction, + LoaderError, + PyPITokenException, + ValidationError, +) +from .token import ( + DateRestriction, + NoopRestriction, + ProjectsRestriction, + Restriction, + Token, +) __all__ = [ # Main classes "Token", "Restriction", # Restriction subclasses + "DateRestriction", "NoopRestriction", "ProjectsRestriction", # Exceptions "PyPITokenException", + "InvalidRestriction", "LoaderError", "ValidationError", ] diff --git a/pypitoken/exceptions.py b/pypitoken/exceptions.py index 6ac9d16..5750afb 100644 --- a/pypitoken/exceptions.py +++ b/pypitoken/exceptions.py @@ -6,6 +6,13 @@ class PyPITokenException(Exception): """ +class InvalidRestriction(PyPITokenException, ValueError): + """ + Exception encoutered while calling `Token.restrict`, due to unexpected + parameters. + """ + + class LoaderError(PyPITokenException): """ Exception encoutered while calling `Token.load`, due to unexpected diff --git a/pypitoken/token.py b/pypitoken/token.py index 7b96be5..8342b88 100644 --- a/pypitoken/token.py +++ b/pypitoken/token.py @@ -1,6 +1,8 @@ import dataclasses +import datetime import functools import json +import time from typing import Any, Dict, Iterable, List, Optional, Type, TypeVar, Union import jsonschema @@ -25,6 +27,7 @@ class Context: """ project: str + now: int = dataclasses.field(default_factory=lambda: int(time.time())) T = TypeVar("T", bound="Restriction") @@ -79,7 +82,7 @@ def _get_subclasses() -> List[Type["Restriction"]]: # We could use __subclasses__ but that could lead to all kinds of funky things, # especially in a security-sensistive library. # Tests will check this against Restriction subclasses though. - return [NoopRestriction, ProjectsRestriction] + return [NoopRestriction, ProjectsRestriction, DateRestriction] @staticmethod def _json_load_caveat(caveat: str) -> Any: @@ -281,6 +284,90 @@ def from_parameters( return None +@dataclasses.dataclass +class DateRestriction(Restriction): + """ + Restrict a `Token` to a single time interval. + + Attributes + ---------- + not_before : + Token is not to be used before this Unix timestamp. + not_after : + Token is not to be used after this Unix timestamp. + + """ + + not_before: int + not_after: int + + @staticmethod + def _get_schema() -> Dict: + return { + "type": "object", + "properties": { + "nbf": {"type": "integer"}, + "exp": {"type": "integer"}, + }, + "required": ["nbf", "exp"], + "additionalProperties": False, + } + + @classmethod + def _extract_kwargs(cls, value: Dict) -> Dict: + return { + "not_before": value["nbf"], + "not_after": value["exp"], + } + + def dump(self) -> Dict: + return {"nbf": self.not_before, "exp": self.not_after} + + def check(self, context: Context) -> None: + now: int = context.now + + if not self.not_before <= now < self.not_after: + raise exceptions.ValidationError( + f"This token can only be used between timestamps " + f"{self.not_before} (incl) and {self.not_after} (excl). " + f"Received: {now}" + ) + + @classmethod + def from_parameters( + cls, + not_before: Optional[Union[datetime.datetime, int]] = None, + not_after: Optional[Union[datetime.datetime, int]] = None, + **kwargs, + ) -> Optional["DateRestriction"]: + if not_before or not_after: + if not (not_before and not_after): + raise exceptions.InvalidRestriction( + "`not_before` and `not_after` parameters must be used together. " + "Either define both or neither. " + f"Received not_before={not_before} and not_after={not_after}" + ) + return cls( + not_before=cls.timestamp_from_parameter(not_before), + not_after=cls.timestamp_from_parameter(not_after), + ) + return None + + @staticmethod + def timestamp_from_parameter(param: Union[datetime.datetime, int]) -> int: + if isinstance(param, int): + return param + # https://docs.python.org/3/library/datetime.html#determining-if-an-object-is-aware-or-naive + naive = param.tzinfo is None or param.tzinfo.utcoffset(param) is None + if naive: + raise exceptions.InvalidRestriction( + "Cannot use a naive datetime. Either provide a timezone or " + "the timestamp directly. " + "Received {param}" + ) + return int(param.timestamp()) + + class Token: """ Create a `Token` (as PyPI itself would do), load an existing @@ -437,6 +524,17 @@ def restrict(self, **kwargs) -> "Token": projects : Restrict the token to uploading releases only for projects with these normalized names, by default None (no restriction) + not_before : + Restrict the token to uploading releases only after the given timestamp + or tz-aware datetime. Must be used with ``not_after``. + not_after : + Restrict the token to uploading releases only before the given timestamp + or tz-aware datetime. Must be used with ``not_before``. + + Raises + ------ + exceptions.InvalidRestriction + If the provided parameters cannot describe a valid description Returns ------- @@ -461,9 +559,7 @@ def dump(self) -> str: return f"{self.prefix}-{self._macaroon.serialize()}" def check( - self, - key: Union[str, bytes], - project: str, + self, key: Union[str, bytes], project: str, now: Optional[int] = None ) -> None: """ Raises pypitoken.ValidationError if the token is invalid. @@ -490,7 +586,10 @@ def check( """ verifier = pymacaroons.Verifier() - context = Context(project=project) + context_kwargs: Dict[str, Any] = {"project": project} + if now: + context_kwargs["now"] = now + context = Context(**context_kwargs) errors: List[Exception] = [] diff --git a/tests/test_token.py b/tests/test_token.py index 2410da7..c006aa4 100644 --- a/tests/test_token.py +++ b/tests/test_token.py @@ -1,4 +1,5 @@ import dataclasses +import datetime import pymacaroons import pytest @@ -182,6 +183,115 @@ def test__ProjectsRestriction__from_parameters__not_empty(): ) == token.ProjectsRestriction(projects=["a", "b"]) +def test__DateRestriction__load_value__pass(): + assert token.DateRestriction._load_value( + value={"nbf": 1_234_567_890, "exp": 1_234_567_900} + ) == token.DateRestriction(not_before=1_234_567_890, not_after=1_234_567_900) + + +@pytest.mark.parametrize( + "value", + [ + {}, + {"nbf": "2000-01-01 00:00:00", "exp": "2100-01-01 00:00:00"}, + {"nbf": "1_234_567_890", "exp": "1_234_567_900"}, + {"nbf": 1_234_567_890}, + {"exp": 1_234_567_890}, + ], +) +def test__DateRestriction__load_value__fail(value): + with pytest.raises(exceptions.LoaderError): + token.DateRestriction._load_value(value=value) + + +def test__DateRestriction__extract_kwargs(): + value = {"nbf": 1_234_567_890, "exp": 1_234_567_900} + kwargs = token.DateRestriction._extract_kwargs(value=value) + assert kwargs == {"not_before": 1_234_567_890, "not_after": 1_234_567_900} + + +def test__DateRestriction__check__pass(): + restriction = token.DateRestriction._load_value( + value={"nbf": 1_234_567_890, "exp": 1_234_567_900} + ) + assert ( + restriction.check(context=token.Context(project="a", now=1_234_567_895)) is None + ) + + +@pytest.mark.parametrize( + "value", + [ + 1_234_567_000, + 1_234_568_000, + ], +) +def test__DateRestriction__check__fail(value): + restriction = token.DateRestriction._load_value( + value={"nbf": 1_234_567_890, "exp": 1_234_567_900} + ) + with pytest.raises(exceptions.ValidationError): + restriction.check(context=token.Context(project="a", now=value)) + + +def test__DateRestriction__dump(): + restriction = token.DateRestriction._load_value( + value={"nbf": 1_234_567_890, "exp": 1_234_567_900} + ) + assert restriction.dump() == { + "nbf": 1_234_567_890, + "exp": 1_234_567_900, + } + + +def test__DateRestriction__from_parameters__empty(): + assert token.DateRestriction.from_parameters() is None + + +@pytest.mark.parametrize( + "kwargs", + [ + {"not_before": 1_234_567_890}, + {"not_after": 1_234_567_900}, + { + "not_before": datetime.datetime(2000, 1, 1), + "not_after": datetime.datetime(2100, 1, 1), + }, + ], +) +def test__DateRestriction__from_parameters__fail(kwargs): + with pytest.raises(exceptions.InvalidRestriction): + assert token.DateRestriction.from_parameters(**kwargs) is None + + +@pytest.mark.parametrize( + "kwargs, expected", + [ + ( + {"not_before": 1_234_567_890, "not_after": 1_234_567_900}, + token.DateRestriction._load_value( + value={"nbf": 1_234_567_890, "exp": 1_234_567_900} + ), + ), + ( + { + "not_before": datetime.datetime( + 2000, 1, 1, tzinfo=datetime.timezone.utc + ), + "not_after": datetime.datetime( + 2100, 1, 1, tzinfo=datetime.timezone.utc + ), + }, + token.DateRestriction._load_value( + value={"nbf": 946_684_800, "exp": 4_102_444_800} + ), + ), + ], +) +def test__DateRestriction__from_parameters__ok(kwargs, expected): + assert token.DateRestriction.from_parameters(**kwargs) == expected + + def test__Restriction__get_subclasses(): # This test ensures we didn't forget to add new restriction classes to # the set. @@ -425,7 +535,14 @@ def test__Token__check__pass(create_token, key): tok = create_token(key=key) tok.restrict(projects=["a", "b"]) - tok.check(key=key, project="a") + tok.check(key=key, project="a", now=1_234_567_890) + + +def test__Token__check__pass__optional_now(create_token): + + tok = create_token(key="ohsosecret") + tok.restrict(not_before=1_000_000_000, not_after=3_000_000_000) + tok.check(key="ohsosecret", project="a") def test__Token__check__fail__signature(create_token): From 35ec4da49708de060b276d04c5e17832f4c60359 Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 29 May 2022 02:31:57 +0200 Subject: [PATCH 3/4] Documentation --- docs/discussions.rst | 10 ++++++---- docs/howto.rst | 15 ++++++++++----- docs/reference.rst | 6 ++++++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/docs/discussions.rst b/docs/discussions.rst index e2d400c..85b0d67 100644 --- a/docs/discussions.rst +++ b/docs/discussions.rst @@ -35,8 +35,7 @@ The specification doesn't say what the caveat strings should be, but the idea is each caveat is adding a restriction to the original macaroon, so that when the bearer has to send the macaroon, they only delegate the smallest fraction of their power. -It's up to the Macaroon original minter to decide how to understand the caveats, -but when they receive a macaroon for +It's up to the Macaroon original minter to decide how to understand the caveats. .. _HMAC: https://en.wikipedia.org/wiki/HMAC @@ -65,7 +64,7 @@ then from the database, the key and user are extracted. The macaroon is checked the key and caveats are checked against the upload information. If the macaroon is valid, then PyPI checks if the user has upload rights on the package, and then proceeds. -The caveats are json-encoded strings, and as of March 2021, they come in 2 flavors: +The caveats are json-encoded strings, and as of May 2022, they come in 3 flavors: - ``"{"version": 1, "permissions": "user"}"`` (note: it's the string ``"user"`` here, not a placeholder for the username), which is always met. It's represented in this @@ -74,6 +73,10 @@ The caveats are json-encoded strings, and as of March 2021, they come in 2 flavo ``""`` is a placeholder here). It's met if the project we upload is among the ones listed in the caveats. It's represented by the `ProjectsRestriction`. +- ``"{"nbf": , "exp": }"``. It's met if we try uploading + the project between ``nbf`` (included) and ``exp`` (excluded). It's + represented by the + `DateRestriction`. The projects restriction that PyPI implements is limited to: @@ -107,7 +110,6 @@ The restrictions planned for the future are: - Version-based restriction - Filename-based restriction - Hash-sum-based restriction -- Time-window-based restriction - IP-based restriction - One-time-use restriction (this will require Warehouse to remember a value) - Somehow restricting to uploads coming from a given project's CI diff --git a/docs/howto.rst b/docs/howto.rst index c76b800..2ad57b8 100644 --- a/docs/howto.rst +++ b/docs/howto.rst @@ -108,7 +108,7 @@ elsewhere. Of course, just creating a macaroon with this library is not enough to have it be valid on PyPI: valid macaroons need to exist in the database and this -library only handles the computation part, not the stroring part. +library only handles the computation part, not the storing part. Create a token -------------- @@ -123,10 +123,12 @@ Use `Token.create`, `Token.restrict`, `Token.dump`:: prefix="pypi", ) - # Use either - token.restrict(projects=["project-normalized-name"]) # project-specific token - # Or + # Use token.restrict() # user-wide token + # Or + token.restrict(projects=["project-normalized-name"]) # project-specific token + # You can also restrict the token in time: + token.restrict(not_before=timestamp_or_tz_aware_dt, not_after=timestamp_or_tz_aware_dt) token_to_display = token.dump() @@ -151,7 +153,7 @@ Use `Token.load`, `Token.check`:: try: # The project the user is currently uploading - token.check(project="project-normalize-name") + token.check(project="project-normalize-name", now=int(time.time())) except pypitoken.ValidationError: display_error(exc) return Http403() @@ -164,3 +166,6 @@ If you find a case where the exception is not as helpful as it should be, and yo believe the program has more information but it was lost during the exception bubbling phase, or if the information in the exception is not appropriate to be shown back to the user, this will be considered a ``pypitoken`` bug, feel free to open an issue. + +You may omit the ``now`` parameter in the `Token.check` call, it will default +to the current integer timestamp. That said, it's ok to be explicit. diff --git a/docs/reference.rst b/docs/reference.rst index 0f5eb4c..bc0949f 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -24,6 +24,9 @@ should not have to call the methods directly. Use `Token.restrict` and .. autoclass:: pypitoken.ProjectsRestriction :show-inheritance: +.. autoclass:: pypitoken.DateRestriction + :show-inheritance: + Exceptions ---------- @@ -36,3 +39,6 @@ Exceptions .. autoclass:: pypitoken.ValidationError :show-inheritance: + +.. autoclass:: pypitoken.InvalidRestriction + :show-inheritance: From 16dced972f5e0d36716abe453ef6f397c12c6b2b Mon Sep 17 00:00:00 2001 From: Joachim Jablon Date: Sun, 29 May 2022 19:52:18 +0200 Subject: [PATCH 4/4] Fix signature on Token.restrict (improve documentation) --- pypitoken/token.py | 13 ++++++++++++- pypitoken/utils.py | 27 +++++++++++++++++++++++++++ tests/test_token.py | 7 +++++++ tests/test_utils.py | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 pypitoken/utils.py create mode 100644 tests/test_utils.py diff --git a/pypitoken/token.py b/pypitoken/token.py index 8342b88..8821da3 100644 --- a/pypitoken/token.py +++ b/pypitoken/token.py @@ -8,7 +8,7 @@ import jsonschema import pymacaroons -from pypitoken import exceptions +from pypitoken import exceptions, utils PREFIX = "pypi" @@ -149,6 +149,12 @@ def from_parameters(cls: Type[T], **kwargs) -> Optional[T]: """ raise NotImplementedError + @classmethod + def restriction_parameters(cls): + return utils.merge_parameters( + *(subclass.from_parameters for subclass in cls._get_subclasses()) + ) + @classmethod def restrictions_from_parameters(cls, **kwargs) -> Iterable["Restriction"]: """ @@ -666,3 +672,8 @@ def restrictions(self) -> List[Restriction]: Restriction.load_json(caveat=caveat.caveat_id) for caveat in self._macaroon.caveats ] + + +utils.replace_signature( + method=Token.restrict, parameters=Restriction.restriction_parameters() +) diff --git a/pypitoken/utils.py b/pypitoken/utils.py new file mode 100644 index 0000000..e55718d --- /dev/null +++ b/pypitoken/utils.py @@ -0,0 +1,27 @@ +import inspect +from typing import Callable, List + + +def merge_parameters(*callables: Callable) -> List[inspect.Parameter]: + """ + Return a list of Parameters object matching every parameters from input callables. + """ + return [ + param + for func in callables + for _, param in inspect.signature(func).parameters.items() + if param.kind != inspect.Parameter.VAR_KEYWORD + ] + + +def replace_signature(method: Callable, parameters: List[inspect.Parameter]) -> None: + """ + On the received method, keep the self parameter. Replace the other parameters + with the list reciend in ``parameters``. + + Currently only supports methods. + """ + signature = inspect.signature(method) + method.__signature__ = inspect.signature(method).replace( # type: ignore + parameters=[signature.parameters["self"]] + parameters + ) diff --git a/tests/test_token.py b/tests/test_token.py index c006aa4..25f9106 100644 --- a/tests/test_token.py +++ b/tests/test_token.py @@ -1,5 +1,6 @@ import dataclasses import datetime +import inspect import pymacaroons import pytest @@ -579,3 +580,9 @@ def test__Token__restrictions(create_token): token.ProjectsRestriction(projects=["a", "b"]), token.ProjectsRestriction(projects=["a", "d"]), ] + + +def test_Token__restrict__signature(): + assert ( + "projects" in inspect.Signature.from_callable(token.Token.restrict).parameters + ) diff --git a/tests/test_utils.py b/tests/test_utils.py new file mode 100644 index 0000000..844dccd --- /dev/null +++ b/tests/test_utils.py @@ -0,0 +1,39 @@ +import inspect + +from pypitoken import utils + + +def test__merge_parameters(): + def a(b: int) -> str: + pass + + def c(*, d: float) -> None: + pass + + result = utils.merge_parameters(a, c) + + assert result == [ + inspect.Parameter( + "b", kind=inspect.Parameter.POSITIONAL_OR_KEYWORD, annotation=int + ), + inspect.Parameter("d", kind=inspect.Parameter.KEYWORD_ONLY, annotation=float), + ] + + +def test__replace_signature(): + def a(b: int) -> str: + pass + + def c(*, d: float, **kwargs) -> None: + pass + + class E: + def f(self, **kwargs) -> int: # Input + pass + + def g(self, b: int, *, d: float) -> int: # What we expect + pass + + utils.replace_signature(E.f, utils.merge_parameters(a, c)) + + assert inspect.Signature.from_callable(E.f) == inspect.Signature.from_callable(E.g)