refactor(signserver): replace raw SQL with repository interfaces

Extract all direct database access into three repository interfaces (SignUserRepo, SignCharacterRepo, SignSessionRepo) matching the pattern established in channelserver. This surfaces 9 previously silenced errors that are now logged with structured context, and makes the sign server testable with mock repos instead of go-sqlmock. Security fix: GetFriends now uses parameterized ANY($1) queries instead of string-concatenated WHERE clauses (SQL injection vector).
2026-03-27 18:12:50 +01:00 · 2026-02-22 16:30:24 +01:00
parent 53b5bb3b96
commit b3f75232a3
11 changed files with 1193 additions and 435 deletions
--- a/server/signserver/dbutils.go
+++ b/server/signserver/dbutils.go
@@ -5,7 +5,6 @@ import (
 	"errors"
 	"erupe-ce/common/mhfcourse"
 	"erupe-ce/common/token"
-	"strings"
 	"time"

 	"go.uber.org/zap"
@@ -13,34 +12,20 @@ import (
 )

 func (s *Server) newUserChara(uid uint32) error {
-	var numNewChars int
-	err := s.db.QueryRow("SELECT COUNT(*) FROM characters WHERE user_id = $1 AND is_new_character = true", uid).Scan(&numNewChars)
+	numNewChars, err := s.charRepo.CountNewCharacters(uid)
 	if err != nil {
 		return err
 	}

 	// prevent users with an uninitialised character from creating more
 	if numNewChars >= 1 {
-		return err
+		return nil
 	}

-	_, err = s.db.Exec(`
-		INSERT INTO characters (
-			user_id, is_female, is_new_character, name, unk_desc_string,
-			hr, gr, weapon_type, last_login)
-		VALUES($1, False, True, '', '', 0, 0, 0, $2)`,
-		uid,
-		uint32(time.Now().Unix()),
-	)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	return s.charRepo.CreateCharacter(uid, uint32(time.Now().Unix()))
 }

 func (s *Server) registerDBAccount(username string, password string) (uint32, error) {
-	var uid uint32
 	s.logger.Info("Creating user", zap.String("User", username))

 	// Create salted hash of user password
@@ -49,7 +34,7 @@ func (s *Server) registerDBAccount(username string, password string) (uint32, er
 		return 0, err
 	}

-	err = s.db.QueryRow("INSERT INTO users (username, password, return_expires) VALUES ($1, $2, $3) RETURNING id", username, string(passwordHash), time.Now().Add(time.Hour*24*30)).Scan(&uid)
+	uid, err := s.userRepo.Register(username, string(passwordHash), time.Now().Add(time.Hour*24*30))
 	if err != nil {
 		return 0, err
 	}
@@ -57,81 +42,65 @@ func (s *Server) registerDBAccount(username string, password string) (uint32, er
 	return uid, nil
 }

-type character struct {
-	ID             uint32 `db:"id"`
-	IsFemale       bool   `db:"is_female"`
-	IsNewCharacter bool   `db:"is_new_character"`
-	Name           string `db:"name"`
-	UnkDescString  string `db:"unk_desc_string"`
-	HR             uint16 `db:"hr"`
-	GR             uint16 `db:"gr"`
-	WeaponType     uint16 `db:"weapon_type"`
-	LastLogin      uint32 `db:"last_login"`
-}
-
 func (s *Server) getCharactersForUser(uid uint32) ([]character, error) {
-	characters := make([]character, 0)
-	err := s.db.Select(&characters, "SELECT id, is_female, is_new_character, name, unk_desc_string, hr, gr, weapon_type, last_login FROM characters WHERE user_id = $1 AND deleted = false ORDER BY id", uid)
-	if err != nil {
-		return nil, err
-	}
-	return characters, nil
+	return s.charRepo.GetForUser(uid)
 }

 func (s *Server) getReturnExpiry(uid uint32) time.Time {
-	var returnExpiry, lastLogin time.Time
-	_ = s.db.Get(&lastLogin, "SELECT COALESCE(last_login, now()) FROM users WHERE id=$1", uid)
+	var returnExpiry time.Time
+	lastLogin, err := s.userRepo.GetLastLogin(uid)
+	if err != nil {
+		s.logger.Warn("Failed to get last login", zap.Uint32("uid", uid), zap.Error(err))
+		lastLogin = time.Now()
+	}
 	if time.Now().Add((time.Hour * 24) * -90).After(lastLogin) {
 		returnExpiry = time.Now().Add(time.Hour * 24 * 30)
-		_, _ = s.db.Exec("UPDATE users SET return_expires=$1 WHERE id=$2", returnExpiry, uid)
+		if err := s.userRepo.UpdateReturnExpiry(uid, returnExpiry); err != nil {
+			s.logger.Warn("Failed to update return expiry", zap.Uint32("uid", uid), zap.Error(err))
+		}
 	} else {
-		err := s.db.Get(&returnExpiry, "SELECT return_expires FROM users WHERE id=$1", uid)
+		returnExpiry, err = s.userRepo.GetReturnExpiry(uid)
 		if err != nil {
 			returnExpiry = time.Now()
-			_, _ = s.db.Exec("UPDATE users SET return_expires=$1 WHERE id=$2", returnExpiry, uid)
+			if err := s.userRepo.UpdateReturnExpiry(uid, returnExpiry); err != nil {
+				s.logger.Warn("Failed to update return expiry (fallback)", zap.Uint32("uid", uid), zap.Error(err))
+			}
 		}
 	}
-	_, _ = s.db.Exec("UPDATE users SET last_login=$1 WHERE id=$2", time.Now(), uid)
+	if err := s.userRepo.UpdateLastLogin(uid, time.Now()); err != nil {
+		s.logger.Warn("Failed to update last login", zap.Uint32("uid", uid), zap.Error(err))
+	}
 	return returnExpiry
 }

 func (s *Server) getLastCID(uid uint32) uint32 {
-	var lastPlayed uint32
-	_ = s.db.QueryRow("SELECT last_character FROM users WHERE id=$1", uid).Scan(&lastPlayed)
+	lastPlayed, err := s.userRepo.GetLastCharacter(uid)
+	if err != nil {
+		s.logger.Warn("Failed to get last character", zap.Uint32("uid", uid), zap.Error(err))
+		return 0
+	}
 	return lastPlayed
 }

 func (s *Server) getUserRights(uid uint32) uint32 {
-	var rights uint32
-	if uid != 0 {
-		_ = s.db.QueryRow("SELECT rights FROM users WHERE id=$1", uid).Scan(&rights)
-		_, rights = mhfcourse.GetCourseStruct(rights, s.erupeConfig.DefaultCourses)
+	if uid == 0 {
+		return 0
 	}
+	rights, err := s.userRepo.GetRights(uid)
+	if err != nil {
+		s.logger.Warn("Failed to get user rights", zap.Uint32("uid", uid), zap.Error(err))
+		return 0
+	}
+	_, rights = mhfcourse.GetCourseStruct(rights, s.erupeConfig.DefaultCourses)
 	return rights
 }

-type members struct {
-	CID  uint32 // Local character ID
-	ID   uint32 `db:"id"`
-	Name string `db:"name"`
-}
-
 func (s *Server) getFriendsForCharacters(chars []character) []members {
 	friends := make([]members, 0)
 	for _, char := range chars {
-		friendsCSV := ""
-		_ = s.db.QueryRow("SELECT friends FROM characters WHERE id=$1", char.ID).Scan(&friendsCSV)
-		friendsSlice := strings.Split(friendsCSV, ",")
-		friendQuery := "SELECT id, name FROM characters WHERE id="
-		for i := 0; i < len(friendsSlice); i++ {
-			friendQuery += friendsSlice[i]
-			if i+1 != len(friendsSlice) {
-				friendQuery += " OR id="
-			}
-		}
-		charFriends := make([]members, 0)
-		err := s.db.Select(&charFriends, friendQuery)
+		charFriends, err := s.charRepo.GetFriends(char.ID)
 		if err != nil {
+			s.logger.Warn("Failed to get friends", zap.Uint32("charID", char.ID), zap.Error(err))
 			continue
 		}
 		for i := range charFriends {
@@ -145,79 +114,56 @@ func (s *Server) getFriendsForCharacters(chars []character) []members {
 func (s *Server) getGuildmatesForCharacters(chars []character) []members {
 	guildmates := make([]members, 0)
 	for _, char := range chars {
-		var inGuild int
-		_ = s.db.QueryRow("SELECT count(*) FROM guild_characters WHERE character_id=$1", char.ID).Scan(&inGuild)
-		if inGuild > 0 {
-			var guildID int
-			err := s.db.QueryRow("SELECT guild_id FROM guild_characters WHERE character_id=$1", char.ID).Scan(&guildID)
-			if err != nil {
-				continue
-			}
-			charGuildmates := make([]members, 0)
-			err = s.db.Select(&charGuildmates, "SELECT character_id AS id, c.name FROM guild_characters gc JOIN characters c ON c.id = gc.character_id WHERE guild_id=$1 AND character_id!=$2", guildID, char.ID)
-			if err != nil {
-				continue
-			}
-			for i := range charGuildmates {
-				charGuildmates[i].CID = char.ID
-			}
-			guildmates = append(guildmates, charGuildmates...)
+		charGuildmates, err := s.charRepo.GetGuildmates(char.ID)
+		if err != nil {
+			s.logger.Warn("Failed to get guildmates", zap.Uint32("charID", char.ID), zap.Error(err))
+			continue
 		}
+		for i := range charGuildmates {
+			charGuildmates[i].CID = char.ID
+		}
+		guildmates = append(guildmates, charGuildmates...)
 	}
 	return guildmates
 }

-func (s *Server) deleteCharacter(cid int, token string, tokenID uint32) error {
-	if !s.validateToken(token, tokenID) {
+func (s *Server) deleteCharacter(cid int, tok string, tokenID uint32) error {
+	if !s.validateToken(tok, tokenID) {
 		return errors.New("invalid token")
 	}
-	var isNew bool
-	err := s.db.QueryRow("SELECT is_new_character FROM characters WHERE id = $1", cid).Scan(&isNew)
+	isNew, err := s.charRepo.IsNewCharacter(cid)
 	if err != nil {
 		return err
 	}
 	if isNew {
-		_, err = s.db.Exec("DELETE FROM characters WHERE id = $1", cid)
-	} else {
-		_, err = s.db.Exec("UPDATE characters SET deleted = true WHERE id = $1", cid)
+		return s.charRepo.HardDelete(cid)
 	}
-	if err != nil {
-		return err
-	}
-	return nil
+	return s.charRepo.SoftDelete(cid)
 }

 func (s *Server) registerUidToken(uid uint32) (uint32, string, error) {
 	_token := token.Generate(16)
-	var tid uint32
-	err := s.db.QueryRow(`INSERT INTO sign_sessions (user_id, token) VALUES ($1, $2) RETURNING id`, uid, _token).Scan(&tid)
+	tid, err := s.sessionRepo.RegisterUID(uid, _token)
 	return tid, _token, err
 }

 func (s *Server) registerPsnToken(psn string) (uint32, string, error) {
 	_token := token.Generate(16)
-	var tid uint32
-	err := s.db.QueryRow(`INSERT INTO sign_sessions (psn_id, token) VALUES ($1, $2) RETURNING id`, psn, _token).Scan(&tid)
+	tid, err := s.sessionRepo.RegisterPSN(psn, _token)
 	return tid, _token, err
 }

-func (s *Server) validateToken(token string, tokenID uint32) bool {
-	query := `SELECT count(*) FROM sign_sessions WHERE token = $1`
-	if tokenID > 0 {
-		query += ` AND id = $2`
-	}
-	var exists int
-	err := s.db.QueryRow(query, token, tokenID).Scan(&exists)
-	if err != nil || exists == 0 {
+func (s *Server) validateToken(tok string, tokenID uint32) bool {
+	valid, err := s.sessionRepo.Validate(tok, tokenID)
+	if err != nil {
+		s.logger.Warn("Failed to validate token", zap.Error(err))
 		return false
 	}
-	return true
+	return valid
 }

 func (s *Server) validateLogin(user string, pass string) (uint32, RespID) {
-	var uid uint32
-	var passDB string
-	err := s.db.QueryRow(`SELECT id, password FROM users WHERE username = $1`, user).Scan(&uid, &passDB)
+	uid, passDB, err := s.userRepo.GetCredentials(user)
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {
 			s.logger.Info("User not found", zap.String("User", user))
@@ -225,26 +171,25 @@ func (s *Server) validateLogin(user string, pass string) (uint32, RespID) {
 				uid, err = s.registerDBAccount(user, pass)
 				if err == nil {
 					return uid, SIGN_SUCCESS
-				} else {
-					return 0, SIGN_EABORT
 				}
+				return 0, SIGN_EABORT
 			}
 			return 0, SIGN_EAUTH
 		}
 		return 0, SIGN_EABORT
-	} else {
-		if bcrypt.CompareHashAndPassword([]byte(passDB), []byte(pass)) == nil {
-			var bans int
-			err = s.db.QueryRow(`SELECT count(*) FROM bans WHERE user_id=$1 AND expires IS NULL`, uid).Scan(&bans)
-			if err == nil && bans > 0 {
-				return uid, SIGN_EELIMINATE
-			}
-			err = s.db.QueryRow(`SELECT count(*) FROM bans WHERE user_id=$1 AND expires > now()`, uid).Scan(&bans)
-			if err == nil && bans > 0 {
-				return uid, SIGN_ESUSPEND
-			}
-			return uid, SIGN_SUCCESS
-		}
+	}
+
+	if bcrypt.CompareHashAndPassword([]byte(passDB), []byte(pass)) != nil {
 		return 0, SIGN_EPASS
 	}
+
+	bans, err := s.userRepo.CountPermanentBans(uid)
+	if err == nil && bans > 0 {
+		return uid, SIGN_EELIMINATE
+	}
+	bans, err = s.userRepo.CountActiveBans(uid)
+	if err == nil && bans > 0 {
+		return uid, SIGN_ESUSPEND
+	}
+	return uid, SIGN_SUCCESS
 }