feat: fail when the JSON input has extra fields (#211)

internal/common/validator.go

@@ -9,6 +9,8 @@ import (
 	"github.com/gofiber/fiber/v2"
 
 	"github.com/go-playground/validator/v10"
+
+	"github.com/italia/developers-italia-api/internal/jsondecoder"
 )
 
 const (
@@ -63,7 +65,11 @@ func ValidateStruct(validateStruct interface{}) []ValidationError {
 
 func ValidateRequestEntity(ctx *fiber.Ctx, request interface{}, errorMessage string) error {
 	if err := ctx.BodyParser(request); err != nil {
-		return Error(fiber.StatusBadRequest, errorMessage, "invalid json")
+		if errors.Is(err, jsondecoder.ErrUnknownField) {
+			return Error(fiber.StatusUnprocessableEntity, errorMessage, err.Error())
+		}
+
+		return Error(fiber.StatusBadRequest, errorMessage, "invalid or malformed JSON")
 	}
 
 	if err := ValidateStruct(request); err != nil {

internal/handlers/logs.go

@@ -84,51 +84,47 @@ func (p *Log) GetLog(ctx *fiber.Ctx) error {
 
 // PostLog creates a new log.
 func (p *Log) PostLog(ctx *fiber.Ctx) error {
-	logReq := new(common.Log)
+	const errMsg = "can't create Log"
 
-	if err := ctx.BodyParser(&logReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't create Log", "invalid json")
-	}
+	logReq := new(common.Log)
 
-	if err := common.ValidateStruct(*logReq); err != nil {
-		return common.ErrorWithValidationErrors(fiber.StatusUnprocessableEntity, "can't create Log", err)
+	if err := common.ValidateRequestEntity(ctx, logReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	log := models.Log{ID: utils.UUIDv4(), Message: logReq.Message}
 
 	if err := p.db.Create(&log).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't create Log", "db error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "db error")
 	}
 
 	return ctx.JSON(&log)
 }
 
 // PatchLog updates the log with the given ID.
 func (p *Log) PatchLog(ctx *fiber.Ctx) error {
-	logReq := new(common.Log)
+	const errMsg = "can't update Log"
 
-	if err := ctx.BodyParser(logReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't update Log", "invalid json")
-	}
+	logReq := new(common.Log)
 
-	if err := common.ValidateStruct(*logReq); err != nil {
-		return common.ErrorWithValidationErrors(fiber.StatusUnprocessableEntity, "can't update Log", err)
+	if err := common.ValidateRequestEntity(ctx, logReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	log := models.Log{}
 
 	if err := p.db.First(&log, "id = ?", ctx.Params("id")).Error; err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			return common.Error(fiber.StatusNotFound, "can't update Log", "Log was not found")
+			return common.Error(fiber.StatusNotFound, errMsg, "Log was not found")
 		}
 
-		return common.Error(fiber.StatusInternalServerError, "can't update Log", "internal server error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "internal server error")
 	}
 
 	log.Message = logReq.Message
 
 	if err := p.db.Updates(&log).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't update Log", "db error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "db error")
 	}
 
 	return ctx.JSON(&log)
@@ -198,6 +194,8 @@ func (p *Log) GetSoftwareLogs(ctx *fiber.Ctx) error {
 
 // PostSoftwareLog creates a new log associated to a Software with the given ID and returns any error encountered.
 func (p *Log) PostSoftwareLog(ctx *fiber.Ctx) error {
+	const errMsg = "can't create Log"
+
 	logReq := new(common.Log)
 
 	software := models.Software{}
@@ -213,12 +211,8 @@ func (p *Log) PostSoftwareLog(ctx *fiber.Ctx) error {
 		)
 	}
 
-	if err := ctx.BodyParser(&logReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't create Log", "invalid json")
-	}
-
-	if err := common.ValidateStruct(*logReq); err != nil {
-		return common.ErrorWithValidationErrors(fiber.StatusUnprocessableEntity, "can't create Log", err)
+	if err := common.ValidateRequestEntity(ctx, logReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	table := models.Software{}.TableName()
@@ -231,7 +225,7 @@ func (p *Log) PostSoftwareLog(ctx *fiber.Ctx) error {
 	}
 
 	if err := p.db.Create(&log).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't create Log", "db error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "db error")
 	}
 
 	return ctx.JSON(&log)

internal/handlers/software.go

@@ -149,16 +149,12 @@ func (p *Software) GetSoftware(ctx *fiber.Ctx) error {
 
 // PostSoftware creates a new software.
 func (p *Software) PostSoftware(ctx *fiber.Ctx) error {
-	softwareReq := new(common.SoftwarePost)
+	const errMsg = "can't create Software"
 
-	if err := ctx.BodyParser(&softwareReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't create Software", "invalid json")
-	}
+	softwareReq := new(common.SoftwarePost)
 
-	if err := common.ValidateStruct(*softwareReq); err != nil {
-		return common.ErrorWithValidationErrors(
-			fiber.StatusUnprocessableEntity, "can't create Software", err,
-		)
+	if err := common.ValidateRequestEntity(ctx, softwareReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	aliases := []models.SoftwareURL{}
@@ -180,14 +176,16 @@ func (p *Software) PostSoftware(ctx *fiber.Ctx) error {
 	}
 
 	if err := p.db.Create(&software).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't create Software", err.Error())
+		return common.Error(fiber.StatusInternalServerError, errMsg, err.Error())
 	}
 
 	return ctx.JSON(&software)
 }
 
 // PatchSoftware updates the software with the given ID.
-func (p *Software) PatchSoftware(ctx *fiber.Ctx) error { //nolint:cyclop // mostly error handling ifs
+func (p *Software) PatchSoftware(ctx *fiber.Ctx) error {
+	const errMsg = "can't update Software"
+
 	softwareReq := new(common.SoftwarePatch)
 	software := models.Software{}
 
@@ -202,14 +200,8 @@ func (p *Software) PatchSoftware(ctx *fiber.Ctx) error { //nolint:cyclop // most
 		return common.Error(fiber.StatusInternalServerError, "can't update Software", "internal server error")
 	}
 
-	if err := ctx.BodyParser(softwareReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't update Software", "invalid json")
-	}
-
-	if err := common.ValidateStruct(*softwareReq); err != nil {
-		return common.ErrorWithValidationErrors(
-			fiber.StatusUnprocessableEntity, "can't update Software", err,
-		)
+	if err := common.ValidateRequestEntity(ctx, softwareReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	// Slice of urls that we expect in the database after the PATCH (url + aliases)

internal/handlers/webhooks.go

@@ -114,18 +114,14 @@ func (p *Webhook[T]) GetSingleResourceWebhooks(ctx *fiber.Ctx) error {
 // PostSingleResourceWebhook creates a new webhook associated to resources
 // (fe. Software, Publishers) and returns any error encountered.
 func (p *Webhook[T]) PostResourceWebhook(ctx *fiber.Ctx) error {
+	const errMsg = "can't create Webhook"
+
 	webhookReq := new(common.Webhook)
 
 	var resource T
 
-	if err := ctx.BodyParser(&webhookReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't create Webhook", "invalid json")
-	}
-
-	if err := common.ValidateStruct(*webhookReq); err != nil {
-		return common.ErrorWithValidationErrors(
-			fiber.StatusUnprocessableEntity, "can't create Webhook", err,
-		)
+	if err := common.ValidateRequestEntity(ctx, webhookReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	webhook := models.Webhook{
@@ -137,7 +133,7 @@ func (p *Webhook[T]) PostResourceWebhook(ctx *fiber.Ctx) error {
 	}
 
 	if err := p.db.Create(&webhook).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't create Webhook", "db error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "db error")
 	}
 
 	return ctx.JSON(&webhook)
@@ -146,6 +142,8 @@ func (p *Webhook[T]) PostResourceWebhook(ctx *fiber.Ctx) error {
 // PostResourceWebhook creates a new webhook associated to a resource with the given ID
 // (fe. a specific Software or Publisher) and returns any error encountered.
 func (p *Webhook[T]) PostSingleResourceWebhook(ctx *fiber.Ctx) error {
+	const errMsg = "can't create Webhook"
+
 	webhookReq := new(common.Webhook)
 
 	var resource T
@@ -162,14 +160,8 @@ func (p *Webhook[T]) PostSingleResourceWebhook(ctx *fiber.Ctx) error {
 		)
 	}
 
-	if err := ctx.BodyParser(&webhookReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't create Webhook", "invalid json")
-	}
-
-	if err := common.ValidateStruct(*webhookReq); err != nil {
-		return common.ErrorWithValidationErrors(
-			fiber.StatusUnprocessableEntity, "can't create Webhook", err,
-		)
+	if err := common.ValidateRequestEntity(ctx, webhookReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	webhook := models.Webhook{
@@ -181,44 +173,40 @@ func (p *Webhook[T]) PostSingleResourceWebhook(ctx *fiber.Ctx) error {
 	}
 
 	if err := p.db.Create(&webhook).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't create Webhook", "db error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "db error")
 	}
 
 	return ctx.JSON(&webhook)
 }
 
 // PatchWebhook updates the webhook with the given ID.
 func (p *Webhook[T]) PatchWebhook(ctx *fiber.Ctx) error {
-	webhookReq := new(common.Webhook)
+	const errMsg = "can't update Webhook"
 
-	if err := ctx.BodyParser(webhookReq); err != nil {
-		return common.Error(fiber.StatusBadRequest, "can't update Webhook", "invalid json")
-	}
+	webhookReq := new(common.Webhook)
 
-	if err := common.ValidateStruct(*webhookReq); err != nil {
-		return common.ErrorWithValidationErrors(
-			fiber.StatusUnprocessableEntity, "can't update Webhook", err,
-		)
+	if err := common.ValidateRequestEntity(ctx, webhookReq, errMsg); err != nil {
+		return err //nolint:wrapcheck
 	}
 
 	webhook := models.Webhook{}
 
 	if err := p.db.First(&webhook, "id = ?", ctx.Params("id")).Error; err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
-			return common.Error(fiber.StatusNotFound, "can't update Webhook", "Webhook was not found")
+			return common.Error(fiber.StatusNotFound, errMsg, "Webhook was not found")
 		}
 
 		return common.Error(
 			fiber.StatusInternalServerError,
-			"can't update Webhook",
+			errMsg,
 			fiber.ErrInternalServerError.Message,
 		)
 	}
 
 	webhook.URL = webhookReq.URL
 
 	if err := p.db.Updates(&webhook).Error; err != nil {
-		return common.Error(fiber.StatusInternalServerError, "can't update Webhook", "db error")
+		return common.Error(fiber.StatusInternalServerError, errMsg, "db error")
 	}
 
 	return ctx.JSON(&webhook)

internal/jsondecoder/jsondecoder.go

@@ -0,0 +1,43 @@
+package jsondecoder
+
+import (
+	"bytes"
+	"encoding/json"
+	"errors"
+	"strings"
+)
+
+var (
+	ErrExtraDataAfterDecoding = errors.New("extra data after decoding")
+	ErrUnknownField           = errors.New("unknown field in JSON input")
+)
+
+// UnmarshalDisallowUnknownFieldsUnmarshal parses the JSON-encoded data
+// and stores the result in the value pointed to by v like json.Unmarshal,
+// but with DisallowUnknownFields() set by default for extra security.
+func UnmarshalDisallowUnknownFields(data []byte, v interface{}) error {
+	dec := json.NewDecoder(bytes.NewReader(data))
+	dec.DisallowUnknownFields()
+
+	if err := dec.Decode(v); err != nil {
+		// Ugly, but the encoding/json uses a dynamic error here
+		if strings.HasPrefix(err.Error(), "json: unknown field ") {
+			return ErrUnknownField
+		}
+
+		// we want to provide an alternative implementation, with the
+		// unwrapped errors
+		//nolint:wrapcheck
+		return err
+	}
+
+	// Check if there's any data left in the decoder's buffer.
+	// This ensures that there's no extra JSON after the main object
+	// otherwise something like '{"foo": 1}{"bar": 2}' or even '{}garbage'
+	// will not error out.
+	if dec.More() {
+		return ErrExtraDataAfterDecoding
+	}
+
+	return nil
+}

main.go

@@ -11,6 +11,7 @@ import (
 	"github.com/italia/developers-italia-api/internal/common"
 	"github.com/italia/developers-italia-api/internal/database"
 	"github.com/italia/developers-italia-api/internal/handlers"
+	"github.com/italia/developers-italia-api/internal/jsondecoder"
 	"github.com/italia/developers-italia-api/internal/middleware"
 	"github.com/italia/developers-italia-api/internal/models"
 	"github.com/italia/developers-italia-api/internal/webhooks"
@@ -54,6 +55,9 @@ func Setup() *fiber.App {
 
 	app := fiber.New(fiber.Config{
 		ErrorHandler: common.CustomErrorHandler,
+		// Fiber doesn't set DisallowUnknownFields by default
+		// (https://github.com/gofiber/fiber/issues/2601)
+		JSONDecoder: jsondecoder.UnmarshalDisallowUnknownFields,
 	})
 
 	// Automatically recover panics in handlers