From a25a40fac6161d3ccc9b88ec300d712afe576ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Gro=C3=9Fe?= Date: Sat, 2 Dec 2023 17:38:51 +0000 Subject: [PATCH] fix: don't constrain middlewares' context-keys to strings `context` recommends using "unexported type" as context keys to avoid collisions https://pkg.go.dev/github.com/gofiber/fiber/v2#Ctx.Locals. The official go blog also recommends this https://go.dev/blog/context. `fiber.Ctx.Locals(key any, value any)` correctly allows consumers to use unexported types or e.g. strings. But some fiber middlewares constrain their context-keys to `string` in their "default config structs", making it impossible to use unexported types. This PR removes the `string` _constraint_ from all middlewares, allowing to now use unexported types as per the official guidelines. However the default value is still a string, so it's not a breaking change, and anyone still using strings as context keys is not affected. --- docs/api/middleware/basicauth.md | 4 ++-- docs/api/middleware/csrf.md | 4 ++-- docs/api/middleware/keyauth.md | 2 +- middleware/adaptor/adaptor_test.go | 5 +++-- middleware/basicauth/config.go | 8 ++++---- middleware/csrf/config.go | 6 +++--- middleware/csrf/csrf.go | 2 +- middleware/idempotency/idempotency.go | 6 ++++-- middleware/keyauth/config.go | 4 ++-- 9 files changed, 22 insertions(+), 19 deletions(-) diff --git a/docs/api/middleware/basicauth.md b/docs/api/middleware/basicauth.md index 0e90eafed04..d0f3609bdef 100644 --- a/docs/api/middleware/basicauth.md +++ b/docs/api/middleware/basicauth.md @@ -67,8 +67,8 @@ app.Use(basicauth.New(basicauth.Config{ | Realm | `string` | Realm is a string to define the realm attribute of BasicAuth. The realm identifies the system to authenticate against and can be used by clients to save credentials. | `"Restricted"` | | Authorizer | `func(string, string) bool` | Authorizer defines a function to check the credentials. It will be called with a username and password and is expected to return true or false to indicate approval. | `nil` | | Unauthorized | `fiber.Handler` | Unauthorized defines the response body for unauthorized responses. | `nil` | -| ContextUsername | `string` | ContextUsername is the key to store the username in Locals. | `"username"` | -| ContextPassword | `string` | ContextPassword is the key to store the password in Locals. | `"password"` | +| ContextUsername | `interface{}` | ContextUsername is the key to store the username in Locals. | `"username"` | +| ContextPassword | `interface{}` | ContextPassword is the key to store the password in Locals. | `"password"` | ## Default Config diff --git a/docs/api/middleware/csrf.md b/docs/api/middleware/csrf.md index 2c30ed5cbed..536e8a4b2d2 100644 --- a/docs/api/middleware/csrf.md +++ b/docs/api/middleware/csrf.md @@ -152,14 +152,14 @@ app.Use(csrf.New(csrf.Config{ | Storage | `fiber.Storage` | Store is used to store the state of the middleware. | `nil` | | Session | `*session.Store` | Session is used to store the state of the middleware. Overrides Storage if set. | `nil` | | SessionKey | `string` | SessionKey is the key used to store the token within the session. | "fiber.csrf.token" | -| ContextKey | `string` | Context key to store the generated CSRF token into the context. If left empty, the token will not be stored within the context. | "" | +| ContextKey | `inteface{}` | Context key to store the generated CSRF token into the context. If left empty, the token will not be stored within the context. | "" | | KeyGenerator | `func() string` | KeyGenerator creates a new CSRF token. | utils.UUID | | CookieExpires | `time.Duration` (Deprecated) | Deprecated: Please use Expiration. | 0 | | Cookie | `*fiber.Cookie` (Deprecated) | Deprecated: Please use Cookie* related fields. | `nil` | | TokenLookup | `string` (Deprecated) | Deprecated: Please use KeyLookup. | "" | | ErrorHandler | `fiber.ErrorHandler` | ErrorHandler is executed when an error is returned from fiber.Handler. | DefaultErrorHandler | | Extractor | `func(*fiber.Ctx) (string, error)` | Extractor returns the CSRF token. If set, this will be used in place of an Extractor based on KeyLookup. | Extractor based on KeyLookup | -| HandlerContextKey | `string` | HandlerContextKey is used to store the CSRF Handler into context. | "fiber.csrf.handler" | +| HandlerContextKey | `interface{}` | HandlerContextKey is used to store the CSRF Handler into context. | "fiber.csrf.handler" | ### Default Config diff --git a/docs/api/middleware/keyauth.md b/docs/api/middleware/keyauth.md index ecabe122e70..1a719c134d0 100644 --- a/docs/api/middleware/keyauth.md +++ b/docs/api/middleware/keyauth.md @@ -221,7 +221,7 @@ curl --header "Authorization: Bearer my-super-secret-key" http://localhost:3000 | KeyLookup | `string` | KeyLookup is a string in the form of "`:`" that is used to extract key from the request. | "header:Authorization" | | AuthScheme | `string` | AuthScheme to be used in the Authorization header. | "Bearer" | | Validator | `func(*fiber.Ctx, string) (bool, error)` | Validator is a function to validate the key. | A function for key validation | -| ContextKey | `string` | Context key to store the bearer token from the token into context. | "token" | +| ContextKey | `interface{}` | Context key to store the bearer token from the token into context. | "token" | ## Default Config diff --git a/middleware/adaptor/adaptor_test.go b/middleware/adaptor/adaptor_test.go index 340c05c04b8..6e843a4158a 100644 --- a/middleware/adaptor/adaptor_test.go +++ b/middleware/adaptor/adaptor_test.go @@ -36,7 +36,8 @@ func Test_HTTPHandler(t *testing.T) { expectedURL, err := url.ParseRequestURI(expectedRequestURI) utils.AssertEqual(t, nil, err) - expectedContextKey := "contextKey" + type contextKeyType string + expectedContextKey := contextKeyType("contextKey") expectedContextValue := "contextValue" callsCount := 0 @@ -293,7 +294,7 @@ func testFiberToHandlerFunc(t *testing.T, checkDefaultPort bool, app ...*fiber.A utils.AssertEqual(t, expectedResponseBody, string(w.body), "Body") } -func setFiberContextValueMiddleware(next fiber.Handler, key string, value interface{}) fiber.Handler { +func setFiberContextValueMiddleware(next fiber.Handler, key interface{}, value interface{}) fiber.Handler { return func(c *fiber.Ctx) error { c.Locals(key, value) return next(c) diff --git a/middleware/basicauth/config.go b/middleware/basicauth/config.go index 3845e91538a..d69f48be67f 100644 --- a/middleware/basicauth/config.go +++ b/middleware/basicauth/config.go @@ -44,12 +44,12 @@ type Config struct { // ContextUser is the key to store the username in Locals // // Optional. Default: "username" - ContextUsername string + ContextUsername interface{} // ContextPass is the key to store the password in Locals // // Optional. Default: "password" - ContextPassword string + ContextPassword interface{} } // ConfigDefault is the default config @@ -95,10 +95,10 @@ func configDefault(config ...Config) Config { return c.SendStatus(fiber.StatusUnauthorized) } } - if cfg.ContextUsername == "" { + if cfg.ContextUsername == nil { cfg.ContextUsername = ConfigDefault.ContextUsername } - if cfg.ContextPassword == "" { + if cfg.ContextPassword == nil { cfg.ContextPassword = ConfigDefault.ContextPassword } return cfg diff --git a/middleware/csrf/config.go b/middleware/csrf/config.go index 539e84967c7..9ab6cab0632 100644 --- a/middleware/csrf/config.go +++ b/middleware/csrf/config.go @@ -93,7 +93,7 @@ type Config struct { // If left empty, token will not be stored in context. // // Optional. Default: "" - ContextKey string + ContextKey interface{} // KeyGenerator creates a new CSRF token // @@ -124,7 +124,7 @@ type Config struct { // HandlerContextKey is used to store the CSRF Handler into context // // Default: "fiber.csrf.handler" - HandlerContextKey string + HandlerContextKey interface{} } const HeaderName = "X-Csrf-Token" @@ -204,7 +204,7 @@ func configDefault(config ...Config) Config { if cfg.SessionKey == "" { cfg.SessionKey = ConfigDefault.SessionKey } - if cfg.HandlerContextKey == "" { + if cfg.HandlerContextKey == nil { cfg.HandlerContextKey = ConfigDefault.HandlerContextKey } diff --git a/middleware/csrf/csrf.go b/middleware/csrf/csrf.go index f4dee74c8ea..939daaea79a 100644 --- a/middleware/csrf/csrf.go +++ b/middleware/csrf/csrf.go @@ -129,7 +129,7 @@ func New(config ...Config) fiber.Handler { c.Vary(fiber.HeaderCookie) // Store the token in the context if a context key is specified - if cfg.ContextKey != "" { + if cfg.ContextKey != nil { c.Locals(cfg.ContextKey, token) } diff --git a/middleware/idempotency/idempotency.go b/middleware/idempotency/idempotency.go index 604f867c764..5affc59620d 100644 --- a/middleware/idempotency/idempotency.go +++ b/middleware/idempotency/idempotency.go @@ -12,9 +12,11 @@ import ( // Inspired by https://datatracker.ietf.org/doc/html/draft-ietf-httpapi-idempotency-key-header-02 // and https://github.com/penguin-statistics/backend-next/blob/f2f7d5ba54fc8a58f168d153baa17b2ad4a14e45/internal/pkg/middlewares/idempotency.go +type localsKeys string + const ( - localsKeyIsFromCache = "idempotency_isfromcache" - localsKeyWasPutToCache = "idempotency_wasputtocache" + localsKeyIsFromCache localsKeys = "idempotency_isfromcache" + localsKeyWasPutToCache localsKeys = "idempotency_wasputtocache" ) func IsFromCache(c *fiber.Ctx) bool { diff --git a/middleware/keyauth/config.go b/middleware/keyauth/config.go index 39d71b6305d..c762d72ce64 100644 --- a/middleware/keyauth/config.go +++ b/middleware/keyauth/config.go @@ -41,7 +41,7 @@ type Config struct { // Context key to store the bearertoken from the token into context. // Optional. Default: "token". - ContextKey string + ContextKey interface{} } // ConfigDefault is the default config @@ -87,7 +87,7 @@ func configDefault(config ...Config) Config { if cfg.Validator == nil { panic("fiber: keyauth middleware requires a validator function") } - if cfg.ContextKey == "" { + if cfg.ContextKey == nil { cfg.ContextKey = ConfigDefault.ContextKey }