AOSSIE-Org / AOSSIE-Org/DebateAI
[BUG]: Unchecked type assertions in admin/RBAC middleware panic (500) instead of returning 401/403
- Ngôn ngữ chính
- TypeScript
- Star
- 84
- Fork
- 198
- Merge trung bình
- 2 ngày 19 giờ
- Pull request đã merge (30 ngày)
- 30
Mô tả
### 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
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Hướng nghiên cứu
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.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- go
- Lĩnh vực
- authentication, authorization, backend
- Loại issue
- Lỗi
- Độ khó
- 2/5
- Thời gian dự kiến
- 1-3 giờ
- Mức độ hoạt động
- Sôi nổi
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức phù hợp với người mới
- 82/100