diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f6651a11..1881de9e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ All notable changes to this project are documented in this file. The format is b - Scaffold new modules with `core/` and `web/` areas and mirrored nested Python tests; existing modules migrate individually. See `docs/module-layout.md`. - `mail` follows the module layout: `mail.core.{constants,settings,state,data_compatibility}` and `mail.delivery.{sender,runtime}`. `mail.settings` and `mail.constants` still import, with a `DeprecationWarning`, until 0.0.46. +- `site_lock` follows the module layout: `site_lock.core.{constants,settings,state}` and `site_lock.web.{middleware,page,rate_limit}` (the unlock template moves with the page). `site_lock.settings` still imports, with a `DeprecationWarning`, until 0.0.46. ### Fixed diff --git a/docs/module-layout.md b/docs/module-layout.md index 1288ce1b3..d5caae551 100644 --- a/docs/module-layout.md +++ b/docs/module-layout.md @@ -210,6 +210,6 @@ when a later module PR moves files. | permissions | 12 | 14 | flat | | settings | 31 | 43 | flat | | sharing | 13 | 6 | flat | -| site_lock | 8 | 5 | flat | +| site_lock | 2 (+1 shim) | 0 | follows this layout | | tenants | 19 | 23 | flat | | users | 31 | 97 | flat | diff --git a/host/tests/test_module_layout.py b/host/tests/test_module_layout.py index e6664c9a8..5adbcc8a9 100644 --- a/host/tests/test_module_layout.py +++ b/host/tests/test_module_layout.py @@ -127,11 +127,6 @@ services.py settings.py tenant_cleanup.py user_cleanup.py """.split() ), - "site_lock": frozenset( - """ - constants.py middleware.py page.py rate_limit.py settings.py state.py templates - """.split() - ), "tenants": frozenset( """ _deletion.py _membership_ops.py _owner_guard.py components constants.py deps.py diff --git a/modules/settings/tests/test_lenient_related_fields_validators.py b/modules/settings/tests/test_lenient_related_fields_validators.py index d6c3a57f5..7daf471dc 100644 --- a/modules/settings/tests/test_lenient_related_fields_validators.py +++ b/modules/settings/tests/test_lenient_related_fields_validators.py @@ -14,7 +14,7 @@ from simple_module_core.settings_hydrate import RELATED_FIELDS_CTX, build_settings from simple_module_core.stored_settings import construct_leniently from simple_module_hosting.host_settings import HostSettings -from site_lock.settings import SiteLockSettings +from site_lock.core.settings import SiteLockSettings from users.settings import UsersSettings diff --git a/modules/site_lock/site_lock/core/__init__.py b/modules/site_lock/site_lock/core/__init__.py new file mode 100644 index 000000000..755db354f --- /dev/null +++ b/modules/site_lock/site_lock/core/__init__.py @@ -0,0 +1 @@ +"""What the site lock builds on: constants, settings and app state.""" diff --git a/modules/site_lock/site_lock/constants.py b/modules/site_lock/site_lock/core/constants.py similarity index 100% rename from modules/site_lock/site_lock/constants.py rename to modules/site_lock/site_lock/core/constants.py diff --git a/modules/site_lock/site_lock/core/settings.py b/modules/site_lock/site_lock/core/settings.py new file mode 100644 index 000000000..01afc3f6b --- /dev/null +++ b/modules/site_lock/site_lock/core/settings.py @@ -0,0 +1,55 @@ +"""Site Lock module settings — DB-backed via ``register_module_settings``. + +The field is named ``password`` so the settings admin UI masks it +automatically (``settings._module_settings.is_secret_field``). +""" + +from __future__ import annotations + +from pydantic import model_validator +from pydantic_settings import SettingsConfigDict +from simple_module_core.settings_base import DbBackedSettings +from simple_module_core.stored_settings import field_error + +_BLANK_PASSWORD_MSG = "password must be set before enabling the site lock" + + +class SiteLockSettings(DbBackedSettings): + """Site-wide shared-password gate configuration.""" + + # ``DbBackedSettings`` (not ``BaseSettings``) so the DB is genuinely the + # only source: omitting ``env_prefix`` would leave pydantic-settings + # reading each field from its bare name — GH #283. + model_config = SettingsConfigDict(extra="ignore") + + enabled: bool = False + password: str = "" + message: str = "" + + @model_validator(mode="after") + def _password_required_when_enabled(self) -> SiteLockSettings: + """Refuse to gate the site behind an empty password. + + Without this, flipping ``enabled`` on before setting a password would + lock every visitor out from behind a secret that is the empty string. + Raising here makes ``apply_changes_and_reload`` reject the change so + the settings screen shows a validation error instead. + + The error is deliberately pinned to ``enabled`` rather than raised as a + bare ``ValueError``. A plain raise from a model validator carries + ``loc=[]``, and the shared settings form only renders errors it can + attach to a field (``ModuleForm.onSave`` keys them by ``loc[-1]``). + With an empty ``loc`` the 422 was silently swallowed: the admin saw the + toggle stay on, no error, and had every reason to believe the site was + locked when it was still wide open. + """ + if self.enabled and not self.password.strip(): + raise field_error( + type(self), + "enabled", + "site_lock_password_required", + _BLANK_PASSWORD_MSG, + value=self.enabled, + related=["enabled", "password"], + ) + return self diff --git a/modules/site_lock/site_lock/state.py b/modules/site_lock/site_lock/core/state.py similarity index 84% rename from modules/site_lock/site_lock/state.py rename to modules/site_lock/site_lock/core/state.py index add309fda..a14d09955 100644 --- a/modules/site_lock/site_lock/state.py +++ b/modules/site_lock/site_lock/core/state.py @@ -4,9 +4,9 @@ from dataclasses import dataclass, field -from site_lock import constants as c -from site_lock.rate_limit import AttemptLimiter -from site_lock.settings import SiteLockSettings +from site_lock.core import constants as c +from site_lock.core.settings import SiteLockSettings +from site_lock.web.rate_limit import AttemptLimiter def _default_limiter() -> AttemptLimiter: diff --git a/modules/site_lock/site_lock/module.py b/modules/site_lock/site_lock/module.py index 88cab113f..aef3ce5cd 100644 --- a/modules/site_lock/site_lock/module.py +++ b/modules/site_lock/site_lock/module.py @@ -8,7 +8,7 @@ from simple_module_core.module_settings import register_module_settings from simple_module_hosting.session import preserve_across_sign_in -from site_lock import constants as c +from site_lock.core import constants as c if TYPE_CHECKING: from fastapi import FastAPI @@ -26,8 +26,8 @@ class SiteLockModule(ModuleBase): ) def register_settings(self, app: FastAPI) -> None: - from site_lock.settings import SiteLockSettings - from site_lock.state import SiteLockState + from site_lock.core.settings import SiteLockSettings + from site_lock.core.state import SiteLockState # Signing in replaces the session; the unlock marker must outlive it. preserve_across_sign_in(app, c.SESSION_KEY) @@ -39,6 +39,6 @@ def register_settings(self, app: FastAPI) -> None: ) def register_middleware(self, app: FastAPI) -> None: - from site_lock.middleware import SiteLockMiddleware + from site_lock.web.middleware import SiteLockMiddleware app.add_middleware(SiteLockMiddleware) diff --git a/modules/site_lock/site_lock/settings.py b/modules/site_lock/site_lock/settings.py index 01afc3f6b..6e1356493 100644 --- a/modules/site_lock/site_lock/settings.py +++ b/modules/site_lock/site_lock/settings.py @@ -1,55 +1,11 @@ -"""Site Lock module settings — DB-backed via ``register_module_settings``. - -The field is named ``password`` so the settings admin UI masks it -automatically (``settings._module_settings.is_secret_field``). -""" +"""Moved to :mod:`site_lock.core.settings`; this alias goes in 0.0.46 (module layout).""" from __future__ import annotations -from pydantic import model_validator -from pydantic_settings import SettingsConfigDict -from simple_module_core.settings_base import DbBackedSettings -from simple_module_core.stored_settings import field_error - -_BLANK_PASSWORD_MSG = "password must be set before enabling the site lock" - - -class SiteLockSettings(DbBackedSettings): - """Site-wide shared-password gate configuration.""" - - # ``DbBackedSettings`` (not ``BaseSettings``) so the DB is genuinely the - # only source: omitting ``env_prefix`` would leave pydantic-settings - # reading each field from its bare name — GH #283. - model_config = SettingsConfigDict(extra="ignore") - - enabled: bool = False - password: str = "" - message: str = "" - - @model_validator(mode="after") - def _password_required_when_enabled(self) -> SiteLockSettings: - """Refuse to gate the site behind an empty password. +import importlib +import sys - Without this, flipping ``enabled`` on before setting a password would - lock every visitor out from behind a secret that is the empty string. - Raising here makes ``apply_changes_and_reload`` reject the change so - the settings screen shows a validation error instead. +from simple_module_core.deprecation import deprecated_module - The error is deliberately pinned to ``enabled`` rather than raised as a - bare ``ValueError``. A plain raise from a model validator carries - ``loc=[]``, and the shared settings form only renders errors it can - attach to a field (``ModuleForm.onSave`` keys them by ``loc[-1]``). - With an empty ``loc`` the 422 was silently swallowed: the admin saw the - toggle stay on, no error, and had every reason to believe the site was - locked when it was still wide open. - """ - if self.enabled and not self.password.strip(): - raise field_error( - type(self), - "enabled", - "site_lock_password_required", - _BLANK_PASSWORD_MSG, - value=self.enabled, - related=["enabled", "password"], - ) - return self +deprecated_module("site_lock.settings", remove_in="0.0.46", use="site_lock.core.settings") +sys.modules[__name__] = importlib.import_module("site_lock.core.settings") diff --git a/modules/site_lock/site_lock/web/__init__.py b/modules/site_lock/site_lock/web/__init__.py new file mode 100644 index 000000000..d585f3714 --- /dev/null +++ b/modules/site_lock/site_lock/web/__init__.py @@ -0,0 +1 @@ +"""The HTTP side of the lock: the gate middleware, the unlock page and its attempt limiter.""" diff --git a/modules/site_lock/site_lock/middleware.py b/modules/site_lock/site_lock/web/middleware.py similarity index 98% rename from modules/site_lock/site_lock/middleware.py rename to modules/site_lock/site_lock/web/middleware.py index 4f999194b..0d5d70f73 100644 --- a/modules/site_lock/site_lock/middleware.py +++ b/modules/site_lock/site_lock/web/middleware.py @@ -28,8 +28,8 @@ ) from starlette.types import ASGIApp, Receive, Scope, Send -from site_lock import constants as c -from site_lock.page import render_unlock_page, safe_next +from site_lock.core import constants as c +from site_lock.web.page import render_unlock_page, safe_next logger = logging.getLogger(__name__) diff --git a/modules/site_lock/site_lock/page.py b/modules/site_lock/site_lock/web/page.py similarity index 94% rename from modules/site_lock/site_lock/page.py rename to modules/site_lock/site_lock/web/page.py index 877dc1041..ebd075257 100644 --- a/modules/site_lock/site_lock/page.py +++ b/modules/site_lock/site_lock/web/page.py @@ -25,7 +25,7 @@ # ``safe_next`` is imported, not defined here: the implementation moved to # ``simple_module_core.redirect_safety`` once AuthMiddleware needed the same -# rules. Re-exported so existing ``site_lock.page.safe_next`` callers keep +# rules. Re-exported so existing ``site_lock.web.page.safe_next`` callers keep # working. __all__ = ["render_unlock_page", "safe_next"] diff --git a/modules/site_lock/site_lock/rate_limit.py b/modules/site_lock/site_lock/web/rate_limit.py similarity index 100% rename from modules/site_lock/site_lock/rate_limit.py rename to modules/site_lock/site_lock/web/rate_limit.py diff --git a/modules/site_lock/site_lock/templates/unlock.html b/modules/site_lock/site_lock/web/templates/unlock.html similarity index 100% rename from modules/site_lock/site_lock/templates/unlock.html rename to modules/site_lock/site_lock/web/templates/unlock.html diff --git a/modules/site_lock/tests/test_site_lock_settings.py b/modules/site_lock/tests/core/settings/test_site_lock_settings.py similarity index 96% rename from modules/site_lock/tests/test_site_lock_settings.py rename to modules/site_lock/tests/core/settings/test_site_lock_settings.py index 39e022736..9aa9d8b89 100644 --- a/modules/site_lock/tests/test_site_lock_settings.py +++ b/modules/site_lock/tests/core/settings/test_site_lock_settings.py @@ -4,7 +4,7 @@ import pytest from pydantic import ValidationError -from site_lock.settings import SiteLockSettings +from site_lock.core.settings import SiteLockSettings def test_disabled_by_default() -> None: diff --git a/modules/site_lock/tests/test_site_lock_integration.py b/modules/site_lock/tests/web/middleware/test_site_lock_integration.py similarity index 96% rename from modules/site_lock/tests/test_site_lock_integration.py rename to modules/site_lock/tests/web/middleware/test_site_lock_integration.py index 665a6a568..fce162c53 100644 --- a/modules/site_lock/tests/test_site_lock_integration.py +++ b/modules/site_lock/tests/web/middleware/test_site_lock_integration.py @@ -9,8 +9,8 @@ from __future__ import annotations import pytest -from site_lock import constants as c -from site_lock.settings import SiteLockSettings +from site_lock.core import constants as c +from site_lock.core.settings import SiteLockSettings def test_module_state_is_mounted_and_disabled_by_default(app) -> None: diff --git a/modules/site_lock/tests/test_site_lock_middleware.py b/modules/site_lock/tests/web/middleware/test_site_lock_middleware.py similarity index 97% rename from modules/site_lock/tests/test_site_lock_middleware.py rename to modules/site_lock/tests/web/middleware/test_site_lock_middleware.py index 2c687e342..15718cc8a 100644 --- a/modules/site_lock/tests/test_site_lock_middleware.py +++ b/modules/site_lock/tests/web/middleware/test_site_lock_middleware.py @@ -4,10 +4,10 @@ import httpx from fastapi import FastAPI -from site_lock import constants as c -from site_lock.middleware import SiteLockMiddleware, password_fingerprint -from site_lock.settings import SiteLockSettings -from site_lock.state import SiteLockState +from site_lock.core import constants as c +from site_lock.core.settings import SiteLockSettings +from site_lock.core.state import SiteLockState +from site_lock.web.middleware import SiteLockMiddleware, password_fingerprint from starlette.middleware.sessions import SessionMiddleware from starlette.responses import JSONResponse diff --git a/modules/site_lock/tests/test_site_lock_page.py b/modules/site_lock/tests/web/page/test_site_lock_page.py similarity index 96% rename from modules/site_lock/tests/test_site_lock_page.py rename to modules/site_lock/tests/web/page/test_site_lock_page.py index e462a6a97..038e4fa83 100644 --- a/modules/site_lock/tests/test_site_lock_page.py +++ b/modules/site_lock/tests/web/page/test_site_lock_page.py @@ -3,7 +3,7 @@ from __future__ import annotations import pytest -from site_lock.page import render_unlock_page, safe_next +from site_lock.web.page import render_unlock_page, safe_next @pytest.mark.parametrize( diff --git a/modules/site_lock/tests/test_site_lock_rate_limit.py b/modules/site_lock/tests/web/rate_limit/test_site_lock_rate_limit.py similarity index 97% rename from modules/site_lock/tests/test_site_lock_rate_limit.py rename to modules/site_lock/tests/web/rate_limit/test_site_lock_rate_limit.py index 5412def3d..eb3347d01 100644 --- a/modules/site_lock/tests/test_site_lock_rate_limit.py +++ b/modules/site_lock/tests/web/rate_limit/test_site_lock_rate_limit.py @@ -2,7 +2,7 @@ from __future__ import annotations -from site_lock.rate_limit import AttemptLimiter +from site_lock.web.rate_limit import AttemptLimiter def _limiter() -> AttemptLimiter: