Security hardening: go-live review fixes
- TV tokens upgraded from 4 to 6 digits; Regen Token button in Admin - Nginx rate limiting on TV dashboard and WebSocket endpoints - Login lockout after 5 failed attempts (15 min); clears on admin password reset - HSTS header added; CSP unsafe-inline removed from script-src; CORS restricted to explicit methods/headers - Dependency CVE fixes: PyJWT 2.12.0, aiomysql 0.3.0, cryptography 46.0.5, python-multipart 0.0.22 - datetime.utcnow() replaced with datetime.now(timezone.utc) throughout - SQL identifier whitelist for startup migration queries - README updated: security notes section, lockout docs, token regen, NPM proxy guidance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import logging
|
||||
import random
|
||||
from contextlib import asynccontextmanager
|
||||
|
||||
@@ -19,9 +20,36 @@ from app.websocket.manager import manager
|
||||
|
||||
settings = get_settings()
|
||||
|
||||
logging.basicConfig(
|
||||
level=logging.INFO,
|
||||
format="%(asctime)s %(levelname)s %(name)s: %(message)s",
|
||||
)
|
||||
logger = logging.getLogger("homeschool")
|
||||
|
||||
|
||||
_ALLOWED_IDENTIFIERS: set[str] = {
|
||||
# tables
|
||||
"schedule_blocks", "children", "users", "subjects", "daily_sessions", "timer_events",
|
||||
# columns
|
||||
"duration_minutes", "break_time_enabled", "break_time_minutes", "strikes",
|
||||
"timezone", "is_system", "last_active_at", "strikes_last_reset", "tv_token",
|
||||
"failed_login_attempts", "locked_until",
|
||||
# index names
|
||||
"ix_daily_sessions_session_date", "ix_daily_sessions_is_active", "ix_timer_events_event_type",
|
||||
# index columns
|
||||
"session_date", "is_active", "event_type",
|
||||
}
|
||||
|
||||
|
||||
def _check_identifier(*names: str) -> None:
|
||||
for name in names:
|
||||
if name not in _ALLOWED_IDENTIFIERS:
|
||||
raise ValueError(f"Disallowed SQL identifier: {name!r}")
|
||||
|
||||
|
||||
async def _add_column_if_missing(conn, table: str, column: str, definition: str):
|
||||
"""Add a column to a table, silently ignoring if it already exists (MySQL 1060)."""
|
||||
_check_identifier(table, column)
|
||||
try:
|
||||
await conn.execute(text(f"ALTER TABLE {table} ADD COLUMN {column} {definition}"))
|
||||
except OperationalError as e:
|
||||
@@ -29,6 +57,16 @@ async def _add_column_if_missing(conn, table: str, column: str, definition: str)
|
||||
raise
|
||||
|
||||
|
||||
async def _add_index_if_missing(conn, index_name: str, table: str, column: str):
|
||||
"""Create an index, silently ignoring if it already exists (MySQL 1061)."""
|
||||
_check_identifier(index_name, table, column)
|
||||
try:
|
||||
await conn.execute(text(f"CREATE INDEX {index_name} ON {table} ({column})"))
|
||||
except OperationalError as e:
|
||||
if e.orig.args[0] != 1061: # 1061 = Duplicate key name
|
||||
raise
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def lifespan(app: FastAPI):
|
||||
# Create tables on startup (Alembic handles migrations in prod, this is a safety net)
|
||||
@@ -44,6 +82,12 @@ async def lifespan(app: FastAPI):
|
||||
await _add_column_if_missing(conn, "users", "last_active_at", "DATETIME NULL")
|
||||
await _add_column_if_missing(conn, "children", "strikes_last_reset", "DATE NULL")
|
||||
await _add_column_if_missing(conn, "children", "tv_token", "INT NULL")
|
||||
await _add_column_if_missing(conn, "users", "failed_login_attempts", "INT NOT NULL DEFAULT 0")
|
||||
await _add_column_if_missing(conn, "users", "locked_until", "DATETIME NULL")
|
||||
# Idempotent index additions
|
||||
await _add_index_if_missing(conn, "ix_daily_sessions_session_date", "daily_sessions", "session_date")
|
||||
await _add_index_if_missing(conn, "ix_daily_sessions_is_active", "daily_sessions", "is_active")
|
||||
await _add_index_if_missing(conn, "ix_timer_events_event_type", "timer_events", "event_type")
|
||||
|
||||
# Backfill tv_token for existing children that don't have one
|
||||
from app.database import AsyncSessionLocal
|
||||
@@ -53,7 +97,7 @@ async def lifespan(app: FastAPI):
|
||||
used_tokens = set()
|
||||
for child in children_without_token:
|
||||
while True:
|
||||
token = random.randint(1000, 9999)
|
||||
token = random.randint(100000, 999999)
|
||||
if token not in used_tokens:
|
||||
existing = await db.execute(select(Child).where(Child.tv_token == token))
|
||||
if not existing.scalar_one_or_none():
|
||||
@@ -87,9 +131,9 @@ async def lifespan(app: FastAPI):
|
||||
app = FastAPI(
|
||||
title="Homeschool API",
|
||||
version="1.0.0",
|
||||
docs_url="/api/docs",
|
||||
redoc_url="/api/redoc",
|
||||
openapi_url="/api/openapi.json",
|
||||
docs_url="/api/docs" if settings.docs_enabled else None,
|
||||
redoc_url="/api/redoc" if settings.docs_enabled else None,
|
||||
openapi_url="/api/openapi.json" if settings.docs_enabled else None,
|
||||
lifespan=lifespan,
|
||||
)
|
||||
|
||||
@@ -97,8 +141,8 @@ app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=settings.cors_origins_list,
|
||||
allow_credentials=True,
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
allow_methods=["GET", "POST", "PATCH", "DELETE", "OPTIONS"],
|
||||
allow_headers=["Content-Type", "Authorization"],
|
||||
)
|
||||
|
||||
# Routers
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
from datetime import datetime
|
||||
from sqlalchemy import String, Boolean, DateTime
|
||||
from sqlalchemy import String, Boolean, DateTime, Integer
|
||||
from sqlalchemy.orm import Mapped, mapped_column, relationship
|
||||
from app.models.base import Base, TimestampMixin
|
||||
|
||||
@@ -15,6 +15,8 @@ class User(TimestampMixin, Base):
|
||||
is_admin: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||
timezone: Mapped[str] = mapped_column(String(64), nullable=False, default="UTC")
|
||||
last_active_at: Mapped[datetime | None] = mapped_column(DateTime, nullable=True, default=None)
|
||||
failed_login_attempts: Mapped[int] = mapped_column(Integer, nullable=False, default=0)
|
||||
locked_until: Mapped[datetime | None] = mapped_column(DateTime, nullable=True, default=None)
|
||||
|
||||
children: Mapped[list["Child"]] = relationship("Child", back_populates="user") # noqa: F821
|
||||
subjects: Mapped[list["Subject"]] = relationship("Subject", back_populates="user") # noqa: F821
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import logging
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select, delete
|
||||
|
||||
logger = logging.getLogger("homeschool.admin")
|
||||
|
||||
from app.auth.jwt import create_admin_token, create_access_token, hash_password
|
||||
from app.config import get_settings
|
||||
from app.dependencies import get_db, get_admin_user
|
||||
@@ -16,6 +19,7 @@ async def admin_login(body: dict):
|
||||
username = body.get("username", "")
|
||||
password = body.get("password", "")
|
||||
if username != settings.admin_username or password != settings.admin_password:
|
||||
logger.warning("Failed super-admin login attempt for username=%s", username)
|
||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid admin credentials")
|
||||
token = create_admin_token({"sub": "admin"})
|
||||
return {"access_token": token, "token_type": "bearer"}
|
||||
@@ -87,6 +91,8 @@ async def reset_user_password(
|
||||
if not user:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found")
|
||||
user.hashed_password = hash_password(new_password)
|
||||
user.failed_login_attempts = 0
|
||||
user.locked_until = None
|
||||
await db.commit()
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
from datetime import datetime
|
||||
import logging
|
||||
from datetime import datetime, timezone, timedelta
|
||||
from fastapi import APIRouter, Depends, HTTPException, Response, Request, status
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select
|
||||
|
||||
logger = logging.getLogger("homeschool.auth")
|
||||
|
||||
from app.auth.jwt import (
|
||||
create_access_token,
|
||||
create_refresh_token,
|
||||
@@ -22,7 +25,7 @@ REFRESH_COOKIE = "refresh_token"
|
||||
COOKIE_OPTS = {
|
||||
"httponly": True,
|
||||
"samesite": "lax",
|
||||
"secure": False, # set True in production with HTTPS
|
||||
"secure": True,
|
||||
}
|
||||
|
||||
|
||||
@@ -57,16 +60,41 @@ async def register(body: RegisterRequest, response: Response, db: AsyncSession =
|
||||
return TokenResponse(access_token=access)
|
||||
|
||||
|
||||
_LOGIN_MAX_ATTEMPTS = 5
|
||||
_LOGIN_LOCKOUT_MINUTES = 15
|
||||
|
||||
|
||||
@router.post("/login", response_model=TokenResponse)
|
||||
async def login(body: LoginRequest, response: Response, db: AsyncSession = Depends(get_db)):
|
||||
result = await db.execute(select(User).where(User.email == body.email))
|
||||
user = result.scalar_one_or_none()
|
||||
if not user or not verify_password(body.password, user.hashed_password):
|
||||
if not user:
|
||||
raise HTTPException(status_code=401, detail="Invalid credentials")
|
||||
|
||||
now = datetime.now(timezone.utc).replace(tzinfo=None)
|
||||
|
||||
if user.locked_until and user.locked_until > now:
|
||||
remaining = int((user.locked_until - now).total_seconds() / 60) + 1
|
||||
logger.warning("Locked account login attempt for email=%s", body.email)
|
||||
raise HTTPException(status_code=429, detail=f"Account locked. Try again in {remaining} minute(s).")
|
||||
|
||||
if not user.is_active:
|
||||
logger.warning("Login attempt on disabled account email=%s", body.email)
|
||||
raise HTTPException(status_code=403, detail="This account has been disabled. Please contact your administrator.")
|
||||
|
||||
user.last_active_at = datetime.utcnow()
|
||||
if not verify_password(body.password, user.hashed_password):
|
||||
user.failed_login_attempts += 1
|
||||
if user.failed_login_attempts >= _LOGIN_MAX_ATTEMPTS:
|
||||
user.locked_until = now + timedelta(minutes=_LOGIN_LOCKOUT_MINUTES)
|
||||
logger.warning("Account locked for email=%s after %d failed attempts", body.email, user.failed_login_attempts)
|
||||
else:
|
||||
logger.warning("Failed login attempt %d/%d for email=%s", user.failed_login_attempts, _LOGIN_MAX_ATTEMPTS, body.email)
|
||||
await db.commit()
|
||||
raise HTTPException(status_code=401, detail="Invalid credentials")
|
||||
|
||||
user.failed_login_attempts = 0
|
||||
user.locked_until = None
|
||||
user.last_active_at = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
access = create_access_token({"sub": str(user.id)})
|
||||
@@ -95,7 +123,7 @@ async def refresh_token(request: Request, response: Response, db: AsyncSession =
|
||||
if not user:
|
||||
raise HTTPException(status_code=401, detail="User not found")
|
||||
|
||||
user.last_active_at = datetime.utcnow()
|
||||
user.last_active_at = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
access = create_access_token({"sub": str(user.id)})
|
||||
|
||||
@@ -52,7 +52,7 @@ async def list_children(
|
||||
|
||||
async def _generate_tv_token(db: AsyncSession) -> int:
|
||||
while True:
|
||||
token = random.randint(1000, 9999)
|
||||
token = random.randint(100000, 999999)
|
||||
result = await db.execute(select(Child).where(Child.tv_token == token))
|
||||
if not result.scalar_one_or_none():
|
||||
return token
|
||||
@@ -134,6 +134,24 @@ async def update_strikes(
|
||||
return child
|
||||
|
||||
|
||||
@router.post("/{child_id}/regenerate-token", response_model=ChildOut)
|
||||
async def regenerate_tv_token(
|
||||
child_id: int,
|
||||
current_user: User = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
result = await db.execute(
|
||||
select(Child).where(Child.id == child_id, Child.user_id == current_user.id)
|
||||
)
|
||||
child = result.scalar_one_or_none()
|
||||
if not child:
|
||||
raise HTTPException(status_code=404, detail="Child not found")
|
||||
child.tv_token = await _generate_tv_token(db)
|
||||
await db.commit()
|
||||
await db.refresh(child)
|
||||
return child
|
||||
|
||||
|
||||
@router.delete("/{child_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||
async def delete_child(
|
||||
child_id: int,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
"""Shared timer-elapsed computation used by sessions and dashboard routers."""
|
||||
from datetime import datetime
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select
|
||||
@@ -39,7 +39,7 @@ async def compute_block_elapsed(
|
||||
last_start = None
|
||||
running = last_start is not None
|
||||
if running:
|
||||
elapsed += (datetime.utcnow() - last_start).total_seconds()
|
||||
elapsed += (datetime.now(timezone.utc) - last_start).total_seconds()
|
||||
|
||||
# is_paused is True whenever the timer is not actively running —
|
||||
# covers: explicitly paused, never started, or only selected.
|
||||
@@ -75,7 +75,7 @@ async def compute_break_elapsed(
|
||||
last_start = None
|
||||
running = last_start is not None
|
||||
if running:
|
||||
elapsed += (datetime.utcnow() - last_start).total_seconds()
|
||||
elapsed += (datetime.now(timezone.utc) - last_start).total_seconds()
|
||||
|
||||
is_paused = not running
|
||||
return int(elapsed), is_paused
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
fastapi==0.115.0
|
||||
uvicorn[standard]==0.30.6
|
||||
sqlalchemy[asyncio]==2.0.35
|
||||
aiomysql==0.2.0
|
||||
python-jose[cryptography]==3.3.0
|
||||
aiomysql==0.3.0
|
||||
PyJWT==2.12.0
|
||||
cryptography==46.0.5
|
||||
passlib[bcrypt]==1.7.4
|
||||
bcrypt==3.2.2
|
||||
pydantic-settings==2.5.2
|
||||
alembic==1.13.3
|
||||
python-multipart==0.0.12
|
||||
python-multipart==0.0.22
|
||||
email-validator==2.2.0
|
||||
|
||||
Reference in New Issue
Block a user