Dynamic CORS handling based on path params #2510

Activity

1. jonasps commented on Sep 1, 2023

   

   jonasps

   on Sep 1, 2023

   More actions

   Hi, you have an interesting problem, tough I believe that the CORS settings should only depend on the value from origin and nothing else. Mabey there is a better way of restricting the access to your application instead of CORS setting?
2. aldas commented on Sep 1, 2023

   

   aldas

   on Sep 1, 2023

   Last edited by aldas

   Contributor

   More actions

   I'm building an authentication service that needs to use a certain parameter in the request's path parameters (c.Param("something")) to determine which URLs are allowed for CORS purposes.

   To be honest - I probably do not understand exactly the use-case but if we are talking about authentication I think you can achieve this by adding your authentication middleware before CORS middleware and detect things that you need.

   or you could conditionally execute cors like that

   func main() {
   	e := echo.New()
   
   	e.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
   		cors := middleware.CORS()
   		return func(c echo.Context) error {
   			param := c.Param("something")
   			if param == "nope" { // stop middleware/handler chain
   				return echo.ErrUnauthorized
   			}
   			if param == "skip" {
   				return next(c) // execute next middleware without CORS
   			}
   			return cors(next)(c) // execute CORS and then next middleware
   		}
   	})
   
   	e.GET("/:something", func(c echo.Context) error {
   		return c.String(http.StatusOK, "Hi")
   	})
   
   	if err := e.Start(":8080"); err != nil && !errors.Is(err, http.ErrServerClosed) {
   		e.Logger.Fatal(err)
   	}
   }

   👍React with 👍1Reacted by Jonas Sandström
3. jub0bs commented on Sep 12, 2023

   

   jub0bs

   on Sep 12, 2023

   Last edited by jub0bs

   More actions

   @bakatz I strongly believe that custom callbacks are a bad idea for CORS middleware. Why not simply be more selective as to which paths you apply the CORS middleware to at the router level?

   @aldas You write

   you can achieve this by adding your authentication middleware before CORS middleware

   However, since preflight requests are not themselves authenticated, CORS middleware must in general come before any authentication middleware.
4. aldas commented on Sep 12, 2023

   

   aldas

   on Sep 12, 2023

   Contributor

   More actions

   I would say - everything depends on the situation. Most of the time frameworks do not try to guess user intentions/requirements. If it were so then the CORS middleware would be mandatory/added implicitly.

   CORS middleware must in general come before any authentication middleware.

   there is always "skip" functionality you can add for cases when methodtype=OPTIONS as these request do not contain auth.

   I really do not know what @bakatz is building, but it could be some kind of highly dynamic virtual host supporting server and dynamically changing routes application with "weird" requirements - in that case when you are sure what you are doing you can bend the middleware/handler chain by adding conditions.

   Problem with (web)frameworks is that there are so many different (valid or not) user requirements so eventually every single line is configurable and writing configuration could be longer than writing your middleware from the scratch.
5. jub0bs commented on Sep 12, 2023

   

   jub0bs

   on Sep 12, 2023

   Last edited by jub0bs

   More actions

   If it were so then the CORS middleware would be mandatory/added implicitly

   Why? Why not adopt secure defaults (i.e. no CORS configuration whatsoever) instead?

   there is always "skip" functionality you can add for cases when methodtype=OPTIONS as these request do not contain auth.

   Careful, though: not all OPTIONS requests are preflight requests, and some OPTIONS requests may very well carry credentials.

   I had to check but the (erroneous) assumption that all OPTIONS requests are preflight requests is present in the CORS middleware. Perhaps I should open a separate issue about that...
6. aldas commented on Sep 12, 2023

   

   aldas

   on Sep 12, 2023

   Contributor

   More actions

   @jub0bs if there is a case where request is considered incorrectly preflight please create issue (preferably with example).

   currently it consider method + origin although Mozilla says

   it is an OPTIONS request, using two or three HTTP request headers: Access-Control-Request-Method, Origin, and optionally Access-Control-Request-Headers.

   but this probably is not something catastrophically wrong with that as long as browser requests are covered and non-browser calls (api2api etc) do not send "origin" nothing serious happens as CORS is important for browsers and OPTIONS for API2API calls is quite uncommon or we would have tickets about that. We usually have tickets that CORS does not work when CURLing application etc.
7. bakatz commented on Sep 12, 2023

   

   bakatz

   on Sep 12, 2023

   ContributorAuthor

   More actions

   I'm just going to close this - the solution I implemented already works and after some further consideration I don't think it's appropriate to add this to the echo codebase anyway, as it's a problem unique to what I'm doing.

   Thanks for the feedback.

   👍React with 👍1Reacted by jub0bs
8. bakatz

   closed this as completedon Sep 12, 2023
9. jub0bs commented on Sep 12, 2023

   

   jub0bs

   on Sep 12, 2023

   Last edited by jub0bs

   More actions

   @aldas The absence of ticket about this doesn't guarantee that the implementation is sound. I'll create a separate issue (with examples) when time allows.