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

Enhance Reproxy Support for X-Forwarded Headers #172

Closed
rashpile opened this issue Feb 1, 2024 · 5 comments
Closed

Enhance Reproxy Support for X-Forwarded Headers #172

rashpile opened this issue Feb 1, 2024 · 5 comments

Comments

@rashpile
Copy link

rashpile commented Feb 1, 2024

In certain applications, it is crucial to be aware of the complete request URL, including the host, port, and scheme. These details are typically relayed via the headers X-Forwarded-Host, X-Forwarded-Port, and X-Forwarded-Proto. While Reproxy currently handles the X-Forwarded-Host header effectively, passing the original hostname, it does not accommodate for the port and scheme, which are essential in mixed protocol scenarios.

Use Case:
Our scenario involves deploying a Keycloak service with Reproxy. Internally, the Keycloak container exposes port 8080 and operates over HTTP. However, the public URL is configured as https://idp.example.com/, with Reproxy directing traffic to the Keycloak service. Keycloak is utilized for user authentication, with the frontend URL passing through Reproxy, while client authentication occurs directly via the container's hostname.

Observed Issue:
When attempting to access the Keycloak admin page, users are incorrectly redirected to http://idp.example.com:8080/. This behavior indicates a mismatch in the scheme and port handling between the internal service (Keycloak) and the external access method (Reproxy).

Posible Solutions:

  • Automatic Header Handling: Implement functionality in Reproxy to automatically append X-Forwarded-Port: 443 and X-Forwarded-Proto: https headers when SSL is in use. This would ensure that services behind Reproxy are aware of the original request's protocol and port, even when the internal service operates over a different protocol or port.

  • Custom Header Configuration: Introduce a new parameter in Reproxy's configuration to allow the addition of custom request headers. This would provide users with the flexibility to specify any necessary headers based on their unique deployment scenarios.

@umputun
Copy link
Owner

umputun commented Feb 1, 2024

Appending X-Forwarded-Port and X-Forwarded-Proto is the right thing to do, regardless of the second point.

Regarding custom headers - it is fine with me, too; we already have the ProxyHeaders param, allowing us to set custom "outgoing" headers. Having a similar one for incoming headers makes sense.

Let me know if you want to deal with this and provide a PR with the change(s)

@eugenelitvinov
Copy link

This PR created a regression where X-Forwarded-Proto and X-Forwarded-Port headers are newly added even if existed in the original request. In my scenario, Reproxy instance is behind CloudFlare which adds these headers already. Now when request arrives to my target application, it complains about multiple headers with the same key. Not sure if there is a clear RFC for this, but appending additional values in the comma separated list appears to be a commonly used approach.

@umputun umputun changed the title [Discussion] Enhance Reproxy Support for X-Forwarded Headers Enhance Reproxy Support for X-Forwarded Headers Feb 6, 2024
@umputun
Copy link
Owner

umputun commented Feb 6, 2024

Thanks for the report. I don't think appending additional values in the comma-separated list is appropriate in this case. I believe the right thing to do is to skip setting those headers if they are already present.

@umputun
Copy link
Owner

umputun commented Feb 6, 2024

fixed in v1.1.1, pls check it out

@eugenelitvinov
Copy link

Thanks for the prompt fix, switched to v1.1.1 and things are back to normal!

@umputun umputun closed this as completed Feb 6, 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

No branches or pull requests

3 participants