Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use reverse proxy IP for users #7368

Open
wants to merge 1 commit into
base: release-2.1
Choose a base branch
from

Conversation

MarcoCLA
Copy link

@MarcoCLA MarcoCLA commented Mar 1, 2022

The real client IP is not used in $user_info. This causes incorrect behavior when used behind a reverse proxy.
For example only one guest will be visible (as only the IP of the reverse proxy is used) and all logging is linked to that IP.

Again a small hotfix. A better aproach would be to use a proper getClientIP() function which processes the headers and checks for the reverse proxy IP.

Example: Symphony getClientIP()

Signed-off-by: MarcoCLA <marco@twilp.nl>
@jdarwood007
Copy link
Member

This is the reason we have $user_info['ip'] and $user_info['ip2']. If incorrect stats/guests online are being detected because of a single ip, then we should be fixing that. In reality, guests online shouldn't be determined by their ip, but by their session. Some countries alone have a limited number of IPv4 and use what is essentially a CGNAT.

@Sesquipedalian
Copy link
Member

@MarcoCLA, please open an issue to describe in more detail the problem that this PR is meant to address.

@MarcoCLA
Copy link
Author

MarcoCLA commented Mar 3, 2022

What is the use-case of $user_info['ip'] in the current implementation ($_SERVER['REMOTE_ADDR'])?

If a forwarded ip is verified, I see no reason to use the REMOTE_ADDR instead of the validated forwarded ip.

@Sesquipedalian Sesquipedalian added this to the The future milestone Apr 4, 2022
@Sesquipedalian
Copy link
Member

Added to "The future" for further consideration. It will help a great deal if we are given a more fulsome description of the problem in a proper issue report.

@sbulen sbulen added the Not enough details Issue doesn't provide enough details/documentation for it to be reproduced. label Jul 23, 2022
@jdarwood007 jdarwood007 modified the milestones: The future, 3.0 Alpha 5 Jan 22, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Not enough details Issue doesn't provide enough details/documentation for it to be reproduced.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants