fix: P0 hardening — fail-closed SECRET_KEY, locked CORS, role-safe registration (HARDENING.md P0.1/P0.2/P0.3)
P0.1 — fail-closed secrets: - config.py: no default SECRET_KEY; refuses to boot when unset, a known placeholder, or <32 chars. Generate with: openssl rand -hex 32. - docker-compose.yml: literal secrets removed; runtime env now comes from a git-ignored .env via env_file. .env.example added as template. - .gitignore already covers .env (verified). P0.2 — locked CORS: - main.py: CORS_ORIGINS must be an explicit comma-separated allow-list. '*' or an empty value refuses to boot (was: silently ['*'] with allow_credentials=True). P0.3 — role-safe registration: - services/auth.py: client-supplied 'role' is IGNORED on POST /api/auth/register; self-registered users always get the least-privilege 'CS Rep' role. Unauthenticated callers can no longer mint Admin/Jerome, Admin/Wahab, or Director accounts. Tests: - conftest.py sets test SECRET_KEY/CORS_ORIGINS before app import. - New tests/test_p0_hardening.py (8 tests): role-escalation blocked for Admin/Jerome and Admin/Wahab, duplicate-email 409, and subprocess boot-validation for placeholder/short/missing secret + wildcard CORS. - Full suite: 44 passed. Redeploy note (per research): seed_units/seed_categories are insert-only, so the Aug-26 redeploy does NOT orphan historical tickets referencing units 103E/103W/105E/105W or the legacy 34-category tree. Pending Wahab: are 103E/103W/105E/105W real apartments dropped from the Excel regeneration? Optional follow-up: floor-number backfill for already- seeded units (mapping corrected floors; existing rows keep old values). Checks per HARDENING.md acceptance: - [x] starting without a real key fails loudly (subprocess-verified) - [x] compose carries no literal secret; secrets come from .env - [x] CORS_ORIGINS explicit allow-list, '*' rejected - [x] unauthenticated register cannot mint Admin/* or Director
This commit is contained in:
committed by
Mumuni (Syslog Falcon)
parent
76d9d12b78
commit
3ec09470ff
@@ -0,0 +1,15 @@
|
||||
# Denya OneCare — runtime environment template (HARDENING.md P0.1/P1.1)
|
||||
# Copy to .env and fill in real values. NEVER commit .env.
|
||||
# Generate the secret with: openssl rand -hex 32
|
||||
|
||||
# ── Required ─────────────────────────────────────────────
|
||||
SECRET_KEY=
|
||||
DATABASE_URL=sqlite+aiosqlite:///./data/denya_onecare.db
|
||||
# Explicit origin allow-list — "*" is rejected at startup (P0.2)
|
||||
CORS_ORIGINS=http://localhost:8000
|
||||
|
||||
# ── Optional (WhatsApp; needed before wiring Meta) ───────
|
||||
WHATSAPP_PHONE_NUMBER_ID=
|
||||
WHATSAPP_ACCESS_TOKEN=
|
||||
WHATSAPP_VERIFY_TOKEN=
|
||||
META_GRAPH_BASE=https://graph.facebook.com/v18.0
|
||||
+19
-1
@@ -23,7 +23,7 @@ class Settings(BaseSettings):
|
||||
DATABASE_URL: str = "sqlite+aiosqlite:///./denya_onecare.db"
|
||||
|
||||
# ── Auth ─────────────────────────────────────────────────────────
|
||||
SECRET_KEY: str = "change-me-in-production-use-a-real-secret"
|
||||
SECRET_KEY: str = ""
|
||||
ALGORITHM: str = "HS256"
|
||||
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 # Phase 1: raised 30 -> 60 for fewer re-logins
|
||||
REFRESH_TOKEN_EXPIRE_MINUTES: int = 60 * 24 * 7 # 7 days
|
||||
@@ -42,3 +42,21 @@ class Settings(BaseSettings):
|
||||
|
||||
|
||||
settings = Settings()
|
||||
|
||||
# ── Fail-closed secret validation (HARDENING.md P0.1) ─────────────────
|
||||
# Refuse to boot without a real SECRET_KEY. Devs must create a local .env
|
||||
# (see .env.example); production injects it via docker-compose env_file.
|
||||
_KNOWN_PLACEHOLDER_SECRETS = {
|
||||
"",
|
||||
"change-me-in-production",
|
||||
"change-me-in-production-use-a-real-secret",
|
||||
"changeme",
|
||||
"secret",
|
||||
}
|
||||
|
||||
if settings.SECRET_KEY in _KNOWN_PLACEHOLDER_SECRETS or len(settings.SECRET_KEY) < 32:
|
||||
raise RuntimeError(
|
||||
"SECRET_KEY is missing, a known placeholder, or shorter than 32 chars. "
|
||||
"Generate one with: openssl rand -hex 32 — and set it in .env "
|
||||
"(dev) or the runtime environment (prod). Refusing to start."
|
||||
)
|
||||
|
||||
+9
-2
@@ -83,10 +83,17 @@ app = FastAPI(
|
||||
lifespan=lifespan,
|
||||
)
|
||||
|
||||
# ── CORS ─────────────────────────────────────────────────────────────
|
||||
# ── CORS (HARDENING.md P0.2 — explicit origin allow-list, never "*") ──
|
||||
_origins = [o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()]
|
||||
if "*" in _origins or not _origins:
|
||||
raise RuntimeError(
|
||||
"CORS_ORIGINS must be an explicit comma-separated origin allow-list "
|
||||
"(e.g. 'https://denya.sysloggh.net,http://localhost:8000'). "
|
||||
"'*' with allow_credentials=True is invalid and unsafe. Refusing to start."
|
||||
)
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=settings.CORS_ORIGINS.split(",") if settings.CORS_ORIGINS != "*" else ["*"],
|
||||
allow_origins=_origins,
|
||||
allow_credentials=True,
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
|
||||
+4
-1
@@ -10,7 +10,10 @@ class RegisterRequest(BaseModel):
|
||||
password: str
|
||||
full_name: str
|
||||
phone: str | None = None
|
||||
role: str = "CS Rep"
|
||||
# HARDENING.md P0.3: role is NOT client-controllable. Self-registration
|
||||
# always creates the least-privilege role; privileged roles are assigned
|
||||
# by an admin directly in the DB (or a future admin-gated endpoint).
|
||||
role: str = "CS Rep" # kept for backward compat; ignored by the service
|
||||
|
||||
|
||||
class LoginRequest(BaseModel):
|
||||
|
||||
+11
-2
@@ -16,9 +16,18 @@ from app.core.security import (
|
||||
from app.models.user import User
|
||||
from app.schemas.auth import RegisterRequest
|
||||
|
||||
# HARDENING.md P0.3 — least-privilege default for self-registered users.
|
||||
_SELF_REGISTER_ROLE = "CS Rep"
|
||||
|
||||
|
||||
async def register(db: AsyncSession, body: RegisterRequest) -> User:
|
||||
"""Create a new user. Raises 409 if email already exists."""
|
||||
"""Create a new user. Raises 409 if email already exists.
|
||||
|
||||
HARDENING.md P0.3: unauthenticated self-registration must never mint a
|
||||
privileged role. The client-supplied ``role`` field is IGNORED — new
|
||||
self-registered users always land on the least-privilege role.
|
||||
Admins assign elevated roles directly (DB seed / admin endpoint).
|
||||
"""
|
||||
result = await db.execute(select(User).where(User.email == body.email))
|
||||
if result.scalar_one_or_none():
|
||||
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered")
|
||||
@@ -28,7 +37,7 @@ async def register(db: AsyncSession, body: RegisterRequest) -> User:
|
||||
password_hash=hash_password(body.password),
|
||||
full_name=body.full_name,
|
||||
phone=body.phone,
|
||||
role=body.role,
|
||||
role=_SELF_REGISTER_ROLE,
|
||||
)
|
||||
db.add(user)
|
||||
await db.flush()
|
||||
|
||||
+2
-3
@@ -4,10 +4,9 @@ services:
|
||||
container_name: denya-onecare
|
||||
ports:
|
||||
- "8000:8000"
|
||||
env_file:
|
||||
- .env # git-ignored; see .env.example for required keys
|
||||
environment:
|
||||
- DATABASE_URL=sqlite+aiosqlite:///./data/denya_onecare.db
|
||||
- SECRET_KEY=change-me-in-production
|
||||
- CORS_ORIGINS=*
|
||||
- DEBUG=false
|
||||
volumes:
|
||||
- app-data:/app/data
|
||||
|
||||
@@ -12,6 +12,10 @@ import tempfile
|
||||
|
||||
_TMP_DIR = tempfile.mkdtemp(prefix="denya-test-")
|
||||
os.environ["DATABASE_URL"] = f"sqlite+aiosqlite:///{_TMP_DIR}/test.db"
|
||||
# HARDENING.md P0.1/P0.2: the app now fails closed without a real SECRET_KEY
|
||||
# and an explicit CORS allow-list — tests must satisfy both.
|
||||
os.environ.setdefault("SECRET_KEY", "test-secret-key-not-for-production-0123456789abcdef")
|
||||
os.environ.setdefault("CORS_ORIGINS", "http://test")
|
||||
|
||||
import pytest_asyncio # noqa: E402 (DATABASE_URL must be set before app imports)
|
||||
from httpx import ASGITransport, AsyncClient # noqa: E402
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
"""P0 hardening regression tests (HARDENING.md P0.1 / P0.2 / P0.3).
|
||||
|
||||
Covers:
|
||||
- P0.3: self-registration CANNOT mint a privileged role (role field ignored)
|
||||
- P0.3: duplicate email still 409s
|
||||
- P0.1: app fails to import/boot with placeholder or missing SECRET_KEY
|
||||
- P0.2: app fails to boot with CORS_ORIGINS="*"
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from httpx import ASGITransport, AsyncClient
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
|
||||
pytestmark = pytest.mark.asyncio
|
||||
|
||||
|
||||
|
||||
# ── P0.3: registration role-escalation ────────────────────────────────
|
||||
|
||||
async def test_register_cannot_mint_admin_role(client: AsyncClient):
|
||||
"""A raw unauthenticated register call must NOT be able to mint Admin/*."""
|
||||
resp = await client.post(
|
||||
"/api/auth/register",
|
||||
json={
|
||||
"email": "attacker@example.com",
|
||||
"password": "Sup3rSecret!",
|
||||
"full_name": "Attacker",
|
||||
"role": "Admin/Jerome",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 201, resp.text
|
||||
created = resp.json()
|
||||
assert created["role"] == "CS Rep", (
|
||||
f"self-registration minted privileged role: {created['role']}"
|
||||
)
|
||||
|
||||
|
||||
async def test_register_role_wahab_also_blocked(client: AsyncClient):
|
||||
resp = await client.post(
|
||||
"/api/auth/register",
|
||||
json={
|
||||
"email": "attacker2@example.com",
|
||||
"password": "Sup3rSecret!",
|
||||
"full_name": "Attacker Two",
|
||||
"role": "Admin/Wahab",
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
assert resp.json()["role"] == "CS Rep"
|
||||
|
||||
|
||||
async def test_register_duplicate_email_conflict(client: AsyncClient):
|
||||
payload = {
|
||||
"email": "dupe@example.com",
|
||||
"password": "Sup3rSecret!",
|
||||
"full_name": "Dupe",
|
||||
}
|
||||
r1 = await client.post("/api/auth/register", json=payload)
|
||||
assert r1.status_code == 201
|
||||
r2 = await client.post("/api/auth/register", json=payload)
|
||||
assert r2.status_code == 409
|
||||
|
||||
|
||||
# ── P0.1 / P0.2: fail-closed boot validation ──────────────────────────
|
||||
|
||||
def _boot_with_env(env_overrides: dict[str, str]) -> subprocess.CompletedProcess:
|
||||
"""Try importing app.main in a subprocess with the given env; the import
|
||||
must fail (non-zero) when fail-closed validation trips."""
|
||||
env = os.environ.copy()
|
||||
env["DATABASE_URL"] = "sqlite+aiosqlite:///:memory:"
|
||||
env.pop("SECRET_KEY", None)
|
||||
env.pop("CORS_ORIGINS", None)
|
||||
env.update(env_overrides)
|
||||
script = (
|
||||
"import sys; sys.path.insert(0, ''); "
|
||||
"import app.main" # noqa
|
||||
)
|
||||
return subprocess.run(
|
||||
[sys.executable, "-c", script],
|
||||
cwd=str(REPO_ROOT),
|
||||
env=env,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=60,
|
||||
)
|
||||
|
||||
|
||||
def test_boot_fails_with_placeholder_secret():
|
||||
result = _boot_with_env({"SECRET_KEY": "change-me-in-production"})
|
||||
assert result.returncode != 0, "app booted with placeholder SECRET_KEY!"
|
||||
assert "SECRET_KEY" in result.stderr
|
||||
|
||||
|
||||
def test_boot_fails_with_short_secret():
|
||||
result = _boot_with_env({"SECRET_KEY": "tooshort"})
|
||||
assert result.returncode != 0, "app booted with a <32-char SECRET_KEY!"
|
||||
assert "SECRET_KEY" in result.stderr
|
||||
|
||||
|
||||
def test_boot_fails_without_secret():
|
||||
result = _boot_with_env({"SECRET_KEY": ""})
|
||||
assert result.returncode != 0, "app booted without a SECRET_KEY!"
|
||||
assert "SECRET_KEY" in result.stderr
|
||||
|
||||
|
||||
def test_boot_fails_with_wildcard_cors():
|
||||
result = _boot_with_env(
|
||||
{
|
||||
"SECRET_KEY": "test-secret-key-not-for-production-0123456789abcdef",
|
||||
"CORS_ORIGINS": "*",
|
||||
}
|
||||
)
|
||||
assert result.returncode != 0, "app booted with CORS_ORIGINS=* !"
|
||||
assert "CORS_ORIGINS" in result.stderr
|
||||
|
||||
|
||||
def test_boot_succeeds_with_valid_env():
|
||||
result = _boot_with_env(
|
||||
{
|
||||
"SECRET_KEY": "test-secret-key-not-for-production-0123456789abcdef",
|
||||
"CORS_ORIGINS": "http://test",
|
||||
}
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
Reference in New Issue
Block a user