diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 2b0cf13..b75dce7 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -324,6 +324,7 @@ async def login( # If TOTP is enabled, issue a short-lived MFA challenge token if user.totp_enabled: mfa_token = create_mfa_token(user.id) + await db.commit() return { "authenticated": False, "totp_required": True, @@ -514,15 +515,15 @@ async def auth_status( config = config_result.scalar_one_or_none() registration_open = config.allow_registration if config else False - # Check if authenticated user has passkeys registered (Q-04) + # Check if authenticated user has passkeys registered (Q-04, W-2: EXISTS not COUNT) has_passkeys = False if authenticated and u: pk_result = await db.execute( - select(func.count()).select_from(PasskeyCredential).where( + select(PasskeyCredential.id).where( PasskeyCredential.user_id == u.id - ) + ).limit(1) ) - has_passkeys = pk_result.scalar_one() > 0 + has_passkeys = pk_result.scalar_one_or_none() is not None return { "authenticated": authenticated, @@ -565,6 +566,7 @@ async def verify_password_endpoint( valid, new_hash = await averify_password_with_upgrade(data.password, current_user.password_hash) if not valid: await record_failed_login(db, current_user) + await db.commit() raise HTTPException(status_code=401, detail="Invalid password") if new_hash: @@ -591,6 +593,7 @@ async def change_password( valid, _ = await averify_password_with_upgrade(data.old_password, current_user.password_hash) if not valid: await record_failed_login(db, current_user) + await db.commit() raise HTTPException(status_code=401, detail="Invalid current password") if data.new_password == data.old_password: diff --git a/backend/app/routers/passkeys.py b/backend/app/routers/passkeys.py index 27dd26b..8a95470 100644 --- a/backend/app/routers/passkeys.py +++ b/backend/app/routers/passkeys.py @@ -24,7 +24,7 @@ import json import logging from datetime import datetime -from fastapi import APIRouter, Depends, HTTPException, Request, Response +from fastapi import APIRouter, Depends, HTTPException, Path, Request, Response from pydantic import BaseModel, ConfigDict, Field from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy import select @@ -287,23 +287,16 @@ async def passkey_login_complete( except (json.JSONDecodeError, KeyError): raise HTTPException(status_code=401, detail="Authentication failed") - # Look up credential by ID + # Look up credential + user in a single JOIN query (W-1 perf fix) result = await db.execute( - select(PasskeyCredential).where( - PasskeyCredential.credential_id == raw_id_b64 - ) + select(PasskeyCredential, User) + .join(User, User.id == PasskeyCredential.user_id) + .where(PasskeyCredential.credential_id == raw_id_b64) ) - credential = result.scalar_one_or_none() - if not credential: - raise HTTPException(status_code=401, detail="Authentication failed") - - # Load user - user_result = await db.execute( - select(User).where(User.id == credential.user_id) - ) - user = user_result.scalar_one_or_none() - if not user: + row = result.one_or_none() + if not row: raise HTTPException(status_code=401, detail="Authentication failed") + credential, user = row.tuple() # Check account lockout (C-03) await check_account_lockout(user) @@ -361,6 +354,14 @@ async def passkey_login_complete( _, token = await create_db_session(db, user, client_ip, user_agent) set_session_cookie(response, token) + # B-01: Handle must_change_password and mfa_enforce_pending flags + result_data: dict = {"authenticated": True} + if user.must_change_password: + result_data["must_change_password"] = True + # Passkey satisfies MFA — if mfa_enforce_pending, clear it (before commit) + if user.mfa_enforce_pending: + user.mfa_enforce_pending = False + await log_audit_event( db, action="passkey.login_success", actor_id=user.id, detail={"credential_name": credential.name}, @@ -368,15 +369,6 @@ async def passkey_login_complete( ) await db.commit() - # B-01: Handle must_change_password and mfa_enforce_pending flags - result_data: dict = {"authenticated": True} - if user.must_change_password: - result_data["must_change_password"] = True - # Passkey satisfies MFA — if mfa_enforce_pending, clear it - if user.mfa_enforce_pending: - user.mfa_enforce_pending = False - await db.commit() - return result_data @@ -411,8 +403,8 @@ async def list_passkeys( @router.delete("/{credential_id}") async def delete_passkey( - credential_id: int, - data: PasskeyDeleteRequest, + credential_id: int = Path(ge=1, le=2147483647), + data: PasskeyDeleteRequest = ..., request: Request, db: AsyncSession = Depends(get_db), current_user: User = Depends(get_current_user), diff --git a/backend/app/services/passkey.py b/backend/app/services/passkey.py index fda3a1b..307b1c8 100644 --- a/backend/app/services/passkey.py +++ b/backend/app/services/passkey.py @@ -43,8 +43,11 @@ _challenge_serializer = URLSafeTimedSerializer( salt="webauthn-challenge-v1", ) -# Thread-safe nonce cache for single-use enforcement -# Keys: nonce string, Values: expiry timestamp +# Thread-safe nonce cache for single-use enforcement. +# Keys: nonce string, Values: expiry timestamp. +# NOTE: This is process-local. If scaling to multiple uvicorn workers, +# move nonce tracking to Redis or a DB table with unique constraint. +# Current deployment: single worker (Dockerfile --workers 1). _used_nonces: dict[str, float] = {} _nonce_lock = threading.Lock() diff --git a/backend/app/services/session.py b/backend/app/services/session.py index d687fb9..ae33bda 100644 --- a/backend/app/services/session.py +++ b/backend/app/services/session.py @@ -43,19 +43,25 @@ async def check_account_lockout(user: User) -> None: async def record_failed_login(db: AsyncSession, user: User) -> None: - """Increment failure counter; lock account after 10 failures.""" + """Increment failure counter; lock account after 10 failures. + + Does NOT commit — caller owns the transaction boundary. + """ user.failed_login_count += 1 if user.failed_login_count >= 10: user.locked_until = datetime.now() + timedelta(minutes=30) - await db.commit() + await db.flush() async def record_successful_login(db: AsyncSession, user: User) -> None: - """Reset failure counter and update last_login_at.""" + """Reset failure counter and update last_login_at. + + Does NOT commit — caller owns the transaction boundary. + """ user.failed_login_count = 0 user.locked_until = None user.last_login_at = datetime.now() - await db.commit() + await db.flush() async def create_db_session( diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 7bbfa77..3b1ca90 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -187,5 +187,5 @@ server { add_header Content-Security-Policy "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; img-src 'self' data:; font-src 'self' https://fonts.gstatic.com; connect-src 'self';" always; add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; # PT-I03: Restrict unnecessary browser APIs - add_header Permissions-Policy "camera=(), microphone=(), geolocation=(), interest-cohort=(), publickey-credentials-get=*, publickey-credentials-create=*" always; + add_header Permissions-Policy "camera=(), microphone=(), geolocation=(), interest-cohort=(), publickey-credentials-get=(self), publickey-credentials-create=(self)" always; } diff --git a/frontend/src/components/settings/PasskeySection.tsx b/frontend/src/components/settings/PasskeySection.tsx index 935606d..4fc7818 100644 --- a/frontend/src/components/settings/PasskeySection.tsx +++ b/frontend/src/components/settings/PasskeySection.tsx @@ -90,7 +90,7 @@ function PasskeyDeleteButton({ credential, onDelete, isDeleting }: DeleteConfirm aria-label={`Remove passkey ${credential.name}`} > {confirming ? ( - Sure? + Sure? ) : ( )} @@ -269,7 +269,7 @@ export default function PasskeySection() { {hasPasskeys && ( - + {passkeys.length} registered )} @@ -292,14 +292,14 @@ export default function PasskeySection() { )} - + Added {formatDate(pk.created_at)} · Last used {formatRelativeTime(pk.last_used_at)} ))}