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

support for adding CORS headers? #110

Closed
boyvinall opened this issue Jun 13, 2016 · 7 comments
Closed

support for adding CORS headers? #110

boyvinall opened this issue Jun 13, 2016 · 7 comments

Comments

@boyvinall
Copy link

Hi

Wanted to get your thoughts on possibility of some config options for adding CORS support. As a dev hack in one system I'm currently using nginx in front of fabio to do this, but that just seems wrong :) I don't really want to be adding CORS directly into the backend API as it's specific to the deployment. Using the consul backend, I'm thinking this should probably be supported via consul tags, so that it can be enabled only for specific services if needed. I might be able to spend some time on this if you think it's useful.

Thanks

@magiconair
Copy link
Contributor

That sounds like a good idea although you then make your service behavior dependent on fabio instead of nginx. This is the same as the strip prefix discussion but I get the impression that enough people want this so lets at least provide a mechanism to provide this properly.

Can you provide an example request including the headers fabio should add? Using tags will probably not work in all cases since a service can register multiple prefixes in different domains, e.g. a.com/foo and b.org/bar for which you may need to provide different headers.

Before you start coding lets hash out first what this could look like. Pls have a look at my last comment on #42 which suggests a format for providing options.

@magiconair
Copy link
Contributor

I did some digging and started a proposal for a more generic syntax which would allow configuration of a whole bunch of issues. See #111 for details.

@magiconair
Copy link
Contributor

@boyvinall Could you provide an example of the headers which would need to be added?

@boyvinall
Copy link
Author

Hi @magiconair, sorry for going quiet, got swamped with some things. A really simplistic example would be something like this:

Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: *
Access-Control-Allow-Headers: Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization

(I should say, I'm not hugely familiar with CORS, I just needed to add support for it to something that also fabio - and it seemed to me that this might be a nice feature-add.) The * above might be better tied down somewhat to say exactly which origin domains and HTTP methods are permitted. As I got a little further into this, I started to suspect that this feature request might actually be more generalised as adding in any desired response headers - indeed, the relevant bit of my (still development) nginx config is basically this:

server {
    listen *:80 default_server;
    listen *:443 default_server ssl;
    ssl_certificate        /path/to/cert-chain.pem;
    ssl_certificate_key    /path/to/privkey.pem;
    server_name _;
    add_header 'Access-Control-Allow-Origin' '*' always;
    add_header 'Access-Control-Allow-Methods' '*' always;
    add_header 'Access-Control-Allow-Headers' 'Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization';

    location / {
        if ($request_method = 'OPTIONS') {
            return 200;
        }
        proxy_pass "http://127.0.0.1:8000";
    }
}

So, it's just adding the headers and maybe forcing an empty HTTP 200 if you get an OPTIONS request.

@discobean
Copy link

Managing CORS though fabio would certainly be something that we would use

@far-blue
Copy link

I think the OPTIONS request really should be passed on to the backend server rather than just 200'ing. Services not written with CORS in mind tend not to handle the OPTIONS preflight but those written for CORS often make use of it for validation checks.

@boyvinall
Copy link
Author

I'm going to close this as I don't need it any more, and also I think the more generic header support mentioned in #168 should cope with it.

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

No branches or pull requests

4 participants