Skip to content

Commit 6d356b0

Browse files
committed
fix: resolve 15 backend bugs across security, logic, and code quality
Critical fixes: - Fix scoreboard export producing wrong rankings (missing sort) - Auth middleware now rejects banned/deleted users via Redis-cached DB status check, preventing stale JWT abuse for up to 7 days - Practice mode now excludes active contest challenges to prevent leaking live challenge data - Fix IsChallengeVisible returning stale err variable Medium fixes: - SearchChallenges now enforces contest visibility (was leaking active contest challenges via search endpoint) - Fix CSV injection in scoreboard export (escape team name quotes) - Fix solo leaderboard including users with 0 distinct solves (len(subs) → len(seen)) - Contest analytics now validates collaborator_contest_id to prevent cross-contest unauthorized queries - Fix AdjustUserScoreRequest.Delta binding:required rejecting 0 with wrong error message Low/cleanup fixes: - Replace custom parseInt with strconv.Atoi (overflow protection) - Inline all dead oid/objID variable aliases from MongoDB migration across contest_admin_service, hint_service, team_service, admin_user_handler - Handle ignored encoder.Encode errors in export and account handlers - Add 10s timeout to webhook HTTP client to prevent goroutine leaks
1 parent 236062d commit 6d356b0

14 files changed

Lines changed: 176 additions & 78 deletions

backend/internal/handlers/account_handler.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,5 +177,7 @@ func (h *AccountHandler) ExportAccountData(c *gin.Context) {
177177

178178
encoder := json.NewEncoder(c.Writer)
179179
encoder.SetIndent("", " ")
180-
encoder.Encode(export)
180+
if err := encoder.Encode(export); err != nil {
181+
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to encode export data"})
182+
}
181183
}

backend/internal/handlers/admin_user_handler.go

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,7 @@ type UpdateUserStatusRequest struct {
177177
// @Router /admin/users/{id}/status [put]
178178
func (h *AdminUserHandler) UpdateUserStatus(c *gin.Context) {
179179
id := c.Param("id")
180-
objID := id
181-
if objID == "" {
180+
if id == "" {
182181
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid user ID"})
183182
return
184183
}
@@ -192,7 +191,7 @@ func (h *AdminUserHandler) UpdateUserStatus(c *gin.Context) {
192191
return
193192
}
194193
update := map[string]interface{}{"status": req.Status, "ban_reason": req.BanReason}
195-
err := h.userRepo.UpdateFields(objID, update)
194+
err := h.userRepo.UpdateFields(id, update)
196195
if err != nil {
197196
utils.RespondWithError(c, http.StatusInternalServerError, err.Error(), err)
198197
return
@@ -218,8 +217,7 @@ type UpdateUserRoleRequest struct {
218217
// @Router /admin/users/{id}/role [put]
219218
func (h *AdminUserHandler) UpdateUserRole(c *gin.Context) {
220219
id := c.Param("id")
221-
objID := id
222-
if objID == "" {
220+
if id == "" {
223221
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid user ID"})
224222
return
225223
}
@@ -240,7 +238,7 @@ func (h *AdminUserHandler) UpdateUserRole(c *gin.Context) {
240238
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid role. Must be 'admin' or 'user'"})
241239
return
242240
}
243-
err := h.userRepo.UpdateFields(objID, map[string]interface{}{"role": req.Role})
241+
err := h.userRepo.UpdateFields(id, map[string]interface{}{"role": req.Role})
244242
if err != nil {
245243
utils.RespondWithError(c, http.StatusInternalServerError, err.Error(), err)
246244
return
@@ -250,7 +248,7 @@ func (h *AdminUserHandler) UpdateUserRole(c *gin.Context) {
250248

251249
// AdjustUserScoreRequest represents a manual user score adjustment
252250
type AdjustUserScoreRequest struct {
253-
Delta int `json:"delta" binding:"required"`
251+
Delta int `json:"delta"`
254252
Reason string `json:"reason"`
255253
}
256254

@@ -333,8 +331,7 @@ func (h *AdminUserHandler) AdjustUserScore(c *gin.Context) {
333331
// @Router /admin/users/{id} [delete]
334332
func (h *AdminUserHandler) DeleteUser(c *gin.Context) {
335333
userID := c.Param("id")
336-
objID := userID
337-
if objID == "" {
334+
if userID == "" {
338335
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid user ID"})
339336
return
340337
}
@@ -364,7 +361,7 @@ func (h *AdminUserHandler) DeleteUser(c *gin.Context) {
364361
// physically removing the record. This preserves data integrity for historical records
365362
// like submissions and team memberships. The user will still appear in total counts.
366363
// For a full purge, implement data cleanup in related tables (submissions, teams, etc.)
367-
err = h.userRepo.UpdateFields(objID, map[string]interface{}{"status": "deleted"})
364+
err = h.userRepo.UpdateFields(userID, map[string]interface{}{"status": "deleted"})
368365
if err != nil {
369366
utils.RespondWithError(c, http.StatusInternalServerError, err.Error(), err)
370367
return

backend/internal/handlers/challenge_handler.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,17 +387,39 @@ func (h *ChallengeHandler) SearchChallenges(c *gin.Context) {
387387
return
388388
}
389389

390-
// Determine current user
390+
// Determine current user and their team
391391
var userID string
392+
var teamID *string
392393
if userIDStr, exists := c.Get("user_id"); exists {
393394
userID = userIDStr.(string)
395+
if h.teamRepo != nil && userID != "" {
396+
if team, err := h.teamRepo.FindTeamByMemberID(userID); err == nil && team != nil {
397+
teamID = &team.ID
398+
}
399+
}
400+
}
401+
402+
// Build a set of visible challenge IDs to enforce contest visibility
403+
visibleSet := make(map[string]bool)
404+
hasActiveContest := false
405+
if h.contestAdminService != nil {
406+
if visibleIDs, err := h.contestAdminService.GetVisibleChallengeIDs(time.Now(), teamID); err == nil && len(visibleIDs) > 0 {
407+
hasActiveContest = true
408+
for _, id := range visibleIDs {
409+
visibleSet[id] = true
410+
}
411+
}
394412
}
395413

396414
var result []ChallengePublicResponse
397415
for _, ch := range challenges {
398416
if !ch.IsPublished {
399417
continue
400418
}
419+
// If there's an active contest, only show challenges that are visible to this user
420+
if hasActiveContest && !visibleSet[ch.ID] {
421+
continue
422+
}
401423
isSolved := false
402424
if h.submissionRepo != nil && userID != "" {
403425
if sub, _ := h.submissionRepo.FindByChallengeAndUser(ch.ID, userID); sub != nil {

backend/internal/handlers/contest_analytics_handler.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ type CategoryAnalytics struct {
8080
func (h *ContestAnalyticsHandler) GetContestAnalytics(c *gin.Context) {
8181
contestID := c.Param("contest_id")
8282

83+
// Validate that the collaborator is authorized for this specific contest
84+
if collabContestID, exists := c.Get("collaborator_contest_id"); exists {
85+
if collabContestID.(string) != contestID {
86+
c.JSON(http.StatusForbidden, gin.H{"error": "You are not a collaborator for this contest"})
87+
return
88+
}
89+
}
90+
8391
// Get registered teams count
8492
teamIDs, _ := h.teamContestRegistrationRepo.GetContestTeams(contestID)
8593
totalTeams := len(teamIDs)
@@ -198,6 +206,14 @@ func (h *ContestAnalyticsHandler) GetContestAnalytics(c *gin.Context) {
198206
func (h *ContestAnalyticsHandler) GetContestParticipantStats(c *gin.Context) {
199207
contestID := c.Param("contest_id")
200208

209+
// Validate that the collaborator is authorized for this specific contest
210+
if collabContestID, exists := c.Get("collaborator_contest_id"); exists {
211+
if collabContestID.(string) != contestID {
212+
c.JSON(http.StatusForbidden, gin.H{"error": "You are not a collaborator for this contest"})
213+
return
214+
}
215+
}
216+
201217
teamIDs, _ := h.teamContestRegistrationRepo.GetContestTeams(contestID)
202218

203219
type TeamStat struct {

backend/internal/handlers/export_handler.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"net/http"
7+
"sort"
78
"strings"
89

910
"github.com/Uttam-Mahata/RootAccess/backend/internal/repositories"
@@ -128,7 +129,9 @@ func (h *ExportHandler) ExportScoreboardJSON(c *gin.Context) {
128129

129130
encoder := json.NewEncoder(c.Writer)
130131
encoder.SetIndent("", " ")
131-
encoder.Encode(entries)
132+
if err := encoder.Encode(entries); err != nil {
133+
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to encode scoreboard"})
134+
}
132135
}
133136

134137
// ExportScoreboardCSV exports the scoreboard as downloadable CSV
@@ -147,7 +150,9 @@ func (h *ExportHandler) ExportScoreboardCSV(c *gin.Context) {
147150
var sb strings.Builder
148151
sb.WriteString("Rank,Team Name,Score,Solve Count,Member Count\n")
149152
for _, e := range entries {
150-
sb.WriteString(fmt.Sprintf("%d,\"%s\",%d,%d,%d\n", e.Rank, e.TeamName, e.Score, e.SolveCount, e.MemberCount))
153+
// Escape double quotes in team name to prevent CSV injection
154+
escapedName := strings.ReplaceAll(e.TeamName, "\"", "\"\"")
155+
sb.WriteString(fmt.Sprintf("%d,\"%s\",%d,%d,%d\n", e.Rank, escapedName, e.Score, e.SolveCount, e.MemberCount))
151156
}
152157
c.String(http.StatusOK, sb.String())
153158
}
@@ -183,7 +188,11 @@ func (h *ExportHandler) buildScoreboardEntries() []ScoreboardExportEntry {
183188
}
184189

185190
// Sort by score descending
186-
var result []ScoreboardExportEntry
191+
sort.Slice(entries, func(i, j int) bool {
192+
return entries[i].score > entries[j].score
193+
})
194+
195+
result := make([]ScoreboardExportEntry, 0, len(entries))
187196
for i, e := range entries {
188197
result = append(result, ScoreboardExportEntry{
189198
Rank: i + 1,
@@ -194,8 +203,5 @@ func (h *ExportHandler) buildScoreboardEntries() []ScoreboardExportEntry {
194203
})
195204
}
196205

197-
if result == nil {
198-
result = []ScoreboardExportEntry{}
199-
}
200206
return result
201207
}

backend/internal/handlers/practice_handler.go

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package handlers
22

33
import (
44
"net/http"
5+
"time"
56

67
"github.com/Uttam-Mahata/RootAccess/backend/internal/repositories"
78
"github.com/Uttam-Mahata/RootAccess/backend/internal/services"
@@ -14,13 +15,15 @@ type PracticeHandler struct {
1415
contestAdminService *services.ContestAdminService
1516
challengeRepo *repositories.ChallengeRepository
1617
submissionRepo *repositories.SubmissionRepository
18+
contestEntityRepo *repositories.ContestEntityRepository
1719
}
1820

19-
func NewPracticeHandler(contestAdminService *services.ContestAdminService, challengeRepo *repositories.ChallengeRepository, submissionRepo *repositories.SubmissionRepository) *PracticeHandler {
21+
func NewPracticeHandler(contestAdminService *services.ContestAdminService, challengeRepo *repositories.ChallengeRepository, submissionRepo *repositories.SubmissionRepository, contestEntityRepo *repositories.ContestEntityRepository) *PracticeHandler {
2022
return &PracticeHandler{
2123
contestAdminService: contestAdminService,
2224
challengeRepo: challengeRepo,
2325
submissionRepo: submissionRepo,
26+
contestEntityRepo: contestEntityRepo,
2427
}
2528
}
2629

@@ -53,6 +56,19 @@ func (h *PracticeHandler) GetPracticeChallenges(c *gin.Context) {
5356
return
5457
}
5558

59+
// Build a set of challenge IDs that belong to currently active/running contests
60+
// so we can exclude them from practice mode
61+
activeContestChallenges := make(map[string]bool)
62+
if h.contestAdminService != nil {
63+
// Get challenges visible in the active contest (without team filter — we want ALL active IDs)
64+
// We pass a dummy non-nil teamID to get the full set for any registered team
65+
if ids, err := h.contestAdminService.GetVisibleChallengeIDs(time.Now(), nil); err == nil {
66+
for _, id := range ids {
67+
activeContestChallenges[id] = true
68+
}
69+
}
70+
}
71+
5672
// Determine the user's own solves
5773
var userID string
5874
if uid, exists := c.Get("user_id"); exists {
@@ -67,12 +83,24 @@ func (h *PracticeHandler) GetPracticeChallenges(c *gin.Context) {
6783
}
6884
}
6985

70-
// Filter: only published challenges (which serve as practice when not part of an active contest)
86+
// Filter: only published challenges NOT in an active contest
7187
var result []PracticeChallenge
7288
for _, ch := range challenges {
7389
if !ch.IsPublished {
7490
continue
7591
}
92+
// Skip challenges that are part of an active/running contest
93+
if activeContestChallenges[ch.ID] {
94+
continue
95+
}
96+
// Also skip challenges in contests that haven't ended yet (by checking ContestID)
97+
if ch.ContestID != "" && h.contestEntityRepo != nil {
98+
if contest, err := h.contestEntityRepo.FindByID(ch.ContestID); err == nil && contest != nil {
99+
if time.Now().Before(contest.EndTime) {
100+
continue // Contest hasn't ended yet
101+
}
102+
}
103+
}
76104

77105
result = append(result, PracticeChallenge{
78106
ID: ch.ID,

backend/internal/handlers/scoreboard_handler.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package handlers
22

33
import (
44
"net/http"
5+
"strconv"
56
"time"
67

78
"github.com/Uttam-Mahata/RootAccess/backend/internal/repositories"
@@ -134,7 +135,7 @@ func (h *ScoreboardHandler) GetTeamStatistics(c *gin.Context) {
134135

135136
days := 30
136137
if daysParam := c.Query("days"); daysParam != "" {
137-
if parsedDays := parseInt(daysParam); parsedDays > 0 {
138+
if parsedDays, err := strconv.Atoi(daysParam); err == nil && parsedDays > 0 {
138139
days = parsedDays
139140
}
140141
}
@@ -216,14 +217,3 @@ func (h *ScoreboardHandler) GetScoreboardContests(c *gin.Context) {
216217
c.JSON(http.StatusOK, gin.H{"contests": result})
217218
}
218219

219-
func parseInt(s string) int {
220-
var result int
221-
for _, char := range s {
222-
if char >= '0' && char <= '9' {
223-
result = result*10 + int(char-'0')
224-
} else {
225-
return 0
226-
}
227-
}
228-
return result
229-
}

backend/internal/handlers/solo_leaderboard_handler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func (h *SoloLeaderboardHandler) GetSoloLeaderboard(c *gin.Context) {
9292
totalScore += challengePoints[sub.ChallengeID]
9393
}
9494

95-
if totalScore > 0 || len(subs) > 0 {
95+
if totalScore > 0 || len(seen) > 0 {
9696
scores = append(scores, SoloPlayerScore{
9797
UserID: user.ID,
9898
Username: user.Username,

backend/internal/middleware/auth_middleware.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package middleware
22

33
import (
4+
"context"
45
"fmt"
56
"net/http"
7+
"time"
68

79
"github.com/Uttam-Mahata/RootAccess/backend/internal/config"
10+
"github.com/Uttam-Mahata/RootAccess/backend/internal/database"
11+
"github.com/Uttam-Mahata/RootAccess/backend/internal/repositories"
812
"github.com/gin-gonic/gin"
913
"github.com/golang-jwt/jwt/v5"
1014
)
@@ -57,6 +61,18 @@ func AuthMiddleware(cfg *config.Config) gin.HandlerFunc {
5761
return
5862
}
5963

64+
userID, _ := claims["user_id"].(string)
65+
66+
// Verify user status from DB (with short Redis cache to avoid per-request DB hits)
67+
if userID != "" {
68+
status := getUserStatusCached(userID)
69+
if status == "banned" || status == "deleted" {
70+
c.JSON(http.StatusForbidden, gin.H{"error": "Your account has been " + status})
71+
c.Abort()
72+
return
73+
}
74+
}
75+
6076
c.Set("user_id", claims["user_id"])
6177
c.Set("username", claims["username"])
6278
c.Set("email", claims["email"])
@@ -66,6 +82,39 @@ func AuthMiddleware(cfg *config.Config) gin.HandlerFunc {
6682
}
6783
}
6884

85+
// getUserStatusCached returns the user's account status, using Redis as a short-lived
86+
// cache (10s TTL) to avoid hitting the DB on every single request.
87+
func getUserStatusCached(userID string) string {
88+
cacheKey := "user_status:" + userID
89+
90+
// Try Redis first
91+
if database.Registry != nil && database.Registry.General != nil {
92+
ctx := context.Background()
93+
if cached, err := database.Registry.General.Get(ctx, cacheKey).Result(); err == nil {
94+
return cached
95+
}
96+
}
97+
98+
// Fallback to DB
99+
if database.TursoDB != nil {
100+
userRepo := repositories.NewUserRepository(database.TursoDB)
101+
if user, err := userRepo.FindByID(userID); err == nil {
102+
status := user.Status
103+
if status == "" {
104+
status = "active"
105+
}
106+
// Cache in Redis for 10 seconds
107+
if database.Registry != nil && database.Registry.General != nil {
108+
ctx := context.Background()
109+
database.Registry.General.Set(ctx, cacheKey, status, 10*time.Second)
110+
}
111+
return status
112+
}
113+
}
114+
115+
return "active" // fail open if DB is unreachable
116+
}
117+
69118
func AdminMiddleware() gin.HandlerFunc {
70119
return func(c *gin.Context) {
71120
role, exists := c.Get("role")

backend/internal/routes/routes.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func SetupRouter(cfg *config.Config) *gin.Engine {
175175
accountHandler := handlers.NewAccountHandler(userRepo, teamRepo, submissionRepo, achievementRepo, writeupRepo)
176176
soloLeaderboardHandler := handlers.NewSoloLeaderboardHandler(userRepo, teamRepo, submissionRepo, challengeRepo)
177177
exportHandler := handlers.NewExportHandler(challengeRepo, submissionRepo, userRepo, teamRepo)
178-
practiceHandler := handlers.NewPracticeHandler(contestAdminService, challengeRepo, submissionRepo)
178+
practiceHandler := handlers.NewPracticeHandler(contestAdminService, challengeRepo, submissionRepo, contestEntityRepo)
179179
contestAnalyticsHandler := handlers.NewContestAnalyticsHandler(submissionRepo, challengeRepo, teamRepo, userRepo, teamContestRegistrationRepo, contestSolveRepo, roundChallengeRepo, contestRoundRepo)
180180
challengeRatingHandler := handlers.NewChallengeRatingHandler(database.TursoDB)
181181
webhookService := services.NewWebhookService(cfg.DiscordWebhookURL, cfg.SlackWebhookURL)

0 commit comments

Comments
 (0)