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

refactor ProxyFix middleware #1314

Merged
merged 1 commit into from May 28, 2018
Merged

refactor ProxyFix middleware #1314

merged 1 commit into from May 28, 2018

Conversation

@davidism
Copy link
Member

@davidism davidism commented May 28, 2018

  • Support multiple values for all headers.
  • Configured the number of proxies to trust for each header separately. num_proxies is deprecated. For backwards compatibility with the common case, x_for defaults to 1.
  • The original WSGI environ values are stored in a dict under werkzeug.proxy_fix.orig instead of individual keys. The keys in the dict match the environ keys.
  • Parametrize tests and rewrite docs for ProxyFix and get_host.

I want to add support for the standard Forwarded header.

I considered refactoring the somewhat repetitious code in __call__ with a for loop, but it ended up being about as complicated and seemed harder to follow.

Eventually this should be moved out of contrib into a new werkzeug.middleware.proxy_fix module.

@davidism davidism added this to the 0.15 milestone May 28, 2018
@davidism davidism self-assigned this May 28, 2018
@davidism davidism force-pushed the proxyfix branch from 5c1e7b6 to 7fc0e93 May 28, 2018
support multiple values for all headers
configure trust for each header separately
store original values in dict in environ
parametrize tests for ProxyFix and get_host
rewrite docs for ProxyFix and get_host
@davidism davidism force-pushed the proxyfix branch from 7fc0e93 to 6cbed92 May 28, 2018
@davidism davidism merged commit 3496bff into master May 28, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@davidism davidism deleted the proxyfix branch May 28, 2018
Yajo added a commit to Tecnativa/odoo that referenced this pull request Feb 28, 2019
Without this patch, Odoo is compatible only with one single inverse proxy in front of it.

However, when several proxies are chained in front of it, and all of them use the [proxy protocol][1], Odoo would understand the latest one's IP is the remote IP that is originating the HTTP request.

When several proxies are chained, Werkzeug receives this HTTP header:

    X-Forwarded-For: 1.2.3.4, 5.6.7.8

It means that 1.2.3.4 is the request originator, and 5.6.7.8 is the 1st proxy.

If using [Werkzeug >= 0.14.1][2], which logs the correct IP, the logs would end up with:

    2019-02-14 10:15:09,042 37 INFO prod werkzeug: 5.6.7.8 - - [14/Feb/2019 10:15:09] "GET /web/login HTTP/1.1" 200 -

After this patch, if you use the proper Werkzeug version and configure Odoo with `--proxy-trusted-x-for=2`, you'd get instead:

    2019-02-14 10:15:09,042 37 INFO prod werkzeug: 1.2.3.4 - - [14/Feb/2019 10:15:09] "GET /web/login HTTP/1.1" 200 -

This patch is prepared to be expanded later, when Werkzeug 0.15 is released, with the `ProxyFix` refactoring made in pallets/werkzeug#1314 available, and further options for other proxy protocol headers can be added in a similar fashion.

[1]: https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt
[2]: odoo#18052 (comment)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant