276 lines
7.1 KiB
Markdown
276 lines
7.1 KiB
Markdown
# ✅ 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
|
|
|
|
```go
|
|
// 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) :
|
|
```go
|
|
userID := c.MustGet("user_id").(uuid.UUID)
|
|
if userID == uuid.Nil {
|
|
response.Unauthorized(c, "unauthorized")
|
|
return
|
|
}
|
|
```
|
|
|
|
**Après** (fail-secure) :
|
|
```go
|
|
// 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
|
|
|
|
```bash
|
|
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
|
|
```go
|
|
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
|
|
```go
|
|
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
|
|
```go
|
|
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
|
|
|
|
```bash
|
|
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)
|
|
|
|
```go
|
|
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)
|
|
|
|
```go
|
|
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
|
|
|
|
```bash
|
|
go build ./internal/core/track/...
|
|
```
|
|
|
|
**Résultat** : ✅ **Compilation réussie**
|
|
|
|
### Tests unitaires
|
|
|
|
```bash
|
|
go test ./internal/core/track -run TestTrackHandler_NoPanicWhenUserIDMissing -v
|
|
```
|
|
|
|
**Résultat** : ✅ **Tous les tests passent (13/13)**
|
|
|
|
### Tests complets
|
|
|
|
```bash
|
|
go test ./internal/core/track -v -count=1
|
|
```
|
|
|
|
**Résultat** : ✅ **Tous les tests passent**
|
|
|
|
### Vérification absence de c.MustGet()
|
|
|
|
```bash
|
|
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
|
|
```bash
|
|
go build ./internal/core/track/...
|
|
```
|
|
|
|
### Tests spécifiques
|
|
```bash
|
|
go test ./internal/core/track -run TestTrackHandler_NoPanicWhenUserIDMissing -v -count=1
|
|
go test ./internal/core/track -run TestTrackHandler_GetUserID -v -count=1
|
|
```
|
|
|
|
### Tests complets
|
|
```bash
|
|
go test ./internal/core/track -v -count=1
|
|
```
|
|
|
|
### Vérification absence de c.MustGet()
|
|
```bash
|
|
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 `_` :
|
|
|
|
```go
|
|
if _, ok := h.getUserID(c); !ok {
|
|
return // Erreur déjà envoyée par getUserID
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
**Statut final** : ✅ **P1-RES-003 IMPLÉMENTÉ ET VALIDÉ**
|