fix: 3차 리뷰 LOW — 에러 메시지 일관성, Redis 타임아웃, 입력 검증
Some checks failed
Server CI/CD / lint-and-build (push) Failing after 30s
Server CI/CD / deploy (push) Has been skipped

- 5개 핸들러 err.Error() → 제네릭 메시지 (Login, Refresh, SSAFY, Ticket, BossRaid)
- Redis context.Background() → WithTimeout 5s (10곳)
- SprintMultiplier 범위 검증 추가
- 방어적 문서화 (SSAFY 충돌, zip bomb, body limit prefix, 로그 주입)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-03-15 19:05:17 +09:00
parent 9504bf37de
commit 423e2832a0
7 changed files with 51 additions and 20 deletions

View File

@@ -1,6 +1,7 @@
package auth package auth
import ( import (
"log"
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
@@ -70,7 +71,8 @@ func (h *Handler) Login(c *fiber.Ctx) error {
accessToken, refreshToken, user, err := h.svc.Login(req.Username, req.Password) accessToken, refreshToken, user, err := h.svc.Login(req.Username, req.Password)
if err != nil { if err != nil {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) log.Printf("Login failed (username=%s): %v", req.Username, err)
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "아이디 또는 비밀번호가 올바르지 않습니다"})
} }
c.Cookie(&fiber.Cookie{ c.Cookie(&fiber.Cookie{
@@ -106,7 +108,8 @@ func (h *Handler) Refresh(c *fiber.Ctx) error {
newAccessToken, newRefreshToken, err := h.svc.Refresh(refreshTokenStr) newAccessToken, newRefreshToken, err := h.svc.Refresh(refreshTokenStr)
if err != nil { if err != nil {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) log.Printf("Refresh failed: %v", err)
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "토큰 갱신에 실패했습니다"})
} }
c.Cookie(&fiber.Cookie{ c.Cookie(&fiber.Cookie{
@@ -219,7 +222,8 @@ func (h *Handler) SSAFYCallback(c *fiber.Ctx) error {
accessToken, refreshToken, user, err := h.svc.SSAFYLogin(req.Code, req.State) accessToken, refreshToken, user, err := h.svc.SSAFYLogin(req.Code, req.State)
if err != nil { if err != nil {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) log.Printf("SSAFY login failed: %v", err)
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "SSAFY 로그인에 실패했습니다"})
} }
c.Cookie(&fiber.Cookie{ c.Cookie(&fiber.Cookie{
@@ -263,7 +267,8 @@ func (h *Handler) RedeemLaunchTicket(c *fiber.Ctx) error {
} }
token, err := h.svc.RedeemLaunchTicket(req.Ticket) token, err := h.svc.RedeemLaunchTicket(req.Ticket)
if err != nil { if err != nil {
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": err.Error()}) log.Printf("RedeemLaunchTicket failed: %v", err)
return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{"error": "유효하지 않거나 만료된 티켓입니다"})
} }
return c.JSON(fiber.Map{"token": token}) return c.JSON(fiber.Map{"token": token})
} }

View File

@@ -95,7 +95,9 @@ func (s *Service) issueAccessToken(user *User) (string, error) {
} }
key := fmt.Sprintf("session:%d", user.ID) key := fmt.Sprintf("session:%d", user.ID)
if err := s.rdb.Set(context.Background(), key, tokenStr, expiry).Err(); err != nil { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := s.rdb.Set(ctx, key, tokenStr, expiry).Err(); err != nil {
return "", fmt.Errorf("세션 저장에 실패했습니다") return "", fmt.Errorf("세션 저장에 실패했습니다")
} }
@@ -119,7 +121,9 @@ func (s *Service) issueRefreshToken(user *User) (string, error) {
} }
key := fmt.Sprintf("refresh:%d", user.ID) key := fmt.Sprintf("refresh:%d", user.ID)
if err := s.rdb.Set(context.Background(), key, tokenStr, refreshTokenExpiry).Err(); err != nil { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := s.rdb.Set(ctx, key, tokenStr, refreshTokenExpiry).Err(); err != nil {
return "", fmt.Errorf("리프레시 토큰 저장에 실패했습니다") return "", fmt.Errorf("리프레시 토큰 저장에 실패했습니다")
} }
@@ -145,7 +149,9 @@ func (s *Service) Refresh(refreshTokenStr string) (newAccessToken, newRefreshTok
// Redis에서 저장된 리프레시 토큰과 비교 // Redis에서 저장된 리프레시 토큰과 비교
key := fmt.Sprintf("refresh:%d", claims.UserID) key := fmt.Sprintf("refresh:%d", claims.UserID)
stored, err := s.rdb.Get(context.Background(), key).Result() refreshCtx, refreshCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer refreshCancel()
stored, err := s.rdb.Get(refreshCtx, key).Result()
if err != nil || stored != refreshTokenStr { if err != nil || stored != refreshTokenStr {
return "", "", fmt.Errorf("만료되었거나 유효하지 않은 리프레시 토큰입니다") return "", "", fmt.Errorf("만료되었거나 유효하지 않은 리프레시 토큰입니다")
} }
@@ -171,7 +177,8 @@ func (s *Service) Refresh(refreshTokenStr string) (newAccessToken, newRefreshTok
} }
func (s *Service) Logout(userID uint) error { func (s *Service) Logout(userID uint) error {
ctx := context.Background() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
sessionKey := fmt.Sprintf("session:%d", userID) sessionKey := fmt.Sprintf("session:%d", userID)
refreshKey := fmt.Sprintf("refresh:%d", userID) refreshKey := fmt.Sprintf("refresh:%d", userID)
return s.rdb.Del(ctx, sessionKey, refreshKey).Err() return s.rdb.Del(ctx, sessionKey, refreshKey).Err()
@@ -191,10 +198,11 @@ func (s *Service) DeleteUser(id uint) error {
} }
// Clean up Redis sessions for deleted user // Clean up Redis sessions for deleted user
ctx := context.Background() delCtx, delCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer delCancel()
sessionKey := fmt.Sprintf("session:%d", id) sessionKey := fmt.Sprintf("session:%d", id)
refreshKey := fmt.Sprintf("refresh:%d", id) refreshKey := fmt.Sprintf("refresh:%d", id)
s.rdb.Del(ctx, sessionKey, refreshKey) s.rdb.Del(delCtx, sessionKey, refreshKey)
// TODO: Clean up wallet and profile data via cross-service calls // TODO: Clean up wallet and profile data via cross-service calls
// (walletCreator/profileCreator are creation-only; deletion callbacks are not yet wired up) // (walletCreator/profileCreator are creation-only; deletion callbacks are not yet wired up)
@@ -214,7 +222,8 @@ func (s *Service) CreateLaunchTicket(userID uint) (string, error) {
// Store ticket → userID mapping in Redis with 30s TTL // Store ticket → userID mapping in Redis with 30s TTL
key := fmt.Sprintf("launch_ticket:%s", ticket) key := fmt.Sprintf("launch_ticket:%s", ticket)
ctx := context.Background() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := s.rdb.Set(ctx, key, userID, 30*time.Second).Err(); err != nil { if err := s.rdb.Set(ctx, key, userID, 30*time.Second).Err(); err != nil {
return "", fmt.Errorf("store ticket: %w", err) return "", fmt.Errorf("store ticket: %w", err)
} }
@@ -225,7 +234,8 @@ func (s *Service) CreateLaunchTicket(userID uint) (string, error) {
// The ticket is deleted immediately after use (one-time). // The ticket is deleted immediately after use (one-time).
func (s *Service) RedeemLaunchTicket(ticket string) (string, error) { func (s *Service) RedeemLaunchTicket(ticket string) (string, error) {
key := fmt.Sprintf("launch_ticket:%s", ticket) key := fmt.Sprintf("launch_ticket:%s", ticket)
ctx := context.Background() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
// Atomically get and delete (one-time use) // Atomically get and delete (one-time use)
userIDStr, err := s.rdb.GetDel(ctx, key).Result() userIDStr, err := s.rdb.GetDel(ctx, key).Result()
@@ -290,7 +300,8 @@ func (s *Service) GetSSAFYLoginURL() (string, error) {
// Store state in Redis with 5-minute TTL for one-time verification // Store state in Redis with 5-minute TTL for one-time verification
key := fmt.Sprintf("ssafy_state:%s", state) key := fmt.Sprintf("ssafy_state:%s", state)
ctx := context.Background() ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := s.rdb.Set(ctx, key, "1", 5*time.Minute).Err(); err != nil { if err := s.rdb.Set(ctx, key, "1", 5*time.Minute).Err(); err != nil {
return "", fmt.Errorf("state 저장 실패: %w", err) return "", fmt.Errorf("state 저장 실패: %w", err)
} }
@@ -379,7 +390,9 @@ func (s *Service) SSAFYLogin(code, state string) (accessToken, refreshToken stri
return "", "", nil, fmt.Errorf("state 파라미터가 필요합니다") return "", "", nil, fmt.Errorf("state 파라미터가 필요합니다")
} }
stateKey := fmt.Sprintf("ssafy_state:%s", state) stateKey := fmt.Sprintf("ssafy_state:%s", state)
val, err := s.rdb.GetDel(context.Background(), stateKey).Result() stateCtx, stateCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer stateCancel()
val, err := s.rdb.GetDel(stateCtx, stateKey).Result()
if err != nil || val != "1" { if err != nil || val != "1" {
return "", "", nil, fmt.Errorf("유효하지 않거나 만료된 state 파라미터입니다") return "", "", nil, fmt.Errorf("유효하지 않거나 만료된 state 파라미터입니다")
} }
@@ -411,6 +424,9 @@ func (s *Service) SSAFYLogin(code, state string) (accessToken, refreshToken stri
ssafyID := userInfo.UserID ssafyID := userInfo.UserID
// SSAFY ID에서 영문 소문자+숫자만 추출하여 안전한 username 생성 // SSAFY ID에서 영문 소문자+숫자만 추출하여 안전한 username 생성
// NOTE: Username collision is handled by the DB unique constraint.
// If collision occurs, the transaction will rollback and return a generic error.
// A retry with random suffix could improve UX but is not critical.
safeID := sanitizeForUsername(ssafyID) safeID := sanitizeForUsername(ssafyID)
if safeID == "" { if safeID == "" {
safeID = hex.EncodeToString(randomBytes[:8]) safeID = hex.EncodeToString(randomBytes[:8])
@@ -480,7 +496,9 @@ func (s *Service) VerifyToken(tokenStr string) (string, error) {
} }
key := fmt.Sprintf("session:%d", claims.UserID) key := fmt.Sprintf("session:%d", claims.UserID)
stored, err := s.rdb.Get(context.Background(), key).Result() verifyCtx, verifyCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer verifyCancel()
stored, err := s.rdb.Get(verifyCtx, key).Result()
if err != nil || stored != tokenStr { if err != nil || stored != tokenStr {
return "", fmt.Errorf("만료되었거나 로그아웃된 세션입니다") return "", fmt.Errorf("만료되었거나 로그아웃된 세션입니다")
} }

View File

@@ -40,7 +40,7 @@ func (h *Handler) RequestEntry(c *fiber.Ctx) error {
room, err := h.svc.RequestEntry(req.Usernames, req.BossID) room, err := h.svc.RequestEntry(req.Usernames, req.BossID)
if err != nil { if err != nil {
return bossError(c, fiber.StatusConflict, err.Error(), err) return bossError(c, fiber.StatusConflict, "보스 레이드 입장에 실패했습니다", err)
} }
return c.Status(fiber.StatusCreated).JSON(fiber.Map{ return c.Status(fiber.StatusCreated).JSON(fiber.Map{
@@ -67,7 +67,7 @@ func (h *Handler) StartRaid(c *fiber.Ctx) error {
room, err := h.svc.StartRaid(req.SessionName) room, err := h.svc.StartRaid(req.SessionName)
if err != nil { if err != nil {
return bossError(c, fiber.StatusBadRequest, err.Error(), err) return bossError(c, fiber.StatusBadRequest, "레이드 시작에 실패했습니다", err)
} }
return c.JSON(fiber.Map{ return c.JSON(fiber.Map{
@@ -93,7 +93,7 @@ func (h *Handler) CompleteRaid(c *fiber.Ctx) error {
room, results, err := h.svc.CompleteRaid(req.SessionName, req.Rewards) room, results, err := h.svc.CompleteRaid(req.SessionName, req.Rewards)
if err != nil { if err != nil {
return bossError(c, fiber.StatusBadRequest, err.Error(), err) return bossError(c, fiber.StatusBadRequest, "레이드 완료 처리에 실패했습니다", err)
} }
return c.JSON(fiber.Map{ return c.JSON(fiber.Map{
@@ -119,7 +119,7 @@ func (h *Handler) FailRaid(c *fiber.Ctx) error {
room, err := h.svc.FailRaid(req.SessionName) room, err := h.svc.FailRaid(req.SessionName)
if err != nil { if err != nil {
return bossError(c, fiber.StatusBadRequest, err.Error(), err) return bossError(c, fiber.StatusBadRequest, "레이드 실패 처리에 실패했습니다", err)
} }
return c.JSON(fiber.Map{ return c.JSON(fiber.Map{
@@ -175,7 +175,7 @@ func (h *Handler) RequestEntryAuth(c *fiber.Ctx) error {
room, tokens, err := h.svc.RequestEntryWithTokens(req.Usernames, req.BossID) room, tokens, err := h.svc.RequestEntryWithTokens(req.Usernames, req.BossID)
if err != nil { if err != nil {
return bossError(c, fiber.StatusConflict, err.Error(), err) return bossError(c, fiber.StatusConflict, "보스 레이드 입장에 실패했습니다", err)
} }
return c.Status(fiber.StatusCreated).JSON(fiber.Map{ return c.Status(fiber.StatusCreated).JSON(fiber.Map{

View File

@@ -139,6 +139,8 @@ func (s *Service) Upload(filename string, body io.Reader, baseURL string) (*Info
return info, s.repo.Save(info) return info, s.repo.Save(info)
} }
// NOTE: No size limit on decompressed entry. This is admin-only so
// the risk is minimal. For defense-in-depth, consider io.LimitReader.
func hashGameExeFromZip(zipPath string) string { func hashGameExeFromZip(zipPath string) string {
r, err := zip.OpenReader(zipPath) r, err := zip.OpenReader(zipPath)
if err != nil { if err != nil {

View File

@@ -95,6 +95,7 @@ func (h *Handler) InternalSaveGameData(c *fiber.Ctx) error {
} }
if err := h.svc.SaveGameDataByUsername(username, &req); err != nil { if err := h.svc.SaveGameDataByUsername(username, &req); err != nil {
// Username from internal API (ServerAuth protected) — low risk of injection
log.Printf("게임 데이터 저장 실패 (username=%s): %v", username, err) log.Printf("게임 데이터 저장 실패 (username=%s): %v", username, err)
return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "서버 오류가 발생했습니다"}) return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{"error": "서버 오류가 발생했습니다"})
} }

View File

@@ -26,6 +26,9 @@ func validateGameData(data *GameDataRequest) error {
if data.AttackRange != nil && (*data.AttackRange < 0 || *data.AttackRange > 100) { if data.AttackRange != nil && (*data.AttackRange < 0 || *data.AttackRange > 100) {
return fmt.Errorf("attack_range must be 0-100") return fmt.Errorf("attack_range must be 0-100")
} }
if data.SprintMultiplier != nil && (*data.SprintMultiplier < 0 || *data.SprintMultiplier > 10) {
return fmt.Errorf("sprint_multiplier must be 0-10")
}
if data.PlayTimeDelta != nil && *data.PlayTimeDelta < 0 { if data.PlayTimeDelta != nil && *data.PlayTimeDelta < 0 {
return fmt.Errorf("플레이 시간 변화량은 0 이상이어야 합니다") return fmt.Errorf("플레이 시간 변화량은 0 이상이어야 합니다")
} }

View File

@@ -11,6 +11,8 @@ import (
// bypass this check. Fiber's global BodyLimit provides the final safety net. // bypass this check. Fiber's global BodyLimit provides the final safety net.
// Paths matching any of the excludePrefixes are skipped (e.g. upload endpoints // Paths matching any of the excludePrefixes are skipped (e.g. upload endpoints
// that legitimately need the global 4GB limit). // that legitimately need the global 4GB limit).
// NOTE: excludePrefixes uses HasPrefix matching. Ensure no unintended
// routes share the same prefix as upload endpoints.
func BodyLimit(maxBytes int, excludePrefixes ...string) fiber.Handler { func BodyLimit(maxBytes int, excludePrefixes ...string) fiber.Handler {
return func(c *fiber.Ctx) error { return func(c *fiber.Ctx) error {
for _, prefix := range excludePrefixes { for _, prefix := range excludePrefixes {