-
Notifications
You must be signed in to change notification settings - Fork 8
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
Has there been a change to the CORS configuration on the catalog? #673
Comments
An update. My default browser is firefox and the error in the screenshot above is from there. However, after some googling I found out that chrome provides a more accurate error. Specifically, with the above config the error in chrome is:
Note that the header has two values: '*' and 'http://dev.local:9000' which is not correct. This issue suggests that perhaps the backend is setting the header and chrome is merging them. I'm just spitballing here and will keep tinkering on my end. |
OK - I think your stack is adding the 'Access-Control-Allow-Origin' header somewhere and it shouldn't be. If I change the nginx configuration above from:
to (basically I hide that header coming from the catalog at my proxy server)
It all works and the browser doesn't get an incorrect allow origin header. For the sake of documentation, Allow-Origin '*' (wildcard) is not allowed when |
@marcolarosa I had a look at it, and I haven't yet figured this one out. I can't find any sign of how CORS has changed lately. |
ok. Since i'm seeing it on the graphql endpoint is it possible that the graphql middleware is automatically setting it? I ask this given that graphql is an API query language so i could believe that the default configuration is to allow anyone to talk to the API. |
Hi @marcolarosa, /graphql is routed through Rails, same as the catalog, so the same headers are added (Including a single Access-Control-Allow-Origin: *). This hasn't been changed recently. |
So my thoughts are that CORS headers shouldn't be set at all. Nabu is not an API being used my many frontends or other services. It's a standalone catalog server and even though the viewer uses it as an API, it doesn't need CORS headers set in production as the viewer is hosted at the same domain. The only time CORS headers are needed are during development b/c dev happens at a different URL to the catalog. However, that's easy to get around by just running an nginx proxy that sets the required headers. And indeed that's what I've been doing since the beginning. Aside from all that, having allow * forbids sending credentials so it doesn't work anyway as the session cookie won't get transferred so nothing will load from Nabu anyway. Hope that makes sense and helps. |
@marcolarosa is this still an outstanding issue? |
@enwardy Not really sure how to answer the question. It doesn't look like anything has been done to fix the issue so I would think that it is still outstanding. Something in the stack is setting CORS headers and I don't think they should be set at all as per #673 (comment). As far as I'm concerned, someone needs to decide whether CORS headers should be set and if so, how it should be configured so that credentials can be sent otherwise they're useless anyway. |
In order to develop the viewer I have a local nginx installation that sets up the CORS configuration for my request so that I don't bump in to CORS issues with my development instance not being hosted at the same domain as the catalog.
This doesn't seem to work anymore so I'm wondering if there have been any server changes. Following is my nginx config that proxies to Nabu: (note that it doesn't matter what I have in the allow origin field. It always fails).
And in the browser console I see:
The text was updated successfully, but these errors were encountered: