Manage the password in the app; close remaining audit findings

Password management moves out of the CLI entirely. The credential is now a salted scrypt hash in the database (so it survives rebuilds, living in the /data volume) rather than an environment variable; PARTS_PASSWORD is demoted to a bootstrap value that stops working the moment a password is set in the UI. Every token carries a session epoch, so changing the password — or "sign out other devices" — invalidates outstanding cookies while keeping the browser that made the change signed in. A banner nags until the handed-over password is replaced. app/admin.py remains for the one case the UI cannot cover, a forgotten password. Audit findings: - Taxonomy update and delete scanned affected parts before taking the write lock, so a concurrent rename could leave the search index matching a name the UI no longer showed. All four routes now lock first; removing the lock again makes the new test fail exactly that way. - History of a missing part returned 200 with an empty list; now 404. - Infinity and NaN passed ge=0 and failed at the database. They are rejected as 422 now, and the validation error handler no longer chokes trying to echo a non-finite value back. Checks go from 134 to 181. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-24 10:31:06 -04:00
parent 7bdf276342
commit ab82b5e9a9
11 changed files with 626 additions and 49 deletions
@@ -9,12 +9,14 @@ consumer of the same endpoints rather than a fork of them.
 import asyncio
 import hashlib
 import json
+import math
 import os
 import sqlite3
 from contextlib import asynccontextmanager
 from typing import Annotated, Any, Literal

 from fastapi import Depends, FastAPI, HTTPException, Query, Request, Response
+from fastapi.exceptions import RequestValidationError
 from fastapi.responses import HTMLResponse, JSONResponse
 from fastapi.staticfiles import StaticFiles
 from pydantic import AfterValidator, BaseModel, Field
@@ -69,10 +71,10 @@ class PartIn(BaseModel):
    location_id: int | None = None
    manufacturer: Text = Field(default="", max_length=200)
    mpn: Text = Field(default="", max_length=120)
-    quantity: float = Field(default=0, ge=0)
+    quantity: float = Field(default=0, ge=0, allow_inf_nan=False)
    unit: RequiredText = Field(default="pcs", max_length=20)
-    min_quantity: float | None = Field(default=None, ge=0)
-    cost_each: float | None = Field(default=None, ge=0)
+    min_quantity: float | None = Field(default=None, ge=0, allow_inf_nan=False)
+    cost_each: float | None = Field(default=None, ge=0, allow_inf_nan=False)
    datasheet_url: Text = Field(default="", max_length=1000)
    product_url: Text = Field(default="", max_length=1000)
    notes: Text = Field(default="", max_length=4000)
@@ -87,10 +89,10 @@ class PartPatch(BaseModel):
    location_id: int | None = None
    manufacturer: Text | None = Field(default=None, max_length=200)
    mpn: Text | None = Field(default=None, max_length=120)
-    quantity: float | None = Field(default=None, ge=0)
+    quantity: float | None = Field(default=None, ge=0, allow_inf_nan=False)
    unit: RequiredText | None = Field(default=None, max_length=20)
-    min_quantity: float | None = Field(default=None, ge=0)
-    cost_each: float | None = Field(default=None, ge=0)
+    min_quantity: float | None = Field(default=None, ge=0, allow_inf_nan=False)
+    cost_each: float | None = Field(default=None, ge=0, allow_inf_nan=False)
    datasheet_url: Text | None = Field(default=None, max_length=1000)
    product_url: Text | None = Field(default=None, max_length=1000)
    notes: Text | None = Field(default=None, max_length=4000)
@@ -109,7 +111,7 @@ PART_COLUMNS = [


 class AdjustIn(BaseModel):
-    delta: float
+    delta: float = Field(allow_inf_nan=False)
    reason: Text = Field(default="", max_length=300)


@@ -126,6 +128,11 @@ class LoginIn(BaseModel):
    password: str = Field(max_length=500)


+class PasswordChangeIn(BaseModel):
+    current_password: str = Field(max_length=500)
+    new_password: str = Field(max_length=500)
+
+
 # --- helpers ----------------------------------------------------------------

 def _specs_for(conn, part_ids: list[int]) -> dict[int, list[dict]]:
@@ -240,6 +247,32 @@ def _check_parent(conn, table: str, node_id: int, parent_id: int | None, label:
        raise HTTPException(400, f"A {label} cannot be moved beneath itself")


+# --- error handling ---------------------------------------------------------
+
+def _json_safe(value):
+    """Make a validation-error payload encodable.
+
+    FastAPI echoes the rejected value back in the error detail. When that value
+    is Infinity or NaN — the very thing being rejected — the JSON encoder
+    refuses it and a clean 422 turns into a serialisation failure. Stringify
+    anything the encoder can't represent.
+    """
+    if isinstance(value, float):
+        return value if math.isfinite(value) else str(value)
+    if isinstance(value, dict):
+        return {str(k): _json_safe(v) for k, v in value.items()}
+    if isinstance(value, (list, tuple, set)):
+        return [_json_safe(v) for v in value]
+    if isinstance(value, (str, int, bool)) or value is None:
+        return value
+    return str(value)
+
+
+@app.exception_handler(RequestValidationError)
+def validation_error(request: Request, exc: RequestValidationError):
+    return JSONResponse({"detail": _json_safe(exc.errors())}, status_code=422)
+
+
 # --- middleware -------------------------------------------------------------

@app.middleware("http")
@@ -268,19 +301,35 @@ async def security_headers(request: Request, call_next):

 # --- auth routes ------------------------------------------------------------

+def _set_session_cookie(response: Response, request: Request, conn):
+    response.set_cookie(
+        auth.COOKIE_NAME,
+        auth.issue_token(conn),
+        max_age=auth.session_days() * 86400,
+        httponly=True,
+        samesite="lax",
+        secure=auth.is_https(request),
+        path="/",
+    )
+
+
@app.get("/api/me")
-def me(request: Request):
+def me(request: Request, conn=Depends(db.get_db)):
    if not auth.auth_enabled():
-        return {"authenticated": True, "auth_required": False}
+        return {"authenticated": True, "auth_required": False, "using_bootstrap_password": False}
    token = request.cookies.get(auth.COOKIE_NAME)
+    authenticated = bool(token and auth.token_valid(token, conn))
    return {
-        "authenticated": bool(token and auth.token_valid(token)),
+        "authenticated": authenticated,
        "auth_required": True,
+        # Surfaced so the UI can nag until the handed-over password is replaced.
+        "using_bootstrap_password": authenticated and auth.using_bootstrap_password(conn),
+        "min_password_length": auth.MIN_PASSWORD_LENGTH,
    }


@app.post("/api/login")
-async def login(body: LoginIn, response: Response, request: Request):
+async def login(body: LoginIn, response: Response, request: Request, conn=Depends(db.get_db)):
    if not auth.auth_enabled():
        return {"authenticated": True, "auth_required": False}

@@ -290,7 +339,7 @@ async def login(body: LoginIn, response: Response, request: Request):
            429, "Too many failed attempts", headers={"Retry-After": str(retry_after)}
        )

-    if not auth.check_password(body.password):
+    if not auth.check_password(conn, body.password):
        auth.record_failure()
        # Async sleep, so a burst of guesses can't tie up the worker threadpool
        # that real requests need.
@@ -298,16 +347,52 @@ async def login(body: LoginIn, response: Response, request: Request):
        raise HTTPException(401, "Incorrect password")

    auth.clear_failures()
-    response.set_cookie(
-        auth.COOKIE_NAME,
-        auth.issue_token(),
-        max_age=auth.session_days() * 86400,
-        httponly=True,
-        samesite="lax",
-        secure=auth.is_https(request),
-        path="/",
-    )
-    return {"authenticated": True, "auth_required": True}
+    _set_session_cookie(response, request, conn)
+    return {
+        "authenticated": True,
+        "auth_required": True,
+        "using_bootstrap_password": auth.using_bootstrap_password(conn),
+    }
+
+
+@app.post("/api/password", dependencies=[Depends(auth.require_auth)])
+async def change_password(
+    body: PasswordChangeIn, response: Response, request: Request, conn=Depends(db.get_db)
+):
+    """Change the password from inside the app.
+
+    Requires the current password even though the caller already holds a valid
+    session: a borrowed browser tab should not be enough to lock the owner out.
+    """
+    if not auth.auth_enabled():
+        raise HTTPException(400, "Authentication is disabled, so there is no password to change")
+
+    if not auth.check_password(conn, body.current_password):
+        auth.record_failure()
+        await asyncio.sleep(0.5)
+        raise HTTPException(403, "Current password is incorrect")
+
+    problem = auth.password_problem(body.new_password)
+    if problem:
+        raise HTTPException(422, problem)
+    if body.new_password == body.current_password:
+        raise HTTPException(422, "New password must be different from the current one")
+
+    auth.clear_failures()
+    # Bumps the session epoch, so every cookie issued before now stops working.
+    auth.set_password(conn, body.new_password)
+    # Re-issue for the caller, so changing the password doesn't log you out of
+    # the tab you changed it in.
+    _set_session_cookie(response, request, conn)
+    return {"changed": True, "other_sessions_signed_out": True}
+
+
+@app.post("/api/sessions/revoke", dependencies=[Depends(auth.require_auth)])
+def revoke_sessions(response: Response, request: Request, conn=Depends(db.get_db)):
+    """Sign out every other device, keeping this one signed in."""
+    auth.bump_epoch(conn)
+    _set_session_cookie(response, request, conn)
+    return {"revoked": True}


@app.post("/api/logout")
@@ -503,6 +588,8 @@ def adjust_part(part_id: int, body: AdjustIn, conn=Depends(db.get_db)):

@app.get("/api/parts/{part_id}/history", dependencies=[Depends(auth.require_auth)])
 def part_history(part_id: int, conn=Depends(db.get_db), limit: int = Query(default=50, ge=1, le=500)):
+    if conn.execute("SELECT 1 FROM parts WHERE id = ?", (part_id,)).fetchone() is None:
+        raise HTTPException(404, "Part not found")
    rows = conn.execute(
        "SELECT * FROM stock_log WHERE part_id = ? ORDER BY id DESC LIMIT ?", (part_id, limit)
    ).fetchall()
@@ -578,6 +665,10 @@ def create_category(body: NodeIn, conn=Depends(db.get_db)):

@app.patch("/api/categories/{cat_id}", dependencies=[Depends(auth.require_auth)])
 def update_category(cat_id: int, body: NodeIn, conn=Depends(db.get_db)):
+    # Before the first read: the affected-parts scan and the reindex that
+    # follows it have to see one consistent tree, or a concurrent write can slip
+    # between them and leave the search index describing the old taxonomy.
+    db.begin_immediate(conn)
    _require_node(conn, "categories", cat_id, "Category")
    _check_parent(conn, "categories", cat_id, body.parent_id, "category")
    affected = db.parts_under(conn, "categories", cat_id)
@@ -596,6 +687,10 @@ def update_category(cat_id: int, body: NodeIn, conn=Depends(db.get_db)):

@app.delete("/api/categories/{cat_id}", dependencies=[Depends(auth.require_auth)])
 def delete_category(cat_id: int, conn=Depends(db.get_db)):
+    # Before the first read: the affected-parts scan and the reindex that
+    # follows it have to see one consistent tree, or a concurrent write can slip
+    # between them and leave the search index describing the old taxonomy.
+    db.begin_immediate(conn)
    _require_node(conn, "categories", cat_id, "Category")
    # Capture the parts first: ON DELETE SET NULL means they survive as
    # uncategorised rather than vanish, but their indexed path must be rebuilt.
@@ -626,6 +721,10 @@ def create_location(body: NodeIn, conn=Depends(db.get_db)):

@app.patch("/api/locations/{loc_id}", dependencies=[Depends(auth.require_auth)])
 def update_location(loc_id: int, body: NodeIn, conn=Depends(db.get_db)):
+    # Before the first read: the affected-parts scan and the reindex that
+    # follows it have to see one consistent tree, or a concurrent write can slip
+    # between them and leave the search index describing the old taxonomy.
+    db.begin_immediate(conn)
    _require_node(conn, "locations", loc_id, "Location")
    _check_parent(conn, "locations", loc_id, body.parent_id, "location")
    affected = db.parts_under(conn, "locations", loc_id)
@@ -642,6 +741,10 @@ def update_location(loc_id: int, body: NodeIn, conn=Depends(db.get_db)):

@app.delete("/api/locations/{loc_id}", dependencies=[Depends(auth.require_auth)])
 def delete_location(loc_id: int, conn=Depends(db.get_db)):
+    # Before the first read: the affected-parts scan and the reindex that
+    # follows it have to see one consistent tree, or a concurrent write can slip
+    # between them and leave the search index describing the old taxonomy.
+    db.begin_immediate(conn)
    _require_node(conn, "locations", loc_id, "Location")
    affected = db.parts_under(conn, "locations", loc_id)
    conn.execute("DELETE FROM locations WHERE id = ?", (loc_id,))