roosterwijs/SECURITY-AUDIT.md

Security & Bug Audit — Roosterwijs

Date: 2026-06-09 · Scope: backend (Django), frontend (React), Docker/nginx deployment. Verdict: code quality is good and the architecture is clean, but the system is not safe to expose on a network yet. One critical gap (no authentication) and two high-risk items must be fixed before the test server holds real student data.

STATUS 2026-06-10: all findings below are FIXED (verified with 13 functional tests + frontend build). Remaining for you: (1) run npm install once in frontend/ and commit package-lock.json so Docker switches to npm ci; (2) set DJANGO_SECURE=1 in .env once the server runs behind HTTPS. Note #2 (TLS itself) still requires a certificate/reverse proxy on the server.

---

CRITICAL

1. The entire API is unauthenticated

REST_FRAMEWORK has no DEFAULT_PERMISSION_CLASSES, so DRF defaults to AllowAny. Anyone who can reach port 8080 can, anonymously:

• read, create, edit and delete all persons — including student names and the free-text opmerkingen field (sensitive special-education data → AVG/GDPR);
• delete groups and subgroups;
• enable/disable modules via POST /api/modules/<key>/state/.

Fix:

# settings.py
REST_FRAMEWORK = {
    "DEFAULT_PERMISSION_CLASSES": ["rest_framework.permissions.IsAuthenticated"],
    "DEFAULT_AUTHENTICATION_CLASSES": [
        "rest_framework.authentication.SessionAuthentication",
    ],
    ...
}

Then add a login flow to the React app and send the CSRF token (see Low #11 — these must land together, otherwise all writes break). This naturally belongs to the accounts module, but a minimal session login must protect the API before that module is fully built — security cannot be an optional plugin.

---

HIGH

2. No TLS anywhere; admin login travels in plain text

docker-compose.yml exposes plain HTTP on 8080 and .env.example shows http:// CSRF origins. Django admin passwords and all student data cross the network unencrypted.

Fix: terminate TLS (reverse proxy or certs in nginx), then enable in settings.py, driven by an env flag:

if _env_bool("DJANGO_SECURE", not DEBUG):
    SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https")
    SECURE_SSL_REDIRECT = True
    SESSION_COOKIE_SECURE = True
    CSRF_COOKIE_SECURE = True
    SECURE_HSTS_SECONDS = 31536000

3. SECRET_KEY silently falls back to a known dev key

If DJANGO_SECRET_KEY is missing in production, the app runs with "dev-only-change-me-in-productie" — sessions and password-reset tokens become forgeable, with no warning.

Fix: fail hard:

SECRET_KEY = os.environ.get("DJANGO_SECRET_KEY", "")
if not SECRET_KEY:
    if _env_bool("DJANGO_DEBUG", False):
        SECRET_KEY = "dev-only-change-me-in-productie"
    else:
        raise RuntimeError("DJANGO_SECRET_KEY ontbreekt; weiger te starten zonder geheime sleutel.")

(Note: DEBUG is read after this block today — reorder accordingly.)

---

MEDIUM

4. Browsable API enabled in production

BrowsableAPIRenderer gives visitors a friendly UI to explore and submit writes. Gate it: include it in DEFAULT_RENDERER_CLASSES only when DEBUG is true.

5. Bug — module toggle truthiness

plugins/views.py: enabled = bool(request.data.get("enabled", False)). A form-encoded or string payload "false" becomes True. Parse explicitly:

enabled = request.data.get("enabled")
if not isinstance(enabled, bool):
    return Response({"detail": "enabled moet true of false zijn."}, status=400)

6. Bug — role change leaves stale subgroup membership

Persoon.save() clears groep when the rol is no longer leerling, but M2M subgroepen membership stays (limit_choices_to only affects admin forms, not the API). A teacher can remain "lid" of a niveaugroepje.

Fix: in Persoon.save() (after super().save()): if self.rol != Rol.LEERLING: self.subgroepen.clear() — or validate in PersoonSerializer.update().

7. No rate limiting

No DRF throttling and no nginx limit_req. Add DEFAULT_THROTTLE_CLASSES/RATES (e.g. "anon": "20/min", "user": "200/min") — cheap insurance once the API is internet-reachable.

8. Backend container runs as root

Add to the Dockerfile after COPY . .:

RUN useradd --create-home appuser && chown -R appuser /app
USER appuser

(Static volume permissions: run collectstatic before switching user, or chown the volume in entrypoint.)

---

LOW

9. Race in sync_states()

Three gunicorn workers can pass the key not in existing check simultaneously → IntegrityError on the unique key. Use ModuleState.objects.get_or_create(key=spec.key, defaults={"enabled": spec.default_enabled}).

10. Missing security headers on the SPA

Django adds X-Frame-Options/nosniff only to /api and /admin. Add to the nginx location / block: X-Content-Type-Options: nosniff, X-Frame-Options: DENY, Referrer-Policy: same-origin, and later a CSP.

11. Frontend sends no CSRF token

api.js never sends X-CSRFToken. Harmless today (anonymous = no CSRF check), but the moment Fix #1 lands, every POST/PATCH/DELETE will 403. Read the csrftoken cookie and set the header in request(). Ship together with #1.

12. npm install in Docker build

Use npm ci with a committed package-lock.json for reproducible, tamper-evident builds.

13. No logging / audit trail

No LOGGING config; with student data you want a record of logins and destructive actions. Minimal console logging config now; an audit-log module fits the plugin architecture later.

---

Verified non-issues

• .gitignore and backend/.dockerignore correctly exclude .env, db.sqlite3 (image and repo stay clean).
• CORS is locked to localhost dev origins; production traffic is same-origin via nginx — correct.
• ALLOWED_HOSTS from env with safe defaults; nginx forwards Host so host-header spoofing is caught.
• Dependency-aware module enable/disable logic in plugins/services.py is sound (checked both directions).
• Postgres is not port-mapped to the host; only nginx is exposed.
• Strong password validators active; DEBUG defaults to off.

Recommended order of work

1. #1 + #11 together (auth + CSRF in frontend) — blocks everything else being meaningful.
2. #3 (fail hard on missing SECRET_KEY) — one-file change, do immediately.
3. #2 (TLS + secure cookies) when the test server gets a hostname.
4. #5, #6, #9 (bugs) — small, independent fixes.
5. #4, #7, #8, #10, #12, #13 as hardening follow-ups.