diff --git a/CHANGELOG.md b/CHANGELOG.md index a423100..1579a91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,25 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **`cryptography` and `redis` accept their current major versions**, here and + in the generated project: `cryptography<51` and `redis<9`. The challenge store + dropped two casts and widened one that redis 8's own type hints now contradict, + which is all the newer driver asked for. + +### Fixed + +- **The in-process challenge store no longer grows for the life of the process.** + Nothing expires a key in a dict, and `LocMemChallengeStore` dropped a lapsed + record only when something looked that same key up again -- which a code that + is requested and never used, and every rate-limit counter, never are. One per + address, per destination and per account tried were held until a restart, so an + anonymous caller could grow a worker's memory by asking. The two write paths + sweep what has expired first. + ## [2.1.0] - 2026-09-30 ### Added diff --git a/pyproject.toml b/pyproject.toml index 9804e38..4f8e78e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,7 +56,7 @@ django-ninja-starter = "django_ninja_starter.cli:main" [project.optional-dependencies] dev = [ "Django>=5.2,<7.0", - "cryptography>=45,<48", + "cryptography>=45,<51", "build>=1.2", "django-ninja>=1.4,<2.0", "django-socio-grpc>=0.25,<1.0", @@ -69,7 +69,7 @@ dev = [ "python-dotenv>=1.1,<2.0", "PyJWT>=2.10,<3.0", "pyotp>=2.9,<3.0", - "redis>=5.0,<7.0", + "redis>=5.0,<9.0", "fakeredis>=2.26,<3.0", "mypy-protobuf>=3.6", "ruff>=0.11", diff --git a/src/django_ninja_starter/template/pyproject.toml b/src/django_ninja_starter/template/pyproject.toml index 13e383c..099f87d 100644 --- a/src/django_ninja_starter/template/pyproject.toml +++ b/src/django_ninja_starter/template/pyproject.toml @@ -24,7 +24,7 @@ requires-python = ">=3.12" # opens. Each is imported only by the module its feature loads, so leaving an # extra out costs nothing until something actually reaches for it. dependencies = [ - "cryptography>=45,<48", + "cryptography>=45,<51", "Django>=5.2,<7.0", "django-ninja>=1.4,<2.0", "django-socio-grpc>=0.25,<1.0", @@ -38,7 +38,7 @@ dependencies = [ # Social sign-in, for talking to the providers. oauth = ["httpx>=0.28,<1.0"] # The challenge store that survives a restart and reaches every worker. -redis = ["redis>=5.0,<7.0"] +redis = ["redis>=5.0,<9.0"] # Authenticator-app second factors. totp = ["pyotp>=2.9,<3.0"] # An ASGI server and a WebSocket protocol implementation, for the notification diff --git a/src/django_ninja_starter/template/src/infrastructure/auth/core/challenges.py b/src/django_ninja_starter/template/src/infrastructure/auth/core/challenges.py index ddc897a..2617d6e 100644 --- a/src/django_ninja_starter/template/src/infrastructure/auth/core/challenges.py +++ b/src/django_ninja_starter/template/src/infrastructure/auth/core/challenges.py @@ -173,7 +173,7 @@ def create( record = _record(purpose, subject, channel, destination, metadata, code_hash) key = self._key(ticket) pipeline = self.client.pipeline() - pipeline.hset(key, mapping=record) + pipeline.hset(key, mapping=cast("Any", record)) pipeline.expire(key, lifetime) pipeline.execute() return ticket @@ -181,7 +181,7 @@ def create( def verify(self, ticket: str, code: str, *, purpose: str) -> Challenge: record = self._load(ticket, purpose) key = self._key(ticket) - attempts = cast(int, self.client.hincrby(key, "attempts", 1)) + attempts = self.client.hincrby(key, "attempts", 1) if attempts > settings.AUTH_CHALLENGE_MAX_ATTEMPTS: self.client.delete(key) raise ChallengeAttemptsExhausted("Too many incorrect codes. Request a new one.") @@ -201,7 +201,7 @@ def fail(self, ticket: str) -> int: key = self._key(ticket) if not self.client.exists(key): return 0 - attempts = cast(int, self.client.hincrby(key, "attempts", 1)) + attempts = self.client.hincrby(key, "attempts", 1) if attempts > settings.AUTH_CHALLENGE_MAX_ATTEMPTS: self.client.delete(key) return attempts @@ -223,12 +223,27 @@ class LocMemChallengeStore: Deliberately not safe across processes: a multi-worker deployment would hand step two to a worker that never saw step one. + + Unlike Redis, nothing here expires a key on its own: a lapsed record is + dropped when something looks it up again, and most never are. A code that is + requested and never used, and every rate-limit counter -- one per address, + per destination, per account tried -- would otherwise be held for the life of + the process, which is memory an anonymous caller can grow by asking. So the + two write paths sweep first. """ def __init__(self) -> None: self._records: dict[str, tuple[float, dict[str, str]]] = {} self._counters: dict[str, tuple[float, int]] = {} + def _sweep(self) -> None: + # ponytail: a full pass over both dicts per write. They stay small + # because of it; a store that needs better than that wants Redis, which + # is the other implementation in this module. + now = time.monotonic() + self._records = {k: v for k, v in self._records.items() if v[0] > now} + self._counters = {k: v for k, v in self._counters.items() if v[0] > now} + def _live(self, key: str) -> dict[str, str] | None: entry = self._records.get(key) if entry is None: @@ -260,6 +275,7 @@ def create( metadata: dict[str, Any] | None = None, ttl: int | None = None, ) -> str: + self._sweep() ticket = generate_ticket() lifetime = ttl if ttl is not None else settings.AUTH_CHALLENGE_TTL_SECONDS code_hash = hash_code(ticket, purpose, code) if code else "" @@ -301,6 +317,7 @@ def discard(self, ticket: str) -> None: self._records.pop(ticket_key(ticket), None) def increment(self, key: str, ttl: int) -> int: + self._sweep() entry = self._counters.get(key) now = time.monotonic() if entry is None or entry[0] <= now: diff --git a/src/django_ninja_starter/template/src/infrastructure/auth/core/tests/test_internals.py b/src/django_ninja_starter/template/src/infrastructure/auth/core/tests/test_internals.py index 1ff0e09..3ab2bed 100644 --- a/src/django_ninja_starter/template/src/infrastructure/auth/core/tests/test_internals.py +++ b/src/django_ninja_starter/template/src/infrastructure/auth/core/tests/test_internals.py @@ -196,3 +196,21 @@ def test_corrupt_metadata_degrades_to_an_empty_mapping(db: None) -> None: store._records[ticket_key(ticket)][1]["metadata"] = "{not json" assert store.read(ticket, purpose="pending").metadata == {} + + +def test_the_in_process_store_does_not_hoard_what_has_expired() -> None: + """Nothing expires a key in a dict, so the write paths have to drop them. + + A code nobody comes back for, and a counter per address tried, would + otherwise be held until the process restarted. + """ + from infrastructure.auth.core.challenges import LocMemChallengeStore + + store = LocMemChallengeStore() + store.create(purpose="pending", subject="7", ttl=-1) + store.increment("attempts:gone", -1) + + store.create(purpose="pending", subject="8") + + assert len(store._records) == 1 + assert store._counters == {} diff --git a/src/infrastructure/auth/core/challenges.py b/src/infrastructure/auth/core/challenges.py index ddc897a..2617d6e 100644 --- a/src/infrastructure/auth/core/challenges.py +++ b/src/infrastructure/auth/core/challenges.py @@ -173,7 +173,7 @@ def create( record = _record(purpose, subject, channel, destination, metadata, code_hash) key = self._key(ticket) pipeline = self.client.pipeline() - pipeline.hset(key, mapping=record) + pipeline.hset(key, mapping=cast("Any", record)) pipeline.expire(key, lifetime) pipeline.execute() return ticket @@ -181,7 +181,7 @@ def create( def verify(self, ticket: str, code: str, *, purpose: str) -> Challenge: record = self._load(ticket, purpose) key = self._key(ticket) - attempts = cast(int, self.client.hincrby(key, "attempts", 1)) + attempts = self.client.hincrby(key, "attempts", 1) if attempts > settings.AUTH_CHALLENGE_MAX_ATTEMPTS: self.client.delete(key) raise ChallengeAttemptsExhausted("Too many incorrect codes. Request a new one.") @@ -201,7 +201,7 @@ def fail(self, ticket: str) -> int: key = self._key(ticket) if not self.client.exists(key): return 0 - attempts = cast(int, self.client.hincrby(key, "attempts", 1)) + attempts = self.client.hincrby(key, "attempts", 1) if attempts > settings.AUTH_CHALLENGE_MAX_ATTEMPTS: self.client.delete(key) return attempts @@ -223,12 +223,27 @@ class LocMemChallengeStore: Deliberately not safe across processes: a multi-worker deployment would hand step two to a worker that never saw step one. + + Unlike Redis, nothing here expires a key on its own: a lapsed record is + dropped when something looks it up again, and most never are. A code that is + requested and never used, and every rate-limit counter -- one per address, + per destination, per account tried -- would otherwise be held for the life of + the process, which is memory an anonymous caller can grow by asking. So the + two write paths sweep first. """ def __init__(self) -> None: self._records: dict[str, tuple[float, dict[str, str]]] = {} self._counters: dict[str, tuple[float, int]] = {} + def _sweep(self) -> None: + # ponytail: a full pass over both dicts per write. They stay small + # because of it; a store that needs better than that wants Redis, which + # is the other implementation in this module. + now = time.monotonic() + self._records = {k: v for k, v in self._records.items() if v[0] > now} + self._counters = {k: v for k, v in self._counters.items() if v[0] > now} + def _live(self, key: str) -> dict[str, str] | None: entry = self._records.get(key) if entry is None: @@ -260,6 +275,7 @@ def create( metadata: dict[str, Any] | None = None, ttl: int | None = None, ) -> str: + self._sweep() ticket = generate_ticket() lifetime = ttl if ttl is not None else settings.AUTH_CHALLENGE_TTL_SECONDS code_hash = hash_code(ticket, purpose, code) if code else "" @@ -301,6 +317,7 @@ def discard(self, ticket: str) -> None: self._records.pop(ticket_key(ticket), None) def increment(self, key: str, ttl: int) -> int: + self._sweep() entry = self._counters.get(key) now = time.monotonic() if entry is None or entry[0] <= now: diff --git a/src/infrastructure/auth/core/tests/test_internals.py b/src/infrastructure/auth/core/tests/test_internals.py index 1ff0e09..3ab2bed 100644 --- a/src/infrastructure/auth/core/tests/test_internals.py +++ b/src/infrastructure/auth/core/tests/test_internals.py @@ -196,3 +196,21 @@ def test_corrupt_metadata_degrades_to_an_empty_mapping(db: None) -> None: store._records[ticket_key(ticket)][1]["metadata"] = "{not json" assert store.read(ticket, purpose="pending").metadata == {} + + +def test_the_in_process_store_does_not_hoard_what_has_expired() -> None: + """Nothing expires a key in a dict, so the write paths have to drop them. + + A code nobody comes back for, and a counter per address tried, would + otherwise be held until the process restarted. + """ + from infrastructure.auth.core.challenges import LocMemChallengeStore + + store = LocMemChallengeStore() + store.create(purpose="pending", subject="7", ttl=-1) + store.increment("attempts:gone", -1) + + store.create(purpose="pending", subject="8") + + assert len(store._records) == 1 + assert store._counters == {}