Skip to content

Commit

Permalink
🎨 Style!: Update CSRF and Limiter to remove repetitive names (#2846)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nickajacks1 authored Feb 10, 2024
1 parent 70067a1 commit 97da409
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 35 deletions.
2 changes: 0 additions & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions docs/api/middleware/csrf.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:<name>`.
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:<name>`.
:::

:::note
Expand Down Expand Up @@ -206,7 +206,7 @@ var ConfigDefault = Config{
Expiration: 1 * time.Hour,
KeyGenerator: utils.UUIDv4,
ErrorHandler: defaultErrorHandler,
Extractor: CsrfFromHeader(HeaderName),
Extractor: FromHeader(HeaderName),
SessionKey: "csrfToken",
}
```
Expand All @@ -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",
}
Expand Down
12 changes: 6 additions & 6 deletions middleware/csrf/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ var ConfigDefault = Config{
Expiration: 1 * time.Hour,
KeyGenerator: utils.UUIDv4,
ErrorHandler: defaultErrorHandler,
Extractor: CsrfFromHeader(HeaderName),
Extractor: FromHeader(HeaderName),
SessionKey: "csrfToken",
}

Expand Down Expand Up @@ -169,23 +169,23 @@ 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")
}
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
}
}
Expand Down
25 changes: 13 additions & 12 deletions middleware/csrf/csrf.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ var (
dummyValue = []byte{'+'}
)

type CSRFHandler struct {
// Handler handles
type Handler struct {
config *Config
sessionManager *sessionManager
storageManager *storageManager
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
20 changes: 10 additions & 10 deletions middleware/csrf/extractors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand All @@ -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 == "" {
Expand All @@ -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 == "" {
Expand All @@ -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 == "" {
Expand All @@ -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 == "" {
Expand Down
2 changes: 1 addition & 1 deletion middleware/limiter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion middleware/limiter/limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const (
xRateLimitReset = "X-RateLimit-Reset"
)

type LimiterHandler interface {
type Handler interface {
New(config Config) fiber.Handler
}

Expand Down

0 comments on commit 97da409

Please sign in to comment.