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) <noreply@anthropic.com>
This commit is contained in:
Kyle 2026-03-17 23:02:59 +08:00
parent ab84c7bc53
commit 53101d1401
2 changed files with 26 additions and 26 deletions

View File

@ -20,7 +20,7 @@ Security:
import asyncio import asyncio
import secrets import secrets
import logging import logging
from datetime import datetime, timedelta from datetime import datetime
from typing import Optional from typing import Optional
from fastapi import APIRouter, Depends, HTTPException, Request, Response from fastapi import APIRouter, Depends, HTTPException, Request, Response
@ -40,7 +40,13 @@ from app.services.auth import (
verify_mfa_token, verify_mfa_token,
verify_mfa_enforce_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 ( from app.services.totp import (
generate_totp_secret, generate_totp_secret,
encrypt_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") raise HTTPException(status_code=400, detail="TOTP not configured for this account")
# Check account lockout (shared counter with password failures) # Check account lockout (shared counter with password failures)
if user.locked_until and datetime.now() < user.locked_until: await check_account_lockout(user)
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.",
)
# --- Backup code path --- # --- Backup code path ---
if data.backup_code: if data.backup_code:
normalized = data.backup_code.strip().upper() normalized = data.backup_code.strip().upper()
valid = await _verify_backup_code(db, user.id, normalized) valid = await _verify_backup_code(db, user.id, normalized)
if not valid: if not valid:
user.failed_login_count += 1 await record_failed_login(db, user)
if user.failed_login_count >= 10:
user.locked_until = datetime.now() + timedelta(minutes=30)
await db.commit() await db.commit()
raise HTTPException(status_code=401, detail="Invalid backup code") raise HTTPException(status_code=401, detail="Invalid backup code")
# Backup code accepted — reset lockout counter and issue session # Backup code accepted — reset lockout counter and issue session
user.failed_login_count = 0 await record_successful_login(db, user)
user.locked_until = None
user.last_login_at = datetime.now()
await db.commit()
ip = get_client_ip(request) ip = get_client_ip(request)
user_agent = request.headers.get("user-agent") user_agent = request.headers.get("user-agent")
_, token = await create_db_session(db, user, ip, user_agent) _, token = await create_db_session(db, user, ip, user_agent)
set_session_cookie(response, token) set_session_cookie(response, token)
await db.commit()
return {"authenticated": True} return {"authenticated": True}
# --- TOTP code path --- # --- TOTP code path ---
matched_window = verify_totp_code(user.totp_secret, data.code) matched_window = verify_totp_code(user.totp_secret, data.code)
if matched_window is None: if matched_window is None:
user.failed_login_count += 1 await record_failed_login(db, user)
if user.failed_login_count >= 10:
user.locked_until = datetime.now() + timedelta(minutes=30)
await db.commit() await db.commit()
raise HTTPException(status_code=401, detail="Invalid code") 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) totp_record = TOTPUsage(user_id=user.id, code=data.code, window=matched_window)
db.add(totp_record) db.add(totp_record)
try: try:
await db.commit() await db.flush()
except IntegrityError: except IntegrityError:
await db.rollback() await db.rollback()
raise HTTPException(status_code=401, detail="Code already used — wait for the next code") 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 # Success — reset lockout counter, update last_login_at, issue full session
user.failed_login_count = 0 await record_successful_login(db, user)
user.locked_until = None
user.last_login_at = datetime.now()
await db.commit()
ip = get_client_ip(request) ip = get_client_ip(request)
user_agent = request.headers.get("user-agent") user_agent = request.headers.get("user-agent")
_, token = await create_db_session(db, user, ip, user_agent) _, token = await create_db_session(db, user, ip, user_agent)
set_session_cookie(response, token) set_session_cookie(response, token)
await db.commit()
return {"authenticated": True} return {"authenticated": True}

View File

@ -1,5 +1,5 @@
import { useState, useEffect } from 'react'; import { useState, useEffect } from 'react';
import { Outlet } from 'react-router-dom'; import { Outlet, useNavigate } from 'react-router-dom';
import { toast } from 'sonner'; import { toast } from 'sonner';
import { Menu } from 'lucide-react'; import { Menu } from 'lucide-react';
import { useTheme } from '@/hooks/useTheme'; import { useTheme } from '@/hooks/useTheme';
@ -20,6 +20,7 @@ function AppContent({ mobileOpen, setMobileOpen }: {
}) { }) {
const { isLocked, isLockResolved } = useLock(); const { isLocked, isLockResolved } = useLock();
const { hasPasskeys } = useAuth(); const { hasPasskeys } = useAuth();
const navigate = useNavigate();
usePrefetch(isLockResolved && !isLocked); usePrefetch(isLockResolved && !isLocked);
// Post-login passkey prompt — show once per session if user has no passkeys // 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.getItem('passkey-prompt-shown')
) { ) {
sessionStorage.setItem('passkey-prompt-shown', '1'); 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(() => { const [collapsed, setCollapsed] = useState(() => {
try { return JSON.parse(localStorage.getItem('umbra-sidebar-collapsed') || 'false'); } try { return JSON.parse(localStorage.getItem('umbra-sidebar-collapsed') || 'false'); }
catch { return false; } catch { return false; }