Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross domain requests #7

Open
turboMaCk opened this issue Oct 19, 2015 · 5 comments
Open

Cross domain requests #7

turboMaCk opened this issue Oct 19, 2015 · 5 comments

Comments

@turboMaCk
Copy link
Contributor

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

@florianheinemann
Copy link
Owner

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?

@turboMaCk
Copy link
Contributor Author

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)

@florianheinemann
Copy link
Owner

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

@turboMaCk
Copy link
Contributor Author

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).

@turboMaCk
Copy link
Contributor Author

My implementation is here: turboMaCk/koa-sslify#5
It's little bit different – I've used options for methods whitelist setup and also add support for internal redirect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants