Skip to content

fix CORS support for reverse web proxy deployments #998

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

Open
wants to merge 3 commits into
base: testing
Choose a base branch
from

Conversation

kx1t
Copy link

@kx1t kx1t commented Apr 9, 2025

This fixes CORS support for use with SSL via reverse web proxies (like nginx) (further: "rwp") for container deployments. The fix was tested both via a rwp, as well as directly to ensure it doesn't break any non-webproxy deployments.

Specifically, it adds the Python flask-cors module to the container and applies CORS enhancements to the creation of the app and the socketio.

This fix does NOT fix the issue/complexity of having websockets work via a rwp -- the transport mechanism will fall back to http(s) polling. However, this works fine.

How to test --

  1. A radiosonde_auto_rx container using reverse-web-proxy with this fix has been deployed to https://kx1t.com/trenton-sonde
  2. The rwp deployed at kx1t.com uses nginx with the following location definition (and radiosonde_auto_rx is deployed at http://10.244.95.84:88/):
location /trenton-sonde/ {
    proxy_pass http://10.244.95.84:88/;
    proxy_http_version 1.1;
    proxy_redirect / /trenton-sonde/;
    proxy_set_header  X-Forwarded-Prefix /trenton-sonde;
}
  1. I tested the rwp deployment listed in (1) as well as the direct deployment to http://10.244.95.84:88; via the rwp, socketio uses polling; using the direct link, socketio uses websockets. In both cases, the application works equally well.

Last -- I additionally tested the enhancement that was suggested in #834 to change app.wsgi_app in web.py to:

app.wsgi_app = ProxyFix(app.wsgi_app, x_for=2, x_port=2, x_host=2, x_proto=2, x_prefix=2)

Unfortunately, that completely breaks socketio connections - the app works fine with the current code around app.wsgi_app

darksidelemm and others added 3 commits December 15, 2024 11:17
auto_rx 1.8.0 release - Full ka9q-radio support!
v1.8.1 Release - RS41 Multi-GNSS Support, ka9q-radio updates
@argilo
Copy link
Contributor

argilo commented Apr 9, 2025

Why is CORS necessary? Shouldn't all the content be loaded from the same origin, from the browser's perspective?

@xssfox
Copy link

xssfox commented Apr 10, 2025

@kx1t can you please provide a har recording for stock radiosonde autorx (eg when cors is failing)

@xssfox
Copy link

xssfox commented Apr 10, 2025

More than likely CORS was showing an error because of the sockets.io problem you mentioned on discord. The CORS error was likely masking the real error. I overwrote the cors header in chrome and tested on your site and it works fine without it.

image

@kx1t
Copy link
Author

kx1t commented Apr 11, 2025

@xssfox note that both my web proxy and the auto_rx instance for my site allow '*' for CORS, so overwriting the CORS header probably doesn't / shouldn't do anything as it's set to accept anything. The instances that I have deployed all include the fix of this PR already.

From what I read online, the CORS permissions should be added in any case -- they won't hurt for non-rwp deployments, and they will ensure that rwp deployments work error-free

@argilo
Copy link
Contributor

argilo commented Apr 11, 2025

From what I read online, the CORS permissions should be added in any case

Why? CORS is only needed in cases where you want to allow another website ("origin") to access content served by yours, and that shouldn't be the case for reverse web proxy deployments where everything is served from the proxy's origin.

@argilo
Copy link
Contributor

argilo commented Apr 11, 2025

Access-Control-Allow-Origin: * disables a browser security feature, so I don't think we should do that without good reason.

@xssfox
Copy link

xssfox commented Apr 11, 2025

@xssfox note that both my web proxy and the auto_rx instance for my site allow '*' for CORS, so overwriting the CORS header probably doesn't / shouldn't do anything as it's set to accept anything. The instances that I have deployed all include the fix of this PR already.

From what I read online, the CORS permissions should be added in any case -- they won't hurt for non-rwp deployments, and they will ensure that rwp deployments work error-free

I was overwriting it to simulate no cors header to demonstrate that it works fine without.

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

Successfully merging this pull request may close these issues.

4 participants