CORS middleware doesnt automatically handle OPTIONS routes for groups anymore since upgrade to v5

[philicious]

Not sure if this intended behavior or a regression:

I have had code working with echo v4 like

r := e.Group("/myroute", middleware.CORSWithConfig(middleware.CORSConfig{
	AllowOrigins: []string{
		"http://localhost:3000", // local frontend
		"https://mydomain.de",
	},
	AllowHeaders: []string{echo.HeaderOrigin, echo.HeaderContentType, echo.HeaderAccept, echo.HeaderAuthorization},
}))

r.GET("", controller.GetStuff)
...

now after upgrading to v5 I suddenly experience CORS errors

Access to XMLHttpRequest at XX from origin 'https://mydomain.de' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource.

and indeed the header is missing.

A fix was to explicitly add routes for OPTIONS on all groups with CORS middleware

func addPreflight(g *echo.Group) {
	h := func(c *echo.Context) error {
		return c.NoContent(http.StatusNoContent)
	}

	g.OPTIONS("", h)
	g.OPTIONS("/", h)
	g.OPTIONS("/*", h)
}

further investigation showed the CORS middleware HandlerFunc isnt invoked at all for OPTIONS-method if not explicitly added to routes in case CORS is added to a group.

known good version: v4.15.1

however I didnt see mentions of that in migration guide. did I miss them? is this new expected behavior?

[aldas]

This is not actually CORS middleware related. In v5 the group does not register implicit RouteNotFound routes on same path and if there are no routes the middleware will not be run.

This is what happened when you created group with middleware in v4 (g.Use is called also during group creation)

echo/group.go

Lines 22 to 33 in 6f3a84a

	func (g *Group) Use(middleware ...MiddlewareFunc) {
	g.middleware = append(g.middleware, middleware...)
	if len(g.middleware) == 0 {
	return
	}
	// group level middlewares are different from Echo `Pre` and `Use` middlewares (those are global). Group level middlewares
	// are only executed if they are added to the Router with route.
	// So we register catch all route (404 is a safe way to emulate route match) for this group and now during routing the
	// Router would find route to match our request path and therefore guarantee the middleware(s) will get executed.
	g.RouteNotFound("", NotFoundHandler)
	g.RouteNotFound("/*", NotFoundHandler)
	}

and this is v5

echo/group.go

Lines 22 to 24 in 0143b9d

	func (g *Group) Use(middleware ...MiddlewareFunc) {
	g.middleware = append(g.middleware, middleware...)
	}

I think at very least we need to update docs about this

[philicious]

@aldas thanks for the swift response.
I agree it should be at least documented in the Migration Guide and probably also in the CORS doc, as Preflight requests are tightly coupled to CORS (https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request)

I would even say this v5 change is debatable as it reduces the simplicity of the CORS middleware and requires deeper knowledge on user side of how browsers deal with CORS. As typically users dont implement preflight calls themselves in a web frontend.
(Its also confusing if middleware behaves differently in a group)

What was the motivation of removing the catch-all? Maybe it could be satisfied while preserving former CORS handling.

Also, what is the suggested way to handle preflight requests now? Just adding options-routes as I did?

[aldas]

well, reason to remove catch-all 404 routes on group was because these were confusing for some routing related cases - as it is so hidden feature. and seems when it comes to CORS we have "whack-a-mole situation" when it comes to groups.

I do not know the proper solution at the moment, but I will update the migration guide and add remark about it in website.

another thing - CORS middleware on group is a thing that seems to be overlooked. When it comes to me personally - I generally slam it on whole application and ... and knowing all the use-cases and finding solution that works with all of them is quite hard.

[aldas]

btw - could you describe the use-case why you decided to add CORS on specific route? is it something generic as "/api"?

[philicious]

btw - could you describe the use-case why you decided to add CORS on specific route? is it something generic as "/api"?

basically I have a public endpoint that uses less strict CORS and afterwards I group the private endpoints with more strict CORS

	e := echo.New()
....

	// the public endpoint
	e.GET("/config", controller.GetConfig, middleware.CORSWithConfig(middleware.CORSConfig{
		AllowOrigins: []string{
			"*",
		},
		AllowHeaders: []string{echo.HeaderOrigin, echo.HeaderContentType, echo.HeaderAccept, echo.HeaderCookie, echo.HeaderAuthorization},
	}))

	// the private endpoints
	k := e.Group("/private1", middleware.CORSWithConfig(middleware.CORSConfig{
		AllowOrigins: []string{
			"http://localhost:3000", // local frontend
			"https://my-domain.de",
		},
		AllowHeaders: []string{echo.HeaderOrigin, echo.HeaderContentType, echo.HeaderAccept, echo.HeaderAuthorization},
	}))

       // CRUD routes...
       k.POST("/foo", controller.CreateFoo)

[aldas]

Thanks, I did not even think of multiple different CORS configurations per application.

Anyway, go with explicit OPTIONS route at the moment and I think what we can do to improve the situation.

[philicious]

alright. I am happy to have come to a conclusion and a fix for now. thanks for that 👍

looking forward to any updates in the future that might come regarding this. feel free to close the issue after updating docs or so