diff --git a/.env.example b/.env.example index ccd3166..e39d22a 100644 --- a/.env.example +++ b/.env.example @@ -1,24 +1,51 @@ -# Docker registry URL (used in docker-compose.yml) -REGISTRY_URL=registry.kucharczyk.xyz +# ============================================================================= +# Django application settings (read by timetracker/config.py) +# +# Resolution priority, highest first: +# SECRET_KEY__FILE -> env var -> .env -> settings.ini -> built-in default +# See docs/configuration.md for the full reference. +# ============================================================================= -# Container timezone +# Turn DEBUG off in production. Defaults on for local development. +# (The old PROD=1 variable still works but is deprecated; prefer DEBUG.) +DEBUG=false + +# Secret key. Required in production; an insecure default is used in DEBUG. +# For Docker/K8s secrets, point SECRET_KEY__FILE at a mounted file instead. +SECRET_KEY=change-me-to-a-long-random-string +# SECRET_KEY__FILE=/run/secrets/timetracker_secret_key + +# Public URL of the site. Derives ALLOWED_HOSTS and CSRF_TRUSTED_ORIGINS. +APP_URL=https://tracker.kucharczyk.xyz + +# Optional explicit overrides (comma-separated). When set they win over APP_URL. +# Useful behind a reverse proxy, e.g. ALLOWED_HOSTS=* +# ALLOWED_HOSTS=* +# CSRF_TRUSTED_ORIGINS=https://tracker.kucharczyk.xyz + +# Container timezone. TZ=Europe/Prague -# User/group IDs for container (used in entrypoint.sh) +# Directory holding the SQLite database (defaults to the project root). +DATA_DIR=/home/timetracker/app/data + +# ============================================================================= +# Container / entrypoint-only settings (read by entrypoint.sh, NOT by Django) +# ============================================================================= + +# User/group IDs the container process runs as. PUID=1000 PGID=100 -# External port mapping -TIMETRACKER_EXTERNAL_PORT=8000 - -# Django production mode (set to "1" for production) -PROD=1 - -# Database directory (defaults to project root) -DATA_DIR=/home/timetracker/app/data - -# CSRF trusted origins -CSRF_TRUSTED_ORIGINS=https://tracker.kucharczyk.xyz - -# Create a default admin/admin superuser on startup (for initial setup only) +# Create an admin/admin superuser on startup (for initial setup only). CREATE_DEFAULT_SUPERUSER=false + +# ============================================================================= +# docker-compose-only settings (compose file substitution, not the app) +# ============================================================================= + +# Docker registry URL (used in docker-compose.yml). +REGISTRY_URL=registry.kucharczyk.xyz + +# External port mapping. +TIMETRACKER_EXTERNAL_PORT=8000 diff --git a/.gitea/workflows/staging.yml b/.gitea/workflows/staging.yml index 2dfbfb3..e47589c 100644 --- a/.gitea/workflows/staging.yml +++ b/.gitea/workflows/staging.yml @@ -37,7 +37,7 @@ jobs: -e DATA_DIR=/home/timetracker/app/data \ -e STAGING=true \ -e "SECRET_KEY=${STAGING_SECRET_KEY}" \ - -e "CSRF_TRUSTED_ORIGINS=https://${HOST}" \ + -e "APP_URL=https://${HOST}" \ -v "timetracker-staging-${SLUG}:/home/timetracker/app/data" \ -l "caddy=${HOST}" \ -l 'caddy.reverse_proxy={{ upstreams 8000 }}' \ diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 3eb47cd..e3e4fcd 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -43,9 +43,10 @@ jobs: # Per-app SECRET_KEY so each staging instance is independent and no # session cookie is shared across instances or with production. SECRET_KEY="staging-${SLUG}-$(head -c16 /dev/urandom | base64 | tr -dc 'a-zA-Z0-9')" + # APP_URL derives both ALLOWED_HOSTS and CSRF_TRUSTED_ORIGINS. flyctl secrets set --app "$APP" --stage \ "SECRET_KEY=${SECRET_KEY}" \ - "CSRF_TRUSTED_ORIGINS=https://${HOST}" + "APP_URL=https://${HOST}" - name: Deploy run: flyctl deploy --app "$APP" --config fly.staging.toml --remote-only --yes diff --git a/.gitignore b/.gitignore index 1b8c576..ba084e7 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,10 @@ data/ dist/ .DS_Store .python-version + +# Local configuration (may contain secrets); examples are committed instead +.env +/settings.ini .direnv .hermes/ diff --git a/CLAUDE.md b/CLAUDE.md index 65267fc..eddb493 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,15 +141,20 @@ Docker-based: multi-stage Dockerfile (uv builder → Node assets stage → slim ### Database -SQLite with WAL journal mode. Connection timeout 20s. The `DATA_DIR` env var controls the database file location. Migrations live in `games/migrations/`. There are `GeneratedField`s on the models — these are computed by the database engine and cannot be written from application code. +SQLite with WAL journal mode. Connection timeout 20s. The `DATA_DIR` setting controls the database file location and is read consistently by both `settings.py` and `entrypoint.sh` (same env var + matching default). Migrations live in `games/migrations/`. There are `GeneratedField`s on the models — these are computed by the database engine and cannot be written from application code. ### Configuration -- `DEBUG` is `True` unless `PROD` env var is set -- `TIME_ZONE` defaults to `Europe/Prague` in debug, otherwise reads `TZ` env var (default `UTC`) -- Django Admin, Debug Toolbar, and `django_extensions` are only available in `DEBUG` mode -- `CSRF_TRUSTED_ORIGINS` is parsed from a comma-separated env var -- `DATA_DIR` env var sets the SQLite database location (defaults to `BASE_DIR`) +All configurable Django settings are read through `config()` in `timetracker/config.py`, never via bare `os.environ` in `settings.py`. Full reference: `docs/configuration.md`. + +- **Resolution priority** (highest first): `NAME__FILE` (opt-in file secret) → `NAME` env var → `.env` → `settings.ini` (`[timetracker]` section) → in-code default. Missing + no default = `ImproperlyConfigured`. +- `config(name, *, default, cast, allow_file, required_in_prod)`: `cast` handles `bool`/`list`/`int`/`Path`/callable; `allow_file=True` honors `NAME__FILE` (contents `.strip()`-ed); `required_in_prod=True` hard-fails when missing and DEBUG is off. +- `DEBUG` defaults `True` (dev), turned off with `DEBUG=false`. `PROD` is a **deprecated alias** kept for one release. +- `SECRET_KEY` is required in production (insecure default only in DEBUG); supports `SECRET_KEY__FILE`. +- `APP_URL` derives `ALLOWED_HOSTS` and `CSRF_TRUSTED_ORIGINS` when those aren't set explicitly; the two are never merged (different security checks) and each can be overridden directly. +- `TIME_ZONE` reads `TZ` (defaults `Europe/Prague` in debug, `UTC` in prod). +- Django Admin, Debug Toolbar, and `django_extensions` are only available in `DEBUG` mode. +- **Container/entrypoint-only** flags (`PUID`, `PGID`, `CREATE_DEFAULT_SUPERUSER`, `STAGING`, `LOAD_SAMPLE_DATA`) live in `entrypoint.sh`, not the Python config — they are bootstrap concerns, not Django settings. - django-q2 cluster: 1 worker, 60s timeout, 120s retry, ORM broker ### Testing @@ -180,6 +185,7 @@ Pytest settings are in `pyproject.toml` under `[tool.pytest.ini_options]` (`DJAN - **Components are nodes; use the named builders** — build with `Div()`, `Span()`, `Element("tag", ...)`, etc., which return `Node` objects. For a tag with no builder, add it to the whitelist in `primitives.py` (one line) or use `Element("tag", attrs, children)`. Use `Fragment(a, b, ...)` to group siblings (never `str(a)+str(b)`, which flattens the tree and drops media). Wrap trusted pre-rendered HTML in `Safe(html)` (the `mark_safe` analogue). - **JS-bearing components declare `Media`, they don't rely on the view** — give a component `class Media: js = (...)` (a `BaseComponent`) or `return node.with_media(Media(js=...))`. `Page()` collects and emits it. Never re-add `scripts=ModuleScript(...)` threading in a view for a component that can declare its own dependency. - **Filter views** accept `?filter=` (structured) and fall back to `?search_string=` (free-text). New filter criteria go in `games/filters.py`; new criterion types go in `common/criteria.py`. +- **Read settings via `config()`** — new Django settings go through `config()` from `timetracker/config.py`, never bare `os.environ.get` in `settings.py`. Declare `cast`/`allow_file`/`required_in_prod` explicitly. Container-bootstrap flags belong in `entrypoint.sh`, not the Python config. See `docs/configuration.md`. - **Signals handle side-effects** — do not manually recalculate `Game.playtime` or `Purchase.num_purchases`; the signals in `games/signals.py` do this on save/delete. - **Button colors**: `blue` (primary action), `red` (destructive), `gray` (secondary), `green` (positive). Icon buttons use `icon=True`. - **Inline Alpine.js** is used for client-side reactivity in domain components (`GameStatusSelector`, `SessionDeviceSelector`). The pattern is `x-data="{...}"` with `fetchWithHtmxTriggers()` for PATCH API calls. diff --git a/docker-compose.no-caddy.yml b/docker-compose.no-caddy.yml index 4aad4bb..5239d13 100644 --- a/docker-compose.no-caddy.yml +++ b/docker-compose.no-caddy.yml @@ -7,8 +7,11 @@ services: dockerfile: Dockerfile container_name: timetracker environment: + - DEBUG=false - TZ=Europe/Prague - - CSRF_TRUSTED_ORIGINS="https://tracker.kucharczyk.xyz" + # APP_URL drives ALLOWED_HOSTS and CSRF_TRUSTED_ORIGINS unless overridden. + # Behind your own reverse proxy you may also set ALLOWED_HOSTS=* directly. + - APP_URL=https://tracker.kucharczyk.xyz user: "1000" # volumes: # - "db:/home/timetracker/app/src/timetracker/db.sqlite3" diff --git a/docker-compose.yml b/docker-compose.yml index bf710ff..87831ae 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,14 +1,17 @@ --- services: timetracker: - image: ${REGISTRY_URL:-registry.kucharczyk.xyz}/timetracker:1.7.0 + image: ${REGISTRY_URL:-registry.kucharczyk.xyz}/timetracker:latest build: context: . dockerfile: Dockerfile container_name: timetracker environment: + - DEBUG=${DEBUG:-false} + - SECRET_KEY=${SECRET_KEY} - TZ=${TZ:-Europe/Prague} - - CSRF_TRUSTED_ORIGINS=https://tracker.kucharczyk.xyz + # APP_URL drives ALLOWED_HOSTS and CSRF_TRUSTED_ORIGINS unless overridden. + - APP_URL=${APP_URL:-http://localhost:8000} - PUID=${PUID:-1000} - PGID=${PGID:-100} - DATA_DIR=${DATA_DIR:-/home/timetracker/app/data} diff --git a/docs/configuration.md b/docs/configuration.md new file mode 100644 index 0000000..59c019a --- /dev/null +++ b/docs/configuration.md @@ -0,0 +1,124 @@ +# Configuration + +All configurable Django settings are read through a single helper, +`config()` in [`timetracker/config.py`](../timetracker/config.py). It resolves +each value from a fixed chain of sources so the same setting can come from an +environment variable, a `.env` file, an `.ini` file, or a built-in default — +without any per-setting special-casing in `settings.py`. + +## Resolution priority + +For a setting named `NAME`, the first source that provides a value wins: + +| Priority | Source | Notes | +|---------:|--------|-------| +| 1 | `NAME__FILE` env var | Path to a file; its *stripped* contents are the value. Opt-in per setting (`allow_file=True`). For Docker/Kubernetes secrets. | +| 2 | `NAME` env var | A real process environment variable. | +| 3 | `.env` file | `KEY=value` lines (see [.env syntax](#env-syntax)). | +| 4 | `settings.ini` file | The `[timetracker]` section, parsed with `configparser`. | +| 5 | `default` | The in-code fallback in `settings.py`. | + +If no source supplies a value and no `default` is defined, startup fails with +`ImproperlyConfigured` rather than silently using an empty value. + +**Worked example.** With `VALUE` set in the environment *and* in `.env` *and* +in `settings.ini`, the environment variable wins. Remove it and `.env` wins; +remove that and `settings.ini` wins; remove that and the code default applies. + +## Settings reference + +| Setting | Cast | Default | `__FILE`? | Description | +|---------|------|---------|:---------:|-------------| +| `SECRET_KEY` | str | insecure dev key | yes | Django secret key. **Required in production** (DEBUG off) — a missing value is a hard error, not a silent insecure fallback. | +| `DEBUG` | bool | `true` (dev) | no | Debug mode. Turn **off** in production. Defaults on for local development. | +| `APP_URL` | str | `http://localhost:8000` | no | Public URL of the site. Derives `ALLOWED_HOSTS` and `CSRF_TRUSTED_ORIGINS` when those are not set explicitly. | +| `ALLOWED_HOSTS` | list | derived from `APP_URL` | no | Comma-separated hostnames. Overrides the `APP_URL` derivation. | +| `CSRF_TRUSTED_ORIGINS` | list | derived from `APP_URL` | no | Comma-separated full origins (`https://host`). Overrides the `APP_URL` derivation. | +| `TZ` | str | `Europe/Prague` (dev) / `UTC` (prod) | no | Time zone. | +| `DATA_DIR` | path | project root | no | Directory holding the SQLite database. Also read by `entrypoint.sh`. | + +`cast` understands `bool` (`true/1/yes/on` → `True`), `list` (comma-separated, +whitespace-trimmed, empty items dropped), `int`, `Path`, or any callable. + +## APP_URL, ALLOWED_HOSTS and CSRF + +`ALLOWED_HOSTS` and `CSRF_TRUSTED_ORIGINS` guard different things — the `Host` +header versus cross-origin requests — so they are **never merged**. For the +common case you set only `APP_URL` and both are derived: + +``` +APP_URL=https://tracker.example.com +# -> ALLOWED_HOSTS = ["tracker.example.com"] +# -> CSRF_TRUSTED_ORIGINS = ["https://tracker.example.com"] +``` + +Power users override either independently. A typical reverse-proxy setup: + +``` +ALLOWED_HOSTS=* +CSRF_TRUSTED_ORIGINS=https://tracker.example.com +``` + +## Secrets and `__FILE` + +Secret managers (Docker secrets, Kubernetes) mount secrets as files. For any +setting that opts in (currently `SECRET_KEY`), point a `*__FILE` variable at +the mounted path: + +``` +SECRET_KEY__FILE=/run/secrets/timetracker_secret_key +``` + +The file contents are read and `.strip()`-ed. The strip matters: editors and +`echo` often append a trailing newline, and a stray `\n` inside `SECRET_KEY` +would silently invalidate every signed cookie/token when the file is recreated +without it. + +## .env syntax + +```dotenv +# full-line comment +KEY=value +export KEY=value # optional leading "export" +QUOTED="value with spaces" # surrounding quotes are stripped +SINGLE='also fine' +WITH_HASH="a # b" # '#' inside quotes is literal +INLINE=value # trailing comment after an unquoted value is dropped +``` + +Deliberately **not** supported (documented limits, not bugs): + +- variable interpolation (`${OTHER}`) +- multiline values + +File locations default to `.env` and `settings.ini` at the project root and +can be moved with the `ENV_FILE` / `INI_FILE` environment variables. Missing +files are ignored, so env-only deployments need neither. A `.env` file used by +`docker-compose` for `${VAR}` substitution is the same file Django reads in +local development; inside the container, real environment variables apply. + +See [`.env.example`](../.env.example) and +[`settings.ini.example`](../settings.ini.example) for starting points. + +## Container / entrypoint-only variables + +These are consumed by [`entrypoint.sh`](../entrypoint.sh) during container +bootstrap, **not** by Django. They are intentionally not part of the Python +config — moving them there would buy nothing and force a bash↔Python bridge. + +| Variable | Default | Purpose | +|----------|---------|---------| +| `PUID` / `PGID` | `1000` / `100` | uid/gid the container process runs as. | +| `DATA_DIR` | `/home/timetracker/app/data` | Database directory. Shared with Django via the same env var + matching default. | +| `CREATE_DEFAULT_SUPERUSER` | `false` | Create an `admin`/`admin` superuser on first start. | +| `STAGING` | `false` | Scrub copied sessions / django-q schedule on staging. | +| `LOAD_SAMPLE_DATA` | `false` | Seed sample fixtures when the database is empty. | + +## Migrating from the old config + +- `PROD=1` → `DEBUG=false`. `PROD` still works as a **deprecated alias** for + one release and emits a `DeprecationWarning`. +- `ALLOWED_HOSTS` is now configurable (it was previously hard-coded to `*`). + After upgrading, set `APP_URL` (or `ALLOWED_HOSTS` explicitly) or the host + will be rejected. Reverse-proxy deployments that relied on `*` should set + `ALLOWED_HOSTS=*`. diff --git a/entrypoint.sh b/entrypoint.sh index 0dd3c08..1fdb7c9 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -1,8 +1,16 @@ #!/bin/bash set -euo pipefail +# Container-bootstrap configuration. These variables are consumed only by this +# entrypoint, NOT by Django (see timetracker/config.py for the app settings): +# PUID/PGID — uid/gid the container process runs as +# DATA_DIR — writable dir for the SQLite database (kept in +# sync with Django via the same env var + default) +# CREATE_DEFAULT_SUPERUSER — create an admin/admin user on first start +# STAGING / LOAD_SAMPLE_DATA — staging-only data bootstrap (see below) PUID=${PUID:-1000} PGID=${PGID:-100} +DATA_DIR=${DATA_DIR:-/home/timetracker/app/data} USERHOME=$(grep timetracker /etc/passwd | cut -d ":" -f6) usermod -d "/root" timetracker @@ -10,11 +18,11 @@ groupmod -o -g "$PGID" timetracker usermod -o -u "$PUID" timetracker usermod -d "${USERHOME}" timetracker -mkdir -p /home/timetracker/app/data /var/log/supervisor +mkdir -p "$DATA_DIR" /var/log/supervisor chmod 755 /home/timetracker/app chmod 755 /home/timetracker/app/.venv -chown "$PUID:$PGID" /home/timetracker/app/data +chown "$PUID:$PGID" "$DATA_DIR" chown "$PUID:$PGID" /var/log/supervisor python manage.py migrate @@ -49,6 +57,6 @@ if not User.objects.filter(username='admin').exists(): " fi -chown -R "$PUID:$PGID" /home/timetracker/app/data +chown -R "$PUID:$PGID" "$DATA_DIR" exec /usr/bin/supervisord -c /etc/supervisor/conf.d/supervisor.conf diff --git a/fly.staging.toml b/fly.staging.toml index 82e2668..f488ce1 100644 --- a/fly.staging.toml +++ b/fly.staging.toml @@ -11,7 +11,7 @@ primary_region = "ams" dockerfile = "Dockerfile" [env] - PROD = "1" + DEBUG = "false" TZ = "Europe/Prague" DATA_DIR = "/home/timetracker/app/data" LOAD_SAMPLE_DATA = "true" diff --git a/settings.ini.example b/settings.ini.example new file mode 100644 index 0000000..be10470 --- /dev/null +++ b/settings.ini.example @@ -0,0 +1,15 @@ +# Alternative to a .env file for non-Docker / bare-metal deployments. +# Copy to settings.ini (next to manage.py) or point INI_FILE at it. +# Real environment variables and a .env file both take precedence over this. +# See docs/configuration.md for the full reference. + +[timetracker] +DEBUG = false +SECRET_KEY = change-me-to-a-long-random-string +APP_URL = https://tracker.kucharczyk.xyz +TZ = Europe/Prague +DATA_DIR = /var/lib/timetracker + +# Optional explicit overrides (comma-separated); win over APP_URL when set. +# ALLOWED_HOSTS = * +# CSRF_TRUSTED_ORIGINS = https://tracker.kucharczyk.xyz diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..0a9506d --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,198 @@ +"""Tests for the configuration reader in ``timetracker/config.py``.""" + +import pytest +from django.core.exceptions import ImproperlyConfigured + +from timetracker import config as config_module +from timetracker.config import config + + +@pytest.fixture(autouse=True) +def _clear_caches(): + """Each test sees freshly parsed files.""" + config_module.reset_caches() + yield + config_module.reset_caches() + + +@pytest.fixture +def env_file(tmp_path, monkeypatch): + def _write(contents: str): + path = tmp_path / ".env" + path.write_text(contents) + monkeypatch.setenv("ENV_FILE", str(path)) + config_module.reset_caches() + return path + + return _write + + +@pytest.fixture +def ini_file(tmp_path, monkeypatch): + def _write(contents: str): + path = tmp_path / "settings.ini" + path.write_text(contents) + monkeypatch.setenv("INI_FILE", str(path)) + config_module.reset_caches() + return path + + return _write + + +def test_default_returned_when_unset(): + assert config("TOTALLY_UNSET_VALUE", default="fallback") == "fallback" + + +def test_missing_without_default_raises(): + with pytest.raises(ImproperlyConfigured): + config("TOTALLY_UNSET_VALUE") + + +def test_env_var_overrides_default(monkeypatch): + monkeypatch.setenv("SOME_SETTING", "from-env") + assert config("SOME_SETTING", default="fallback") == "from-env" + + +def test_priority_env_beats_files(monkeypatch, env_file, ini_file): + ini_file("[timetracker]\nVALUE = from-ini\n") + env_file("VALUE=from-dotenv\n") + monkeypatch.setenv("VALUE", "from-env") + assert config("VALUE") == "from-env" + + +def test_priority_dotenv_beats_ini(env_file, ini_file): + ini_file("[timetracker]\nVALUE = from-ini\n") + env_file("VALUE=from-dotenv\n") + assert config("VALUE") == "from-dotenv" + + +def test_priority_ini_beats_default(ini_file): + ini_file("[timetracker]\nVALUE = from-ini\n") + assert config("VALUE", default="fallback") == "from-ini" + + +def test_ini_preserves_key_case(ini_file): + ini_file("[timetracker]\nSECRET_KEY = abc\n") + assert config("SECRET_KEY") == "abc" + + +# --- __FILE secret pointer ------------------------------------------------- + + +def test_file_pointer_read_and_stripped(tmp_path, monkeypatch): + secret = tmp_path / "secret" + secret.write_text("super-secret-value\n") # trailing newline must be stripped + monkeypatch.setenv("SECRET_KEY__FILE", str(secret)) + assert config("SECRET_KEY", allow_file=True) == "super-secret-value" + + +def test_file_pointer_ignored_without_allow_file(tmp_path, monkeypatch): + secret = tmp_path / "secret" + secret.write_text("ignored") + monkeypatch.setenv("SECRET_KEY__FILE", str(secret)) + assert config("SECRET_KEY", default="fallback") == "fallback" + + +def test_file_pointer_beats_env(tmp_path, monkeypatch): + secret = tmp_path / "secret" + secret.write_text("from-file") + monkeypatch.setenv("SECRET_KEY__FILE", str(secret)) + monkeypatch.setenv("SECRET_KEY", "from-env") + assert config("SECRET_KEY", allow_file=True) == "from-file" + + +# --- casting --------------------------------------------------------------- + + +@pytest.mark.parametrize( + "raw,expected", + [ + ("true", True), + ("True", True), + ("1", True), + ("yes", True), + ("on", True), + ("false", False), + ("0", False), + ("no", False), + ("", False), + ], +) +def test_cast_bool(monkeypatch, raw, expected): + monkeypatch.setenv("FLAG", raw) + assert config("FLAG", cast=bool) is expected + + +def test_cast_list(monkeypatch): + monkeypatch.setenv("HOSTS", "a.example, b.example , ,c.example") + assert config("HOSTS", cast=list) == ["a.example", "b.example", "c.example"] + + +def test_cast_int(monkeypatch): + monkeypatch.setenv("COUNT", "42") + assert config("COUNT", cast=int) == 42 + + +def test_cast_not_applied_to_default(): + # A None default passes through untouched even with a cast set. + assert config("UNSET", default=None, cast=list) is None + + +# --- required_in_prod ------------------------------------------------------ + + +def test_required_in_prod_raises_when_prod(monkeypatch): + monkeypatch.setenv("DEBUG", "false") + with pytest.raises(ImproperlyConfigured): + config("SECRET_KEY", default="dev-default", required_in_prod=True) + + +def test_required_in_prod_uses_default_in_debug(monkeypatch): + monkeypatch.setenv("DEBUG", "true") + assert config("SECRET_KEY", default="dev-default", required_in_prod=True) == ( + "dev-default" + ) + + +def test_deprecated_prod_var_implies_production(monkeypatch): + monkeypatch.delenv("DEBUG", raising=False) + monkeypatch.setenv("PROD", "1") + with pytest.raises(ImproperlyConfigured): + config("SECRET_KEY", default="dev-default", required_in_prod=True) + + +# --- .env parser edge cases ------------------------------------------------ + + +def test_env_parser_quotes_comments_and_export(env_file): + env_file( + "\n".join( + [ + "# a comment line", + "PLAIN=value", + "export EXPORTED=exported-value", + 'DOUBLE="quoted value"', + "SINGLE='single quoted'", + "INLINE=value # trailing comment", + 'HASH_IN_QUOTES="a # b"', + "EMPTY=", + 'QUOTED_THEN_COMMENT="keep" # drop', + ] + ) + + "\n" + ) + assert config("PLAIN") == "value" + assert config("EXPORTED") == "exported-value" + assert config("DOUBLE") == "quoted value" + assert config("SINGLE") == "single quoted" + assert config("INLINE") == "value" + assert config("HASH_IN_QUOTES") == "a # b" + assert config("EMPTY", default="x") == "" + assert config("QUOTED_THEN_COMMENT") == "keep" + + +def test_missing_files_are_ignored(monkeypatch, tmp_path): + monkeypatch.setenv("ENV_FILE", str(tmp_path / "does-not-exist.env")) + monkeypatch.setenv("INI_FILE", str(tmp_path / "does-not-exist.ini")) + config_module.reset_caches() + assert config("ANYTHING", default="fallback") == "fallback" diff --git a/timetracker/config.py b/timetracker/config.py new file mode 100644 index 0000000..0ffb5fd --- /dev/null +++ b/timetracker/config.py @@ -0,0 +1,195 @@ +""" +Centralized configuration reading for timetracker. + +Every configurable Django setting is resolved through :func:`config`, which +consults several sources in a fixed priority order (highest first): + +1. ``NAME__FILE`` — path to a file whose *stripped* contents are the value. + Only consulted when the setting opts in with + ``allow_file=True``. Intended for Docker/Kubernetes + secrets, which are mounted as files rather than env vars. +2. ``NAME`` — a real process environment variable. +3. ``.env`` file — ``KEY=value`` lines (see the supported syntax below). +4. ``settings.ini`` — the ``[timetracker]`` section, parsed with + :mod:`configparser`. +5. ``default`` — the in-code fallback passed to :func:`config`. + +If no source supplies a value and no ``default`` is given, an +:class:`~django.core.exceptions.ImproperlyConfigured` error is raised. + +``.env`` syntax supported: + +- ``KEY=value`` and ``export KEY=value`` +- blank lines and ``#`` full-line comments +- single- or double-quoted values (the surrounding quotes are stripped); a + ``#`` inside quotes is treated literally +- an inline ``# comment`` after an *unquoted* value + +Deliberately NOT supported (documented limits, not bugs): + +- variable interpolation (``${OTHER}``) +- multiline values + +File locations default to ``.env`` and ``settings.ini`` next to the project +root and can be overridden with the ``ENV_FILE`` / ``INI_FILE`` environment +variables. Missing files are silently ignored so env-only deployments are +unaffected. +""" + +import os +from configparser import ConfigParser +from pathlib import Path +from typing import Any, Callable + +from django.core.exceptions import ImproperlyConfigured + +BASE_DIR = Path(__file__).resolve().parent.parent + +# Sentinel distinguishing "no default supplied" from an explicit ``None``. +NOT_SET: Any = object() + +INI_SECTION = "timetracker" + +_env_file_cache: dict[str, str] | None = None +_ini_file_cache: dict[str, str] | None = None + + +def _unquote(value: str) -> str: + """Strip surrounding quotes, or an inline comment from an unquoted value.""" + if not value: + return value + quote = value[0] + if quote in "\"'": + closing = value.find(quote, 1) + if closing != -1: + return value[1:closing] + # Opening quote with no match: drop it and keep the rest verbatim. + return value[1:] + comment_index = value.find("#") + if comment_index != -1: + value = value[:comment_index] + return value.strip() + + +def _parse_env_file(path: Path) -> dict[str, str]: + values: dict[str, str] = {} + for raw_line in path.read_text().splitlines(): + line = raw_line.strip() + if not line or line.startswith("#"): + continue + if line.startswith("export "): + line = line[len("export ") :].lstrip() + if "=" not in line: + continue + name, _, value = line.partition("=") + name = name.strip() + if not name: + continue + values[name] = _unquote(value.strip()) + return values + + +def _load_env_file() -> dict[str, str]: + global _env_file_cache + if _env_file_cache is None: + path = Path(os.environ.get("ENV_FILE", BASE_DIR / ".env")) + _env_file_cache = _parse_env_file(path) if path.is_file() else {} + return _env_file_cache + + +def _load_ini_file() -> dict[str, str]: + global _ini_file_cache + if _ini_file_cache is None: + path = Path(os.environ.get("INI_FILE", BASE_DIR / "settings.ini")) + if path.is_file(): + parser = ConfigParser() + # Preserve key case; ConfigParser lowercases option names by default. + parser.optionxform = str # type: ignore[assignment, method-assign] + parser.read(path) + _ini_file_cache = ( + dict(parser[INI_SECTION]) if parser.has_section(INI_SECTION) else {} + ) + else: + _ini_file_cache = {} + return _ini_file_cache + + +def reset_caches() -> None: + """Clear parsed-file caches. Intended for use in tests.""" + global _env_file_cache, _ini_file_cache + _env_file_cache = None + _ini_file_cache = None + + +def _cast_value(value: str, cast: Callable[[str], Any] | None) -> Any: + if cast is None: + return value + if cast is bool: + return value.strip().lower() in {"true", "1", "yes", "on"} + if cast is list: + return [item.strip() for item in value.split(",") if item.strip()] + return cast(value) + + +def _resolve_raw(name: str, allow_file: bool) -> str | None: + """Return the first raw string from the source chain, or ``None``.""" + if allow_file: + file_pointer = os.environ.get(f"{name}__FILE") + if file_pointer: + return Path(file_pointer).read_text().strip() + if name in os.environ: + return os.environ[name] + env_file = _load_env_file() + if name in env_file: + return env_file[name] + ini_file = _load_ini_file() + if name in ini_file: + return ini_file[name] + return None + + +def _debug_enabled() -> bool: + """Whether the app runs in DEBUG mode, mirroring ``settings.DEBUG``. + + Defaults to on for local development; turned off by ``DEBUG=false`` or the + deprecated ``PROD`` env var. Used to decide whether ``required_in_prod`` + settings may fall back to a development default. + """ + raw = _resolve_raw("DEBUG", allow_file=False) + if raw is not None: + return _cast_value(raw, bool) + return not bool(os.environ.get("PROD")) + + +def config( + name: str, + *, + default: Any = NOT_SET, + cast: Callable[[str], Any] | None = None, + allow_file: bool = False, + required_in_prod: bool = False, +) -> Any: + """Resolve a configuration value from the source chain. + + Args: + name: The setting / environment variable name. + default: Fallback when no source provides a value. If omitted, a + missing value raises ``ImproperlyConfigured``. + cast: Coercion applied to string values — ``bool``, ``list``, ``int``, + ``Path``, or any callable taking a string. Defaults are returned + untouched. + allow_file: Whether to honor a ``NAME__FILE`` secret pointer. + required_in_prod: When ``True``, a missing value raises in production + (DEBUG off) even if a ``default`` is given, so insecure development + defaults never leak into a deployment. + """ + raw = _resolve_raw(name, allow_file=allow_file) + if raw is None: + if required_in_prod and not _debug_enabled(): + raise ImproperlyConfigured( + f"{name} must be set in production (DEBUG is off)." + ) + if default is NOT_SET: + raise ImproperlyConfigured(f"Required setting {name} is not configured.") + return default + return _cast_value(raw, cast) diff --git a/timetracker/settings.py b/timetracker/settings.py index 014f4a0..87e955b 100644 --- a/timetracker/settings.py +++ b/timetracker/settings.py @@ -11,7 +11,11 @@ https://docs.djangoproject.com/en/4.1/ref/settings/ """ import os +import warnings from pathlib import Path +from urllib.parse import urlparse + +from timetracker.config import config # Build paths inside the project like this: BASE_DIR / 'subdir'. BASE_DIR = Path(__file__).resolve().parent.parent @@ -20,18 +24,44 @@ BASE_DIR = Path(__file__).resolve().parent.parent # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/4.1/howto/deployment/checklist/ +# SECURITY WARNING: don't run with debug turned on in production! +# DEBUG defaults on for local development. Production turns it off via +# DEBUG=false (preferred) or the deprecated PROD env var. +_debug = config("DEBUG", default=None, cast=bool) +if _debug is None: + if os.environ.get("PROD"): + warnings.warn( + "The PROD environment variable is deprecated; set DEBUG=false instead.", + DeprecationWarning, + stacklevel=2, + ) + _debug = False + else: + _debug = True +DEBUG = _debug + # SECURITY WARNING: keep the secret key used in production secret! -# Read from the environment so each deployment (prod, staging) can supply its -# own key; falls back to an insecure default for local development and tests. -SECRET_KEY = os.environ.get( +# Each deployment supplies its own key (env, .env/.ini, or a SECRET_KEY__FILE +# secret); falls back to an insecure default only in DEBUG. Missing in +# production is a hard error rather than a silent insecure fallback. +SECRET_KEY = config( "SECRET_KEY", - "django-insecure-x0_t$gei=_o_p(%%!-db$jezka@y+d67$a8tvw13nl^8$l*t@=", + default="django-insecure-x0_t$gei=_o_p(%%!-db$jezka@y+d67$a8tvw13nl^8$l*t@=", + allow_file=True, + required_in_prod=True, ) -# SECURITY WARNING: don't run with debug turned on in production! -DEBUG = False if os.environ.get("PROD") else True +# ALLOWED_HOSTS and CSRF_TRUSTED_ORIGINS are configured independently (they +# guard different things), but both default off a single user-facing APP_URL +# when not set explicitly. Power users override either one directly — e.g. +# ALLOWED_HOSTS=* behind a reverse proxy while CSRF stays locked to the domain. +APP_URL = config("APP_URL", default="http://localhost:8000") +_app_url = urlparse(APP_URL) -ALLOWED_HOSTS = ["*"] +ALLOWED_HOSTS = config("ALLOWED_HOSTS", default=None, cast=list) or [_app_url.hostname] +CSRF_TRUSTED_ORIGINS = config("CSRF_TRUSTED_ORIGINS", default=None, cast=list) or [ + f"{_app_url.scheme}://{_app_url.netloc}" +] # Application definition @@ -114,7 +144,7 @@ WSGI_APPLICATION = "timetracker.wsgi.application" DATABASES = { "default": { "ENGINE": "django.db.backends.sqlite3", - "NAME": Path(os.environ.get("DATA_DIR", str(BASE_DIR))) / "db.sqlite3", + "NAME": config("DATA_DIR", default=BASE_DIR, cast=Path) / "db.sqlite3", "OPTIONS": { "timeout": 20, "init_command": "PRAGMA synchronous=FULL; PRAGMA journal_mode=WAL;", @@ -147,7 +177,7 @@ AUTH_PASSWORD_VALIDATORS = [ LANGUAGE_CODE = "en-us" -TIME_ZONE = "Europe/Prague" if DEBUG else os.environ.get("TZ", "UTC") +TIME_ZONE = config("TZ", default="Europe/Prague" if DEBUG else "UTC") USE_I18N = True @@ -180,9 +210,3 @@ LOGGING = { "games": {"handlers": ["console"], "level": "INFO", "propagate": False}, }, } - -_csrf_trusted_origins = os.environ.get("CSRF_TRUSTED_ORIGINS") -if _csrf_trusted_origins: - CSRF_TRUSTED_ORIGINS = _csrf_trusted_origins.split(",") -else: - CSRF_TRUSTED_ORIGINS = []