Fix(🩹): error handling in CSRF token storage retrieval

middleware/csrf/csrf.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/gofiber/fiber/v3"
+	"github.com/gofiber/fiber/v3/log"
 )
 
 var (
@@ -18,8 +19,10 @@ var (
 	ErrRefererNoMatch  = errors.New("referer does not match host and is not a trusted origin")
 	ErrOriginInvalid   = errors.New("origin invalid")
 	ErrOriginNoMatch   = errors.New("origin does not match host and is not a trusted origin")
-	errOriginNotFound  = errors.New("origin not supplied or is null") // internal error, will not be returned to the user
-	dummyValue         = []byte{'+'}
+	ErrNotGetStorage   = errors.New("unable to retrieve data from CSRF storage")
+
+	errOriginNotFound = errors.New("origin not supplied or is null") // internal error, will not be returned to the user
+	dummyValue        = []byte{'+'}
 )
 
 // Handler for CSRF middleware
@@ -102,10 +105,9 @@ func New(config ...Config) fiber.Handler {
 		switch c.Method() {
 		case fiber.MethodGet, fiber.MethodHead, fiber.MethodOptions, fiber.MethodTrace:
 			cookieToken := c.Cookies(cfg.CookieName)
-
 			if cookieToken != "" {
-				raw := getRawFromStorage(c, cookieToken, cfg, sessionManager, storageManager)
-
+				// In this case, handling error doesn't make sense because we have validations after the switch.
+				raw, _ := getRawFromStorage(c, cookieToken, cfg, sessionManager, storageManager) //nolint:errcheck //the details are in the comment above
 				if raw != nil {
 					token = cookieToken // Token is valid, safe to set it
 				}
@@ -148,9 +150,10 @@ func New(config ...Config) fiber.Handler {
 				return cfg.ErrorHandler(c, ErrTokenInvalid)
 			}
 
-			raw := getRawFromStorage(c, extractedToken, cfg, sessionManager, storageManager)
+			raw, err := getRawFromStorage(c, extractedToken, cfg, sessionManager, storageManager)
+			if err != nil || raw == nil {
+				log.Error("Failed to retrieve CSRF token: ", err)
 
-			if raw == nil {
 				// If token is not in storage, expire the cookie
 				expireCSRFCookie(c, cfg)
 				// and return an error
@@ -209,7 +212,7 @@ func HandlerFromContext(c fiber.Ctx) *Handler {
 
 // getRawFromStorage returns the raw value from the storage for the given token
 // returns nil if the token does not exist, is expired or is invalid
-func getRawFromStorage(c fiber.Ctx, token string, cfg Config, sessionManager *sessionManager, storageManager *storageManager) []byte {
+func getRawFromStorage(c fiber.Ctx, token string, cfg Config, sessionManager *sessionManager, storageManager *storageManager) ([]byte, error) {
 	if cfg.Session != nil {
 		return sessionManager.getRaw(c, token, dummyValue)
 	}

middleware/csrf/session_manager.go

@@ -26,20 +26,23 @@
 }
 
 // get token from session
-func (m *sessionManager) getRaw(c fiber.Ctx, key string, raw []byte) []byte {
+func (m *sessionManager) getRaw(c fiber.Ctx, key string, raw []byte) ([]byte, error) {
 	sess, err := m.session.Get(c)
 	if err != nil {
-		return nil
+		log.Warn("csrf: failed to get session: ", err)
+		return nil, ErrTokenNotFound
 	}
+
 	token, ok := sess.Get(m.key).(Token)
-	if ok {
-		if token.Expiration.Before(time.Now()) || key != token.Key || !compareTokens(raw, token.Raw) {
-			return nil
-		}
-		return token.Raw
+	if !ok {
+		return nil, ErrTokenInvalid
+	}
+
+	if token.Expiration.Before(time.Now()) || key != token.Key || !compareTokens(raw, token.Raw) {
+		return nil, ErrTokenInvalid
 	}
 
-	return nil
+	return token.Raw, nil
 }
 
 // set token in session

middleware/csrf/storage_manager.go

@@ -1,11 +1,13 @@
 package csrf
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
 	"github.com/gofiber/fiber/v3"
 	"github.com/gofiber/fiber/v3/internal/memory"
+	"github.com/gofiber/fiber/v3/log"
 	"github.com/gofiber/utils/v2"
 )
 
@@ -40,20 +42,35 @@
 }
 
 // get raw data from storage or memory
-func (m *storageManager) getRaw(key string) []byte {
-	var raw []byte
+func (m *storageManager) getRaw(key string) ([]byte, error) {
+	var (
+		raw []byte
+		err error
+	)
 	if m.storage != nil {
-		raw, _ = m.storage.Get(key) //nolint:errcheck // TODO: Do not ignore error
+		raw, err = m.storage.Get(key)
+		if err != nil {
+			return nil, fmt.Errorf("%w: %s", ErrNotGetStorage, err.Error())
+		}
 	} else {
-		raw, _ = m.memory.Get(key).([]byte) //nolint:errcheck // TODO: Do not ignore error
+		var ok bool
+		raw, ok = m.memory.Get(key).([]byte)
+		if !ok {
+			return nil, ErrNotGetStorage
+		}
 	}
-	return raw
+
+	return raw, nil
 }
 
 // set data to storage or memory
 func (m *storageManager) setRaw(key string, raw []byte, exp time.Duration) {
 	if m.storage != nil {
-		_ = m.storage.Set(key, raw, exp) //nolint:errcheck // TODO: Do not ignore error
+		err := m.storage.Set(key, raw, exp)
+		if err != nil {
+			log.Warnf("csrf: failed to save session in storage: %s", err.Error())
+			return
+		}
 	} else {
 		// the key is crucial in crsf and sometimes a reference to another value which can be reused later(pool/unsafe values concept), so a copy is made here
 		m.memory.Set(utils.CopyString(key), raw, exp)
@@ -63,7 +80,11 @@
 // delete data from storage or memory
 func (m *storageManager) delRaw(key string) {
 	if m.storage != nil {
-		_ = m.storage.Delete(key) //nolint:errcheck // TODO: Do not ignore error
+		err := m.storage.Delete(key)
+		if err != nil {
+			log.Warnf("csrf: failed to delete session in storage: %s", err.Error())
+			return
+		}
 	} else {
 		m.memory.Delete(key)
 	}