Ревью и оптимизация реализации пользовательского API

Условие

Необходимо проанализировать представленный фрагмент кода API, отвечающего за работу с пользователями, выявить потенциальные проблемы и предложить улучшения в реализации.

func isValidName(name string) error {
  if len(name) > 50 {
    return errors.New("invalid name length")
  }
  return nil
}

func isValidPhone(phone string) error {
  if len(phone) != 11 {
    return errors.New("invalid phone format")
  }
  return nil
}

type User struct {
  Id         int       `json:"id"`
  Name       string    `json:"name"`
  Phone      string    `json:"phone"`
  IsAdmin    bool      `json:"is_admin"`
  CreatedAt  time.Time `json:"created_at"`
  UpdatedAt  time.Time `json:"updated_at"`
}

type UserRepository interface {
  Find(sql string) *User
  Save(user *User) *User
}

type UserApi struct {
  userRepo UserRepository
  logger   *zap.Logger
}

func NewUserApi(userRepo UserRepository) *UserApi {
  api := &UserApi{
    userRepo: userRepo,
  }

  if os.Getenv("ENV") == "prod" {
    api.logger, _ = zap.NewProduction()
  } else {
    api.logger, _ = zap.NewDevelopment()
  }

  return api
}

func (api *UserApi) ProcessRequest(c *gin.Context) {
  authUserId := c.GetInt("auth_user_id")
  authUser := api.userRepo.Find(fmt.Sprintf("SELECT * FROM user WHERE id = %d", authUserId))

  id := c.Query("id")
  user := api.userRepo.Find(fmt.Sprintf("SELECT * FROM user WHERE id = %s", id))

  if authUser.IsAdmin || authUser.Id == user.Id {
    if c.Request.Method == "POST" {
      bytes, _ := io.ReadAll(c.Request.Body)
      var body map[string]string
      json.Unmarshal(bytes, &body)

      var errs map[string]string

      if err := isValidName(body["name"]); err != nil {
        errs["name"] = err.Error()
      } else {
        user.Name = body["name"]
      }

      if err := isValidPhone(body["phone"]); err != nil {
        errs["phone"] = err.Error()
      } else {
        user.Phone = body["phone"]
      }

      if len(errs) > 0 {
        c.JSON(http.StatusUnprocessableEntity, errs)
        return
      }

      user.UpdatedAt = time.Now()

      api.userRepo.Save(user)
      api.logger.Debug("user saved", zap.Int("user_id", user.Id))
      c.JSON(http.StatusOK, user)
      return
    }
  }

  c.Status(http.StatusForbidden)
}

func main() {
    // It's implemented somehow
    api := NewUserApi(...)
    r := gin.Default()
    r.POST("/users", api.ProcessRequest)
    r.Run()
}

Ответ

Основные дефекты: SQL-инъекция через id, nil-разыменования, запись в неинициализированную errs map и игнорирование ошибок. Нужны типизированный ввод, явная авторизация и репозиторий с context/error.

Инициализируйте errs через make; проверяйте JSON и ограничивайте размер тела. len(name) считает байты, а телефон нельзя проверить только длиной. Валидируйте весь DTO до изменения User. Парсите id, передавайте SQL-параметры, различайте отсутствие пользователя и отказ БД. Проверяйте, что auth middleware действительно установил пользователя, и не принимайте IsAdmin из клиентского ввода.

Find/Save должны возвращать ошибку, logger — корректно инициализироваться. Ответ формируйте по явной схеме, не раскрывая лишние поля. Для конкурентного редактирования предусмотрите версию, а для session-auth — CSRF. Логи не должны содержать телефон, тело запроса и секреты.