H-01: Add global CSRF middleware for all mutating endpoints
Replace the admin-only verify_xhr dependency with a pure ASGI CSRFHeaderMiddleware that validates X-Requested-With: XMLHttpRequest on all POST/PUT/PATCH/DELETE requests globally. Pre-auth endpoints (login, setup, register, totp-verify, enforce-setup/confirm) are exempt. This closes the CSRF gap where non-admin routes accepted requests without origin validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
9f7bbbfcbb
commit
581efa183a
@ -19,6 +19,59 @@ from app.models import system_config as _system_config_model # noqa: F401
|
||||
from app.models import audit_log as _audit_log_model # noqa: F401
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pure ASGI CSRF middleware — SEC-08 (global)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class CSRFHeaderMiddleware:
|
||||
"""
|
||||
Require X-Requested-With: XMLHttpRequest on all state-mutating requests.
|
||||
|
||||
Browsers never send this header cross-origin without a CORS preflight,
|
||||
which our CORS policy blocks. This prevents CSRF attacks from simple
|
||||
form submissions and cross-origin fetches.
|
||||
|
||||
Uses pure ASGI (not BaseHTTPMiddleware) to avoid streaming/memory overhead.
|
||||
"""
|
||||
|
||||
_EXEMPT_PATHS = frozenset({
|
||||
"/health",
|
||||
"/",
|
||||
"/api/auth/login",
|
||||
"/api/auth/setup",
|
||||
"/api/auth/register",
|
||||
"/api/auth/totp-verify",
|
||||
"/api/auth/totp/enforce-setup",
|
||||
"/api/auth/totp/enforce-confirm",
|
||||
})
|
||||
_MUTATING_METHODS = frozenset({"POST", "PUT", "PATCH", "DELETE"})
|
||||
|
||||
def __init__(self, app):
|
||||
self.app = app
|
||||
|
||||
async def __call__(self, scope, receive, send):
|
||||
if scope["type"] == "http":
|
||||
method = scope.get("method", "")
|
||||
path = scope.get("path", "")
|
||||
|
||||
if method in self._MUTATING_METHODS and path not in self._EXEMPT_PATHS:
|
||||
headers = dict(scope.get("headers", []))
|
||||
if headers.get(b"x-requested-with") != b"XMLHttpRequest":
|
||||
body = b'{"detail":"Invalid request origin"}'
|
||||
await send({
|
||||
"type": "http.response.start",
|
||||
"status": 403,
|
||||
"headers": [
|
||||
[b"content-type", b"application/json"],
|
||||
[b"content-length", str(len(body)).encode()],
|
||||
],
|
||||
})
|
||||
await send({"type": "http.response.body", "body": body})
|
||||
return
|
||||
|
||||
await self.app(scope, receive, send)
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def lifespan(app: FastAPI):
|
||||
scheduler = AsyncIOScheduler()
|
||||
@ -47,7 +100,12 @@ app = FastAPI(
|
||||
openapi_url="/openapi.json" if _is_dev else None,
|
||||
)
|
||||
|
||||
# CORS configuration
|
||||
# Middleware stack — added in reverse order (last added = outermost).
|
||||
# CSRF is added first (innermost), then CORS wraps it (outermost).
|
||||
# This ensures CORS headers appear on CSRF 403 responses.
|
||||
app.add_middleware(CSRFHeaderMiddleware)
|
||||
|
||||
# CORS configuration — outermost layer
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=[o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()],
|
||||
|
||||
@ -4,7 +4,7 @@ Admin router — full user management, system config, and audit log.
|
||||
Security measures implemented:
|
||||
SEC-02: Session revocation on role change
|
||||
SEC-05: Block admin self-actions (own role/password/MFA/active status)
|
||||
SEC-08: X-Requested-With header check (verify_xhr) on all state-mutating requests
|
||||
SEC-08: X-Requested-With validation (now handled globally by CSRFHeaderMiddleware)
|
||||
SEC-13: Session revocation + ntfy alert on MFA disable
|
||||
|
||||
All routes require the `require_admin` dependency (which chains through
|
||||
@ -15,7 +15,7 @@ from datetime import datetime
|
||||
from typing import Optional
|
||||
|
||||
import sqlalchemy as sa
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query, Request
|
||||
from fastapi import APIRouter, Depends, HTTPException, Path, Query, Request
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.database import get_db
|
||||
@ -48,26 +48,12 @@ from app.services.audit import log_audit_event
|
||||
from app.services.auth import hash_password
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# CSRF guard — SEC-08
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
async def verify_xhr(request: Request) -> None:
|
||||
"""
|
||||
Lightweight CSRF mitigation: require X-Requested-With on state-mutating
|
||||
requests. Browsers never send this header cross-origin without CORS
|
||||
pre-flight, which our CORS policy blocks.
|
||||
"""
|
||||
if request.method not in ("GET", "HEAD", "OPTIONS"):
|
||||
if request.headers.get("X-Requested-With") != "XMLHttpRequest":
|
||||
raise HTTPException(status_code=403, detail="Invalid request origin")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Router — all endpoints inherit require_admin + verify_xhr
|
||||
# Router — all endpoints inherit require_admin
|
||||
# (SEC-08 CSRF validation is now handled globally by CSRFHeaderMiddleware)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
router = APIRouter(
|
||||
dependencies=[Depends(require_admin), Depends(verify_xhr)],
|
||||
dependencies=[Depends(require_admin)],
|
||||
)
|
||||
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user