[Router] Add options routes when CORS is used

[fridgerator]

When building sites with SPA's, preflight requests are sent before all requests. Currently the only way to handle this is to create separateoptions routes for each route used and a middleware to immediately return a 200 response:

resources "/lists", ListController
options "/lists", ListController, :index
options "/lists/:id", ListController, :show

I haven't looked at how other frameworks handle this, but a user shouldn't have to also include options routes when using CORS.

[drujensen]

@fridgerator Kemal and Kemalyst allows for handlers to happen before the router. This is an example of where that comes in handy since you can look for options or head requests for all requests and properly handle them before requiring a route to be looked up.

@eliasjpr @elorest What do you think? Should we look at allowing handlers to be added before the pipeline takes hold?

[eliasjpr]

I consider that a HEAD request should run thru the entire pipeline since a header can be set at the controller action level or after.

As per the OPTIONS this needs to be handled by the CORS handler and if the current request is a preflight request it should return as soon as the CORS pipe is hit. So it is likely to recommend that this pipe should be the first on the list for a given pipeline. Also I think we should add the Options requests by default unlike having the developer define those routes.

pipeline :api do
   plug CORS.new
   ...
end 

We can also create Preflight pipe to separate concerns.

[NeverHappened]

Tried to add options route as long as developer uses the CORS pipeline.
Not sure if this is the right way though.

#659

[eliasjpr]

@NeverHappened thank you for contributing to this issue. I found your approach viable given the way things are structured.

Personally was considering to just white-list any OPTIONS requests considering that these type of requests are issued in the background almost exclusively by the web browser and are practically invisible to the developer. So if an OPTIONS requests is received and the handler is not present the request does not get handled at all and a RouteNotFound exception is raised, otherwise if the CORS handler is present it then gets processed accordingly.

I have made some improvements to the CORS handler here #665 and would like your input and review. :)

[NeverHappened]

@eliasjpr Cool :)

The only issue I think about is how does it work since request first goes through the

@route = router.match_by_request(@request)

and then if it matches it goes to the pipeline? There are no options route so it will fail. Or didn't I understand something?

[eliasjpr]

@NeverHappened you are correct. What I mean is that we can add an OPTIONS route by default for all routes, just like we do with HEAD routes (See https://github.com/amberframework/amber/blob/master/src/amber/router/router.cr#L35). Then all OPTIONS routes will be matched, at the time of processing an OPTIONS request, we just have to see if CORS is enabled for that pipeline.

[NeverHappened]

@eliasjpr Great! That's a lot better than my solution.

[eliasjpr]

@NeverHappened since you authored the initial PR you can take on the suggested approach. WDYT?

[NeverHappened]

Yes, of course!

[eliasjpr]

Please see the proposed change here https://github.com/amberframework/amber/pull/700/files

[eliasjpr]

Considered fixed with #659 #700

• When defining routes using get, post, put, patch, delete will automatically create OPTIONS routes and HEAD routes for get requests.
• Developers can still define routes using the route methods head and options

To define routes for SPA apps:

Amber::Server.configure do |app|
  pipeline :web do
    # Add CORS pipe, 
    # for initialization options 
    # see https://github.com/amberframework/amber/blob/master/src/amber/pipes/cors.cr#L28 
    plug Amber::Pipe::CORS.new
  end

  routes :web do
    # Define your routes as normal, `get, post, put, patch, delete` 
    # methods will define a OPTIONS route by default
    get "/", HomeController, :static
  end
end