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 <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,7 @@ from sqlalchemy.orm import DeclarativeBase
|
|||||||
from app.config import settings
|
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)
|
AsyncSessionLocal = async_sessionmaker(engine, expire_on_commit=False)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+1
-2
@@ -33,8 +33,7 @@ async def _seed_admin() -> None:
|
|||||||
display_name="Admin",
|
display_name="Admin",
|
||||||
is_admin=True,
|
is_admin=True,
|
||||||
))
|
))
|
||||||
else:
|
elif not user.is_admin:
|
||||||
user.password_hash = hash_password(settings.admin_password)
|
|
||||||
user.is_admin = True
|
user.is_admin = True
|
||||||
await db.commit()
|
await db.commit()
|
||||||
|
|
||||||
|
|||||||
@@ -140,7 +140,7 @@ async def unimpersonate(
|
|||||||
|
|
||||||
result = await db.execute(select(User).where(User.id == admin_id))
|
result = await db.execute(select(User).where(User.id == admin_id))
|
||||||
admin = result.scalar_one_or_none()
|
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")
|
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Admin not found")
|
||||||
|
|
||||||
return Token(access_token=create_token(admin.id))
|
return Token(access_token=create_token(admin.id))
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import logging
|
|||||||
from fastapi import APIRouter, Depends, HTTPException, Request, status
|
from fastapi import APIRouter, Depends, HTTPException, Request, status
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
|
from sqlalchemy.exc import IntegrityError
|
||||||
|
|
||||||
from app.dependencies import get_db
|
from app.dependencies import get_db
|
||||||
from app.limiter import limiter
|
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],
|
display_name=body.display_name or body.email.split("@")[0],
|
||||||
)
|
)
|
||||||
db.add(user)
|
db.add(user)
|
||||||
|
try:
|
||||||
await db.commit()
|
await db.commit()
|
||||||
|
except IntegrityError:
|
||||||
|
await db.rollback()
|
||||||
|
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Email already registered")
|
||||||
await db.refresh(user)
|
await db.refresh(user)
|
||||||
|
|
||||||
logger.info("New user registered: %s", body.email)
|
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)
|
logger.warning("Failed login for: %s", body.email)
|
||||||
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid credentials")
|
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)
|
logger.info("Login: %s (id=%s)", body.email, user.id)
|
||||||
return Token(access_token=create_token(user.id))
|
return Token(access_token=create_token(user.id))
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ router = APIRouter(prefix="/api/public", tags=["public"])
|
|||||||
@limiter.limit("30/minute")
|
@limiter.limit("30/minute")
|
||||||
async def public_stats(request: Request, db: AsyncSession = Depends(get_db)):
|
async def public_stats(request: Request, db: AsyncSession = Depends(get_db)):
|
||||||
users_result = await db.execute(
|
users_result = await db.execute(
|
||||||
select(User).options(selectinload(User.entries))
|
select(User).options(selectinload(User.entries)).limit(500)
|
||||||
)
|
)
|
||||||
users = users_result.scalars().all()
|
users = users_result.scalars().all()
|
||||||
|
|
||||||
|
|||||||
@@ -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 sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from app.dependencies import get_db, get_current_user
|
from app.dependencies import get_db, get_current_user
|
||||||
|
from app.limiter import limiter
|
||||||
from app.models.user import User
|
from app.models.user import User
|
||||||
from app.schemas.user import UserResponse, UserUpdate, PasswordChange
|
from app.schemas.user import UserResponse, UserUpdate, PasswordChange
|
||||||
from app.utils.security import verify_password, hash_password
|
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)
|
@router.put("/me/password", status_code=status.HTTP_204_NO_CONTENT)
|
||||||
|
@limiter.limit("5/minute")
|
||||||
async def change_password(
|
async def change_password(
|
||||||
|
request: Request,
|
||||||
body: PasswordChange,
|
body: PasswordChange,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
current_user: User = Depends(get_current_user),
|
current_user: User = Depends(get_current_user),
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
fastapi==0.115.6
|
fastapi==0.115.6
|
||||||
uvicorn[standard]==0.32.1
|
uvicorn[standard]==0.32.1
|
||||||
sqlalchemy[asyncio]==2.0.36
|
sqlalchemy[asyncio]==2.0.50
|
||||||
aiomysql==0.2.0
|
aiomysql==0.3.2
|
||||||
pydantic-settings==2.7.0
|
pydantic-settings==2.7.0
|
||||||
python-jose[cryptography]==3.3.0
|
python-jose[cryptography]==3.3.0
|
||||||
passlib[bcrypt]==1.7.4
|
passlib[bcrypt]==1.7.4
|
||||||
|
|||||||
Reference in New Issue
Block a user