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

Support proxy mode by checking the X-Forwarded-For header first #963

Merged
merged 2 commits into from
Apr 5, 2018

Conversation

frankiejarrett
Copy link
Contributor

@frankiejarrett frankiejarrett commented Mar 6, 2018

@lukecarbis When in reverse proxy mode, the REMOTE_ADDR value will always show the IP of the proxy server (ATS, Varnish, HAProxy, etc). Most proxies will pass along the real client IP via the X-Forwarded-For header. If it exists, Stream should prefer that as the real IP and only use REMOTE_ADDR as a fallback.

👋 It's been a while 😃

@@ -107,6 +107,10 @@ function( $var ) {
$role = '';
}

// Support proxy mode by checking the `X-Forwarded-For` header first
$ip_address = wp_stream_filter_input( INPUT_SERVER, 'HTTP_X_FORWARDED_FOR', FILTER_VALIDATE_IP );
$ip_address = $ip_address ? $ip_address : wp_stream_filter_input( INPUT_SERVER, 'REMOTE_ADDR', FILTER_VALIDATE_IP );
Copy link
Contributor

Choose a reason for hiding this comment

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

@fjarrett This is cool, thanks. The REMOTE_ADDR is also requested on line 154/158 So will need to be applied there.
Perhaps it might be better to have method to fetch the IP, or even a simpleIP param set in the constructor to prevent code duplication.

@DavidCramer DavidCramer changed the base branch from develop to add/real-client-ip April 5, 2018 07:06
@DavidCramer DavidCramer merged commit 733ef3d into xwp:add/real-client-ip Apr 5, 2018
@DavidCramer DavidCramer mentioned this pull request Apr 5, 2018
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