AOSSIE-Org / AOSSIE-Org/DebateAI
[BUG]: Unchecked type assertions in admin/RBAC middleware panic (500) instead of returning 401/403
- Langage dominant
- TypeScript
- Étoiles
- 84
- Forks
- 198
- Merge moyen
- 2 j 19 h
- PR mergées (30 j)
- 30
Description
### Bug Description
`backend/middlewares/rbac.go` uses several unchecked type assertions on JWT claims and context values. The regular `AuthMiddleware` (`backend/middlewares/auth.go:66`) handles this correctly with the safe two-value form:
```go
email, ok := claims["sub"].(string)
if !ok { /* return 401 */ }
```
But the admin path does not — it asserts directly:
- `rbac.go:166` — `email := claims["sub"].(string)` (in `AdminAuthMiddleware`)
- `rbac.go:199` — `role := adminRole.(string)` (in `RBACMiddleware`)
- `rbac.go:266` — `AdminEmail: adminEmail.(string)` and `AdminID: adminID.(primitive.ObjectID)` (in the admin action logger)
If `claims["sub"]` is missing (`nil`) or not a string — e.g. a validly-signed token issued by a different/legacy flow, or a token that passes signature validation but has unexpected claim types — line 166 panics instead of returning a clean auth error.
**Scope / severity (honest):** `cmd/server/main.go` uses `gin.Default()`, which includes `gin.Recovery()`, so the panic is caught and returned as **HTTP 500** — it does **not** crash the server, and it is **not** an auth bypass. But it's still a real bug:
- Admin endpoints return `500 Internal Server Error` on a malformed token instead of the clean `401`/`403` the normal auth path returns for the same situation.
- It's inconsistent with `AuthMiddleware`, which already handles this case gracefully — indicating an oversight, not intent.
- Returning 500 on auth failures is poor hygiene: it obscures the real cause, adds noise to error logs/alerts, and lets a malformed token cheaply generate 500s against admin routes.
**Fix:** mirror `AuthMiddleware` — use the `, ok` form for each assertion and return 401/403 (or skip logging) when it fails, instead of asserting directly.
### Steps to Reproduce
1. Obtain or craft a JWT signed with the correct secret but whose `sub` claim is absent or not a string (a token that passes signature validation but has unexpected claims).
2. Send it as `Authorization: Bearer ` to an admin-protected endpoint (guarded by `AdminAuthMiddleware`).
3. Observe the request panics at `rbac.go:166` and returns HTTP 500 (caught by gin.Recovery), instead of a clean 401/403.
4. Compare with a normal endpoint under `AuthMiddleware`, which returns a proper 401 ("Invalid token claims") for the equivalent case.
### Logs and Screenshots
Safe (auth.go:66):
email, ok := claims["sub"].(string)
if !ok { c.JSON(401, gin.H{"error": "Invalid token claims"}); c.Abort(); return }
Unchecked (rbac.go):
166: email := claims["sub"].(string)
199: role := adminRole.(string)
266: AdminEmail: adminEmail.(string), AdminID: adminID.(primitive.ObjectID),
### Environment Details
- File: backend/middlewares/rbac.go (AdminAuthMiddleware, RBACMiddleware, admin action logger)
- Reference (correct pattern): backend/middlewares/auth.go:66
- Backend: Go / Gin (gin.Default → gin.Recovery is active, so panics become 500s, not crashes)
- Branch: main
- Note: not a crash or auth bypass; a robustness/error-handling inconsistency that turns malformed-token cases into 500s on admin routes
### Impact
Low - Minor inconvenience
### Code of Conduct
- [x] I have joined the [Discord server](https://discord.gg/hjUhu33uAn) and will post updates there
- [x] I have searched existing issues to avoid duplicates
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Piste de recherche
Start with backend/middlewares/auth.go:66 to compare its safe claims handling, then inspect the three assertion sites in backend/middlewares/rbac.go. Check cmd/server/main.go to confirm the recovery behavior. Done means malformed admin claims produce the appropriate 401/403 response or skip logging without a panic or 500 response.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- go
- Domaine
- authentication, authorization, backend
- Type d'issue
- Bug
- Difficulté
- 2/5
- Temps estimé
- 1-3 heures
- Activité
- Active
- Clarté
- Clairement spécifiée
- Accessibilité débutants
- 82/100