diff --git a/CHANGELOG.md b/CHANGELOG.md index 84a52342..8c9ef2ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,38 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). --- +## v26.06.12 (2026-06-05) + +### Security + +- **JWT now requires `exp`.** `JWTService.decode` (and the JWKS resource-server + validator) verified `exp` only when present, so a token minted with **no `exp`** + was accepted and never expired. `decode` now requires `exp` + (`options={"require": ["exp"]}`), and `JWTService.encode` auto-adds an `exp` + (now + `expiration_seconds`, default 3600) when the payload omits one — so every + issued token expires. +- **OAuth2 client secret compared in constant time.** `AuthorizationServer` + compared the client secret with `!=` (a timing side-channel); it now uses + `secrets.compare_digest`. +- **OAuth2 grant-type confusion fixed.** The token endpoint ignored the client's + registered `authorization_grant_type`, so a client registered for + `authorization_code` could mint `client_credentials` tokens. The + `client_credentials` grant now requires the client to be registered for it + (otherwise `UNAUTHORIZED_CLIENT`); server-unsupported grants still return + `UNSUPPORTED_GRANT_TYPE`. +- **`HttpSecurity` footgun warning.** `build()` now logs a warning when + authorization rules are configured without a terminal `any_request()` rule + (paths matching no rule fall through allowed) — recommending + `.any_request().deny_all()` / `.authenticated()`. + +These surfaced in an adversarial security audit while validating the +`implement-security` skill (which itself validated clean — enforcement proven, +no silent auth bypass). Session-subsystem hardening (session-fixation id +rotation, `secure` cookie default, Redis-deserialization allowlisting) plus a +dedicated `tests/session` suite are tracked as a focused follow-up. + +--- + ## v26.06.11 (2026-06-05) ### Fixed diff --git a/README.md b/README.md index b3fc22ee..4d0c33c6 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Firefly Framework Python 3.12+ License: Apache 2.0 - Version: 26.06.11 + Version: 26.06.12 Type Checked: mypy strict Code Style: Ruff Async First diff --git a/pyproject.toml b/pyproject.toml index 9e399fe7..e2d5928c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ name = "pyfly" # CalVer YY.MM.PATCH — package metadata uses PEP 440 normalized form (26.5.4); # git tag, GitHub release and human-readable display use leading-zero form # (v26.05.04) to match the Java/.NET/Go siblings. -version = "26.6.11" +version = "26.6.12" description = "The official Python implementation of the Firefly Framework — DI, CQRS, EDA, hexagonal architecture, and more." readme = "README.md" license = "Apache-2.0" diff --git a/src/pyfly/__init__.py b/src/pyfly/__init__.py index 3554c159..f334f015 100644 --- a/src/pyfly/__init__.py +++ b/src/pyfly/__init__.py @@ -13,4 +13,4 @@ # limitations under the License. """PyFly — Enterprise Python Framework.""" -__version__ = "26.06.11" +__version__ = "26.06.12" diff --git a/src/pyfly/security/http_security.py b/src/pyfly/security/http_security.py index 9f72cca1..b7b7b7ae 100644 --- a/src/pyfly/security/http_security.py +++ b/src/pyfly/security/http_security.py @@ -31,6 +31,7 @@ from __future__ import annotations +import logging from dataclasses import dataclass, field from enum import Enum, auto from typing import TYPE_CHECKING @@ -38,6 +39,8 @@ if TYPE_CHECKING: from pyfly.web.adapters.starlette.filters.http_security_filter import HttpSecurityFilter +_logger = logging.getLogger(__name__) + # --------------------------------------------------------------------------- # Access rule model @@ -196,4 +199,15 @@ def build(self) -> HttpSecurityFilter: """ from pyfly.web.adapters.starlette.filters.http_security_filter import HttpSecurityFilter + # Footgun guard: without a terminal any_request() rule (patterns == []), a + # path matching no rule falls through ALLOWED. Warn so an operator adds an + # explicit terminal — e.g. .any_request().deny_all() / .authenticated() — + # as the class docstring demonstrates. + if self._rules and not any(not rule.patterns for rule in self._rules): + _logger.warning( + "HttpSecurity has %d authorization rule(s) but no terminal any_request() rule; " + "requests matching no rule are ALLOWED through. Add .any_request().deny_all() " + "(or .authenticated()) to deny unmatched paths.", + len(self._rules), + ) return HttpSecurityFilter(rules=list(self._rules)) diff --git a/src/pyfly/security/jwt.py b/src/pyfly/security/jwt.py index 126fb2cb..bbaf629a 100644 --- a/src/pyfly/security/jwt.py +++ b/src/pyfly/security/jwt.py @@ -15,6 +15,7 @@ from __future__ import annotations +import time from typing import Any import jwt @@ -29,24 +30,42 @@ class JWTService: Args: secret: Secret key for HMAC-based signing. algorithm: JWT algorithm (default: HS256). + expiration_seconds: Default token lifetime; an ``exp`` claim is added on + :meth:`encode` when the payload does not already carry one. """ - def __init__(self, secret: str, algorithm: str = "HS256") -> None: + def __init__(self, secret: str, algorithm: str = "HS256", expiration_seconds: int = 3600) -> None: self._secret = secret self._algorithm = algorithm + self._expiration_seconds = expiration_seconds def encode(self, payload: dict[str, Any]) -> str: - """Encode a payload into a JWT token.""" + """Encode a payload into a JWT token. + + Adds an ``exp`` claim (now + ``expiration_seconds``) when the payload does + not already specify one, so every issued token expires — :meth:`decode` + requires ``exp``. + """ + if "exp" not in payload: + payload = {**payload, "exp": int(time.time()) + self._expiration_seconds} return jwt.encode(payload, self._secret, algorithm=self._algorithm) def decode(self, token: str) -> dict[str, Any]: """Decode and validate a JWT token. + Requires a valid signature **and** an ``exp`` claim — a token minted + without ``exp`` (which would never expire) is rejected. + Raises: - SecurityException: If the token is invalid or expired. + SecurityException: If the token is invalid, expired, or lacks ``exp``. """ try: - return jwt.decode(token, self._secret, algorithms=[self._algorithm]) + return jwt.decode( + token, + self._secret, + algorithms=[self._algorithm], + options={"require": ["exp"]}, + ) except jwt.PyJWTError as exc: raise SecurityException( f"Invalid token: {exc}", diff --git a/src/pyfly/security/oauth2/authorization_server.py b/src/pyfly/security/oauth2/authorization_server.py index dd8a6d22..eee3d5cc 100644 --- a/src/pyfly/security/oauth2/authorization_server.py +++ b/src/pyfly/security/oauth2/authorization_server.py @@ -116,12 +116,23 @@ async def token( Raises: SecurityException: If authentication fails or grant type is unsupported. """ - # Authenticate client + # Authenticate client (constant-time secret comparison to avoid a timing + # side-channel that could leak the client secret). registration = self._client_repository.find_by_registration_id(client_id) - if registration is None or registration.client_secret != client_secret: + if registration is None or not secrets.compare_digest( + registration.client_secret.encode("utf-8"), client_secret.encode("utf-8") + ): raise SecurityException("Invalid client credentials", code="INVALID_CLIENT") if grant_type == "client_credentials": + # The client must be registered for the client_credentials grant to + # mint a client_credentials token — prevents grant-type confusion (a + # client registered only for authorization_code must not use it). + if registration.authorization_grant_type != "client_credentials": + raise SecurityException( + f"Client '{client_id}' is not authorized for grant type 'client_credentials'", + code="UNAUTHORIZED_CLIENT", + ) return await self._handle_client_credentials(registration, scope) elif grant_type == "refresh_token": if refresh_token is None: diff --git a/src/pyfly/security/oauth2/resource_server.py b/src/pyfly/security/oauth2/resource_server.py index 24bf91be..3b4e152a 100644 --- a/src/pyfly/security/oauth2/resource_server.py +++ b/src/pyfly/security/oauth2/resource_server.py @@ -70,6 +70,7 @@ def validate(self, token: str) -> dict[str, Any]: algorithms=self._algorithms, issuer=self._issuer, audience=self._audience, + options={"require": ["exp"]}, ) return payload except jwt.PyJWTError as exc: diff --git a/tests/security/test_authorization_server.py b/tests/security/test_authorization_server.py index 3e93e6ba..072b1005 100644 --- a/tests/security/test_authorization_server.py +++ b/tests/security/test_authorization_server.py @@ -39,6 +39,7 @@ def client_repo() -> InMemoryClientRegistrationRepository: registration_id="test-client", client_id="test-client", client_secret="test-secret", + authorization_grant_type="client_credentials", scopes=["read", "write"], ) return InMemoryClientRegistrationRepository(reg) diff --git a/tests/security/test_oauth2_resource_server.py b/tests/security/test_oauth2_resource_server.py index b098decb..b7ce0f2c 100644 --- a/tests/security/test_oauth2_resource_server.py +++ b/tests/security/test_oauth2_resource_server.py @@ -15,6 +15,7 @@ from __future__ import annotations +import time from unittest.mock import MagicMock, patch import jwt @@ -34,7 +35,9 @@ def _create_test_token(payload: dict, kid: str = "test-kid") -> str: - """Create an RS256-signed JWT for testing.""" + """Create an RS256-signed JWT for testing (adds an ``exp`` claim by default).""" + if "exp" not in payload: + payload = {**payload, "exp": int(time.time()) + 3600} private_pem = _private_key.private_bytes( encoding=serialization.Encoding.PEM, format=serialization.PrivateFormat.PKCS8, diff --git a/tests/security/test_security_hardening.py b/tests/security/test_security_hardening.py new file mode 100644 index 00000000..4d6a9596 --- /dev/null +++ b/tests/security/test_security_hardening.py @@ -0,0 +1,116 @@ +# Copyright 2026 Firefly Software Foundation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Regression tests for security hardening (v26.06.12). + +- JWTService requires an ``exp`` claim on decode and auto-adds one on encode + (a token minted without ``exp`` would otherwise never expire). +- OAuth2 AuthorizationServer enforces the client's registered grant type — a + client registered for ``authorization_code`` cannot mint ``client_credentials`` + tokens — and compares the client secret in constant time. +- HttpSecurity.build() warns when rules are configured without a terminal + ``any_request()`` rule (unmatched paths fall through allowed). +""" + +from __future__ import annotations + +import logging +import time + +import jwt +import pytest + +from pyfly.kernel.exceptions import SecurityException +from pyfly.security.http_security import HttpSecurity +from pyfly.security.jwt import JWTService +from pyfly.security.oauth2.authorization_server import AuthorizationServer, InMemoryTokenStore +from pyfly.security.oauth2.client import ClientRegistration, InMemoryClientRegistrationRepository + +_SECRET = "test-secret-key-minimum-32-chars!" + + +class TestJWTExpRequired: + def test_encode_adds_exp_claim(self) -> None: + svc = JWTService(secret=_SECRET) + payload = svc.decode(svc.encode({"sub": "u1"})) + assert "exp" in payload + + def test_decode_rejects_token_without_exp(self) -> None: + svc = JWTService(secret=_SECRET) + # Minted directly, bypassing encode()'s auto-exp — must be rejected. + no_exp = jwt.encode({"sub": "u1"}, _SECRET, algorithm="HS256") + with pytest.raises(SecurityException, match="Invalid token"): + svc.decode(no_exp) + + def test_explicit_exp_is_preserved(self) -> None: + svc = JWTService(secret=_SECRET) + exp = int(time.time()) + 99 + payload = svc.decode(svc.encode({"sub": "u1", "exp": exp})) + assert payload["exp"] == exp + + +def _server(grant_type: str) -> AuthorizationServer: + reg = ClientRegistration( + registration_id="c", + client_id="c", + client_secret="s3cr3t-value", + authorization_grant_type=grant_type, + scopes=["read"], + ) + return AuthorizationServer( + secret=_SECRET, + client_repository=InMemoryClientRegistrationRepository(reg), + token_store=InMemoryTokenStore(), + issuer="https://auth.example.com", + ) + + +class TestOAuth2GrantTypeEnforcement: + @pytest.mark.asyncio + async def test_registered_client_can_use_client_credentials(self) -> None: + result = await _server("client_credentials").token( + grant_type="client_credentials", client_id="c", client_secret="s3cr3t-value" + ) + assert "access_token" in result + + @pytest.mark.asyncio + async def test_authorization_code_client_cannot_mint_client_credentials(self) -> None: + with pytest.raises(SecurityException, match="not authorized for grant type"): + await _server("authorization_code").token( + grant_type="client_credentials", client_id="c", client_secret="s3cr3t-value" + ) + + @pytest.mark.asyncio + async def test_wrong_secret_rejected(self) -> None: + with pytest.raises(SecurityException, match="Invalid client credentials"): + await _server("client_credentials").token( + grant_type="client_credentials", client_id="c", client_secret="wrong-secret" + ) + + +class TestHttpSecurityTerminalWarning: + def test_warns_without_any_request_rule(self, caplog: pytest.LogCaptureFixture) -> None: + sec = HttpSecurity() + sec.authorize_requests().request_matchers("/api/**").authenticated() + with caplog.at_level(logging.WARNING, logger="pyfly.security.http_security"): + sec.build() + assert any("no terminal any_request" in r.getMessage() for r in caplog.records) + + def test_no_warning_with_any_request_terminal(self, caplog: pytest.LogCaptureFixture) -> None: + sec = HttpSecurity() + builder = sec.authorize_requests() + builder.request_matchers("/api/**").authenticated() + builder.any_request().deny_all() + with caplog.at_level(logging.WARNING, logger="pyfly.security.http_security"): + sec.build() + assert not any("no terminal any_request" in r.getMessage() for r in caplog.records) diff --git a/uv.lock b/uv.lock index 8587329b..4b4ec0e5 100644 --- a/uv.lock +++ b/uv.lock @@ -1967,7 +1967,7 @@ wheels = [ [[package]] name = "pyfly" -version = "26.6.11" +version = "26.6.12" source = { editable = "." } dependencies = [ { name = "pydantic" },