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(http): get client ip from headers #15205

Merged
merged 6 commits into from
Apr 12, 2024

Conversation

flaneur2020
Copy link
Member

@flaneur2020 flaneur2020 commented Apr 10, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

currently the IP we got is retrieved from req.remote_addr, which represents the direct connection's IP address.

but when we deployed with having a proxy up front, it only reprents the proxy's IP.

this PR tries to retrieve the ip from headers including X-Real-IP, X-Forwarded-For, CF-Connecting-IP, these headers can help us retrieve the real ip of the client.

please note that the ip address from the headers are easy to be manipulated by middleman. we have to ensure the proxies are trustworthy, or the middleman may use a fake header to trick the network policy.

(however, it's necessary to trust the proxy in a cloud deployment IMHO.)

Tests

  • Unit Test

Type of change

  • New Feature (non-breaking change which adds functionality)

This change is Reviewable

@flaneur2020 flaneur2020 marked this pull request as draft April 10, 2024 04:02
@github-actions github-actions bot added the pr-feature this PR introduces a new feature to the codebase label Apr 10, 2024
@flaneur2020 flaneur2020 force-pushed the add-ip-address-from-header branch from 804631a to 851d4be Compare April 10, 2024 07:46
@flaneur2020 flaneur2020 marked this pull request as ready for review April 10, 2024 08:12
@BohuTANG BohuTANG merged commit 2a4febd into databendlabs:main Apr 12, 2024
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature this PR introduces a new feature to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants