From 16ef92dd81f30cf7f8bb94af7327c116b7e8b489 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 22 Apr 2026 13:33:10 +0200 Subject: [PATCH] fix(users): honor SM_USERS_BOOTSTRAP_* from .env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README and .env.example tell users to set the first-boot admin credentials in .env, but the documented path silently did nothing: UsersSettings deliberately has no `env_file` (runtime fields come from the DB), and the `os.environ.get(...)` fallback doesn't see `.env`-only vars. Result: fresh `.env` + `make dev` created no admin, and logins failed with no clue why. - Add `simple_module_core.dotenv.parse_dotenv()` and swap the nearly identical parser already inlined in `simple_module_core.__main__` over to it. - In `users.bootstrap`, read the four `SM_USERS_BOOTSTRAP_*` keys from `.env` as a final fallback after settings → os.environ. Centralize the setting-attr → env-var mapping in `BOOTSTRAP_ENV_KEYS` so the test fixture and the resolver stay in sync. - Add commented-out bootstrap hints to `.env.example` and the `sm new`-scaffolded template so the knob is discoverable again (PR #49 trimmed it out). - Isolate users/tests/test_bootstrap.py from the developer's real `.env` via an autouse fixture. --- .claude/settings.local.json | 3 +- .env.example | 8 +++ framework/core/simple_module_core/__main__.py | 18 ++----- framework/core/simple_module_core/dotenv.py | 38 ++++++++++++++ .../templates/host/.env.example | 5 ++ modules/users/tests/test_bootstrap.py | 11 ++++ modules/users/users/bootstrap.py | 50 +++++++++++++------ 7 files changed, 105 insertions(+), 28 deletions(-) create mode 100644 framework/core/simple_module_core/dotenv.py diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 8b989358..aac75902 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -6,7 +6,8 @@ "mcp__plugin_playwright_playwright__browser_evaluate", "Skill(playwright-cli)", "mcp__plugin_context7_context7__query-docs", - "mcp__plugin_context7_context7__resolve-library-id" + "mcp__plugin_context7_context7__resolve-library-id", + "Bash(curl -fsS -o /dev/null http://127.0.0.1:8000/)" ] } } diff --git a/.env.example b/.env.example index eb60feab..3e2b52ed 100644 --- a/.env.example +++ b/.env.example @@ -12,3 +12,11 @@ SM_SECRET_KEY=change-me-in-production # Dev-only: Vite asset URL (ignored in production builds) SM_VITE_DEV_URL=http://localhost:5050 + +# First-boot admin seed (optional). Only applied when the users table is empty. +# Leave unset and use `uv run sm-users create-admin` instead if you prefer. +# SM_USERS_BOOTSTRAP_EMAIL=admin@example.com +# SM_USERS_BOOTSTRAP_PASSWORD=changeme +# Optional second non-admin seed user (handy in dev): +# SM_USERS_BOOTSTRAP_USER_EMAIL=user@example.com +# SM_USERS_BOOTSTRAP_USER_PASSWORD=changeme diff --git a/framework/core/simple_module_core/__main__.py b/framework/core/simple_module_core/__main__.py index 21edfab6..2afb3166 100644 --- a/framework/core/simple_module_core/__main__.py +++ b/framework/core/simple_module_core/__main__.py @@ -25,26 +25,18 @@ run_diagnostics, ) from simple_module_core.discovery import discover_modules, topological_sort +from simple_module_core.dotenv import parse_dotenv def _load_i18n_settings_from_env() -> tuple[list[str], str] | tuple[None, None]: """Return ``(supported_locales, default_locale)`` or ``(None, None)`` if unset. Reads env vars directly to avoid a dependency on ``simple_module_hosting``. - Honors ``.env`` by reading it line-by-line if present in the cwd or - ``SM_PROJECT_ROOT`` (pydantic-settings isn't imported here). + Honors ``.env`` by merging it into ``os.environ`` if present (pydantic- + settings isn't imported here). """ - root = Path(os.environ.get("SM_PROJECT_ROOT") or Path.cwd()) - dotenv = root / ".env" - if dotenv.is_file(): - for raw in dotenv.read_text(encoding="utf-8").splitlines(): - line = raw.strip() - if not line or line.startswith("#") or "=" not in line: - continue - key, _, value = line.partition("=") - key = key.strip() - value = value.strip().strip('"').strip("'") - os.environ.setdefault(key, value) + for key, value in parse_dotenv().items(): + os.environ.setdefault(key, value) supported_raw = os.environ.get("SM_I18N_SUPPORTED_LOCALES") if not supported_raw: diff --git a/framework/core/simple_module_core/dotenv.py b/framework/core/simple_module_core/dotenv.py new file mode 100644 index 00000000..4c75bc11 --- /dev/null +++ b/framework/core/simple_module_core/dotenv.py @@ -0,0 +1,38 @@ +"""Minimal ``.env`` parser — dependency-free. + +Used in places that can't or shouldn't pull in ``pydantic-settings`` (the +diagnostics CLI runs before the host package is imported; the users-module +bootstrap runs after settings are constructed and needs to read values that +``UsersSettings`` deliberately omits from ``env_file``). +""" + +from __future__ import annotations + +import os +from pathlib import Path + + +def parse_dotenv(path: Path | None = None) -> dict[str, str]: + """Parse a ``.env`` file into a dict. Empty dict if the file is missing. + + Values surrounded by matching single or double quotes have the quotes + stripped. Does *not* handle escapes, ``export KEY=…``, or multiline + values — keep the file simple. Does *not* mutate ``os.environ``; the + caller decides whether to merge. + + Without ``path``, looks up ``$SM_PROJECT_ROOT/.env`` (falling back to + ``$CWD/.env``) — the convention used by every tool in this repo. + """ + if path is None: + root = Path(os.environ.get("SM_PROJECT_ROOT") or Path.cwd()) + path = root / ".env" + if not path.is_file(): + return {} + parsed: dict[str, str] = {} + for raw in path.read_text(encoding="utf-8").splitlines(): + line = raw.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, _, value = line.partition("=") + parsed[key.strip()] = value.strip().strip('"').strip("'") + return parsed diff --git a/framework/hosting/simple_module_hosting/templates/host/.env.example b/framework/hosting/simple_module_hosting/templates/host/.env.example index a5e4ff5d..0f78a40f 100644 --- a/framework/hosting/simple_module_hosting/templates/host/.env.example +++ b/framework/hosting/simple_module_hosting/templates/host/.env.example @@ -13,3 +13,8 @@ SM_VITE_DEV_URL=http://localhost:5050 # Optional: JSON array to restrict which installed modules load at boot. # SM_MODULES_ENABLED=["Auth","Products"] + +# First-boot admin seed (optional). Only applied when the users table is empty. +# Leave unset and use `uv run sm-users create-admin` instead if you prefer. +# SM_USERS_BOOTSTRAP_EMAIL=admin@example.com +# SM_USERS_BOOTSTRAP_PASSWORD=changeme diff --git a/modules/users/tests/test_bootstrap.py b/modules/users/tests/test_bootstrap.py index b6fa14d2..f1a449ee 100644 --- a/modules/users/tests/test_bootstrap.py +++ b/modules/users/tests/test_bootstrap.py @@ -8,7 +8,9 @@ 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, @@ -17,6 +19,15 @@ 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/users/bootstrap.py b/modules/users/users/bootstrap.py index 9bb06e8c..334e6b31 100644 --- a/modules/users/users/bootstrap.py +++ b/modules/users/users/bootstrap.py @@ -8,6 +8,7 @@ from fastapi import FastAPI from fastapi_users.password import PasswordHelper +from simple_module_core.dotenv import parse_dotenv from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession @@ -22,6 +23,15 @@ from users.models import Role, User, UserRole from users.settings import UsersSettings +# Maps a UsersSettings attribute to the env var that seeds it on first boot. +# Shared between the bootstrap function and the test fixture that isolates it. +BOOTSTRAP_ENV_KEYS: dict[str, str] = { + "bootstrap_email": "SM_USERS_BOOTSTRAP_EMAIL", + "bootstrap_password": "SM_USERS_BOOTSTRAP_PASSWORD", + "bootstrap_user_email": "SM_USERS_BOOTSTRAP_USER_EMAIL", + "bootstrap_user_password": "SM_USERS_BOOTSTRAP_USER_PASSWORD", +} + logger = logging.getLogger("users.bootstrap") _EVT_CREATED = "users.bootstrap.created" @@ -178,29 +188,41 @@ async def _user_table_is_empty(db: AsyncSession) -> bool: return count == 0 +def _read_dotenv_bootstrap_vars() -> dict[str, str]: + """Return SM_USERS_BOOTSTRAP_* entries from ``.env`` (ignore everything else). + + ``UsersSettings`` deliberately doesn't use ``env_file`` — runtime fields + come from the DB, and pulling the whole ``.env`` in would re-expose every + SMTP/cookie/token secret as an env knob. So we re-read ``.env`` just for + the four documented seed keys. + """ + wanted = set(BOOTSTRAP_ENV_KEYS.values()) + return {k: v for k, v in parse_dotenv().items() if k in wanted} + + async def bootstrap_admin_from_env(app: FastAPI) -> None: """On-startup hook: create admin from env vars iff users table is empty. - Reads ``SM_USERS_BOOTSTRAP_EMAIL`` + ``SM_USERS_BOOTSTRAP_PASSWORD`` via - `UsersSettings`. If either is blank, returns silently. If the table - already has users, returns silently (so restarts don't try to re-bootstrap). + 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). 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 - # Bootstrap fields are one-shot seed inputs, not runtime DB settings — - # prefer the settings value if a test set one explicitly, otherwise fall - # back to the env var so production deployments keep working. - email = settings.bootstrap_email or os.environ.get("SM_USERS_BOOTSTRAP_EMAIL", "") - password = settings.bootstrap_password or os.environ.get("SM_USERS_BOOTSTRAP_PASSWORD", "") - user_email = settings.bootstrap_user_email or os.environ.get( - "SM_USERS_BOOTSTRAP_USER_EMAIL", "" - ) - user_password = settings.bootstrap_user_password or os.environ.get( - "SM_USERS_BOOTSTRAP_USER_PASSWORD", "" - ) + 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() + } + email = resolved["bootstrap_email"] + password = resolved["bootstrap_password"] + user_email = resolved["bootstrap_user_email"] + user_password = resolved["bootstrap_user_password"] if not email or not password: return