3758594896
Addresses three Important-severity review findings inherited from Task 4's plan reference code: - login() now sets secure=True on the session cookie (safe behind the Cloudflare Tunnel, which terminates TLS at the edge). - logout() looks up and deletes the matching AuthSession row before clearing the cookie, so a leaked raw token can no longer be replayed after logout. - login() always performs exactly one verify_password call regardless of whether the username exists (against a module-level dummy hash for nonexistent users), removing the timing oracle that let unauthenticated requests distinguish registered from unregistered usernames. Adds two tests: nonexistent-username login rejection, and logout revoking the session server-side. Also adjusts two cookie-propagation touch points in test_auth.py to manually re-inject the qm_session cookie, since httpx's cookie jar won't auto-attach a Secure cookie to the test transport's plain http://test base_url (a real browser talking to the HTTPS tunnel edge wouldn't have this problem). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013PphXq1s43DNRj1uWKGXof
83 lines
2.9 KiB
Python
83 lines
2.9 KiB
Python
from datetime import datetime, timezone
|
|
|
|
from fastapi import APIRouter, Cookie, Depends, HTTPException, Response, status
|
|
from sqlalchemy import select
|
|
from sqlalchemy.ext.asyncio import AsyncSession
|
|
|
|
from app.db import get_db
|
|
from app.deps import SESSION_COOKIE_NAME, get_current_user
|
|
from app.models.auth_session import AuthSession, SESSION_TTL, generate_session_token, hash_token
|
|
from app.models.user import User
|
|
from app.schemas import LoginRequest, RegisterRequest, UserOut
|
|
from app.security import hash_password, verify_password
|
|
|
|
router = APIRouter(prefix="/auth", tags=["auth"])
|
|
|
|
_DUMMY_PASSWORD_HASH = hash_password("dummy-password-for-timing-safety")
|
|
|
|
|
|
@router.post("/register", response_model=UserOut, status_code=status.HTTP_201_CREATED)
|
|
async def register(payload: RegisterRequest, db: AsyncSession = Depends(get_db)):
|
|
existing = await db.scalar(select(User).where(User.username == payload.username))
|
|
if existing is not None:
|
|
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="username taken")
|
|
|
|
user = User(
|
|
username=payload.username,
|
|
password_hash=hash_password(payload.password),
|
|
email=payload.email,
|
|
)
|
|
db.add(user)
|
|
await db.commit()
|
|
await db.refresh(user)
|
|
return user
|
|
|
|
|
|
@router.post("/login", response_model=UserOut)
|
|
async def login(payload: LoginRequest, response: Response, db: AsyncSession = Depends(get_db)):
|
|
user = await db.scalar(select(User).where(User.username == payload.username))
|
|
if user is None:
|
|
verify_password(payload.password, _DUMMY_PASSWORD_HASH)
|
|
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="invalid credentials")
|
|
if not verify_password(payload.password, user.password_hash):
|
|
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="invalid credentials")
|
|
|
|
raw_token, token_hash = generate_session_token()
|
|
session = AuthSession(
|
|
user_id=user.id,
|
|
token_hash=token_hash,
|
|
expires_at=datetime.now(timezone.utc) + SESSION_TTL,
|
|
)
|
|
db.add(session)
|
|
await db.commit()
|
|
|
|
response.set_cookie(
|
|
SESSION_COOKIE_NAME,
|
|
raw_token,
|
|
httponly=True,
|
|
samesite="lax",
|
|
secure=True,
|
|
max_age=int(SESSION_TTL.total_seconds()),
|
|
)
|
|
return user
|
|
|
|
|
|
@router.post("/logout", status_code=status.HTTP_204_NO_CONTENT)
|
|
async def logout(
|
|
response: Response,
|
|
qm_session: str | None = Cookie(default=None),
|
|
db: AsyncSession = Depends(get_db),
|
|
):
|
|
if qm_session is not None:
|
|
token_hash = hash_token(qm_session)
|
|
session = await db.scalar(select(AuthSession).where(AuthSession.token_hash == token_hash))
|
|
if session is not None:
|
|
await db.delete(session)
|
|
await db.commit()
|
|
response.delete_cookie(SESSION_COOKIE_NAME, httponly=True, samesite="lax", secure=True)
|
|
|
|
|
|
@router.get("/me", response_model=UserOut)
|
|
async def me(user: User = Depends(get_current_user)):
|
|
return user
|