Skip to content

[IMP] developer/reference/cli: --proxy-mode - #6729

Closed
Julien00859 wants to merge 1 commit into
15.0from
15.0-proxy_mode-juc
Closed

Julien00859 wants to merge 1 commit into
15.0from
15.0-proxy_mode-juc

Conversation

@Julien00859

@Julien00859 Julien00859 commented Nov 30, 2023

Copy link
Copy Markdown
Member

Many customers struggle with their web server configuration, notably regarding the --proxy-mode option and the way X-Forwarded-* HTTP request headers are interpreted within Odoo.

The --proxy-mode section has been updated to cover the most common misunderstandings and to give guidances on how to setup a web server.

Odoo always only takes the last entry of the X-Forwarded-* request header because there are situations where it is not possible to determine which last n-th entry to use. Employees might access their odoo database via the internal network: connecting directly to nginx, while customers might access the database via an additional proxy such as cloudflare. The real IP of employees would be the last inside the X-Forwarded-For chain, while the real IP of customers would be the second last entry inside the chain. It would be incorrect to always take the same nth last entry inside the chain. The cloudflare's own IP address must be discarded from the chain. Web servers usually feature a way to ignore trusted IP from the chain, a way so that the real IP of the user is always the last entry inside the chain. Odoo relies on such feature to be active and configured.

Prior discussions about X-Forwarded-For:

All X-Forwarded-* headers are ignored in case the X-Forwarded-Host header is missing (even with --proxy-mode). System admin might be tempted to not set this header and to set Host instead, this is broken as this a user-agent would be able to spoof X-Forwarded-Host and Odoo would use that instead of the correct Host.

Prior discussions about X-Forwarded-Host:

@robodoo

robodoo commented Nov 30, 2023

Copy link
Copy Markdown
Collaborator

@C3POdoo
C3POdoo requested a review from a team November 30, 2023 13:16
@Julien00859
Julien00859 requested a review from odony November 30, 2023 13:19
Comment thread content/developer/reference/cli.rst
@Julien00859

Copy link
Copy Markdown
Member Author

Hello @odoo/doc-review I've met with Olivier and he agrees with the changes. We cn move on with the editorial review and merging it :)

@AntoineVDV AntoineVDV left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kudos for targeting 15.0 👍

@robodoo delegate+

Comment thread content/developer/reference/cli.rst Outdated
Comment thread content/developer/reference/cli.rst Outdated
Comment thread content/developer/reference/cli.rst Outdated

@odony odony left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Julien00859 Julien00859 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many customers struggle with their web server configuration, notably
regarding the `--proxy-mode` option and the way `X-Forwarded-*` HTTP
request headers are interpreted within Odoo.

The `--proxy-mode` section has been updated to cover the most common
misunderstandings and to give guidances on how to setup a web server.

Odoo always only takes the last entry of the `X-Forwarded-*` request
header because there are situations where it is not possible to
determine which last n-th entry to use. Employees might access their
odoo database via the internal network: connecting directly to nginx,
while customers might access the database via an additional proxy such
as cloudflare. The real IP of employees would be the last inside the
`X-Forwarded-For` chain, while the real IP of customers would be the
*second* last entry inside the chain. It would be incorrect to always
take the same nth last entry inside the chain. The cloudflare's own IP
address must be discarded from the chain. Web servers usually feature
a way to ignore trusted IP from the chain, a way so that the real IP
of the user is always the last entry inside the chain. Odoo relies on
such feature to be active and configured.

Prior discussions about `X-Forwarded-For`:
* odoo/odoo#104947
* odoo/odoo#118629
* odoo/odoo#139536

All `X-Forwarded-*` headers are ignored in case the `X-Forwarded-Host`
header is missing (even with `--proxy-mode`). System admin might be
tempted to not set this header and to set `Host` instead, this is
broken as this a user-agent would be able to spoof `X-Forwarded-Host`
and Odoo would use that instead of the correct `Host`.

Prior discussions about `X-Forwarded-Host`:
* odoo/odoo#63277
* odoo/odoo#70117
@Julien00859

Copy link
Copy Markdown
Member Author

@robodoo r+ @fw-bot up to 17.0

@Julien00859

Copy link
Copy Markdown
Member Author

@fw-bot up to master

@fw-bot

fw-bot commented Dec 1, 2023

Copy link
Copy Markdown
Collaborator

Forward-porting to 'master'.

support`_.

It ignores all ``X-Forwarded-*`` headers in case ``X-Forwarded-Host`` is
missing from the request.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this language should be stronger. Something like "A properly configured proxy upstream must provide X-Forwarded-Host or --proxy-mode is ignored.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah shit I only see your message now and the PR as been merged. I don't think using a stronger language is actually needed, the piece of information was missing and people had some expectations, now that the info is there I think they'll configure their proxies accordingly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll see, in case we continue to get messages on the matter we can still open another PR and change the wording :)

robodoo pushed a commit that referenced this pull request Dec 1, 2023
Many customers struggle with their web server configuration, notably
regarding the `--proxy-mode` option and the way `X-Forwarded-*` HTTP
request headers are interpreted within Odoo.

The `--proxy-mode` section has been updated to cover the most common
misunderstandings and to give guidances on how to setup a web server.

Odoo always only takes the last entry of the `X-Forwarded-*` request
header because there are situations where it is not possible to
determine which last n-th entry to use. Employees might access their
odoo database via the internal network: connecting directly to nginx,
while customers might access the database via an additional proxy such
as cloudflare. The real IP of employees would be the last inside the
`X-Forwarded-For` chain, while the real IP of customers would be the
*second* last entry inside the chain. It would be incorrect to always
take the same nth last entry inside the chain. The cloudflare's own IP
address must be discarded from the chain. Web servers usually feature
a way to ignore trusted IP from the chain, a way so that the real IP
of the user is always the last entry inside the chain. Odoo relies on
such feature to be active and configured.

Prior discussions about `X-Forwarded-For`:
* odoo/odoo#104947
* odoo/odoo#118629
* odoo/odoo#139536

All `X-Forwarded-*` headers are ignored in case the `X-Forwarded-Host`
header is missing (even with `--proxy-mode`). System admin might be
tempted to not set this header and to set `Host` instead, this is
broken as this a user-agent would be able to spoof `X-Forwarded-Host`
and Odoo would use that instead of the correct `Host`.

Prior discussions about `X-Forwarded-Host`:
* odoo/odoo#63277
* odoo/odoo#70117

closes #6729

Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
@robodoo robodoo closed this Dec 1, 2023
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