feat: Support forwarded IPs using a configurable header #178
Conversation
pkg/data/data.go
Outdated
| const contextDataKey key = iota | ||
|
|
||
| // GetRemoteIP extracts the remote IP from the request, optionally using a custom header. | ||
| func GetRemoteIP(r *http.Request, sourceIPHeader string) (string, error) { |
There was a problem hiding this comment.
For this I had invision to have a configuration more like Traefik's source criterion which allows choosing which IP in the header is considered.
This will not ban your firewall, for example !
WDYT ?
There was a problem hiding this comment.
Yes, I agree. I listed this as an improvement but I guess I could try to implement it.
There was a problem hiding this comment.
@tomMoulard I just pushed a proposal that more closely matches the source criterion from Traefik. Let me know what you think!
pkg/chain/chain.go
Outdated
| } | ||
|
|
||
| // New creates a new chain. | ||
| func New(final http.Handler, handlers ...ChainHandler) Chain { | ||
| func New(final http.Handler, sourceIPHeader string, handlers ...ChainHandler) Chain { |
There was a problem hiding this comment.
For this, I think it would be better to have a ip extractor interface where:
- if there is a header configured, the ip extractor will extract the IP from the headers
- otherwise, the request's ip will be used.
WDYT ?
There was a problem hiding this comment.
Where would you see this interface going? By interface there, you mean an actual code interface that would be implemented by only one class and used once or an "interface" as a decoupled way of getting the IP? For the latter, would you put this outside the chain handler?
I'm open to streamline this more but I guess I need a bit more context on what you think for the IP extractor.
Co-authored-by: Tom Moulard <[email protected]>
Co-authored-by: Tom Moulard <[email protected]>
Closes #84
Overview
This PR implements a new configuration option for the rules that allows specifying an HTTP header that contains the real remote IP in cases where Traefik is behind a reverse proxy or a CDN (like Cloudflare for instance).
Features
Implementation details
The rules object that hosts the new parameter was not passed to the chain when handling requests. I have chosen to add a parameter to the chain constructor to be able to pass the source IP header that is then used in the data package to get the real IP.
This adds a coupling between the rules and the chain but I thought that the tradeoff was acceptable. It ensures a decoupling with the multiple handlers.
Testing
I have updated the tests to take into account the new argument passed to several functions.
Documentation
I have updated the README to explain the new feature and how it can be used.
Checklist
Improvements
depthparameter to be able to chose the depth at which an IP should be pulled from an HTTP header that contains multiple IPs in case of multiple reverse proxies, just like Traefik does.Related issues