Aller au contenu

PD-240 — Revue de code

1. session-invalidation.service.ts

Points positifs - Utilisation de SCAN + DEL (évite KEYS) et pagination par curseur. - Gestion explicite du cycle de vie Redis (init/quit) et possibilité d’injecter un client pour tests. - Journalisation du nombre de sessions invalidées.

Points négatifs / à améliorer - Important : l’échec d’initialisation Redis lève une exception générique sans code métier ; en contexte PD‑240, cela doit se mapper sur ERR‑240‑SESSION‑INVALIDATION‑FAILED au niveau service/filtre. - Mineur : absence de timeout explicite sur la connexion Redis (risque de blocage en cas de réseau instable). - Mineur : aucune métrique/trace sur la durée d’invalidation (utile pour auditabilité).

Verdict fichier : APPROUVÉ AVEC RÉSERVES

2. user-data-purge.service.ts

Points positifs - Purge effectuée dans une transaction ; réduit le risque d’état partiellement anonymisé. - Journalisation d’audit avec email hashé (pas d’exposition de PII en clair). - Retour d’un résultat {status, purgedAt} conforme au besoin d’observable.

Points négatifs / à améliorer - Important : les erreurs de purge lèvent des Error génériques, non mappées aux codes ERR‑240‑DELETE‑FAILED ; dépendance à un filtre global non visible ici. - Important : l’anonymisation ne couvre que User (email/name/avatar/status/preferences) ; aucune preuve explicite de purge des tables liées (exigence INV‑240‑05). Il faut une couverture RGPD plus large ou une preuve auditée. - Mineur : name/avatarUrl mis à null via cast as unknown as (signal de contournement de types).

Verdict fichier : APPROUVÉ AVEC RÉSERVES

3. delete-account.service.ts

Points positifs - Séquence respecte l’ordre contractuel (sessions → purge → Keycloak). - Utilisation de SCAN/DEL via SessionInvalidationService (pas d’opération Redis coûteuse). - Journalisation structurée des étapes, utile pour auditabilité.

Points négatifs / à améliorer - Critique : en cas d’échec purge RGPD ou suppression Keycloak, l’implémentation retourne ERR‑240‑DELETE‑FAILED avec état dégradé (sessions invalidées, compte non supprimé) — conforme à la spec v4, mais incompatible avec l’exigence « compte non supprimé » et la logique rollback si l’on exigeait un état atomique. Vérifier l’alignement strict avec la version de spec en vigueur. - Important : l’erreur est levée via InternalServerErrorException avec payload {error, message} ; si le filtre ne force pas le format exact contractuel {error, message}, risque de divergence (statusCode, timestamp ajoutés). - Mineur : aucune validation d’input (confirm) ici ; dépendance totale au controller/DTO.

Verdict fichier : APPROUVÉ AVEC RÉSERVES

4. delete-account.controller.ts

Points positifs - Guards appliqués (JWT + reauth) et filtre d’exception dédié. - Endpoint aligné sur le contrat REST (DELETE /user/account). - Journalisation structurée d’entrée/succès.

Points négatifs / à améliorer - Important : le DTO de confirmation est ignoré (@Body() _dto), ce qui masque la validation de confirm côté controller ; risque de contournement si le DTO/pipe n’est pas strictement appliqué. - Important : mapping Keycloak userId = user.sub supposé identique ; absence de résolution explicite d’ID Keycloak (risque d’incohérence si sub != keycloakId). - Mineur : ApiResponse 401 mélange « non authentifié » et « reauth invalide » sans code spécifique (risque d’ambiguïté pour le client).

Verdict fichier : APPROUVÉ AVEC RÉSERVES


Verdict consolidé

APPROUVÉ AVEC RÉSERVES

Motifs principaux : gestion d’erreurs métier non typée, validation de confirmation dépendante du DTO/pipe, couverture RGPD incomplète sur tables liées, et dépendance forte à un filtre global pour le format d’erreur contractuel.