mirror of
https://github.com/zvx-echo6/central.git
synced 2026-06-10 11:54:37 +02:00
1b-8: Wizard redesign (deferred-commit) + map fixes + favicon CSRF race fix (#27)
* feat(wizard): implement deferred-commit pattern for setup wizard Replace the current "POST each step -> DB write -> redirect" architecture with "collect values across steps in a signed cookie, commit everything in one transaction at Finish." Key changes: - Add wizard.py: WizardState dataclass and cookie helpers - csrf.py: Add reuse_or_generate_pre_auth_csrf helper - routes.py: All wizard handlers now use cookie state, no DB writes until finish - middleware.py: Cookie-based wizard step routing instead of DB queries - setup_operator.html: Remove "Operator Already Configured" branch Benefits: - Back navigation works: can return to any step and edit values - Atomic commit: all DB writes happen in single transaction at finish - No orphaned state: failed wizard leaves no DB artifacts - Simpler auth: pre-auth CSRF for all 5 steps (no session until finish) Tests updated for new behavior. 287 tests passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(templates): correct SRI hashes for leaflet.draw assets The integrity hashes for leaflet.draw.css and leaflet.draw.js were incorrect, causing browsers to silently block these resources. This broke the Leaflet.draw toolbar and map rendering for FIRMS/USGS adapter region pickers. Updated both setup_adapters.html and adapters_edit.html with the correct sha512 hashes computed from the actual CDN files. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(gui): return 204 for browser-noise paths to prevent CSRF races Browser requests for /favicon.ico, /apple-touch-icon.png, etc. were triggering parallel GET requests that could race with form loads, causing CSRF token rotation issues. Added BROWSER_NOISE_PATHS constant and early 204 response in both SetupGateMiddleware and SessionMiddleware to short-circuit these requests before any cookie/token handling occurs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Matt Johnson <mj@k7zvx.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
246cd75051
commit
78b6fcf150
8 changed files with 710 additions and 1066 deletions
|
|
@ -1,11 +1,10 @@
|
|||
"""Pre-auth CSRF protection for login and setup/operator pages.
|
||||
"""Pre-auth CSRF protection for login and setup pages.
|
||||
|
||||
These routes cannot use session-bound CSRF because no session exists yet.
|
||||
Uses a simple cookie-based pattern with short-lived tokens.
|
||||
"""
|
||||
|
||||
import secrets
|
||||
from typing import Optional
|
||||
|
||||
from itsdangerous import URLSafeTimedSerializer, BadSignature, SignatureExpired
|
||||
from starlette.requests import Request
|
||||
|
|
@ -34,6 +33,34 @@ def generate_pre_auth_csrf(secret_key: str) -> tuple[str, str]:
|
|||
return plain_token, signed_token
|
||||
|
||||
|
||||
def reuse_or_generate_pre_auth_csrf(
|
||||
request: Request,
|
||||
secret_key: str,
|
||||
) -> tuple[str, str | None]:
|
||||
"""Reuse an existing valid pre-auth CSRF token, or generate new.
|
||||
|
||||
Returns (plain_token, signed_token_for_cookie).
|
||||
If signed_token_for_cookie is None, the existing cookie is
|
||||
still valid and caller should not call set_pre_auth_csrf_cookie.
|
||||
If non-None, caller MUST call set_pre_auth_csrf_cookie with
|
||||
it to persist the new value.
|
||||
"""
|
||||
cookie_value = request.cookies.get(PRE_AUTH_CSRF_COOKIE)
|
||||
if cookie_value:
|
||||
serializer = _get_serializer(secret_key)
|
||||
try:
|
||||
plain_token = serializer.loads(
|
||||
cookie_value,
|
||||
max_age=PRE_AUTH_CSRF_MAX_AGE,
|
||||
)
|
||||
return plain_token, None # reuse existing
|
||||
except (BadSignature, SignatureExpired):
|
||||
pass # fall through to generate
|
||||
|
||||
plain_token, signed_token = generate_pre_auth_csrf(secret_key)
|
||||
return plain_token, signed_token
|
||||
|
||||
|
||||
def set_pre_auth_csrf_cookie(response: Response, signed_token: str) -> None:
|
||||
"""Set the pre-auth CSRF cookie on a response."""
|
||||
response.set_cookie(
|
||||
|
|
|
|||
|
|
@ -16,7 +16,15 @@ SETUP_EXEMPT_PREFIXES = ("/static/", "/setup")
|
|||
|
||||
# Paths that don't require authentication
|
||||
AUTH_EXEMPT_PATHS = {"/setup/operator", "/login", "/health"}
|
||||
AUTH_EXEMPT_PREFIXES = ("/static/",)
|
||||
AUTH_EXEMPT_PREFIXES = ("/static/", "/setup/")
|
||||
|
||||
# Browser-noise paths that trigger CSRF race conditions
|
||||
BROWSER_NOISE_PATHS = {
|
||||
"/favicon.ico",
|
||||
"/apple-touch-icon.png",
|
||||
"/apple-touch-icon-precomposed.png",
|
||||
"/robots.txt",
|
||||
}
|
||||
|
||||
|
||||
def _is_exempt(path: str, exempt_paths: set, exempt_prefixes: tuple) -> bool:
|
||||
|
|
@ -29,33 +37,14 @@ def _is_exempt(path: str, exempt_paths: set, exempt_prefixes: tuple) -> bool:
|
|||
return False
|
||||
|
||||
|
||||
async def _get_wizard_redirect_step(conn) -> str:
|
||||
"""Determine which wizard step to redirect to based on DB state."""
|
||||
# Check if any operators exist
|
||||
op_count = await conn.fetchval("SELECT COUNT(*) FROM config.operators")
|
||||
if op_count == 0:
|
||||
def _get_wizard_redirect_from_cookie(request: Request, csrf_secret: str) -> str:
|
||||
"""Determine wizard redirect step from cookie state."""
|
||||
from central.gui.wizard import get_wizard_state, get_step_route
|
||||
|
||||
state = get_wizard_state(request, csrf_secret)
|
||||
if state is None:
|
||||
return "/setup/operator"
|
||||
|
||||
# Check if system settings have been configured (map_tile_url not default)
|
||||
sys_row = await conn.fetchrow(
|
||||
"SELECT map_tile_url FROM config.system WHERE id = true"
|
||||
)
|
||||
default_tile = "https://tile.openstreetmap.org/{z}/{x}/{y}.png"
|
||||
if sys_row is None or sys_row["map_tile_url"] == default_tile:
|
||||
return "/setup/system"
|
||||
|
||||
# Keys step is optional, so check adapters have been reviewed
|
||||
# We consider adapters reviewed if any adapter has a non-null updated_at
|
||||
# (meaning it was explicitly saved during setup)
|
||||
adapters_touched = await conn.fetchval(
|
||||
"SELECT COUNT(*) FROM config.adapters WHERE updated_at IS NOT NULL"
|
||||
)
|
||||
if adapters_touched == 0:
|
||||
# Go to keys first, then adapters
|
||||
return "/setup/keys"
|
||||
|
||||
# All steps done, go to finish
|
||||
return "/setup/finish"
|
||||
return get_step_route(state.wizard_step)
|
||||
|
||||
|
||||
class SetupGateMiddleware(BaseHTTPMiddleware):
|
||||
|
|
@ -64,6 +53,10 @@ class SetupGateMiddleware(BaseHTTPMiddleware):
|
|||
async def dispatch(self, request: Request, call_next) -> Response:
|
||||
path = request.url.path
|
||||
|
||||
# Short-circuit browser-noise requests that cause CSRF races
|
||||
if path in BROWSER_NOISE_PATHS:
|
||||
return Response(status_code=204)
|
||||
|
||||
# Check setup status from database
|
||||
pool = get_pool()
|
||||
if pool is None:
|
||||
|
|
@ -85,13 +78,16 @@ class SetupGateMiddleware(BaseHTTPMiddleware):
|
|||
if not setup_complete:
|
||||
# Setup not complete - only allow setup paths and static/health
|
||||
if path.startswith("/setup"):
|
||||
# Allow all /setup/* paths (handler will enforce auth)
|
||||
# Allow all /setup/* paths
|
||||
# But /setup with no subpath should redirect to appropriate step
|
||||
if path == "/setup" or path == "/setup/":
|
||||
try:
|
||||
async with pool.acquire() as conn:
|
||||
redirect_step = await _get_wizard_redirect_step(conn)
|
||||
return RedirectResponse(url=redirect_step, status_code=302)
|
||||
from central.bootstrap_config import get_settings
|
||||
settings = get_settings()
|
||||
redirect_step = _get_wizard_redirect_from_cookie(
|
||||
request, settings.csrf_secret
|
||||
)
|
||||
return RedirectResponse(url=redirect_step, status_code=302)
|
||||
except Exception:
|
||||
logger.warning("Failed to determine wizard step", exc_info=True)
|
||||
return RedirectResponse(url="/setup/operator", status_code=302)
|
||||
|
|
@ -118,6 +114,11 @@ class SessionMiddleware(BaseHTTPMiddleware):
|
|||
async def dispatch(self, request: Request, call_next) -> Response:
|
||||
path = request.url.path
|
||||
|
||||
# Short-circuit browser-noise requests (already handled by SetupGateMiddleware,
|
||||
# but this protects if middleware order changes)
|
||||
if path in BROWSER_NOISE_PATHS:
|
||||
return Response(status_code=204)
|
||||
|
||||
# Initialize state
|
||||
request.state.operator = None
|
||||
request.state.csrf_token = None
|
||||
|
|
@ -139,7 +140,7 @@ class SessionMiddleware(BaseHTTPMiddleware):
|
|||
request.state.operator = None
|
||||
request.state.csrf_token = None
|
||||
|
||||
# Check if auth is required
|
||||
# Check if auth is required - setup paths are exempt during wizard
|
||||
if not _is_exempt(path, AUTH_EXEMPT_PATHS, AUTH_EXEMPT_PREFIXES):
|
||||
if request.state.operator is None:
|
||||
return RedirectResponse(url="/login", status_code=302)
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -4,9 +4,9 @@
|
|||
|
||||
{% block head %}
|
||||
<link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" integrity="sha256-p4NxAoJBhIIN+hmNHrzRCf9tD/miZyoHS5obTRR9BMY=" crossorigin="">
|
||||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.css" integrity="sha512-gc3xjCmIy673V6MyOAZhIW93xhM9ei1I+gLbmFjUHIjocENRsLX/QUE1htk5q1XV2D/iie/VQ8DXI6Uj8GB1Og==" crossorigin="anonymous">
|
||||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.css" integrity="sha512-gc3xjCmIy673V6MyOAZhIW93xhM9ei1I+gLbmFjUHIjocENRsLX/QUE1htk5q1XV2D/iie/VQ8DXI6Vu8bexvQ==" crossorigin="anonymous">
|
||||
<script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" integrity="sha256-20nQCchB9co0qIjJZRGuk2/Z9VM+kNiyxNV1lvTlZBo=" crossorigin=""></script>
|
||||
<script src="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.js" integrity="sha512-ozq8xQKq6urvuU6jNgkfqAmT7jKN2XumbrX1JiB3TnF7tI48DPI4Ber9dLJ0ikXiRg9G9Vl2jXwqjZ5LDGQ3g==" crossorigin="anonymous"></script>
|
||||
<script src="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.js" integrity="sha512-ozq8xQKq6urvuU6jNgkfqAmT7jKN2XumbrX1JiB3TnF7tI48DPI4Gy1GXKD/V3EExgAs1V+pRO7vwtS1LHg0Gw==" crossorigin="anonymous"></script>
|
||||
{% endblock %}
|
||||
|
||||
{% block content %}
|
||||
|
|
|
|||
|
|
@ -4,9 +4,9 @@
|
|||
|
||||
{% block head %}
|
||||
<link rel="stylesheet" href="https://unpkg.com/leaflet@1.9.4/dist/leaflet.css" integrity="sha256-p4NxAoJBhIIN+hmNHrzRCf9tD/miZyoHS5obTRR9BMY=" crossorigin="">
|
||||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.css" integrity="sha512-gc3xjCmIy673V6MyOAZhIW93xhM9ei1I+gLbmFjUHIjocENRsLX/QUE1htk5q1XV2D/iie/VQ8DXI6Uj8GB1Og==" crossorigin="anonymous">
|
||||
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.css" integrity="sha512-gc3xjCmIy673V6MyOAZhIW93xhM9ei1I+gLbmFjUHIjocENRsLX/QUE1htk5q1XV2D/iie/VQ8DXI6Vu8bexvQ==" crossorigin="anonymous">
|
||||
<script src="https://unpkg.com/leaflet@1.9.4/dist/leaflet.js" integrity="sha256-20nQCchB9co0qIjJZRGuk2/Z9VM+kNiyxNV1lvTlZBo=" crossorigin=""></script>
|
||||
<script src="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.js" integrity="sha512-ozq8xQKq6urvuU6jNgkfqAmT7jKN2XumbrX1JiB3TnF7tI48DPI4Ber9dLJ0ikXiRg9G9Vl2jXwqjZ5LDGQ3g==" crossorigin="anonymous"></script>
|
||||
<script src="https://cdnjs.cloudflare.com/ajax/libs/leaflet.draw/1.0.4/leaflet.draw.js" integrity="sha512-ozq8xQKq6urvuU6jNgkfqAmT7jKN2XumbrX1JiB3TnF7tI48DPI4Gy1GXKD/V3EExgAs1V+pRO7vwtS1LHg0Gw==" crossorigin="anonymous"></script>
|
||||
{% endblock %}
|
||||
|
||||
{% block content %}
|
||||
|
|
|
|||
|
|
@ -7,17 +7,6 @@
|
|||
{% include "_wizard_header.html" %}
|
||||
{% endwith %}
|
||||
|
||||
{% if existing_operator %}
|
||||
<article>
|
||||
<header>
|
||||
<h1>Operator Already Configured</h1>
|
||||
</header>
|
||||
<p>The operator account <strong>{{ existing_operator.username }}</strong> has been created.</p>
|
||||
<div style="display: flex; gap: 1rem; margin-top: 1rem;">
|
||||
<a href="/setup/system" role="button">Next →</a>
|
||||
</div>
|
||||
</article>
|
||||
{% else %}
|
||||
<article>
|
||||
<header>
|
||||
<h1>Create Operator Account</h1>
|
||||
|
|
@ -53,5 +42,4 @@
|
|||
<button type="submit">Create Operator →</button>
|
||||
</form>
|
||||
</article>
|
||||
{% endif %}
|
||||
{% endblock %}
|
||||
|
|
|
|||
131
src/central/gui/wizard.py
Normal file
131
src/central/gui/wizard.py
Normal file
|
|
@ -0,0 +1,131 @@
|
|||
"""Wizard state management for deferred-commit setup flow.
|
||||
|
||||
The wizard collects configuration across 5 steps and commits everything
|
||||
atomically at the final step. State is carried in a signed cookie.
|
||||
"""
|
||||
|
||||
import base64
|
||||
from dataclasses import dataclass, field, asdict
|
||||
from typing import Any
|
||||
|
||||
from itsdangerous import URLSafeTimedSerializer, BadSignature, SignatureExpired
|
||||
from starlette.requests import Request
|
||||
from starlette.responses import Response
|
||||
|
||||
|
||||
# 1 hour max age for wizard cookie
|
||||
WIZARD_MAX_AGE = 3600
|
||||
WIZARD_COOKIE = "central_wizard"
|
||||
|
||||
|
||||
@dataclass
|
||||
class WizardOperator:
|
||||
"""Operator data collected in step 1."""
|
||||
username: str
|
||||
password_hash: str
|
||||
|
||||
|
||||
@dataclass
|
||||
class WizardSystem:
|
||||
"""System settings collected in step 2."""
|
||||
map_tile_url: str
|
||||
map_attribution: str
|
||||
|
||||
|
||||
@dataclass
|
||||
class WizardApiKey:
|
||||
"""API key collected in step 3."""
|
||||
alias: str
|
||||
encrypted_value_b64: str # base64-encoded encrypted value
|
||||
|
||||
|
||||
@dataclass
|
||||
class WizardAdapter:
|
||||
"""Adapter config collected in step 4."""
|
||||
enabled: bool
|
||||
cadence_s: int
|
||||
settings: dict[str, Any]
|
||||
|
||||
|
||||
@dataclass
|
||||
class WizardState:
|
||||
"""Complete wizard state carried across all steps."""
|
||||
wizard_step: int = 1
|
||||
operator: dict | None = None
|
||||
system: dict | None = None
|
||||
api_keys: list[dict] = field(default_factory=list)
|
||||
adapters: dict[str, dict] | None = None
|
||||
|
||||
def to_dict(self) -> dict:
|
||||
"""Convert to dictionary for serialization."""
|
||||
return {
|
||||
"wizard_step": self.wizard_step,
|
||||
"operator": self.operator,
|
||||
"system": self.system,
|
||||
"api_keys": self.api_keys,
|
||||
"adapters": self.adapters,
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def from_dict(cls, data: dict) -> "WizardState":
|
||||
"""Create from dictionary."""
|
||||
return cls(
|
||||
wizard_step=data.get("wizard_step", 1),
|
||||
operator=data.get("operator"),
|
||||
system=data.get("system"),
|
||||
api_keys=data.get("api_keys", []),
|
||||
adapters=data.get("adapters"),
|
||||
)
|
||||
|
||||
|
||||
def _get_wizard_serializer(secret_key: str) -> URLSafeTimedSerializer:
|
||||
"""Get a timed serializer for wizard state."""
|
||||
return URLSafeTimedSerializer(secret_key, salt="wizard-state")
|
||||
|
||||
|
||||
def get_wizard_state(request: Request, secret_key: str) -> WizardState | None:
|
||||
"""Decode wizard state from cookie.
|
||||
|
||||
Returns WizardState if valid, None if missing/invalid/expired.
|
||||
"""
|
||||
cookie_value = request.cookies.get(WIZARD_COOKIE)
|
||||
if not cookie_value:
|
||||
return None
|
||||
|
||||
serializer = _get_wizard_serializer(secret_key)
|
||||
try:
|
||||
data = serializer.loads(cookie_value, max_age=WIZARD_MAX_AGE)
|
||||
return WizardState.from_dict(data)
|
||||
except (BadSignature, SignatureExpired):
|
||||
return None
|
||||
|
||||
|
||||
def set_wizard_cookie(response: Response, state: WizardState, secret_key: str) -> None:
|
||||
"""Set the wizard state cookie on a response."""
|
||||
serializer = _get_wizard_serializer(secret_key)
|
||||
signed_value = serializer.dumps(state.to_dict())
|
||||
response.set_cookie(
|
||||
WIZARD_COOKIE,
|
||||
signed_value,
|
||||
max_age=WIZARD_MAX_AGE,
|
||||
path="/setup",
|
||||
httponly=True,
|
||||
samesite="lax",
|
||||
)
|
||||
|
||||
|
||||
def clear_wizard_cookie(response: Response) -> None:
|
||||
"""Remove the wizard state cookie."""
|
||||
response.delete_cookie(WIZARD_COOKIE, path="/setup")
|
||||
|
||||
|
||||
def get_step_route(step: int) -> str:
|
||||
"""Get the route for a wizard step number."""
|
||||
routes = {
|
||||
1: "/setup/operator",
|
||||
2: "/setup/system",
|
||||
3: "/setup/keys",
|
||||
4: "/setup/adapters",
|
||||
5: "/setup/finish",
|
||||
}
|
||||
return routes.get(step, "/setup/operator")
|
||||
Loading…
Add table
Add a link
Reference in a new issue