florianheinemann
Cross domain requests
I do not test it but I think that redirecting should not work with cross domain request request because OPTIONS method will produce 403.
https://github.com/florianheinemann/express-sslify/blob/master/index.js#L25
Interesting aspect - I'm not sure at the moment if we should redirect such requests or let them fail. At least from client side, the W3C spec seems to indicate that only GET and HEAD should be redirected without involving the user.
I'm certainly no expert on cross domain requests, but considering that the OPTIONS preflight requests are likely not something that a users would trigger themselves but something that a script would do, I feel like it would be correct to fail that request to tell the developers that they should send this request using save means.
On a different note: I clearly need to add HEAD to the list of requests to be redirected!
Any other thoughts?
hmm interesting. I do not know. I'm working on something pretty similar for koa. Currently it's not handling methods at all, but I want to do this as much compatible with express-sslify as posible.
Maybe we should add new option like wihitelistedMethods or allowedMethods which will be array and made ['GET', 'HEAD'] the default. Then it will be "best practices" by default but still flexible to customize.
the final condition should look like if (isInArray(options.whitelistedMethods, req.method)) and implement isInArray function or use something like lodash for this (then It will make sense to use lodash also for options merging)
Good idea but might bloat the code quite a bit. For each method there might be very specific things to consider. E.g. there simply is no way to redirect a POST request. Not sure about what kind of HTTP flags, etc. would have to be considered for a PUT redirect, ... Put open for any suggestions
sure, you're right... 307 is right status for "redirecting" POST/PUT. So for me it makes sense to respond on OPTIONS with 200 OK if cors are allowed. But I'm not 100% sure. Maybe this is not case at all since middleware like this should be placed before sslify itself.
But there should be support for 307 maybe (default off).
My implementation is here: https://github.com/turboMaCk/koa-sslify/pull/5
It's little bit different – I've used options for methods whitelist setup and also add support for internal redirect