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

fix: cross-site post form submissions are forbidden when using load balancer in front #61

Merged
merged 4 commits into from
Oct 2, 2024

Conversation

KumanoTanaka
Copy link
Contributor

@KumanoTanaka KumanoTanaka commented Jul 7, 2024

In my use case

  • end-client -(https)-> GCP load balancer -(http)-> docker containerized app using svelte-adapter-bun

key points

  • gcp load balancer request to my svelte app by using http://my-domain.com url but https is set to X-Forwarded-Proto. <-- so I want to modify request's base-url on every request. (even ORIGIN === get_origin(request.headers) case).
  • docker containerized app such as k8s, google-cloud cloudrun uses health check request which dose not contain x-forwarded-for header. <-- I want to ignore "ADDRESS_HEADER" check of those health-check requests

(GCP load balancer http-header forwarding rules are described here)

I think this PR will help issue #54

@dihmeetree
Copy link

Thank you for this fix. Was trying to figure this out for like 2 hours lol. Can confirm it fixes the issue 🙂

@kjell0w
Copy link

kjell0w commented Aug 26, 2024

Do we have any updates on this? Will this be merged? Seems like the issue still persists

@vyconm
Copy link

vyconm commented Aug 26, 2024

Please merge this - it's the only thing holding us back from deploying it in production, due to issue #54 which If I understand correctly, is related to this :)

@notramo
Copy link

notramo commented Oct 2, 2024

What about entirely removing the origin checking? It's an insufficient security measure, and is harmful by providing a false sense of security. Origin can be spoofed, so it doesn't protect against sophisticated attacks. An XSRF token form field is a proper solution.

@gornostay25 gornostay25 merged commit ade8792 into gornostay25:master Oct 2, 2024
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.

6 participants