veza/veza-backend-api/P1_RES_003_MUSTGET_FIX_REPORT.md
2025-12-12 21:34:34 -05:00

7.1 KiB

P1-RES-003 — PROTECTION c.MustGet() (Fail-Secure)

Date: 2025-12-12
Objectif: Remplacer tous les usages non protégés de c.MustGet("user_id") par un accès fail-secure pour éviter les panics


📋 RÉSUMÉ

Helper fail-secure créé : getUserID() remplace c.MustGet()
16 occurrences remplacées : Toutes les utilisations de c.MustGet("user_id") dans TrackHandler
Tests complets : 3 tests vérifient l'absence de panic et le retour 401
Compilation réussie : Aucune erreur de compilation


📁 FICHIERS MODIFIÉS

1. internal/core/track/handler.go

  • Helper getUserID() créé (lignes 91-110) : Accès sécurisé avec c.Get() au lieu de c.MustGet()
  • 16 handlers modifiés : Tous utilisent maintenant h.getUserID(c) au lieu de c.MustGet("user_id")
  • Standardisation : Toutes les réponses utilisent response.Unauthorized() (même pattern que le reste du fichier)

Handlers modifiés :

  1. UploadTrack (ligne 129)
  2. GetUploadStatus (ligne 244)
  3. InitiateChunkedUpload (ligne 298)
  4. UploadChunk (ligne 351)
  5. CompleteChunkedUpload (ligne 408)
  6. GetUploadQuota (lignes 597, 613)
  7. ResumeUpload (ligne 649)
  8. UpdateTrack (ligne 852)
  9. DeleteTrack (ligne 934)
  10. BatchDeleteTracks (ligne 999)
  11. BatchUpdateTracks (ligne 1055)
  12. LikeTrack (ligne 1109)
  13. UnlikeTrack (ligne 1142)
  14. CreateShare (ligne 1459)
  15. RevokeShare (ligne 1552)

2. internal/core/track/handler_mustget_test.go (nouveau)

  • TestTrackHandler_NoPanicWhenUserIDMissing : Teste 13 handlers pour vérifier l'absence de panic
  • TestTrackHandler_GetUserID_Returns401WhenMissing : Teste le helper directement
  • TestTrackHandler_GetUserID_Returns401WhenInvalidType : Teste la gestion des types invalides

🔍 IMPLÉMENTATION

Helper fail-secure

// getUserID récupère l'ID utilisateur du contexte de manière sécurisée (fail-secure)
// MOD-P1-RES-003: Remplace c.MustGet() pour éviter les panics
// Retourne false si user_id est absent ou invalide (répond déjà avec 401)
func (h *TrackHandler) getUserID(c *gin.Context) (uuid.UUID, bool) {
	userIDInterface, exists := c.Get("user_id")
	if !exists {
		response.Unauthorized(c, "unauthorized")
		return uuid.Nil, false
	}

	userID, ok := userIDInterface.(uuid.UUID)
	if !ok {
		response.Unauthorized(c, "unauthorized")
		return uuid.Nil, false
	}

	if userID == uuid.Nil {
		response.Unauthorized(c, "unauthorized")
		return uuid.Nil, false
	}

	return userID, true
}

Pattern de remplacement

Avant (peut panic) :

userID := c.MustGet("user_id").(uuid.UUID)
if userID == uuid.Nil {
	response.Unauthorized(c, "unauthorized")
	return
}

Après (fail-secure) :

// MOD-P1-RES-003: Utiliser helper fail-secure au lieu de c.MustGet()
userID, ok := h.getUserID(c)
if !ok {
	return // Erreur déjà envoyée par getUserID
}

🧪 PREUVES (TESTS)

Tests unitaires

go test ./internal/core/track -run TestTrackHandler_NoPanicWhenUserIDMissing -v -count=1

Résultat : Tous les tests passent (13/13)

Test 1 : Aucun handler ne panic si user_id absent

TestTrackHandler_NoPanicWhenUserIDMissing
  • Teste 13 handlers différents
  • Vérifie qu'aucun panic ne se produit
  • Vérifie que la réponse est 401 Unauthorized

Test 2 : Helper retourne 401 si user_id absent

TestTrackHandler_GetUserID_Returns401WhenMissing
  • Vérifie que getUserID() ne panic pas
  • Vérifie que ok est false
  • Vérifie que la réponse est 401

Test 3 : Helper gère les types invalides

TestTrackHandler_GetUserID_Returns401WhenInvalidType
  • Vérifie que getUserID() gère les types invalides (string au lieu de UUID)
  • Vérifie que la réponse est 401

Tests complets

go test ./internal/core/track -v -count=1

Résultat : Tous les tests passent (tests existants + nouveaux tests)


📊 COMPARAISON AVANT/APRÈS

Avant (Risque de panic)

userID := c.MustGet("user_id").(uuid.UUID)  // ❌ Panic si clé absente
if userID == uuid.Nil {
	response.Unauthorized(c, "unauthorized")
	return
}

Problèmes :

  • c.MustGet() peut panic si la clé n'existe pas
  • Type assertion peut panic si le type est incorrect
  • Pas de message d'erreur clair pour debugging

Après (Fail-secure)

userID, ok := h.getUserID(c)  // ✅ Ne panic jamais
if !ok {
	return // Erreur déjà envoyée par getUserID
}

Avantages :

  • Aucun panic possible
  • Gestion explicite des erreurs
  • Réponse structurée 401 si user_id absent/invalide
  • Code plus lisible et maintenable

VALIDATION

Compilation

go build ./internal/core/track/...

Résultat : Compilation réussie

Tests unitaires

go test ./internal/core/track -run TestTrackHandler_NoPanicWhenUserIDMissing -v

Résultat : Tous les tests passent (13/13)

Tests complets

go test ./internal/core/track -v -count=1

Résultat : Tous les tests passent

Vérification absence de c.MustGet()

grep -r "c.MustGet(" internal/core/track/

Résultat : Aucune occurrence de c.MustGet("user_id") restante


🎯 OBJECTIFS ATTEINTS

  • Toutes les occurrences remplacées : 16 handlers utilisent maintenant getUserID()
  • Aucun panic possible : Helper fail-secure avec gestion explicite des erreurs
  • Tests complets : 3 tests vérifient l'absence de panic et le comportement correct
  • Pattern standardisé : Toutes les réponses utilisent response.Unauthorized()
  • Aucun changement fonctionnel : Comportement identique, mais sans risque de panic

📋 COMMANDES DE VALIDATION

Compilation

go build ./internal/core/track/...

Tests spécifiques

go test ./internal/core/track -run TestTrackHandler_NoPanicWhenUserIDMissing -v -count=1
go test ./internal/core/track -run TestTrackHandler_GetUserID -v -count=1

Tests complets

go test ./internal/core/track -v -count=1

Vérification absence de c.MustGet()

grep -r "c.MustGet(" internal/core/track/

📝 DÉTAILS TECHNIQUES

Cas gérés par le helper

  1. Clé absente : c.Get("user_id") retourne exists = false

    • → Retourne uuid.Nil, false et répond avec 401
  2. Type invalide : userIDInterface.(uuid.UUID) échoue

    • → Retourne uuid.Nil, false et répond avec 401
  3. Valeur nil : userID == uuid.Nil

    • → Retourne uuid.Nil, false et répond avec 401
  4. Valeur valide : Toutes les vérifications passent

    • → Retourne userID, true

Handlers où userID n'est pas utilisé

Dans certains handlers (GetUploadStatus, UploadChunk), userID n'est récupéré que pour vérifier l'authentification mais n'est pas utilisé ensuite. Dans ces cas, on utilise _ :

if _, ok := h.getUserID(c); !ok {
	return // Erreur déjà envoyée par getUserID
}

Statut final : P1-RES-003 IMPLÉMENTÉ ET VALIDÉ