Les problèmes de qualité levés par Claude #32

Open
opened 2026-01-14 15:42:14 +01:00 by eloi · 0 comments
Owner
Ce projet présente des défaut de conception et des manquement de qualités logicielle. Il n'utilise pas les règles de l'art des bibliothèques qu'il emploi. Identifie les problèmes et leur impact.  

Audit qualité logicielle - Sisyphe

Ce document identifie les défauts de conception et les écarts aux bonnes pratiques des bibliothèques utilisées (Flask, SQLAlchemy, WTForms).


Problèmes critiques (sécurité/autorisation)

1. Absence de contrôle d'accès basé sur les rôles

Fichier : sisyphe/summary.py:446-482

N'importe quel utilisateur connecté peut modifier les temps facturés. Pas de distinction admin/utilisateur pour les opérations sensibles.

Bonne pratique : Implémenter un système RBAC ou des décorateurs @admin_required.

Impact : Tout utilisateur peut manipuler les données financières.

2. Session trop longue

Fichier : sisyphe/__init__.py:121

app.permanent_session_lifetime = datetime.timedelta(days=365)

Bonne pratique : Sessions de 7-30 jours avec mécanisme de refresh.

Impact : Fenêtre d'attaque élargie pour le vol de session.

3. Coercition non sécurisée

Fichier : sisyphe/summary.py:451

project_id = int(key.replace("billed_", ""))

Bonne pratique : Utiliser la validation de formulaire ou try/except.

Impact : Un input malformé provoque un crash (DoS).


Problèmes d'architecture SQLAlchemy

4. Problème N+1 dans les agrégations

Fichier : sisyphe/models.py:458-503

for entry in time_entries:
    project_name = entry.project.name  # Lazy load à chaque itération
    username = entry.user.username

Bonne pratique : Eager loading avec selectinload() ou joinedload().

Impact : Dégradation de performance proportionnelle au volume de données.

5. Requêtes .all() sans limite

Fichiers : projects.py:27, summary.py:130, etc.

projects = db.session.scalars(
    select(Project).filter_by(is_archived=False).order_by(Project.created_at.desc())
).all()

Bonne pratique : Pagination avec .limit() et .offset().

Impact : Consommation mémoire excessive avec montée en charge.

6. Cascades non définies

Fichier : sisyphe/models.py:68-71

time_entries: Mapped[list["TimeEntry"]] = relationship(back_populates="user")

Bonne pratique : Définir cascade="all, delete-orphan" explicitement.

Impact : Risque d'enregistrements orphelins si un parent est supprimé.

7. Logique métier dans les modèles

Fichier : sisyphe/models.py:79-87

def get_weekly_target_for_date(self, target_date: dt.date) -> float:
    target = db.session.scalar(select(UserWeeklyTarget)...)

Bonne pratique : Requêtes dans une couche service ou repository.

Impact : Modèles difficiles à tester en isolation, couplage fort.


Problèmes WTForms

8. Validation hors du framework

Fichier : sisyphe/contracts.py:71-74

error = contract.validate_fields()
if error:
    flash(error)  # Message global, pas au niveau du champ

Bonne pratique : Validateurs WTForms au niveau des champs ou du formulaire.

Impact : UX dégradée, erreurs non associées aux champs concernés.

9. Conversion manuelle des types

Fichier : sisyphe/contracts.py:57-67

daily_rate=Decimal(form.daily_rate.data) if form.daily_rate.data else None

Bonne pratique : Créer un champ WTForms personnalisé qui retourne directement un Decimal.

Impact : Code verbeux et répétitif.


Problèmes de tests

10. Pas de tests d'autorisation

Les tests vérifient les fonctionnalités mais pas les frontières d'autorisation (ex: "un utilisateur ne peut pas modifier les données d'un autre").

Bonne pratique : Tests exhaustifs des contrôles d'accès pour chaque route.

Impact : Vulnérabilités de sécurité non détectées.

11. CSRF désactivé dans les tests

Fichier : tests/conftest.py:109

"WTF_CSRF_ENABLED": False,

Bonne pratique : Activer CSRF et tester avec des tokens valides.

Impact : Bugs CSRF non détectés par la suite de tests.


Problèmes de qualité de code

12. Duplication de logique d'agrégation

Fichier : sisyphe/summary.py:255-260, 349-354

Le même code de groupement est répété dans yearly() et custom() :

entries_by_project = {}
for entry in time_entries:
    project_name = entry.project.name
    if project_name not in entries_by_project:
        entries_by_project[project_name] = []
    entries_by_project[project_name].append(entry)

Bonne pratique : Extraire dans une fonction utilitaire.

Impact : Maintenance dupliquée, risque de divergence.

13. Fonctions views trop longues

Fichier : sisyphe/summary.py:108-220

monthly() fait 112 lignes avec logique complexe de groupement, calcul de temps facturés et statistiques.

Bonne pratique : Découper en fonctions plus petites et testables.

Impact : Difficile à comprendre et à tester.

14. Gestion de timezone incohérente

Fichier : sisyphe/models.py

Mélange de datetimes timezone-aware (datetime.now(UTC)) et de champs date sans timezone.

Bonne pratique : Utiliser systématiquement des datetimes UTC timezone-aware.

Impact : Bugs potentiels liés aux changements d'heure.


Tableau récapitulatif

Catégorie Sévérité Problèmes Impact principal
Autorisation Haute #1 Manipulation de données financières
Sécurité Haute #2, #3 Vol de session, crash applicatif
Performance SQLAlchemy Moyenne #4, #5 Lenteur avec montée en charge
Architecture SQLAlchemy Moyenne #6, #7 Orphelins, couplage fort
WTForms Basse #8, #9 UX dégradée, verbosité
Tests Moyenne #10, #11 Vulnérabilités non détectées
Qualité de code Basse #12, #13, #14 Dette technique

Priorités recommandées

  1. Immédiat : Implémenter RBAC pour les opérations sensibles (#1)
  2. Court terme : Réduire durée de session (#2), sécuriser les conversions (#3)
  3. Moyen terme : Corriger N+1 (#4), ajouter pagination (#5), tests d'autorisation (#10)
  4. Long terme : Refactoring qualité de code (#12, #13), cascades (#6)
``` Ce projet présente des défaut de conception et des manquement de qualités logicielle. Il n'utilise pas les règles de l'art des bibliothèques qu'il emploi. Identifie les problèmes et leur impact. ``` # Audit qualité logicielle - Sisyphe Ce document identifie les défauts de conception et les écarts aux bonnes pratiques des bibliothèques utilisées (Flask, SQLAlchemy, WTForms). --- ## Problèmes critiques (sécurité/autorisation) ### 1. Absence de contrôle d'accès basé sur les rôles **Fichier** : `sisyphe/summary.py:446-482` N'importe quel utilisateur connecté peut modifier les temps facturés. Pas de distinction admin/utilisateur pour les opérations sensibles. **Bonne pratique** : Implémenter un système RBAC ou des décorateurs `@admin_required`. **Impact** : Tout utilisateur peut manipuler les données financières. ### 2. Session trop longue **Fichier** : `sisyphe/__init__.py:121` ```python app.permanent_session_lifetime = datetime.timedelta(days=365) ``` **Bonne pratique** : Sessions de 7-30 jours avec mécanisme de refresh. **Impact** : Fenêtre d'attaque élargie pour le vol de session. ### 3. Coercition non sécurisée **Fichier** : `sisyphe/summary.py:451` ```python project_id = int(key.replace("billed_", "")) ``` **Bonne pratique** : Utiliser la validation de formulaire ou `try/except`. **Impact** : Un input malformé provoque un crash (DoS). --- ## Problèmes d'architecture SQLAlchemy ### 4. Problème N+1 dans les agrégations **Fichier** : `sisyphe/models.py:458-503` ```python for entry in time_entries: project_name = entry.project.name # Lazy load à chaque itération username = entry.user.username ``` **Bonne pratique** : Eager loading avec `selectinload()` ou `joinedload()`. **Impact** : Dégradation de performance proportionnelle au volume de données. ### 5. Requêtes `.all()` sans limite **Fichiers** : `projects.py:27`, `summary.py:130`, etc. ```python projects = db.session.scalars( select(Project).filter_by(is_archived=False).order_by(Project.created_at.desc()) ).all() ``` **Bonne pratique** : Pagination avec `.limit()` et `.offset()`. **Impact** : Consommation mémoire excessive avec montée en charge. ### 6. Cascades non définies **Fichier** : `sisyphe/models.py:68-71` ```python time_entries: Mapped[list["TimeEntry"]] = relationship(back_populates="user") ``` **Bonne pratique** : Définir `cascade="all, delete-orphan"` explicitement. **Impact** : Risque d'enregistrements orphelins si un parent est supprimé. ### 7. Logique métier dans les modèles **Fichier** : `sisyphe/models.py:79-87` ```python def get_weekly_target_for_date(self, target_date: dt.date) -> float: target = db.session.scalar(select(UserWeeklyTarget)...) ``` **Bonne pratique** : Requêtes dans une couche service ou repository. **Impact** : Modèles difficiles à tester en isolation, couplage fort. --- ## Problèmes WTForms ### 8. Validation hors du framework **Fichier** : `sisyphe/contracts.py:71-74` ```python error = contract.validate_fields() if error: flash(error) # Message global, pas au niveau du champ ``` **Bonne pratique** : Validateurs WTForms au niveau des champs ou du formulaire. **Impact** : UX dégradée, erreurs non associées aux champs concernés. ### 9. Conversion manuelle des types **Fichier** : `sisyphe/contracts.py:57-67` ```python daily_rate=Decimal(form.daily_rate.data) if form.daily_rate.data else None ``` **Bonne pratique** : Créer un champ WTForms personnalisé qui retourne directement un `Decimal`. **Impact** : Code verbeux et répétitif. --- ## Problèmes de tests ### 10. Pas de tests d'autorisation Les tests vérifient les fonctionnalités mais pas les frontières d'autorisation (ex: "un utilisateur ne peut pas modifier les données d'un autre"). **Bonne pratique** : Tests exhaustifs des contrôles d'accès pour chaque route. **Impact** : Vulnérabilités de sécurité non détectées. ### 11. CSRF désactivé dans les tests **Fichier** : `tests/conftest.py:109` ```python "WTF_CSRF_ENABLED": False, ``` **Bonne pratique** : Activer CSRF et tester avec des tokens valides. **Impact** : Bugs CSRF non détectés par la suite de tests. --- ## Problèmes de qualité de code ### 12. Duplication de logique d'agrégation **Fichier** : `sisyphe/summary.py:255-260, 349-354` Le même code de groupement est répété dans `yearly()` et `custom()` : ```python entries_by_project = {} for entry in time_entries: project_name = entry.project.name if project_name not in entries_by_project: entries_by_project[project_name] = [] entries_by_project[project_name].append(entry) ``` **Bonne pratique** : Extraire dans une fonction utilitaire. **Impact** : Maintenance dupliquée, risque de divergence. ### 13. Fonctions views trop longues **Fichier** : `sisyphe/summary.py:108-220` `monthly()` fait 112 lignes avec logique complexe de groupement, calcul de temps facturés et statistiques. **Bonne pratique** : Découper en fonctions plus petites et testables. **Impact** : Difficile à comprendre et à tester. ### 14. Gestion de timezone incohérente **Fichier** : `sisyphe/models.py` Mélange de datetimes timezone-aware (`datetime.now(UTC)`) et de champs `date` sans timezone. **Bonne pratique** : Utiliser systématiquement des datetimes UTC timezone-aware. **Impact** : Bugs potentiels liés aux changements d'heure. --- ## Tableau récapitulatif | Catégorie | Sévérité | Problèmes | Impact principal | |-----------|----------|-----------|------------------| | Autorisation | **Haute** | #1 | Manipulation de données financières | | Sécurité | **Haute** | #2, #3 | Vol de session, crash applicatif | | Performance SQLAlchemy | **Moyenne** | #4, #5 | Lenteur avec montée en charge | | Architecture SQLAlchemy | **Moyenne** | #6, #7 | Orphelins, couplage fort | | WTForms | **Basse** | #8, #9 | UX dégradée, verbosité | | Tests | **Moyenne** | #10, #11 | Vulnérabilités non détectées | | Qualité de code | **Basse** | #12, #13, #14 | Dette technique | --- ## Priorités recommandées 1. **Immédiat** : Implémenter RBAC pour les opérations sensibles (#1) 2. **Court terme** : Réduire durée de session (#2), sécuriser les conversions (#3) 3. **Moyen terme** : Corriger N+1 (#4), ajouter pagination (#5), tests d'autorisation (#10) 4. **Long terme** : Refactoring qualité de code (#12, #13), cascades (#6)
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
yaal/sisyphe#32
No description provided.