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

Proxy: change default offset to take final IP #9859

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

michael-smt
Copy link
Contributor

@michael-smt michael-smt commented Sep 4, 2023

Relates to #9854

Proposed changes

The X-Forwarded-For header is appended to by every proxy between the client and Weblate. The header is split by , in weblate/middleware.py, then the value at the index set by IP_PROXY_OFFSET is passed to Django validators.

Change the default to -1 to take the last entry in the list of addresses. This is the proxy closest to Weblate. Which is assumed to be the most trusted when IP_BEHIND_REVERSE_PROXY is enabled.

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

The MDN web docs also recommends using the final value(s):

If any trusted reverse proxies are between the client and server, the final X-Forwarded-For IP addresses (one for each trusted proxy) are trustworthy, as they were added by trusted proxies.
[...]
Conversely, leftmost (untrusted) values must only be used where there will be no negative impact from the possibility of using spoofed values.

@nijel
Copy link
Member

nijel commented Sep 5, 2023

Can you please document the change as it is possibly breaking one? In docs/changes.rst and in docs/admin/config.rst.

@nijel nijel added this to the 5.0.1 milestone Sep 6, 2023
@nijel nijel self-assigned this Sep 6, 2023
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #9859 (b4ec760) into main (d23693c) will increase coverage by 0.00%.
Report is 16 commits behind head on main.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9859   +/-   ##
=======================================
  Coverage   90.74%   90.74%           
=======================================
  Files         787      787           
  Lines       57005    57015   +10     
  Branches     8872     8874    +2     
=======================================
+ Hits        51729    51739   +10     
  Misses       3715     3715           
  Partials     1561     1561           
Files Changed Coverage
weblate/trans/models/_conf.py 100.00%
weblate/utils/tests/test_middleware.py 100.00%

@nijel nijel merged commit 87c691b into WeblateOrg:main Sep 6, 2023
23 checks passed
@nijel
Copy link
Member

nijel commented Sep 6, 2023

Merged, thanks for your contribution!

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.

2 participants