From 53101d1401a4f0ad6abf7bb6c872096e3928f125 Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Tue, 17 Mar 2026 23:02:59 +0800 Subject: [PATCH] Action deferred review items: TOTP lockout consolidation + toast nav W-04: Replace inline lockout logic in totp.py (3 occurrences of manual failed_login_count/locked_until manipulation) with shared session service calls: check_account_lockout, record_failed_login, record_successful_login. Also fix TOTP replay prevention to use flush() not commit() for atomicity with session creation. S-1: Add "Set up" action button to the post-login passkey prompt toast, navigating to /settings?tab=security (already supported by SettingsPage search params). Co-Authored-By: Claude Opus 4.6 (1M context) --- backend/app/routers/totp.py | 39 ++++++++------------ frontend/src/components/layout/AppLayout.tsx | 13 +++++-- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/backend/app/routers/totp.py b/backend/app/routers/totp.py index 39d84cf..e2d8331 100644 --- a/backend/app/routers/totp.py +++ b/backend/app/routers/totp.py @@ -20,7 +20,7 @@ Security: import asyncio import secrets import logging -from datetime import datetime, timedelta +from datetime import datetime from typing import Optional from fastapi import APIRouter, Depends, HTTPException, Request, Response @@ -40,7 +40,13 @@ from app.services.auth import ( verify_mfa_token, verify_mfa_enforce_token, ) -from app.services.session import create_db_session, set_session_cookie +from app.services.session import ( + create_db_session, + set_session_cookie, + check_account_lockout, + record_failed_login, + record_successful_login, +) from app.services.totp import ( generate_totp_secret, encrypt_totp_secret, @@ -263,42 +269,31 @@ async def totp_verify( raise HTTPException(status_code=400, detail="TOTP not configured for this account") # Check account lockout (shared counter with password failures) - if user.locked_until and datetime.now() < user.locked_until: - remaining = int((user.locked_until - datetime.now()).total_seconds() / 60) + 1 - raise HTTPException( - status_code=423, - detail=f"Account locked. Try again in {remaining} minutes.", - ) + await check_account_lockout(user) # --- Backup code path --- if data.backup_code: normalized = data.backup_code.strip().upper() valid = await _verify_backup_code(db, user.id, normalized) if not valid: - user.failed_login_count += 1 - if user.failed_login_count >= 10: - user.locked_until = datetime.now() + timedelta(minutes=30) + await record_failed_login(db, user) await db.commit() raise HTTPException(status_code=401, detail="Invalid backup code") # Backup code accepted — reset lockout counter and issue session - user.failed_login_count = 0 - user.locked_until = None - user.last_login_at = datetime.now() - await db.commit() + await record_successful_login(db, user) ip = get_client_ip(request) user_agent = request.headers.get("user-agent") _, token = await create_db_session(db, user, ip, user_agent) set_session_cookie(response, token) + await db.commit() return {"authenticated": True} # --- TOTP code path --- matched_window = verify_totp_code(user.totp_secret, data.code) if matched_window is None: - user.failed_login_count += 1 - if user.failed_login_count >= 10: - user.locked_until = datetime.now() + timedelta(minutes=30) + await record_failed_login(db, user) await db.commit() raise HTTPException(status_code=401, detail="Invalid code") @@ -306,21 +301,19 @@ async def totp_verify( totp_record = TOTPUsage(user_id=user.id, code=data.code, window=matched_window) db.add(totp_record) try: - await db.commit() + await db.flush() except IntegrityError: await db.rollback() raise HTTPException(status_code=401, detail="Code already used — wait for the next code") # Success — reset lockout counter, update last_login_at, issue full session - user.failed_login_count = 0 - user.locked_until = None - user.last_login_at = datetime.now() - await db.commit() + await record_successful_login(db, user) ip = get_client_ip(request) user_agent = request.headers.get("user-agent") _, token = await create_db_session(db, user, ip, user_agent) set_session_cookie(response, token) + await db.commit() return {"authenticated": True} diff --git a/frontend/src/components/layout/AppLayout.tsx b/frontend/src/components/layout/AppLayout.tsx index 0d9909f..6c69453 100644 --- a/frontend/src/components/layout/AppLayout.tsx +++ b/frontend/src/components/layout/AppLayout.tsx @@ -1,5 +1,5 @@ import { useState, useEffect } from 'react'; -import { Outlet } from 'react-router-dom'; +import { Outlet, useNavigate } from 'react-router-dom'; import { toast } from 'sonner'; import { Menu } from 'lucide-react'; import { useTheme } from '@/hooks/useTheme'; @@ -20,6 +20,7 @@ function AppContent({ mobileOpen, setMobileOpen }: { }) { const { isLocked, isLockResolved } = useLock(); const { hasPasskeys } = useAuth(); + const navigate = useNavigate(); usePrefetch(isLockResolved && !isLocked); // Post-login passkey prompt — show once per session if user has no passkeys @@ -30,9 +31,15 @@ function AppContent({ mobileOpen, setMobileOpen }: { !sessionStorage.getItem('passkey-prompt-shown') ) { sessionStorage.setItem('passkey-prompt-shown', '1'); - toast.info('Simplify your login \u2014 set up a passkey in Settings', { duration: 8000 }); + toast.info('Simplify your login \u2014 set up a passkey in Settings', { + duration: 8000, + action: { + label: 'Set up', + onClick: () => navigate('/settings?tab=security'), + }, + }); } - }, [isLockResolved, isLocked, hasPasskeys]); + }, [isLockResolved, isLocked, hasPasskeys, navigate]); const [collapsed, setCollapsed] = useState(() => { try { return JSON.parse(localStorage.getItem('umbra-sidebar-collapsed') || 'false'); } catch { return false; }