diff --git a/api/.env.example b/api/.env.example index ae76209..568fc81 100644 --- a/api/.env.example +++ b/api/.env.example @@ -21,8 +21,15 @@ SLACK_WEBHOOK_URL= # SLACK_WEBHOOK_SECURITY= # SLACK_WEBHOOK_PAYMENTS= # SLACK_WEBHOOK_INTEREST= -# Dev-only session token signing (see api/auth.py) — replaced by Clerk later. Use a random value, never reuse across environments. +# Dev-only session token signing (see api/auth.py). Only used when DEV_MODE=true. +# Use a random value, never reuse across environments. SESSION_SECRET=dev-secret-change-me +# Clerk. REQUIRED in production: with DEV_MODE unset or false, the API refuses to +# boot without CLERK_JWKS_URL, because auth would otherwise fall back to the +# dev-only HMAC token above. Leave unset for local work and CI. +# CLERK_JWKS_URL=https:///.well-known/jwks.json +# CLERK_ISSUER=https:// +# CLERK_SECRET_KEY= # Encrypts nfc_tags.aes_key_encrypted at rest (see api/services/nfc_crypto.py). # Generate with: python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" NFC_KEY_ENCRYPTION_SECRET= diff --git a/api/auth.py b/api/auth.py index 3fbba9e..4a33445 100644 --- a/api/auth.py +++ b/api/auth.py @@ -1,8 +1,11 @@ -"""Platform authentication — venue/role identity for /dashboard and /admin routes. +"""Platform authentication: venue/role identity for /dashboard and /admin routes. -DEV-ONLY STUB: gamespec.md specifies Clerk for real auth (2FA, sessions, -roles). Until a Clerk dev instance is wired in, sessions are simple -HMAC-signed tokens issued by POST /api/auth/dev-login (DEV_MODE only). +Two modes. Production verifies real Clerk RS256 JWTs against CLERK_JWKS_URL. +DEV_MODE additionally accepts HMAC-signed tokens issued by POST +/api/auth/dev-login, for local work, CI and the sim tools. _verify_token refuses +the HMAC path outright outside DEV_MODE, so it is unreachable in production even +when CLERK_JWKS_URL is missing; that case warns at import and fails every +dashboard and admin token, while leaving the patron game routes untouched. The contract below (get_current_user / require_role) is the permanent pattern every dashboard/admin route should depend on. Swapping in real @@ -13,6 +16,7 @@ import hashlib import hmac import os +import sys import time from typing import Optional @@ -28,11 +32,21 @@ TOKEN_TTL_SECONDS = 60 * 60 * 12 # 12 hours — dev convenience only # Clerk mode: set CLERK_JWKS_URL (+ CLERK_ISSUER) to verify real Clerk RS256 JWTs. -# Unset -> falls back to the dev-login HMAC token, so dev/CI are unaffected and prod -# "just works" once these env vars exist (the swap this module was designed for). +# Unset -> falls back to the dev-login HMAC token, which is correct for dev/CI and +# unacceptable in production, where _verify_token refuses it outright. CLERK_JWKS_URL = os.getenv("CLERK_JWKS_URL") CLERK_ISSUER = os.getenv("CLERK_ISSUER") CLERK_SECRET_KEY = os.getenv("CLERK_SECRET_KEY") + +# Outside DEV_MODE, Clerk is the only acceptable auth path. Warn loudly here and +# refuse the HMAC path in _verify_token, rather than raising: this module is +# imported by the whole app, so raising would take the patron game routes down +# too, and they never touch Clerk. A missing JWKS URL must break owner and admin +# login only, never a table full of people mid-session. +if os.getenv("DEV_MODE") != "true" and not CLERK_JWKS_URL: + print("WARNING: CLERK_JWKS_URL is not set outside DEV_MODE -- dashboard and " + "admin auth will reject every token until it is configured", + file=sys.stderr) # Auto-provisioning allowlist: a first-login user whose email is here becomes an # 'admin'; everyone else becomes a 'venue_owner' (no venue yet -> setup wizard). ADMIN_EMAILS = {e.strip().lower() for e in os.getenv("ADMIN_EMAILS", "").split(",") if e.strip()} @@ -82,6 +96,13 @@ def _verify_token(token: str) -> str: # tools + the dashboard dev-login). In production, Clerk is the only path. if os.getenv("DEV_MODE") != "true": raise HTTPException(status_code=401, detail="Invalid token") + + # Defence in depth. The boot guard makes an unconfigured production + # unreachable, but the HMAC path below must never run outside DEV_MODE even + # if that guard is later loosened. + if os.getenv("DEV_MODE") != "true": + raise HTTPException(status_code=401, detail="Invalid token") + try: payload, expires_at, signature = base64.urlsafe_b64decode(token.encode()).decode().rsplit(":", 2) except (ValueError, UnicodeDecodeError): diff --git a/api/tests/test_auth_production_guard.py b/api/tests/test_auth_production_guard.py new file mode 100644 index 0000000..7f34aba --- /dev/null +++ b/api/tests/test_auth_production_guard.py @@ -0,0 +1,74 @@ +"""Production must never fall back to the dev HMAC token. + +api/auth.py has two auth paths: real Clerk JWTs (CLERK_JWKS_URL) and dev-login +HMAC tokens. Previously, an unconfigured production silently used the second +one, making SESSION_SECRET the only thing protecting a venue dashboard. + +The fix is a refusal inside _verify_token, not a refusal to start: this module +is imported by every router, so a hard failure here would also take down the +patron game routes, which never use Clerk at all. A missing JWKS URL therefore +warns loudly and breaks owner and admin login only. + +The import checks run in a subprocess so import-time behaviour is observed +cleanly, without touching the suite's already-imported api.auth. +""" +import os +import subprocess +import sys + +import pytest +from fastapi import HTTPException + +import api.auth + + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + + +def _import_auth_with(env_overrides): + """Import api.auth in a clean subprocess. Returns (returncode, stderr).""" + env = {**os.environ, **env_overrides} + for key, value in env_overrides.items(): + if value is None: + env.pop(key, None) + proc = subprocess.run( + [sys.executable, "-c", "import api.auth"], + cwd=REPO_ROOT, env=env, capture_output=True, text=True, + ) + return proc.returncode, proc.stderr + + +def test_production_without_clerk_jwks_warns_but_still_boots(): + """A missing JWKS URL must be loud, but must not take the app down. + + api.auth is imported by every router, so raising here would kill the patron + game routes, which never touch Clerk. The security guarantee is enforced in + _verify_token instead (see the HMAC test below), not by refusing to start. + """ + code, stderr = _import_auth_with({"DEV_MODE": "false", "CLERK_JWKS_URL": None}) + assert code == 0, f"api.auth refused to import, taking patron routes with it:\n{stderr}" + assert "CLERK_JWKS_URL is not set" in stderr + + +def test_production_with_clerk_jwks_boots(): + code, stderr = _import_auth_with({ + "DEV_MODE": "false", + "CLERK_JWKS_URL": "https://example.test/.well-known/jwks.json", + }) + assert code == 0, f"api.auth failed to import with CLERK_JWKS_URL set:\n{stderr}" + + +def test_dev_mode_without_clerk_jwks_still_boots(): + code, stderr = _import_auth_with({"DEV_MODE": "true", "CLERK_JWKS_URL": None}) + assert code == 0, f"api.auth failed to import in DEV_MODE:\n{stderr}" + + +def test_hmac_token_is_rejected_outside_dev_mode(monkeypatch): + """A validly-signed dev token must not authenticate when DEV_MODE is off.""" + token = api.auth.issue_dev_token("user_whatever") + assert api.auth._verify_token(token) == "user_whatever" # DEV_MODE on: accepted + + monkeypatch.setenv("DEV_MODE", "false") + with pytest.raises(HTTPException) as exc: + api.auth._verify_token(token) + assert exc.value.status_code == 401