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

Enable CORS options by default #2188

Open
aaroncox opened this issue Feb 2, 2024 · 4 comments
Open

Enable CORS options by default #2188

aaroncox opened this issue Feb 2, 2024 · 4 comments
Labels

Comments

@aaroncox
Copy link
Contributor

aaroncox commented Feb 2, 2024

We just ran into what we considered to be a mundane API client fix on the wharfkit/antelope library, but it turns out to break functionality with a large portion of API providers out in the wild.

We added Content-Type: application/json to all of our HTTP POST request. Browsers, following this standard, issue either a OPTIONS request alongside of this - which most API providers automatically route to nodeos. The nodeos APIs don't respond to this request with the default config, and so the API request is terminated.

To start moving in a direction where the client SDKs can start using the appropriate content types, I'd propose we set the defaults on these parameters to be open, instead of specifying them:

  • access-control-allow-origin default to *
  • access-control-allow-headers default to * (or a standard CORS subset)

If operators would prefer to have more strict settings, they can then use the config file to override them.

Leap 6 being a hardfork and requiring all nodes to update would be an ideal time for this. It would allow us to fix the library after the release of this next version, without breaking the applications or requiring all API operators to update their nodeos/nginx/etc configurations.

@apporc
Copy link

apporc commented Feb 2, 2024

another option, http-validate-host, it requires the listening address to be the same as Host headers used in request, which is too strict for a local development node, and not necessary in normal deployment with nginx as reverse proxy. Maybe only useful in a deployment where nodeos is the top level endpoint.
Unless we allow set this allowed_host value in configuration, not imply using listening address, listening address is not necessarily the same as the request host. For example we can set nodeos to listen on 127.0.0.1:8080 and use nginx to proxy another [external ip] to it, then the request Host header is the external ip or some domain name, not 127.0.0.1, this will fail the validate.

@ericpassmore
Copy link
Contributor

@aaroncox
Copy link
Contributor Author

Judging by that page, I'd assume no - but honestly I haven't tested it. In our nginx configurations where we set CORS, we'll typically use the normal 3 that are required:

add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS' always;

Maybe we tailor that list specifically to the methods that Leap actually utilizes.

I guess speaking of which, does Leap respond to options requests appropriately with the CORS configuration? I've never directly used CORS via Leap, so I am not sure what the capabilities here are. We've always proxied with nginx.

@matthewdarwin
Copy link

We set CORS in nginx as well.

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

No branches or pull requests

5 participants