respect X-Forwarded-For headers if present

[weickmanna]

A classical setup is to terminate HTTPS on a load balancer, but communicate with HTTP between load balancer and backend service.

In this case, the load balancer would add X-Forwarded-For headers to the HTTP requests, such that the backend application would know e.g. for redirects where to redirect the user to.

For this to work, these headers must be picked up by the Jetty server. This is accomplished by adding the ForwardedRequestCustomizer to the http config:

• If the headers are present, they will be used
• If the headers are not present, nothing changes

As Steve is making heavy use of redirects, we cannot run the app behind an AWS load balancer while terminating HTTPS on the load balancer.

[csamsel]

SteVe will work just fine behind a reverse proxy unless you rewrite the paths, which will also break the charging station connection in most cases and it's therefore a bad idea.

[goekay]

@csamsel i understand your point. still, this customizer does not look harmful, since it will only touch the headers optionally. see javadoc. i think we can accept this. what do say?

[csamsel]

Yeah it's fine for me, as you said it doesn't hurt.

[goekay]

can you please not use star import?

[goekay]

please reference the github PR and add the link for future traceability.

[weickmanna]

To clarify a bit further. Situation is like this:

User -> LB (HTTPS) -> Steve (HTTP)
    \                          |
     -------------- Redirect (HTTP)

Since Steve does not know it runs behind HTTPS, it redirects the user to an HTTP URL. Then the user hits the LB on the HTTP URL, but there is nothing listening on HTTP.

With the forwarded headers, Steve will pick up this info from the LB and redirect to HTTPS instead.

Did not test the effects this might have on charge-point related functionality.

[goekay]

Did not test the effects this might have on charge-point related functionality.

i think, one thing to watch out is the charge points using websocket connections (ocpp-j). these connections must go through all the gates. we have a basic wiki page for this: https:/RWTH-i5-IDSG/steve/wiki/FAQ#how-can-i-run-steve-behind-a-reverse-proxy

[csamsel]

To clarify a bit further. Situation is like this:

User -> LB (HTTPS) -> Steve (HTTP)
    \                          |
     -------------- Redirect (HTTP)

Since Steve does not know it runs behind HTTPS, it redirects the user to an HTTP URL. Then the user hits the LB on the HTTP URL, but there is nothing listening on HTTP.

Thats not true, all redirects are relative in the form of target = /steve/foo/bar and will work with a changed domain or protocol. It will only break if you rewrite the path, e.g. /steve/foo/bar -> /steve2/foo/bar

With the forwarded headers, Steve will pick up this info from the LB and redirect to HTTPS instead.

Did not test the effects this might have on charge-point related functionality.

[mhei]

So after all, only the star import and the back-reference to this PR still needs to be fixed, right?
@weickmanna: Do you still care, or would it be okay to proceed?

[weickmanna]

@mhei sure, feel free to merge this in. This fix made it work on our AWS ECS behind https-enabled load balancer setup.

[mhei]

Sorry, my question was misleading: I wanted to know if you (@weickmanna) are making the requested changes. But I've done it now already as far as I understood them. See referenced commit.

[goekay]

since @weickmanna is unwilling to make the suggested cosmetic changes, i will merge this PR and do them later. thanks all.

[mhei]

Did you noticed my branch with the requested changes?Am 24.06.2022 22:56 schrieb Sevket Gökay ***@***.***>: Merged #570 into master. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[goekay]

yes, thank you, but did not want to add ceremony (by asking you kindly to make another PR that would replace this one etc.) and add any more delay to this one year old open PR.

i hope my action did not create some unnecessary work for your branch.