From 50ee1f9fed867ef5228242f61f85ceda44ce8465 Mon Sep 17 00:00:00 2001 From: derekc Date: Mon, 25 May 2026 00:55:12 -0700 Subject: [PATCH] Fix auth bypasses, race condition, stale connections, and unbounded query - Block disabled accounts at login and unimpersonate (C4, C2) - Catch IntegrityError on register commit to return 409 instead of 500 (C5) - Stop _seed_admin from overwriting existing admin password on restart (C8) - Cap public/stats query at 500 users to bound memory usage (C3) - Rate-limit PUT /me/password at 5/minute (C6) - Bump SQLAlchemy to 2.0.50 and aiomysql to 0.3.2; re-enable pool_pre_ping (C7) Co-Authored-By: Claude Sonnet 4.6 --- backend/app/database.py | 2 +- backend/app/main.py | 3 +-- backend/app/routers/admin.py | 2 +- backend/app/routers/auth.py | 11 ++++++++++- backend/app/routers/public.py | 2 +- backend/app/routers/users.py | 5 ++++- backend/requirements.txt | 4 ++-- 7 files changed, 20 insertions(+), 9 deletions(-) diff --git a/backend/app/database.py b/backend/app/database.py index 69a40f1..ab666ac 100644 --- a/backend/app/database.py +++ b/backend/app/database.py @@ -4,7 +4,7 @@ from sqlalchemy.orm import DeclarativeBase from app.config import settings -engine = create_async_engine(settings.database_url, echo=False, pool_recycle=1800) +engine = create_async_engine(settings.database_url, echo=False, pool_pre_ping=True, pool_recycle=1800) AsyncSessionLocal = async_sessionmaker(engine, expire_on_commit=False) diff --git a/backend/app/main.py b/backend/app/main.py index f4992b6..9d484dd 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -33,8 +33,7 @@ async def _seed_admin() -> None: display_name="Admin", is_admin=True, )) - else: - user.password_hash = hash_password(settings.admin_password) + elif not user.is_admin: user.is_admin = True await db.commit() diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index c8b06af..0bb7d0d 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -140,7 +140,7 @@ async def unimpersonate( result = await db.execute(select(User).where(User.id == admin_id)) admin = result.scalar_one_or_none() - if not admin or not admin.is_admin: + if not admin or not admin.is_admin or admin.is_disabled: raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Admin not found") return Token(access_token=create_token(admin.id)) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 3340cd0..1ce0c76 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -3,6 +3,7 @@ import logging from fastapi import APIRouter, Depends, HTTPException, Request, status from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy import select +from sqlalchemy.exc import IntegrityError from app.dependencies import get_db from app.limiter import limiter @@ -27,7 +28,11 @@ async def register(request: Request, body: UserCreate, db: AsyncSession = Depend display_name=body.display_name or body.email.split("@")[0], ) db.add(user) - await db.commit() + try: + await db.commit() + except IntegrityError: + await db.rollback() + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered") await db.refresh(user) logger.info("New user registered: %s", body.email) @@ -44,5 +49,9 @@ async def login(request: Request, body: LoginRequest, db: AsyncSession = Depends logger.warning("Failed login for: %s", body.email) raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials") + if user.is_disabled: + logger.warning("Login attempt on disabled account: %s", body.email) + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Account disabled") + logger.info("Login: %s (id=%s)", body.email, user.id) return Token(access_token=create_token(user.id)) diff --git a/backend/app/routers/public.py b/backend/app/routers/public.py index 3727414..383e37d 100644 --- a/backend/app/routers/public.py +++ b/backend/app/routers/public.py @@ -16,7 +16,7 @@ router = APIRouter(prefix="/api/public", tags=["public"]) @limiter.limit("30/minute") async def public_stats(request: Request, db: AsyncSession = Depends(get_db)): users_result = await db.execute( - select(User).options(selectinload(User.entries)) + select(User).options(selectinload(User.entries)).limit(500) ) users = users_result.scalars().all() diff --git a/backend/app/routers/users.py b/backend/app/routers/users.py index db7093f..dd0c6a3 100644 --- a/backend/app/routers/users.py +++ b/backend/app/routers/users.py @@ -1,7 +1,8 @@ -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, Request, status from sqlalchemy.ext.asyncio import AsyncSession from app.dependencies import get_db, get_current_user +from app.limiter import limiter from app.models.user import User from app.schemas.user import UserResponse, UserUpdate, PasswordChange from app.utils.security import verify_password, hash_password @@ -33,7 +34,9 @@ async def update_me( @router.put("/me/password", status_code=status.HTTP_204_NO_CONTENT) +@limiter.limit("5/minute") async def change_password( + request: Request, body: PasswordChange, db: AsyncSession = Depends(get_db), current_user: User = Depends(get_current_user), diff --git a/backend/requirements.txt b/backend/requirements.txt index 368fafd..8f757c8 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -1,7 +1,7 @@ fastapi==0.115.6 uvicorn[standard]==0.32.1 -sqlalchemy[asyncio]==2.0.36 -aiomysql==0.2.0 +sqlalchemy[asyncio]==2.0.50 +aiomysql==0.3.2 pydantic-settings==2.7.0 python-jose[cryptography]==3.3.0 passlib[bcrypt]==1.7.4