Refactoring - Bonne version améliorée

This commit is contained in:
dahoud
2026-02-05 14:14:45 +00:00
parent a515963a4a
commit dd4dbe111e
56 changed files with 4274 additions and 2142 deletions

View File

@@ -0,0 +1,198 @@
# Audit intégral Frontend (Flutter) & Backend (Quarkus)
**Date** : 4 février 2026
**Périmètre** : `afterwork` (Flutter), `mic-after-work-server-impl-quarkus-main` (Quarkus)
**Références** : bonnes pratiques Quarkus REST, Flutter clean architecture, REST/JWT, WebSocket, Kafka (recherches web et documentation officielle).
---
## 1. Résumé exécutif
| Domaine | État global | Points critiques |
|--------|-------------|------------------|
| **Sécurité API (auth/authz)** | Critique | Aucune vérification JWT ; userId pris de lURL/body sans preuve didentité |
| **Couches backend** | Partiel | Resource accède parfois au repository ; validation incohérente (manuel vs Bean Validation) |
| **Gestion derreurs backend** | Partiel | Réponse derreur JSON construite à la main (risque dinjection) ; exceptions métier non gérées |
| **Frontend auth** | Critique | Aucun en-tête `Authorization` sur les requêtes API |
| **Frontend architecture** | Correct | data/domain/presentation présents ; pas de couche use-case systématique |
| **WebSocket** | Correct | Heartbeat + reconnexion présents ; pas de backoff exponentiel côté Flutter |
---
## 2. Backend (Quarkus) Analyse détaillée
### 2.1 Architecture des couches
**Recommandation (bonnes pratiques Quarkus)** :
Resource (REST, DTO) → Service (métier) → Repository (persistance). La resource ne doit pas appeler le repository pour de la logique métier.
**Constat :**
- **MessageResource** (lignes 98104, 168174) : appelle directement `usersRepository.findById(userId)` pour vérifier lexistence de lutilisateur, au lieu de déléguer au service (ex. `messageService.getUserConversations(userId)` qui lèverait `UserNotFoundException`).
- **MessageResource** injecte `UsersRepository` en plus de `MessageService` → mélange des responsabilités et duplication de la règle “utilisateur doit exister”.
**Recommandation :** Déplacer la résolution/utilisateur dans `MessageService` et faire lever `UserNotFoundException` ; supprimer linjection de `UsersRepository` dans `MessageResource`.
---
### 2.2 Validation des entrées
**Recommandation :** Utiliser Bean Validation (Hibernate Validator) sur les DTO avec `@Valid` sur les paramètres des endpoints. Éviter la validation manuelle dans la resource.
**Constat :**
- **SendMessageRequestDTO** : pas dannotations `@NotNull`, `@NotBlank` ; validation manuelle via `isValid()`.
- **MessageResource.sendMessage** : pas de `@Valid` ; utilise `request.isValid()` et retourne 400 manuellement.
- Autres ressources (UsersResource, EstablishmentResource, FriendshipResource, etc.) : utilisent correctement `@Valid` et DTO avec contraintes.
**Recommandation :** Ajouter sur `SendMessageRequestDTO` les annotations (`@NotNull` pour senderId, recipientId, `@NotBlank` pour content, etc.) et appeler lendpoint avec `@Valid SendMessageRequestDTO request`. Supprimer `isValid()` et le bloc manuel 400.
---
### 2.3 Authentification et autorisation
**Recommandation (OWASP / JWT)** :
Chaque endpoint protégé doit valider un JWT (ou session) et dériver lidentité du token. Les paramètres comme `userId` dans lURL ne doivent pas être la seule source de vérité : vérifier que le sujet du token correspond à la ressource demandée.
**Constat :**
- Aucun usage de `@RolesAllowed`, `@PermitAll`, ni de filtre/filtre JWT dans le projet.
- Les endpoints utilisent `userId` en `@PathParam` (ex. `/notifications/user/{userId}`, `/messages/conversations/{userId}`) ou dans le body (ex. `SendMessageRequestDTO.senderId`) sans aucune preuve que lappelant est cet utilisateur.
- Le commentaire dans `NotificationResource` indique : *“En production, le userId doit être dérivé du contexte d'authentification (JWT/session), pas de l'URL.”* → non implémenté.
**Impact :** Un attaquant peut lire/modifier les données dun autre utilisateur en devinant ou en énumérant des UUID.
**Recommandation :** Introduire lauthentification JWT (ex. `quarkus-oidc` ou filtre custom), extraire le `userId` (ou subject) du token, et pour chaque endpoint : soit utiliser ce `userId` comme source de vérité, soit vérifier que le `userId` en path/body est égal au sujet du token (pour les rôles appropriés).
---
### 2.4 Gestion globale des exceptions
**Recommandation :** Un seul point de sortie pour les erreurs (ExceptionMapper), réponses en JSON structuré (ex. `{"error": "..."}`) avec échappement correct. Gérer toutes les exceptions métier connues.
**Constat :**
- **GlobalExceptionHandler** : gère `BadRequestException`, `UserNotFoundException`, `EventNotFoundException`, `NotFoundException`, `UnauthorizedException`, `ServerException`, `RuntimeException`, et cas par défaut.
- **FriendshipNotFoundException** et **EstablishmentHasDependenciesException** ne sont pas gérées explicitement → elles tombent dans `RuntimeException` ou “Unexpected error”, avec un message potentiellement générique ou une stack trace.
- **buildResponse** (ligne 6265) :
`entity("{\"error\":\"" + message + "\"}")`
Concaténation directe de `message` dans le JSON. Si `message` contient `"` ou `\`, le JSON est mal formé et peut poser des risques (injection / parsing côté client). Il faut sérialiser le message en JSON (ex. via Jackson/JSON-B) au lieu de concaténer une chaîne.
**Recommandation :**
1) Ajouter des branches pour `FriendshipNotFoundException` (ex. 404) et `EstablishmentHasDependenciesException` (ex. 409 Conflict).
2) Remplacer la concaténation par un DTO derreur sérialisé (ex. `Map.of("error", message)` ou classe dédiée) avec le moteur JSON du framework.
---
### 2.5 Ressources qui gèrent les erreurs en local
**Recommandation :** La resource ne doit pas faire de try/catch générique qui transforme tout en 500. Elle doit déléguer au service ; les exceptions métier doivent être mappées par le GlobalExceptionHandler.
**Constat :**
- **MessageResource** : plusieurs méthodes avec `try { ... } catch (Exception e) { return 500 ... }`. Les exceptions métier (ex. utilisateur inexistant, conversation inexistante) ne sont pas levées sous forme dexceptions typées ; elles sont noyées dans un message générique 500.
**Recommandation :** Faire lever par le service des exceptions métier (ex. `UserNotFoundException`, `NotFoundException`) et supprimer les try/catch larges dans la resource ; laisser le GlobalExceptionHandler produire 404/400/500 de façon cohérente.
---
### 2.6 Kafka (déjà traité)
- Tuning prod (`max.poll.interval.ms`, `max.poll.records`, `session.timeout.ms`) déjà ajouté dans `application-prod.properties`.
- Bonnes pratiques SmallRye : en cas déchec critique après consommation, envisager `message.nack()` et stratégie de commit manuel si nécessaire (au-delà du scope de cet audit).
---
## 3. Frontend (Flutter) Analyse détaillée
### 3.1 Structure (clean architecture)
**Recommandation :** Séparation nette data / domain / presentation ; repositories en abstraction dans domain ; use cases optionnels mais utiles pour une logique métier réutilisable.
**Constat :**
- Présence de `data/` (datasources, models, repositories impl, services), `domain/` (entities, repositories abstraits, usecases partiels), `presentation/` (screens, state_management avec BLoC).
- Les datasources sont bien séparés ; les repositories implémentent les contrats du domain. Use cases présents seulement pour une partie des flux (ex. `get_user`).
**Verdict :** Conforme à une clean architecture légère. On peut étendre progressivement les use cases pour les flux critiques.
---
### 3.2 Appels API et authentification
**Recommandation :** Toute requête vers une API protégée doit envoyer le token (ex. `Authorization: Bearer <token>`). Le token doit être lu depuis un stockage sécurisé et rafraîchi si nécessaire.
**Constat :**
- Aucun datasource (user, notification, chat, event, social, reservation, establishment, etc.) najoute den-tête `Authorization` ou `Bearer`.
- Les headers utilisés sont principalement `Content-Type` et `Accept`. Aucune utilisation de `SecureStorage` (ou équivalent) pour récupérer un token et lattacher aux requêtes.
- LAPI backend nexige aujourdhui pas de JWT ; en revanche, dès que lauth sera activée côté backend, tous les appels devront envoyer le token.
**Recommandation :**
1) Créer un client HTTP unique (wrapper ou interceptor) qui récupère le token (ex. depuis `SecureStorage`) et ajoute `Authorization: Bearer <token>` à chaque requête.
2) Utiliser ce client dans tous les datasources au lieu dutiliser `http.Client` brut sans headers dauth.
3) Gérer le cas “token absent ou expiré” (401) : redirection vers login ou refresh.
---
### 3.3 WebSocket (notifications et chat)
**Recommandation (bonnes pratiques WebSocket)** : Heartbeat régulier, reconnexion avec backoff exponentiel, file dattente des messages en cas de déconnexion si besoin.
**Constat :**
- **RealtimeNotificationService** et **ChatWebSocketService** :
- Connexion avec `WebSocketChannel.connect`.
- Heartbeat toutes les 30 s (`_heartbeatInterval`).
- Reconnexion avec délai fixe (`_initialReconnectDelay = 5 s`) et plafond de tentatives (`_maxReconnectAttempts = 5`).
- Pas de backoff exponentiel (délai constant entre les tentatives). Pour réduire la charge serveur en cas de panne, un backoff exponentiel est préférable.
**Recommandation :** Conserver le heartbeat et la reconnexion ; ajouter un backoff exponentiel (ex. 2s, 4s, 8s, 16s, 30s) pour les tentatives de reconnexion, avec un plafond (ex. 30 s).
---
### 3.4 Gestion des erreurs et parsing
- Les datasources gèrent les timeouts, `SocketException`, et codes HTTP (401, 404, etc.) et lèvent des exceptions métier (ex. `ServerException`, `UnauthorizedException`). Cest cohérent.
- Vérifier que partout où lon parse le body derreur, on utilise une clé unique (ex. `error` ou `message`) alignée avec le backend. Après correction du backend (réponse derreur en JSON structuré), adapter si nécessaire le parsing côté Flutter pour lire `error` ou `message`.
---
## 4. Tableau de synthèse des écarts
| # | Composant | Écart | Sévérité | Action recommandée |
|---|-----------|--------|----------|---------------------|
| 1 | Backend | Aucune auth JWT ; userId pris de lURL/body sans preuve | Critique | Introduire JWT et dériver userId du token |
| 2 | Frontend | Aucun en-tête Authorization sur les requêtes API | Critique | Client HTTP centralisé avec Bearer token |
| 3 | Backend | MessageResource : accès direct au repository + validation manuelle | Moyen | Déléguer au service ; Bean Validation sur SendMessageRequestDTO |
| 4 | Backend | buildResponse : concaténation JSON pour le message derreur | Moyen | Utiliser un DTO/Map sérialisé en JSON |
| 5 | Backend | FriendshipNotFoundException, EstablishmentHasDependenciesException non gérées dans GlobalExceptionHandler | Moyen | Ajouter les branches et codes HTTP appropriés |
| 6 | Backend | MessageResource : try/catch générique qui masque les exceptions métier | Moyen | Lever des exceptions typées et laisser le handler global gérer |
| 7 | Frontend | Reconnexion WebSocket avec délai fixe | Faible | Implémenter backoff exponentiel |
---
## 5. Bonnes pratiques croisées (références)
- **Quarkus REST** : Resource → Service → Repository ; DTO + `@Valid` ; ExceptionMapper unique ; pas de logique métier dans la resource.
- **Sécurité REST/JWT** : Vérifier le token sur chaque requête ; ne pas faire confiance au userId passé par le client pour lautorisation.
- **Flutter** : Clean architecture avec repositories abstraits ; couche data qui envoie toujours lauth (client commun avec token).
- **WebSocket** : Heartbeat + reconnexion avec backoff exponentiel pour limiter la charge et les reconnexions agressives.
---
## 6. Conclusion
Les points les plus critiques concernent **lauthentification et lautorisation** : côté backend, aucun contrôle sur lidentité de lappelant ; côté frontend, aucun token nest envoyé. La cohérence des couches (resource sans accès direct au repository pour la logique métier), la validation (Bean Validation partout, y compris chat), et la gestion derreurs (réponse JSON sûre, exceptions métier gérées centralement) sont à renforcer pour aligner le projet sur les bonnes pratiques et sécuriser la production.
---
## 7. Corrections appliquées (suite à l'audit)
- **GlobalExceptionHandler** : Réponse d'erreur en JSON via ObjectMapper ; prise en charge de `FriendshipNotFoundException` (404) et `EstablishmentHasDependenciesException` (409).
- **SendMessageRequestDTO** : Bean Validation ; suppression de `isValid()`.
- **MessageResource** : `@Valid`, `UsersService` au lieu de `UsersRepository`, suppression des try/catch locaux.
- **MessageService** : `NotFoundException` si conversation ou message absent.
- **JWT** : `JwtService`, token au login (HS256), `UserAuthenticateResponseDTO.token`, config `afterwork.jwt.secret`.
- **Frontend** : `SecureStorage.saveAuthToken`/`getAuthToken`, `ApiClient` (Authorization Bearer), tous datasources + FriendsRepositoryImpl ; sauvegarde du token à l'authentification.
- **WebSocket** : Backoff exponentiel (2^attempt s, max 30 s) dans ChatWebSocketService et RealtimeNotificationService.