From 97da409533060d3a01ee20409935542010b92b86 Mon Sep 17 00:00:00 2001 From: nickajacks1 <128185314+nickajacks1@users.noreply.github.com> Date: Sat, 10 Feb 2024 10:50:29 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20Style!:=20Update=20CSRF=20and=20?= =?UTF-8?q?Limiter=20to=20remove=20repetitive=20names=20(#2846)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit chore!: Update CSRF and Limiter to remove repetitive names The `exported` rule of revive warns to not repeat the package name in method names. For example, prefer `csrf.FromCookie` over `csrf.CsrfFromCookie`. This is a breaking change for v3. It appears that these issues will not be caught by the linter until the `exported` rule is reenabled. This requires comments on all exported symbols, which is a much broader effort. --- .golangci.yml | 2 -- docs/api/middleware/csrf.md | 6 +++--- middleware/csrf/config.go | 12 ++++++------ middleware/csrf/csrf.go | 25 +++++++++++++------------ middleware/csrf/extractors.go | 20 ++++++++++---------- middleware/limiter/config.go | 2 +- middleware/limiter/limiter.go | 2 +- 7 files changed, 34 insertions(+), 35 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c19f219e..3ef22c9e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -109,8 +109,6 @@ linters-settings: disabled: true - name: exported disabled: true - arguments: - - disableStutteringCheck # TODO https://github.com/gofiber/fiber/issues/2816 - name: file-header disabled: true - name: function-result-limit diff --git a/docs/api/middleware/csrf.md b/docs/api/middleware/csrf.md index cb051577..54c2d548 100644 --- a/docs/api/middleware/csrf.md +++ b/docs/api/middleware/csrf.md @@ -31,7 +31,7 @@ By default, the middleware generates and stores tokens using the `fiber.Storage` When the authorization status changes, the previously issued token MUST be deleted, and a new one generated. See [Token Lifecycle](#token-lifecycle) [Deleting Tokens](#deleting-tokens) for more information. :::caution -When using this pattern, it's important to set the `CookieSameSite` option to `Lax` or `Strict` and ensure that the Extractor is not `CsrfFromCookie`, and KeyLookup is not `cookie:`. +When using this pattern, it's important to set the `CookieSameSite` option to `Lax` or `Strict` and ensure that the Extractor is not `FromCookie`, and KeyLookup is not `cookie:`. ::: :::note @@ -206,7 +206,7 @@ var ConfigDefault = Config{ Expiration: 1 * time.Hour, KeyGenerator: utils.UUIDv4, ErrorHandler: defaultErrorHandler, - Extractor: CsrfFromHeader(HeaderName), + Extractor: FromHeader(HeaderName), SessionKey: "csrfToken", } ``` @@ -226,7 +226,7 @@ var ConfigDefault = Config{ Expiration: 1 * time.Hour, KeyGenerator: utils.UUIDv4, ErrorHandler: defaultErrorHandler, - Extractor: CsrfFromHeader(HeaderName), + Extractor: FromHeader(HeaderName), Session: session.Store, SessionKey: "csrfToken", } diff --git a/middleware/csrf/config.go b/middleware/csrf/config.go index cb26cb23..3902555d 100644 --- a/middleware/csrf/config.go +++ b/middleware/csrf/config.go @@ -117,7 +117,7 @@ var ConfigDefault = Config{ Expiration: 1 * time.Hour, KeyGenerator: utils.UUIDv4, ErrorHandler: defaultErrorHandler, - Extractor: CsrfFromHeader(HeaderName), + Extractor: FromHeader(HeaderName), SessionKey: "csrfToken", } @@ -169,15 +169,15 @@ func configDefault(config ...Config) Config { if cfg.Extractor == nil { // By default we extract from a header - cfg.Extractor = CsrfFromHeader(textproto.CanonicalMIMEHeaderKey(selectors[1])) + cfg.Extractor = FromHeader(textproto.CanonicalMIMEHeaderKey(selectors[1])) switch selectors[0] { case "form": - cfg.Extractor = CsrfFromForm(selectors[1]) + cfg.Extractor = FromForm(selectors[1]) case "query": - cfg.Extractor = CsrfFromQuery(selectors[1]) + cfg.Extractor = FromQuery(selectors[1]) case "param": - cfg.Extractor = CsrfFromParam(selectors[1]) + cfg.Extractor = FromParam(selectors[1]) case "cookie": if cfg.Session == nil { log.Warn("[CSRF] Cookie extractor is not recommended without a session store") @@ -185,7 +185,7 @@ func configDefault(config ...Config) Config { if cfg.CookieSameSite == "None" || cfg.CookieSameSite != "Lax" && cfg.CookieSameSite != "Strict" { log.Warn("[CSRF] Cookie extractor is only recommended for use with SameSite=Lax or SameSite=Strict") } - cfg.Extractor = CsrfFromCookie(selectors[1]) + cfg.Extractor = FromCookie(selectors[1]) cfg.CookieName = selectors[1] // Cookie name is the same as the key } } diff --git a/middleware/csrf/csrf.go b/middleware/csrf/csrf.go index 9801afd3..644d3901 100644 --- a/middleware/csrf/csrf.go +++ b/middleware/csrf/csrf.go @@ -17,7 +17,8 @@ var ( dummyValue = []byte{'+'} ) -type CSRFHandler struct { +// Handler handles +type Handler struct { config *Config sessionManager *sessionManager storageManager *storageManager @@ -58,7 +59,7 @@ func New(config ...Config) fiber.Handler { } // Store the CSRF handler in the context - c.Locals(handlerKey, &CSRFHandler{ + c.Locals(handlerKey, &Handler{ config: &cfg, sessionManager: sessionManager, storageManager: storageManager, @@ -98,10 +99,10 @@ func New(config ...Config) fiber.Handler { return cfg.ErrorHandler(c, ErrTokenNotFound) } - // If not using CsrfFromCookie extractor, check that the token matches the cookie + // If not using FromCookie extractor, check that the token matches the cookie // This is to prevent CSRF attacks by using a Double Submit Cookie method // Useful when we do not have access to the users Session - if !isCsrfFromCookie(cfg.Extractor) && !compareStrings(extractedToken, c.Cookies(cfg.CookieName)) { + if !isFromCookie(cfg.Extractor) && !compareStrings(extractedToken, c.Cookies(cfg.CookieName)) { return cfg.ErrorHandler(c, ErrTokenInvalid) } @@ -154,10 +155,10 @@ func TokenFromContext(c fiber.Ctx) string { return token } -// HandlerFromContext returns the CSRFHandler found in the context +// HandlerFromContext returns the Handler found in the context // returns nil if the handler does not exist -func HandlerFromContext(c fiber.Ctx) *CSRFHandler { - handler, ok := c.Locals(handlerKey).(*CSRFHandler) +func HandlerFromContext(c fiber.Ctx) *Handler { + handler, ok := c.Locals(handlerKey).(*Handler) if !ok { return nil } @@ -219,11 +220,11 @@ func setCSRFCookie(c fiber.Ctx, cfg Config, token string, expiry time.Duration) // DeleteToken removes the token found in the context from the storage // and expires the CSRF cookie -func (handler *CSRFHandler) DeleteToken(c fiber.Ctx) error { +func (handler *Handler) DeleteToken(c fiber.Ctx) error { // Get the config from the context config := handler.config if config == nil { - panic("CSRFHandler config not found in context") + panic("CSRF Handler config not found in context") } // Extract token from the client request cookie cookieToken := c.Cookies(config.CookieName) @@ -237,9 +238,9 @@ func (handler *CSRFHandler) DeleteToken(c fiber.Ctx) error { return nil } -// isCsrfFromCookie checks if the extractor is set to ExtractFromCookie -func isCsrfFromCookie(extractor any) bool { - return reflect.ValueOf(extractor).Pointer() == reflect.ValueOf(CsrfFromCookie).Pointer() +// isFromCookie checks if the extractor is set to ExtractFromCookie +func isFromCookie(extractor any) bool { + return reflect.ValueOf(extractor).Pointer() == reflect.ValueOf(FromCookie).Pointer() } // refererMatchesHost checks that the referer header matches the host header diff --git a/middleware/csrf/extractors.go b/middleware/csrf/extractors.go index 1396d2ec..8e4a119f 100644 --- a/middleware/csrf/extractors.go +++ b/middleware/csrf/extractors.go @@ -14,8 +14,8 @@ var ( ErrMissingCookie = errors.New("missing csrf token in cookie") ) -// csrfFromParam returns a function that extracts token from the url param string. -func CsrfFromParam(param string) func(c fiber.Ctx) (string, error) { +// FromParam returns a function that extracts token from the url param string. +func FromParam(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.Params(param) if token == "" { @@ -25,8 +25,8 @@ func CsrfFromParam(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromForm returns a function that extracts a token from a multipart-form. -func CsrfFromForm(param string) func(c fiber.Ctx) (string, error) { +// FromForm returns a function that extracts a token from a multipart-form. +func FromForm(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.FormValue(param) if token == "" { @@ -36,8 +36,8 @@ func CsrfFromForm(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromCookie returns a function that extracts token from the cookie header. -func CsrfFromCookie(param string) func(c fiber.Ctx) (string, error) { +// FromCookie returns a function that extracts token from the cookie header. +func FromCookie(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.Cookies(param) if token == "" { @@ -47,8 +47,8 @@ func CsrfFromCookie(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromHeader returns a function that extracts token from the request header. -func CsrfFromHeader(param string) func(c fiber.Ctx) (string, error) { +// FromHeader returns a function that extracts token from the request header. +func FromHeader(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.Get(param) if token == "" { @@ -58,8 +58,8 @@ func CsrfFromHeader(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromQuery returns a function that extracts token from the query string. -func CsrfFromQuery(param string) func(c fiber.Ctx) (string, error) { +// FromQuery returns a function that extracts token from the query string. +func FromQuery(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := fiber.Query[string](c, param) if token == "" { diff --git a/middleware/limiter/config.go b/middleware/limiter/config.go index 291c85d4..3045282e 100644 --- a/middleware/limiter/config.go +++ b/middleware/limiter/config.go @@ -55,7 +55,7 @@ type Config struct { // LimiterMiddleware is the struct that implements a limiter middleware. // // Default: a new Fixed Window Rate Limiter - LimiterMiddleware LimiterHandler + LimiterMiddleware Handler } // ConfigDefault is the default config diff --git a/middleware/limiter/limiter.go b/middleware/limiter/limiter.go index e3e239f0..f6bba526 100644 --- a/middleware/limiter/limiter.go +++ b/middleware/limiter/limiter.go @@ -11,7 +11,7 @@ const ( xRateLimitReset = "X-RateLimit-Reset" ) -type LimiterHandler interface { +type Handler interface { New(config Config) fiber.Handler }