diff --git a/.verify/screenshot.png b/.verify/screenshot.png new file mode 100644 index 00000000..fb7b42e6 Binary files /dev/null and b/.verify/screenshot.png differ diff --git a/framework/cli/simple_module_cli/templates/host/main.py b/framework/cli/simple_module_cli/templates/host/main.py index b1b9f235..18fd1b1f 100644 --- a/framework/cli/simple_module_cli/templates/host/main.py +++ b/framework/cli/simple_module_cli/templates/host/main.py @@ -4,6 +4,22 @@ via entry_points; add them to this host's pyproject.toml to install them. """ +import os +from pathlib import Path + +from simple_module_core.dotenv import load_dotenv_into_environ + +# Resolve the workspace root from this file's location so the web process +# behaves the same regardless of where uvicorn was launched (the scaffolded +# Makefile uses ``cd host && uvicorn main:app``, but ``uv run --project host`` +# or a wheel deployment may run from elsewhere). chdir up front so cwd-relative +# paths in ``.env`` (e.g. ``sqlite+aiosqlite:///./host/app.db``) resolve +# consistently; load ``.env`` into ``os.environ`` so framework code reading +# ``os.environ.get("SM_…")`` directly sees the same values pydantic does. +_REPO_ROOT = Path(__file__).resolve().parent.parent +os.chdir(_REPO_ROOT) +load_dotenv_into_environ(_REPO_ROOT / ".env") + from simple_module_hosting import Settings, create_app from simple_module_hosting.logging import setup_logging diff --git a/framework/cli/tests/test_scaffolding_host.py b/framework/cli/tests/test_scaffolding_host.py index 66a35366..6864f8f3 100644 --- a/framework/cli/tests/test_scaffolding_host.py +++ b/framework/cli/tests/test_scaffolding_host.py @@ -155,6 +155,28 @@ async def test_env_py_uses_shared_helper(self, tmp_path): assert "make_include_object" in env_py assert "for mod in modules:" not in env_py + async def test_main_py_loads_dotenv_before_settings(self, tmp_path): + """Regression for #158: scaffolded main.py must populate ``os.environ`` + from ``.env`` *before* ``Settings`` is imported, otherwise framework + code reading ``os.environ`` directly (e.g. users.bootstrap's dotenv + fallback under uvicorn launched from ``host/``) silently misses values + that pydantic-settings would have picked up. + """ + from simple_module_cli.scaffolding import create_host + + dest = tmp_path / "demo" + create_host(dest, name="demo", modules=[]) + main_py = (dest / "main.py").read_text(encoding="utf-8") + + assert "load_dotenv_into_environ" in main_py + assert "os.chdir" in main_py + # The load_dotenv call must precede the first ``Settings`` import so + # ``BootstrapSettings``' ``env_file=".env"`` lookup and any direct + # ``os.environ.get(...)`` reads see the same view of the environment. + dotenv_idx = main_py.index("load_dotenv_into_environ(") + settings_import_idx = main_py.index("from simple_module_hosting import") + assert dotenv_idx < settings_import_idx + async def test_cli_create_host_runs_end_to_end(self, tmp_path): """The Click `smpy create-host` command produces a working scaffold.""" from simple_module_cli.cli import app diff --git a/host/main.py b/host/main.py index 29e54a3d..6d7f33b6 100644 --- a/host/main.py +++ b/host/main.py @@ -3,18 +3,21 @@ import os from pathlib import Path -# Publish the project root so the hosting layer can find host/static and -# host/templates whether we're running from the uv workspace or a wheel. -os.environ.setdefault( - "SM_PROJECT_ROOT", - str(Path(__file__).resolve().parent.parent), -) +from simple_module_core.dotenv import load_dotenv_into_environ + +# Publish ``SM_PROJECT_ROOT`` and merge ``.env`` into ``os.environ`` *before* +# any settings import so framework code reading ``os.environ.get("SM_…")`` +# directly (not via pydantic-settings) sees the same values pydantic does — +# keeping precedence (real env wins) consistent across the web process, the +# worker, and one-shot scripts. +os.environ.setdefault("SM_PROJECT_ROOT", str(Path(__file__).resolve().parent.parent)) +load_dotenv_into_environ(Path(os.environ["SM_PROJECT_ROOT"]) / ".env") -from simple_module_hosting import Settings, create_app -from simple_module_hosting.logging import setup_logging +from simple_module_hosting import Settings, create_app # noqa: E402 +from simple_module_hosting.logging import setup_logging # noqa: E402 -from host.routes import router as host_router -from host.routes_i18n import router as i18n_router +from host.routes import router as host_router # noqa: E402 +from host.routes_i18n import router as i18n_router # noqa: E402 settings = Settings() diff --git a/modules/users/tests/conftest.py b/modules/users/tests/conftest.py index 69c837a6..fb574439 100644 --- a/modules/users/tests/conftest.py +++ b/modules/users/tests/conftest.py @@ -29,11 +29,16 @@ def _isolate_users_env(monkeypatch): After the env→DB migration ``UsersSettings()`` no longer reads these values, so this is belt-and-braces: it keeps old shell exports from muddying any ``SM_ENVIRONMENT`` checks or from being misread during - developer spelunking. + developer spelunking. Also stubs out + ``users.bootstrap._read_dotenv_bootstrap_vars`` so the developer's local + ``.env`` can't seed bootstrap creds into tests that exercise the resolver. """ + from users import bootstrap as bootstrap_module + for key in list(os.environ): if key.startswith("SM_USERS_"): monkeypatch.delenv(key, raising=False) + monkeypatch.setattr(bootstrap_module, "_read_dotenv_bootstrap_vars", dict) # --------------------------------------------------------------------------- diff --git a/modules/users/tests/test_bootstrap.py b/modules/users/tests/test_bootstrap.py index f1a449ee..b6fa14d2 100644 --- a/modules/users/tests/test_bootstrap.py +++ b/modules/users/tests/test_bootstrap.py @@ -8,9 +8,7 @@ from fastapi_users.password import PasswordHelper from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from users import bootstrap as bootstrap_module from users.bootstrap import ( - BOOTSTRAP_ENV_KEYS, CreateAdminResult, bootstrap_admin_from_env, create_admin, @@ -19,15 +17,6 @@ from users.models import Role, User, UserRole from users.settings import UsersSettings - -@pytest.fixture(autouse=True) -def _isolate_from_repo_dotenv(monkeypatch: pytest.MonkeyPatch) -> None: - """Prevent the developer's ``.env`` from leaking bootstrap vars into tests.""" - monkeypatch.setattr(bootstrap_module, "_read_dotenv_bootstrap_vars", dict) - for key in BOOTSTRAP_ENV_KEYS.values(): - monkeypatch.delenv(key, raising=False) - - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- diff --git a/modules/users/tests/test_bootstrap_resolution.py b/modules/users/tests/test_bootstrap_resolution.py new file mode 100644 index 00000000..5376b7f0 --- /dev/null +++ b/modules/users/tests/test_bootstrap_resolution.py @@ -0,0 +1,72 @@ +"""Tests for ``resolve_bootstrap_credentials`` — the three-tier resolver shared +between the boot-time admin seeder and the login-page dev-quick-fill UI. + +Regression coverage for issue #159: previously the login page only consulted +``UsersSettings`` + ``os.environ``, so an admin seeded via the ``.env`` +fallback was created but the dev-quick-login button never showed. +""" + +from __future__ import annotations + +import pytest +from users import bootstrap as bootstrap_module +from users.bootstrap import BOOTSTRAP_ENV_KEYS, resolve_bootstrap_credentials +from users.settings import UsersSettings + + +def _bare_users_settings(**overrides: str) -> UsersSettings: + """UsersSettings with all required-field defaults filled in.""" + defaults = { + "reset_password_token_secret": "test-secret", + "verification_token_secret": "test-secret", + } + defaults.update(overrides) + return UsersSettings(**defaults) + + +def test_prefers_settings_over_environ(monkeypatch: pytest.MonkeyPatch) -> None: + """A non-empty UsersSettings field beats the same key on ``os.environ``.""" + monkeypatch.setenv("SM_USERS_BOOTSTRAP_EMAIL", "env@example.com") + settings = _bare_users_settings(bootstrap_email="settings@example.com") + + resolved = resolve_bootstrap_credentials(settings) + + assert resolved["bootstrap_email"] == "settings@example.com" + + +def test_falls_back_to_environ(monkeypatch: pytest.MonkeyPatch) -> None: + """An empty UsersSettings field falls through to ``os.environ``.""" + monkeypatch.setenv("SM_USERS_BOOTSTRAP_EMAIL", "env@example.com") + settings = _bare_users_settings() + + resolved = resolve_bootstrap_credentials(settings) + + assert resolved["bootstrap_email"] == "env@example.com" + + +def test_falls_back_to_dotenv(monkeypatch: pytest.MonkeyPatch) -> None: + """When settings + ``os.environ`` are empty, ``.env`` is consulted last.""" + monkeypatch.setattr( + bootstrap_module, + "_read_dotenv_bootstrap_vars", + lambda: { + "SM_USERS_BOOTSTRAP_EMAIL": "dotenv@example.com", + "SM_USERS_BOOTSTRAP_PASSWORD": "DotenvPass1!", + }, + ) + settings = _bare_users_settings() + + resolved = resolve_bootstrap_credentials(settings) + + assert resolved["bootstrap_email"] == "dotenv@example.com" + assert resolved["bootstrap_password"] == "DotenvPass1!" + + +def test_returns_empty_strings_when_nothing_set() -> None: + """All four keys are present in the result even when unresolved.""" + settings = _bare_users_settings() + + resolved = resolve_bootstrap_credentials(settings) + + assert set(resolved) == set(BOOTSTRAP_ENV_KEYS) + assert all(v == "" for v in resolved.values()) diff --git a/modules/users/tests/test_views.py b/modules/users/tests/test_views.py index 58d0f296..a313168c 100644 --- a/modules/users/tests/test_views.py +++ b/modules/users/tests/test_views.py @@ -67,6 +67,51 @@ async def test_login_response_contains_inertia_component(self, anon_client): data = resp.json() assert data["component"] == "Users/Login" + @pytest.mark.anyio + async def test_dev_accounts_empty_outside_development(self, anon_client): + """``dev_accounts`` MUST NOT surface bootstrap creds in non-dev envs.""" + resp = await anon_client.get( + "/users/login", + headers={"X-Inertia": "true", "X-Inertia-Version": "1.0"}, + ) + assert resp.json()["props"]["dev_accounts"] == [] + + @pytest.mark.anyio + async def test_dev_accounts_resolved_via_dotenv_fallback( + self, anon_client, users_app, monkeypatch + ): + """Regression for #159: login_page surfaces creds seeded only via .env. + + With ``environment=development`` and bootstrap vars present only in + ``.env`` (not on settings, not on ``os.environ``), the buttons must + still appear — otherwise the admin gets seeded by the boot-time hook + but the dev-quick-login UX silently breaks. + """ + from users import bootstrap as bootstrap_module + + monkeypatch.setattr(users_app.state.sm.settings, "environment", "development") + monkeypatch.setattr( + bootstrap_module, + "_read_dotenv_bootstrap_vars", + lambda: { + "SM_USERS_BOOTSTRAP_EMAIL": "admin@example.com", + "SM_USERS_BOOTSTRAP_PASSWORD": "AdminPass1!", + "SM_USERS_BOOTSTRAP_USER_EMAIL": "user@example.com", + "SM_USERS_BOOTSTRAP_USER_PASSWORD": "UserPass1!", + }, + ) + + resp = await anon_client.get( + "/users/login", + headers={"X-Inertia": "true", "X-Inertia-Version": "1.0"}, + ) + + dev_accounts = resp.json()["props"]["dev_accounts"] + assert dev_accounts == [ + {"label": "Admin", "email": "admin@example.com", "password": "AdminPass1!"}, + {"label": "User", "email": "user@example.com", "password": "UserPass1!"}, + ] + class TestRegisterPage: @pytest.mark.anyio diff --git a/modules/users/users/auth_local/views.py b/modules/users/users/auth_local/views.py index 42aad883..eafaec1f 100644 --- a/modules/users/users/auth_local/views.py +++ b/modules/users/users/auth_local/views.py @@ -2,15 +2,22 @@ from __future__ import annotations -import os - from fastapi import APIRouter, HTTPException, Request from inertia import InertiaResponse from simple_module_hosting.inertia_deps import InertiaDep from starlette.responses import RedirectResponse +from users.bootstrap import resolve_bootstrap_credentials + router = APIRouter() +# (label, email-key, password-key) for the dev-quick-login buttons, in the +# order they should appear on the page. +_DEV_ACCOUNT_SPECS: tuple[tuple[str, str, str], ...] = ( + ("Admin", "bootstrap_email", "bootstrap_password"), + ("User", "bootstrap_user_email", "bootstrap_user_password"), +) + _PAGE_LOGIN = "Users/Login" _PAGE_REGISTER = "Users/Register" _PAGE_FORGOT_PASSWORD = "Users/ForgotPassword" @@ -26,27 +33,21 @@ async def login_page(request: Request, inertia: InertiaDep) -> InertiaResponse: users_settings = users_state.settings # In development only, surface the bootstrap credentials as click-to-fill # buttons so manual QA doesn't need to retype them. Never exposed in - # production, regardless of whether the vars are set. + # production, regardless of whether the vars are set. Uses the same + # resolution as the boot-time seeder so the buttons appear iff a seed + # admin would actually be created. dev_accounts: list[dict[str, str]] = [] if request.app.state.sm.settings.is_development: - admin_email = users_settings.bootstrap_email or os.environ.get( - "SM_USERS_BOOTSTRAP_EMAIL", "" - ) - admin_password = users_settings.bootstrap_password or os.environ.get( - "SM_USERS_BOOTSTRAP_PASSWORD", "" - ) - if admin_email and admin_password: - dev_accounts.append( - {"label": "Admin", "email": admin_email, "password": admin_password} - ) - user_email = users_settings.bootstrap_user_email or os.environ.get( - "SM_USERS_BOOTSTRAP_USER_EMAIL", "" - ) - user_password = users_settings.bootstrap_user_password or os.environ.get( - "SM_USERS_BOOTSTRAP_USER_PASSWORD", "" - ) - if user_email and user_password: - dev_accounts.append({"label": "User", "email": user_email, "password": user_password}) + resolved = resolve_bootstrap_credentials(users_settings) + for label, email_key, password_key in _DEV_ACCOUNT_SPECS: + if resolved[email_key] and resolved[password_key]: + dev_accounts.append( + { + "label": label, + "email": resolved[email_key], + "password": resolved[password_key], + } + ) return await inertia.render( _PAGE_LOGIN, { diff --git a/modules/users/users/bootstrap.py b/modules/users/users/bootstrap.py index 334e6b31..5e6883fb 100644 --- a/modules/users/users/bootstrap.py +++ b/modules/users/users/bootstrap.py @@ -200,25 +200,34 @@ def _read_dotenv_bootstrap_vars() -> dict[str, str]: return {k: v for k, v in parse_dotenv().items() if k in wanted} +def resolve_bootstrap_credentials(settings: UsersSettings) -> dict[str, str]: + """Resolve the four bootstrap fields with the same precedence everywhere. + + Order: ``UsersSettings`` (test overrides) → ``os.environ`` (docker/systemd) + → ``.env`` file (documented dev path). Used by both the boot-time admin + seeder and the dev-only login-page quick-fill so the two stay in lockstep. + """ + dotenv_vars = _read_dotenv_bootstrap_vars() + return { + attr: getattr(settings, attr) or os.environ.get(env_key) or dotenv_vars.get(env_key, "") + for attr, env_key in BOOTSTRAP_ENV_KEYS.items() + } + + async def bootstrap_admin_from_env(app: FastAPI) -> None: """On-startup hook: create admin from env vars iff users table is empty. - Resolves each of the four bootstrap fields in order: ``UsersSettings`` - (tests), then ``os.environ`` (docker/systemd), then ``.env`` (documented - dev path). If the admin email or password is still blank, returns - silently — same if the users table already has rows (so restarts don't - try to re-bootstrap). + Resolves each of the four bootstrap fields via + :func:`resolve_bootstrap_credentials`. If the admin email or password is + still blank, returns silently — same if the users table already has rows + (so restarts don't try to re-bootstrap). Optionally also creates a non-admin user from ``SM_USERS_BOOTSTRAP_USER_EMAIL`` + ``SM_USERS_BOOTSTRAP_USER_PASSWORD`` — useful in dev for testing non-admin flows alongside the admin account. """ settings: UsersSettings = app.state.users.settings - dotenv_vars = _read_dotenv_bootstrap_vars() - resolved = { - attr: getattr(settings, attr) or os.environ.get(env_key) or dotenv_vars.get(env_key, "") - for attr, env_key in BOOTSTRAP_ENV_KEYS.items() - } + resolved = resolve_bootstrap_credentials(settings) email = resolved["bootstrap_email"] password = resolved["bootstrap_password"] user_email = resolved["bootstrap_user_email"]