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

Feat: Explicitly set fastcgi_params #979

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

steveworley
Copy link
Contributor

When looking to correctly surface true client IP with a number of reverse proxies in the mix we need access to the X_FORWARDED_FOR header in Drupal. With the default configuration this presents as a single IP address for the last hop in the chain:

$_SERVER['HTTP_X_FORWARDED_FOR']	127.0.0.1 (last hop)

With this change we explicitly set X_FORWARDED_FOR and other useful request identifiers back via fastcgi_params. This allows us to use reverse_proxy_addresses correctly in Drupal (and other applications analogs) to correctly act on the true client IP.

With this change PHP sees the following:

$_SERVER['HTTP_X_FORWARDED_FOR']	127.0.0.1 (last hop), 1.1.1.1 (remote_addr)

$proxy_add_x_forwarded_for appends the remote_addr to the IP list to allow us to access that information. PHP currently does not see the value set in the proxy_set_header and this was the only way I was able to surface this to PHP.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@tobybellwood tobybellwood requested a review from seanhamlin April 23, 2024 04:43
Copy link
Contributor

@seanhamlin seanhamlin left a comment

Choose a reason for hiding this comment

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

Make sense to me. Likely will need to be called out in the release notes that X-Forwarded-For will contain the chain of IPs (which is more normal anyway).

@tobybellwood tobybellwood merged commit 5387e2e into uselagoon:main Apr 24, 2024
1 check passed
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.

None yet

3 participants