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 <noreply@anthropic.com>
This commit is contained in:
parent
aa2d011700
commit
5ad0a610bd
@ -376,6 +376,7 @@ async def auth_status(
|
|||||||
@router.post("/verify-password")
|
@router.post("/verify-password")
|
||||||
async def verify_password(
|
async def verify_password(
|
||||||
data: VerifyPasswordRequest,
|
data: VerifyPasswordRequest,
|
||||||
|
request: Request,
|
||||||
db: AsyncSession = Depends(get_db),
|
db: AsyncSession = Depends(get_db),
|
||||||
current_user: User = Depends(get_current_user),
|
current_user: User = Depends(get_current_user),
|
||||||
):
|
):
|
||||||
@ -383,11 +384,20 @@ async def verify_password(
|
|||||||
Verify the current user's password without changing anything.
|
Verify the current user's password without changing anything.
|
||||||
Used by the frontend lock screen to re-authenticate without a full login.
|
Used by the frontend lock screen to re-authenticate without a full login.
|
||||||
Also handles transparent bcrypt→Argon2id upgrade.
|
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)
|
valid, new_hash = verify_password_with_upgrade(data.password, current_user.password_hash)
|
||||||
if not valid:
|
if not valid:
|
||||||
|
_record_ip_failure(client_ip)
|
||||||
|
await _record_failed_login(db, current_user)
|
||||||
raise HTTPException(status_code=401, detail="Invalid password")
|
raise HTTPException(status_code=401, detail="Invalid password")
|
||||||
|
|
||||||
|
_failed_attempts.pop(client_ip, None)
|
||||||
|
|
||||||
# Persist upgraded hash if migration happened
|
# Persist upgraded hash if migration happened
|
||||||
if new_hash:
|
if new_hash:
|
||||||
current_user.password_hash = new_hash
|
current_user.password_hash = new_hash
|
||||||
|
|||||||
@ -1,3 +1,4 @@
|
|||||||
|
from app.schemas.auth import SetupRequest, LoginRequest, ChangePasswordRequest, VerifyPasswordRequest
|
||||||
from app.schemas.settings import SettingsUpdate, SettingsResponse
|
from app.schemas.settings import SettingsUpdate, SettingsResponse
|
||||||
from app.schemas.todo import TodoCreate, TodoUpdate, TodoResponse
|
from app.schemas.todo import TodoCreate, TodoUpdate, TodoResponse
|
||||||
from app.schemas.calendar_event import CalendarEventCreate, CalendarEventUpdate, CalendarEventResponse
|
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
|
from app.schemas.location import LocationCreate, LocationUpdate, LocationResponse
|
||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
|
"SetupRequest",
|
||||||
|
"LoginRequest",
|
||||||
|
"ChangePasswordRequest",
|
||||||
|
"VerifyPasswordRequest",
|
||||||
"SettingsUpdate",
|
"SettingsUpdate",
|
||||||
"SettingsResponse",
|
"SettingsResponse",
|
||||||
"TodoCreate",
|
"TodoCreate",
|
||||||
|
|||||||
@ -64,3 +64,10 @@ class ChangePasswordRequest(BaseModel):
|
|||||||
|
|
||||||
class VerifyPasswordRequest(BaseModel):
|
class VerifyPasswordRequest(BaseModel):
|
||||||
password: str
|
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
|
||||||
|
|||||||
@ -80,6 +80,7 @@ export default function LockOverlay() {
|
|||||||
<Input
|
<Input
|
||||||
ref={inputRef}
|
ref={inputRef}
|
||||||
type="password"
|
type="password"
|
||||||
|
aria-label="Password"
|
||||||
value={password}
|
value={password}
|
||||||
onChange={(e) => setPassword(e.target.value)}
|
onChange={(e) => setPassword(e.target.value)}
|
||||||
placeholder="Enter password to unlock"
|
placeholder="Enter password to unlock"
|
||||||
|
|||||||
@ -29,6 +29,8 @@ export function LockProvider({ children }: { children: ReactNode }) {
|
|||||||
|
|
||||||
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||||
const lastActivityRef = useRef<number>(Date.now());
|
const lastActivityRef = useRef<number>(Date.now());
|
||||||
|
const activeMutationsRef = useRef(activeMutations);
|
||||||
|
activeMutationsRef.current = activeMutations;
|
||||||
|
|
||||||
const lock = useCallback(() => {
|
const lock = useCallback(() => {
|
||||||
setIsLocked(true);
|
setIsLocked(true);
|
||||||
@ -60,7 +62,7 @@ export function LockProvider({ children }: { children: ReactNode }) {
|
|||||||
|
|
||||||
const resetTimer = () => {
|
const resetTimer = () => {
|
||||||
// Don't lock while TanStack mutations are in flight
|
// Don't lock while TanStack mutations are in flight
|
||||||
if (activeMutations > 0) return;
|
if (activeMutationsRef.current > 0) return;
|
||||||
|
|
||||||
if (timerRef.current) clearTimeout(timerRef.current);
|
if (timerRef.current) clearTimeout(timerRef.current);
|
||||||
timerRef.current = setTimeout(() => {
|
timerRef.current = setTimeout(() => {
|
||||||
@ -92,7 +94,7 @@ export function LockProvider({ children }: { children: ReactNode }) {
|
|||||||
timerRef.current = null;
|
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 (
|
return (
|
||||||
<LockContext.Provider value={{ isLocked, lock, unlock }}>
|
<LockContext.Provider value={{ isLocked, lock, unlock }}>
|
||||||
|
|||||||
@ -1,5 +1,6 @@
|
|||||||
export interface Settings {
|
export interface Settings {
|
||||||
id: number;
|
id: number;
|
||||||
|
user_id: number;
|
||||||
accent_color: string;
|
accent_color: string;
|
||||||
upcoming_days: number;
|
upcoming_days: number;
|
||||||
preferred_name?: string | null;
|
preferred_name?: string | null;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user