fix(auth): corrige 4 failles de la revue de securite sur la PR #90
Anti-enumeration cassee sur /auth/forgot-password : l'envoi SMTP etait synchrone dans le chemin de reponse, donc un email existant prenait plus de temps qu'un email inconnu (et pouvait renvoyer 500 si le relais SMTP echouait, contre 202 sinon). L'envoi part desormais en BackgroundTasks, apres que la reponse 202 a ete envoyee au client, avec un try/except qui logue plutot que de laisser une exception SMTP remonter. confirm_password_reset() ne revalidait pas is_active/kind du compte avant de changer le mot de passe : un compte desactive dans les 15 minutes suivant l'emission du lien pouvait quand meme voir son mot de passe change et son must_change_password efface. Les plages [A-ZA-Y]/[a-za-y] de la regle de complexite incluaient par erreur x et / (U+00D7, U+00F7), donc un mot de passe sans aucune majuscule ou minuscule pouvait passer la validation. Le validateur frontend (JS, \w ASCII) et le validateur backend (Python, \w Unicode) divergeaient sur les caracteres accentues : un mot de passe comme "Securite1" passait cote front puis se faisait rejeter en 422 cote back. Les deux cotes utilisent maintenant le meme jeu explicite de caracteres speciaux (SPECIAL_CHARACTERS, partage aussi avec cli.py).
This commit is contained in:
@@ -14,7 +14,10 @@ from datetime import UTC, datetime, timedelta
|
||||
from typing import NoReturn, Protocol
|
||||
from uuid import UUID, uuid4
|
||||
|
||||
from fastapi import BackgroundTasks
|
||||
|
||||
from app.core.hashing import Argon2Hasher
|
||||
from app.core.logging import get_logger
|
||||
from app.core.mailer import Mailer
|
||||
from app.core.principal import Principal
|
||||
from app.core.roles import AccountKind, Role
|
||||
@@ -34,6 +37,8 @@ from app.repositories.password_reset_token import PasswordResetTokenRepository
|
||||
from app.repositories.refresh_token import RefreshTokenRepository
|
||||
from app.repositories.user import UserRepository
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
|
||||
class Transaction(Protocol):
|
||||
async def commit(self) -> None: ...
|
||||
@@ -225,14 +230,21 @@ class AuthService:
|
||||
return self._session(self._en_principal(rafraichi or compte), secret)
|
||||
|
||||
async def request_password_reset(
|
||||
self, *, email: str, client_ip: str | None, user_agent: str | None
|
||||
self,
|
||||
*,
|
||||
email: str,
|
||||
client_ip: str | None,
|
||||
user_agent: str | None,
|
||||
background_tasks: BackgroundTasks,
|
||||
) -> None:
|
||||
await self._refuse_si_limite_reset(email=email, client_ip=client_ip)
|
||||
|
||||
compte = await self._users.get_by_email(email)
|
||||
# Piège : le hachage factice équilibre le temps de réponse sur un compte inconnu, comme
|
||||
# `authenticate()`. La réponse et sa forme restent identiques dans tous les cas : compte
|
||||
# inconnu, compte inactif, ou email envoyé avec succès.
|
||||
# inconnu, compte inactif, ou email envoyé avec succès. L'envoi SMTP lui-même est différé
|
||||
# en tâche de fond : le laisser dans le chemin de réponse rouvrirait le même oracle par le
|
||||
# temps (aller-retour réseau) et par la forme (500 si le relais SMTP échoue, contre 202).
|
||||
if compte is None or not compte.is_active or compte.kind != AccountKind.HUMAIN.value:
|
||||
await self._hasher.verify_dummy()
|
||||
await self._reset_attempts.record(email=email, client_ip=client_ip)
|
||||
@@ -260,7 +272,13 @@ class AuthService:
|
||||
await self._transaction.commit()
|
||||
|
||||
lien = f"{self._reset_policy.frontend_reset_url}?token={secret}"
|
||||
await self._mailer.send_password_reset_email(to=compte.email, reset_url=lien)
|
||||
background_tasks.add_task(self._envoie_email_reset, compte.email, lien)
|
||||
|
||||
async def _envoie_email_reset(self, email: str, reset_url: str) -> None:
|
||||
try:
|
||||
await self._mailer.send_password_reset_email(to=email, reset_url=reset_url)
|
||||
except Exception:
|
||||
logger.exception("auth.password_reset.mail_failed")
|
||||
|
||||
async def confirm_password_reset(
|
||||
self, *, token: str, new_password: str, client_ip: str | None, user_agent: str | None
|
||||
@@ -269,6 +287,13 @@ class AuthService:
|
||||
if revendique is None:
|
||||
raise InvalidOrExpiredResetTokenError("Lien invalide ou expiré")
|
||||
|
||||
# Piège : le jeton peut avoir été émis avant une désactivation du compte. Sans cette
|
||||
# relecture, un lien encore valide (15 min) changerait quand même le mot de passe d'un
|
||||
# compte désactivé, réutilisable dès sa réactivation.
|
||||
compte = await self._users.get_by_id(revendique.user_id)
|
||||
if compte is None or not compte.is_active or compte.kind != AccountKind.HUMAIN.value:
|
||||
raise InvalidOrExpiredResetTokenError("Lien invalide ou expiré")
|
||||
|
||||
await self._users.update_password(
|
||||
revendique.user_id, await self._hasher.hash(new_password), must_change_password=False
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user