fix(security): consolidation & securite phase 1 (#84, BUG-021 a BUG-034)
- sanitizer XSS serveur (markdown + page de partage) [BUG-021/022] - rate-limit/lockout MFA [BUG-023] - isolation vaults par segments [BUG-024] - caps regex ReDoS [BUG-025] - SSRF webhooks + secrets externalises [BUG-026] - rotation/revocation des jetons [BUG-027] - politique de mot de passe + invalidation sessions [BUG-028] - verrous users.json [BUG-029] - IP reelle dans les audits [BUG-030] - rate-limit par compte [BUG-031] - symlinks hors vault ignores [BUG-032] - recherche simple via inverted index [BUG-033] - token en memoire + cookie HttpOnly, CSP durcie [BUG-034] Tests: pytest 961 passed / 6 skipped, ruff 0, mypy 0, frontend vert.
This commit is contained in:
+116
-16
@@ -8,9 +8,12 @@ import re
|
||||
from fastapi import APIRouter, Body, Depends, HTTPException, Request, Response
|
||||
from pydantic import BaseModel, validator
|
||||
|
||||
from backend.ratelimit import is_rate_limited
|
||||
from backend.ratelimit import is_account_rate_limited, is_rate_limited
|
||||
from backend.ratelimit import record_account_failure as rl_record_account_failure
|
||||
from backend.ratelimit import record_account_success as rl_record_account_success
|
||||
from backend.ratelimit import record_failure as rl_record_failure
|
||||
from backend.ratelimit import record_success as rl_record_success
|
||||
from backend.services.net import get_client_ip
|
||||
|
||||
from .jwt_handler import (
|
||||
ACCESS_TOKEN_EXPIRE_SECONDS,
|
||||
@@ -29,7 +32,7 @@ from .mfa import (
|
||||
verify_totp,
|
||||
)
|
||||
from .middleware import is_auth_enabled, require_admin, require_auth
|
||||
from .password import hash_password, verify_password
|
||||
from .password import hash_password, validate_password_strength, verify_password
|
||||
from .user_store import (
|
||||
create_user,
|
||||
delete_user,
|
||||
@@ -61,9 +64,7 @@ class ChangePasswordRequest(BaseModel):
|
||||
|
||||
@validator("new_password")
|
||||
def password_strength(cls, v):
|
||||
if len(v) < 8:
|
||||
raise ValueError("Minimum 8 caractères")
|
||||
return v
|
||||
return validate_password_strength(v)
|
||||
|
||||
|
||||
class CreateUserRequest(BaseModel):
|
||||
@@ -73,6 +74,10 @@ class CreateUserRequest(BaseModel):
|
||||
role: str = "user"
|
||||
vaults: list[str] = []
|
||||
|
||||
@validator("password")
|
||||
def password_valid(cls, v):
|
||||
return validate_password_strength(v)
|
||||
|
||||
@validator("username")
|
||||
def username_valid(cls, v):
|
||||
if not re.match(r"^[a-zA-Z0-9_-]{2,32}$", v):
|
||||
@@ -93,6 +98,12 @@ class UpdateUserRequest(BaseModel):
|
||||
password: str | None = None
|
||||
role: str | None = None
|
||||
|
||||
@validator("password")
|
||||
def password_valid(cls, v):
|
||||
if v is None:
|
||||
return v
|
||||
return validate_password_strength(v)
|
||||
|
||||
|
||||
# ── Public endpoints ──────────────────────────────────────────────────
|
||||
|
||||
@@ -128,16 +139,21 @@ async def login(body: LoginRequest, response: Response, request: Request):
|
||||
raise HTTPException(403, "Compte désactivé")
|
||||
|
||||
# IP-based rate limiting (10 failures / 15 min per IP)
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
client_ip = get_client_ip(request)
|
||||
if is_rate_limited(client_ip):
|
||||
raise HTTPException(429, "Trop de tentatives depuis cette adresse IP (15min)")
|
||||
|
||||
# BUG-031: per-account budget still applies when the attacker rotates IPs.
|
||||
if is_account_rate_limited(body.username):
|
||||
raise HTTPException(429, "Trop de tentatives sur ce compte (15min)")
|
||||
|
||||
if is_locked(body.username):
|
||||
raise HTTPException(429, "Compte temporairement verrouillé (15min)")
|
||||
|
||||
if not verify_password(body.password, user["password_hash"]):
|
||||
attempts = record_login_failure(body.username)
|
||||
rl_attempts, rl_remaining = rl_record_failure(client_ip)
|
||||
rl_record_account_failure(body.username)
|
||||
remaining = max(0, 5 - attempts)
|
||||
detail = "Identifiants invalides"
|
||||
if 0 < remaining <= 2:
|
||||
@@ -164,9 +180,10 @@ async def login(body: LoginRequest, response: Response, request: Request):
|
||||
def _issue_tokens(user: dict, username: str, remember_me: bool, response: Response) -> dict:
|
||||
"""Issue JWT tokens after successful authentication (password or MFA verified)."""
|
||||
record_login_success(username)
|
||||
rl_record_account_success(username)
|
||||
|
||||
access_token = create_access_token(user)
|
||||
refresh_token, refresh_jti = create_refresh_token(username)
|
||||
refresh_token, refresh_jti = create_refresh_token(username, remember=remember_me)
|
||||
|
||||
import os
|
||||
max_age = 2592000 if remember_me else 604800 # 30d or 7d
|
||||
@@ -208,6 +225,8 @@ async def refresh_token_endpoint(request: Request, response: Response):
|
||||
"""Renew access token via refresh token cookie.
|
||||
|
||||
Called automatically by the frontend when the access token expires.
|
||||
The refresh token is rotated on every use (BUG-027) and rejected if it
|
||||
predates the user's last password change (BUG-028).
|
||||
"""
|
||||
refresh_tok = request.cookies.get("refresh_token")
|
||||
if not refresh_tok:
|
||||
@@ -224,11 +243,38 @@ async def refresh_token_endpoint(request: Request, response: Response):
|
||||
if not user or not user.get("active"):
|
||||
raise HTTPException(401, "Utilisateur introuvable ou inactif")
|
||||
|
||||
# BUG-028: reject refresh tokens issued before the last password change.
|
||||
pca = user.get("password_changed_at")
|
||||
iat = payload.get("iat")
|
||||
if pca is not None and iat is not None:
|
||||
try:
|
||||
stale = int(iat) < int(float(pca))
|
||||
except (TypeError, ValueError):
|
||||
stale = True
|
||||
if stale:
|
||||
raise HTTPException(401, "Session expirée, veuillez vous reconnecter")
|
||||
|
||||
import os
|
||||
|
||||
secure = os.environ.get("OBSIGATE_SECURE_COOKIES", "false").lower() == "true"
|
||||
remember_me = bool(payload.get("remember", False))
|
||||
|
||||
# BUG-027: rotate the refresh token — the old one is now single-use.
|
||||
revoke_token(payload["jti"])
|
||||
new_refresh_token, _new_jti = create_refresh_token(user["username"], remember=remember_me)
|
||||
max_age = 2592000 if remember_me else 604800
|
||||
response.set_cookie(
|
||||
key="refresh_token",
|
||||
value=new_refresh_token,
|
||||
max_age=max_age,
|
||||
httponly=True,
|
||||
samesite="strict",
|
||||
secure=secure,
|
||||
path="/api/auth/refresh",
|
||||
)
|
||||
|
||||
new_access_token = create_access_token(user)
|
||||
|
||||
# Update cookies
|
||||
import os
|
||||
secure = os.environ.get("OBSIGATE_SECURE_COOKIES", "false").lower() == "true"
|
||||
response.set_cookie(
|
||||
key="access_token",
|
||||
value=new_access_token,
|
||||
@@ -251,7 +297,7 @@ async def logout(
|
||||
request: Request,
|
||||
response: Response,
|
||||
):
|
||||
"""Logout: revoke refresh token and delete cookies."""
|
||||
"""Logout: revoke refresh and access tokens, then delete cookies."""
|
||||
refresh_tok = request.cookies.get("refresh_token")
|
||||
if refresh_tok:
|
||||
payload = decode_token(refresh_tok)
|
||||
@@ -261,6 +307,21 @@ async def logout(
|
||||
except Exception:
|
||||
pass # token already revoked
|
||||
|
||||
# BUG-027: revoke the access token too, otherwise it stays valid until expiry.
|
||||
access_tok = None
|
||||
auth_header = request.headers.get("authorization", "")
|
||||
if auth_header.lower().startswith("bearer "):
|
||||
access_tok = auth_header[7:].strip()
|
||||
if not access_tok:
|
||||
access_tok = request.cookies.get("access_token")
|
||||
if access_tok:
|
||||
access_payload = decode_token(access_tok)
|
||||
if access_payload and access_payload.get("type") == "access":
|
||||
try:
|
||||
revoke_token(access_payload["jti"])
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
response.delete_cookie("refresh_token", path="/api/auth/refresh")
|
||||
response.delete_cookie("access_token", path="/")
|
||||
response.delete_cookie("access_token", path="/api") # just in case
|
||||
@@ -313,19 +374,51 @@ async def patch_me(req: UpdateMeRequest, current_user=Depends(require_auth)):
|
||||
@router.post("/change-password")
|
||||
async def change_password(
|
||||
req: ChangePasswordRequest,
|
||||
response: Response,
|
||||
current_user=Depends(require_auth),
|
||||
):
|
||||
"""Change own password."""
|
||||
"""Change own password.
|
||||
|
||||
BUG-028: changing the password invalidates all previously issued tokens;
|
||||
a fresh pair is issued to keep the current session alive.
|
||||
"""
|
||||
user = get_user(current_user["username"])
|
||||
assert user is not None, f"User {current_user['username']} not found"
|
||||
if not verify_password(req.current_password, user["password_hash"]):
|
||||
raise HTTPException(400, "Mot de passe actuel incorrect")
|
||||
update_user(current_user["username"], {"password": req.new_password})
|
||||
return {"message": "Mot de passe mis à jour"}
|
||||
updated = get_user(current_user["username"])
|
||||
result: dict = {"message": "Mot de passe mis à jour"}
|
||||
if updated is not None:
|
||||
result.update(_issue_tokens(updated, updated["username"], False, response))
|
||||
return result
|
||||
|
||||
|
||||
# ── MFA endpoints ────────────────────────────────────────────────────
|
||||
|
||||
def _enforce_mfa_rate_limit(request: Request, username: str) -> str:
|
||||
"""Reject MFA attempts from a rate-limited IP or on a locked account.
|
||||
|
||||
BUG-023: the second-factor endpoints were previously unprotected, making
|
||||
the 6-digit TOTP brute-forceable. Returns the resolved client IP.
|
||||
"""
|
||||
client_ip = get_client_ip(request)
|
||||
if is_rate_limited(client_ip):
|
||||
raise HTTPException(429, "Trop de tentatives depuis cette adresse IP (15min)")
|
||||
if is_account_rate_limited(username):
|
||||
raise HTTPException(429, "Trop de tentatives sur ce compte (15min)")
|
||||
if is_locked(username):
|
||||
raise HTTPException(429, "Compte temporairement verrouillé (15min)")
|
||||
return client_ip
|
||||
|
||||
|
||||
def _record_mfa_failure(client_ip: str, username: str) -> None:
|
||||
"""Record a failed MFA attempt for the IP, the account and the lockout."""
|
||||
record_login_failure(username)
|
||||
rl_record_failure(client_ip)
|
||||
rl_record_account_failure(username)
|
||||
|
||||
|
||||
class MfaVerifyRequest(BaseModel):
|
||||
username: str
|
||||
code: str
|
||||
@@ -601,6 +694,8 @@ async def mfa_webauthn_verify(
|
||||
from .user_store import get_user, update_user
|
||||
from .webauthn_mfa import complete_authentication
|
||||
|
||||
client_ip = _enforce_mfa_rate_limit(request, body.username)
|
||||
|
||||
user = get_user(body.username)
|
||||
if not user:
|
||||
hash_password("dummy_timing_protection")
|
||||
@@ -616,8 +711,10 @@ async def mfa_webauthn_verify(
|
||||
raise ValueError("Credential non enregistré")
|
||||
new_count = complete_authentication(body.username, body.credential, stored)
|
||||
except ValueError as e:
|
||||
_record_mfa_failure(client_ip, body.username)
|
||||
raise HTTPException(401, str(e))
|
||||
except Exception as e:
|
||||
_record_mfa_failure(client_ip, body.username)
|
||||
logger.warning(f"WebAuthn verification failed for {body.username}: {e}")
|
||||
raise HTTPException(401, "Vérification WebAuthn échouée")
|
||||
|
||||
@@ -627,7 +724,6 @@ async def mfa_webauthn_verify(
|
||||
c["sign_count"] = new_count
|
||||
update_user(body.username, {"webauthn_credentials": updated})
|
||||
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
rl_record_success(client_ip)
|
||||
logger.info(f"User '{body.username}' logged in via WebAuthn")
|
||||
return _issue_tokens(user, body.username, body.remember_me, response)
|
||||
@@ -655,6 +751,8 @@ async def mfa_totp_verify(body: MfaVerifyRequest, response: Response, request: R
|
||||
"""
|
||||
from .user_store import get_user
|
||||
|
||||
client_ip = _enforce_mfa_rate_limit(request, body.username)
|
||||
|
||||
user = get_user(body.username)
|
||||
if not user:
|
||||
# Timing-safe: simulate work
|
||||
@@ -665,10 +763,10 @@ async def mfa_totp_verify(body: MfaVerifyRequest, response: Response, request: R
|
||||
raise HTTPException(400, "MFA non activé pour cet utilisateur")
|
||||
|
||||
if not verify_totp(user["mfa_secret"], body.code):
|
||||
_record_mfa_failure(client_ip, body.username)
|
||||
raise HTTPException(401, "Code TOTP invalide")
|
||||
|
||||
# Clear IP rate limit on success
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
rl_record_success(client_ip)
|
||||
|
||||
return _issue_tokens(user, body.username, body.remember_me, response)
|
||||
@@ -682,6 +780,8 @@ async def mfa_recovery_login(body: MfaRecoveryRequest, response: Response, reque
|
||||
"""
|
||||
from .user_store import get_user, update_user
|
||||
|
||||
client_ip = _enforce_mfa_rate_limit(request, body.username)
|
||||
|
||||
user = get_user(body.username)
|
||||
if not user:
|
||||
hash_password("dummy_timing_protection")
|
||||
@@ -696,6 +796,7 @@ async def mfa_recovery_login(body: MfaRecoveryRequest, response: Response, reque
|
||||
|
||||
idx = verify_recovery_code(body.recovery_code, hashed_codes)
|
||||
if idx is None:
|
||||
_record_mfa_failure(client_ip, body.username)
|
||||
raise HTTPException(401, "Code de récupération invalide")
|
||||
|
||||
# Remove used recovery code (single-use)
|
||||
@@ -703,7 +804,6 @@ async def mfa_recovery_login(body: MfaRecoveryRequest, response: Response, reque
|
||||
update_user(body.username, {"mfa_recovery_codes": hashed_codes})
|
||||
|
||||
# Clear IP rate limit
|
||||
client_ip = request.client.host if request.client else "unknown"
|
||||
rl_record_success(client_ip)
|
||||
|
||||
logger.info(f"User '{body.username}' logged in via recovery code")
|
||||
|
||||
Reference in New Issue
Block a user