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

Has there been a change to the CORS configuration on the catalog? #673

Open
marcolarosa opened this issue Aug 8, 2018 · 8 comments
Open

Comments

@marcolarosa
Copy link
Collaborator

marcolarosa commented Aug 8, 2018

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

server {
    listen       80;
    server_name  localhost;

    #charset koi8-r;
    #access_log  /var/log/nginx/log/host.access.log  main;

    location / {
         if ($request_method = 'OPTIONS') {
            add_header 'Access-Control-Allow-Origin' "$http_origin";
            add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
            #
            # Custom headers and headers various browsers *should* be OK with but aren't
            #
            add_header 'Access-Control-Allow-Headers' 'Authorization,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
            #
            # Tell client that this pre-flight info is valid for 20 days
            #
            add_header 'Access-Control-Max-Age' 1728000;
            add_header 'Content-Type' 'text/plain charset=UTF-8';
            add_header 'Content-Length' 0;
            return 204;
         }
         if ($request_method = 'POST') {
            add_header 'Access-Control-Allow-Origin' "$http_origin";
            add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
            add_header 'Access-Control-Allow-Headers' 'Authorization,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
            add_header 'Access-Control-Expose-Headers' 'Authorization,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
         }
         if ($request_method = 'GET') {
            add_header 'Access-Control-Allow-Origin' "$http_origin";
            add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
            add_header 'Access-Control-Allow-Headers' 'Authorization,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
            add_header 'Access-Control-Expose-Headers' 'Authorization,DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Content-Range,Range';
         }
         proxy_pass http://catalog.paradisec.org.au;
    }

    #error_page  404              /404.html;

    # redirect server error pages to the static page /50x.html
    #
    error_page   500 502 503 504  /50x.html;
    location = /50x.html {
        root   /usr/share/nginx/html;
    }
}

And in the browser console I see:
screen shot 2018-08-08 at 12 19 24 pm

@marcolarosa
Copy link
Collaborator Author

marcolarosa commented Aug 8, 2018

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:

Failed to load http://catalog.paradisec.org.au/graphql: The 'Access-Control-Allow-Origin' header 
contains multiple values '*, http://dev.local:9000', but only one is allowed. Origin 'http://dev.local:9000' 
is therefore not allowed access. Have the server send the header with a valid value, or, if an opaque 
response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS 
disabled.

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.

screen shot 2018-08-08 at 3 59 10 pm

@marcolarosa
Copy link
Collaborator Author

marcolarosa commented Aug 8, 2018

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:

proxy_pass http://catalog.paradisec.org.au;

to (basically I hide that header coming from the catalog at my proxy server)

proxy_hide_header 'Access-Control-Allow-Origin';
proxy_pass http://catalog.paradisec.org.au;

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
'Access-Control-Allow-Credentials' is true.

@agrimmtt
Copy link
Contributor

@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. /etc/nginx/nginx.conf doesn't have anything, nor does doc/server.md in the Rails app, and set_access_headers in ApplicationController was last touched in May 2015, if not earlier. I'll see if working on #665 will help work this out.

@marcolarosa
Copy link
Collaborator Author

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.

@pd-terem
Copy link

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.
Would you like us to make a change to this header (for either/both Graphql and Catalog URLs)? If so, let us know what you'd like it changed to.

@marcolarosa
Copy link
Collaborator Author

marcolarosa commented Aug 21, 2018

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.

@enwardy
Copy link

enwardy commented Feb 26, 2019

@marcolarosa is this still an outstanding issue?
Anything further you would like to add to it?

@enwardy enwardy assigned enwardy and unassigned agrimmtt Feb 26, 2019
@marcolarosa
Copy link
Collaborator Author

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

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

6 participants