From 5ad0a610bd47639260156bee0a0764358e6b5ee4 Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Wed, 25 Feb 2026 18:20:42 +0800 Subject: [PATCH] Address all QA review warnings and suggestions for lock screen feature - [C-1] Add rate limiting and account lockout to /verify-password endpoint - [W-3] Add max length validator (128 chars) to VerifyPasswordRequest - [W-1] Move activeMutations to ref in useLock to prevent timer thrashing - [W-5] Add user_id field to frontend Settings interface - [S-1] Export auth schemas from schemas registry - [S-3] Add aria-label to LockOverlay password input Co-Authored-By: Claude Opus 4.6 --- backend/app/routers/auth.py | 10 ++++++++++ backend/app/schemas/__init__.py | 5 +++++ backend/app/schemas/auth.py | 7 +++++++ frontend/src/components/layout/LockOverlay.tsx | 1 + frontend/src/hooks/useLock.tsx | 6 ++++-- frontend/src/types/index.ts | 1 + 6 files changed, 28 insertions(+), 2 deletions(-) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 8515fa6..2b2c0b6 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -376,6 +376,7 @@ async def auth_status( @router.post("/verify-password") async def verify_password( data: VerifyPasswordRequest, + request: Request, db: AsyncSession = Depends(get_db), current_user: User = Depends(get_current_user), ): @@ -383,11 +384,20 @@ async def verify_password( Verify the current user's password without changing anything. Used by the frontend lock screen to re-authenticate without a full login. Also handles transparent bcrypt→Argon2id upgrade. + Shares the same rate-limit and lockout guards as /login. """ + client_ip = request.client.host if request.client else "unknown" + _check_ip_rate_limit(client_ip) + await _check_account_lockout(current_user) + valid, new_hash = verify_password_with_upgrade(data.password, current_user.password_hash) if not valid: + _record_ip_failure(client_ip) + await _record_failed_login(db, current_user) raise HTTPException(status_code=401, detail="Invalid password") + _failed_attempts.pop(client_ip, None) + # Persist upgraded hash if migration happened if new_hash: current_user.password_hash = new_hash diff --git a/backend/app/schemas/__init__.py b/backend/app/schemas/__init__.py index fee7b6d..b4e5fa8 100644 --- a/backend/app/schemas/__init__.py +++ b/backend/app/schemas/__init__.py @@ -1,3 +1,4 @@ +from app.schemas.auth import SetupRequest, LoginRequest, ChangePasswordRequest, VerifyPasswordRequest from app.schemas.settings import SettingsUpdate, SettingsResponse from app.schemas.todo import TodoCreate, TodoUpdate, TodoResponse from app.schemas.calendar_event import CalendarEventCreate, CalendarEventUpdate, CalendarEventResponse @@ -8,6 +9,10 @@ from app.schemas.person import PersonCreate, PersonUpdate, PersonResponse from app.schemas.location import LocationCreate, LocationUpdate, LocationResponse __all__ = [ + "SetupRequest", + "LoginRequest", + "ChangePasswordRequest", + "VerifyPasswordRequest", "SettingsUpdate", "SettingsResponse", "TodoCreate", diff --git a/backend/app/schemas/auth.py b/backend/app/schemas/auth.py index 198eb89..806835d 100644 --- a/backend/app/schemas/auth.py +++ b/backend/app/schemas/auth.py @@ -64,3 +64,10 @@ class ChangePasswordRequest(BaseModel): class VerifyPasswordRequest(BaseModel): password: str + + @field_validator("password") + @classmethod + def validate_length(cls, v: str) -> str: + if len(v) > 128: + raise ValueError("Password must be 128 characters or fewer") + return v diff --git a/frontend/src/components/layout/LockOverlay.tsx b/frontend/src/components/layout/LockOverlay.tsx index 281faf7..3108ded 100644 --- a/frontend/src/components/layout/LockOverlay.tsx +++ b/frontend/src/components/layout/LockOverlay.tsx @@ -80,6 +80,7 @@ export default function LockOverlay() { setPassword(e.target.value)} placeholder="Enter password to unlock" diff --git a/frontend/src/hooks/useLock.tsx b/frontend/src/hooks/useLock.tsx index 4dc01f4..fed762d 100644 --- a/frontend/src/hooks/useLock.tsx +++ b/frontend/src/hooks/useLock.tsx @@ -29,6 +29,8 @@ export function LockProvider({ children }: { children: ReactNode }) { const timerRef = useRef | null>(null); const lastActivityRef = useRef(Date.now()); + const activeMutationsRef = useRef(activeMutations); + activeMutationsRef.current = activeMutations; const lock = useCallback(() => { setIsLocked(true); @@ -60,7 +62,7 @@ export function LockProvider({ children }: { children: ReactNode }) { const resetTimer = () => { // Don't lock while TanStack mutations are in flight - if (activeMutations > 0) return; + if (activeMutationsRef.current > 0) return; if (timerRef.current) clearTimeout(timerRef.current); timerRef.current = setTimeout(() => { @@ -92,7 +94,7 @@ export function LockProvider({ children }: { children: ReactNode }) { timerRef.current = null; } }; - }, [settings?.auto_lock_enabled, settings?.auto_lock_minutes, isLocked, lock, activeMutations]); + }, [settings?.auto_lock_enabled, settings?.auto_lock_minutes, isLocked, lock]); return ( diff --git a/frontend/src/types/index.ts b/frontend/src/types/index.ts index b47ec34..510aacd 100644 --- a/frontend/src/types/index.ts +++ b/frontend/src/types/index.ts @@ -1,5 +1,6 @@ export interface Settings { id: number; + user_id: number; accent_color: string; upcoming_days: number; preferred_name?: string | null;