Allow header support in Router, MethodNotFoundHandler (405) and CORS middleware

Author: aldas

context.go

@@ -210,6 +210,13 @@ type (
 	}
 )
 
+const (
+	// ContextKeyHeaderAllow is set by Router for getting value for `Allow` header in later stages of handler call chain.
+	// Allow header is mandatory for status 405 (method not found) and useful for OPTIONS method requests.
+	// It is added to context only when Router does not find matching method handler for request.
+	ContextKeyHeaderAllow = "____echo____header_allow"
+)
+
 const (
 	defaultMemory = 32 << 20 // 32 MB
 	indexPage     = "index.html"

echo.go

@@ -190,8 +190,11 @@ const (
 
 // Headers
 const (
-	HeaderAccept              = "Accept"
-	HeaderAcceptEncoding      = "Accept-Encoding"
+	HeaderAccept         = "Accept"
+	HeaderAcceptEncoding = "Accept-Encoding"
+	// HeaderAllow is header field that lists the set of methods advertised as supported by the target resource.
+	// Allow header is mandatory for status 405 (method not found) and useful OPTIONS method responses.
+	// See: https://datatracker.ietf.org/doc/html/rfc7231#section-7.4.1
 	HeaderAllow               = "Allow"
 	HeaderAuthorization       = "Authorization"
 	HeaderContentDisposition  = "Content-Disposition"
@@ -302,6 +305,13 @@ var (
 	}
 
 	MethodNotAllowedHandler = func(c Context) error {
+		// 'Allow' header RFC: https://datatracker.ietf.org/doc/html/rfc7231#section-7.4.1
+		// >> An origin server MUST generate an Allow field in a 405 (Method Not Allowed) response
+		//    and MAY do so in any other response.
+		routerAllowMethods, ok := c.Get(ContextKeyHeaderAllow).(string)
+		if ok && routerAllowMethods != "" {
+			c.Response().Header().Set(HeaderAllow, routerAllowMethods)
+		}
 		return ErrMethodNotAllowed
 	}
 )

echo_test.go

@@ -716,13 +716,16 @@ func TestEchoNotFound(t *testing.T) {
 
 func TestEchoMethodNotAllowed(t *testing.T) {
 	e := New()
+
 	e.GET("/", func(c Context) error {
 		return c.String(http.StatusOK, "Echo!")
 	})
 	req := httptest.NewRequest(http.MethodPost, "/", nil)
 	rec := httptest.NewRecorder()
 	e.ServeHTTP(rec, req)
+
 	assert.Equal(t, http.StatusMethodNotAllowed, rec.Code)
+	assert.Equal(t, "OPTIONS, GET", rec.Header().Get(HeaderAllow))
 }
 
 func TestEchoContext(t *testing.T) {

middleware/cors.go

@@ -29,6 +29,8 @@ type (
 		// AllowMethods defines a list methods allowed when accessing the resource.
 		// This is used in response to a preflight request.
 		// Optional. Default value DefaultCORSConfig.AllowMethods.
+		// If `allowMethods` is left empty will fill for preflight request `Access-Control-Allow-Methods` header value
+		// from `Allow` header that echo.Router set into context.
 		AllowMethods []string `yaml:"allow_methods"`
 
 		// AllowHeaders defines a list of request headers that can be used when
@@ -41,6 +43,8 @@ type (
 		// a response to a preflight request, this indicates whether or not the
 		// actual request can be made using credentials.
 		// Optional. Default value false.
+		// Security: avoid using `AllowCredentials = true` with `AllowOrigins = *`.
+		// See http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
 		AllowCredentials bool `yaml:"allow_credentials"`
 
 		// ExposeHeaders defines a whitelist headers that clients are allowed to
@@ -80,7 +84,9 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 	if len(config.AllowOrigins) == 0 {
 		config.AllowOrigins = DefaultCORSConfig.AllowOrigins
 	}
+	hasCustomAllowMethods := true
 	if len(config.AllowMethods) == 0 {
+		hasCustomAllowMethods = false
 		config.AllowMethods = DefaultCORSConfig.AllowMethods
 	}
 
@@ -109,10 +115,28 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 			origin := req.Header.Get(echo.HeaderOrigin)
 			allowOrigin := ""
 
-			preflight := req.Method == http.MethodOptions
 			res.Header().Add(echo.HeaderVary, echo.HeaderOrigin)
 
-			// No Origin provided
+			// Preflight request is an OPTIONS request, using three HTTP request headers: Access-Control-Request-Method,
+			// Access-Control-Request-Headers, and the Origin header. See: https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
+			// For simplicity we just consider method type and later `Origin` header.
+			preflight := req.Method == http.MethodOptions
+
+			// Although router adds special handler in case of OPTIONS method we avoid calling next for OPTIONS in this middleware
+			// as CORS requests do not have cookies / authentication headers by default, so we could get stuck in auth
+			// middlewares by calling next(c).
+			// But we still want to send `Allow` header as response in case of Non-CORS OPTIONS request as router default
+			// handler does.
+			routerAllowMethods := ""
+			if preflight {
+				tmpAllowMethods, ok := c.Get(echo.ContextKeyHeaderAllow).(string)
+				if ok && tmpAllowMethods != "" {
+					routerAllowMethods = tmpAllowMethods
+					c.Response().Header().Set(echo.HeaderAllow, routerAllowMethods)
+				}
+			}
+
+			// No Origin provided. This is (probably) not request from actual browser - proceed executing middleware chain
 			if origin == "" {
 				if !preflight {
 					return next(c)
@@ -145,19 +169,15 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 					}
 				}
 
-				// Check allowed origin patterns
-				for _, re := range allowOriginPatterns {
-					if allowOrigin == "" {
-						didx := strings.Index(origin, "://")
-						if didx == -1 {
-							continue
-						}
-						domAuth := origin[didx+3:]
-						// to avoid regex cost by invalid long domain
-						if len(domAuth) > 253 {
-							break
-						}
-
+				checkPatterns := false
+				if allowOrigin == "" {
+					// to avoid regex cost by invalid (long) domains (253 is domain name max limit)
+					if len(origin) <= (253+3+4) && strings.Contains(origin, "://") {
+						checkPatterns = true
+					}
+				}
+				if checkPatterns {
+					for _, re := range allowOriginPatterns {
 						if match, _ := regexp.MatchString(re, origin); match {
 							allowOrigin = origin
 							break
@@ -174,12 +194,13 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 				return c.NoContent(http.StatusNoContent)
 			}
 
+			res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowOrigin)
+			if config.AllowCredentials {
+				res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
+			}
+
 			// Simple request
 			if !preflight {
-				res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowOrigin)
-				if config.AllowCredentials {
-					res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
-				}
 				if exposeHeaders != "" {
 					res.Header().Set(echo.HeaderAccessControlExposeHeaders, exposeHeaders)
 				}
@@ -189,11 +210,13 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
 			// Preflight request
 			res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestMethod)
 			res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestHeaders)
-			res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowOrigin)
-			res.Header().Set(echo.HeaderAccessControlAllowMethods, allowMethods)
-			if config.AllowCredentials {
-				res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
+
+			if !hasCustomAllowMethods && routerAllowMethods != "" {
+				res.Header().Set(echo.HeaderAccessControlAllowMethods, routerAllowMethods)
+			} else {
+				res.Header().Set(echo.HeaderAccessControlAllowMethods, allowMethods)
 			}
+
 			if allowHeaders != "" {
 				res.Header().Set(echo.HeaderAccessControlAllowHeaders, allowHeaders)
 			} else {