Repository: gin-gonic/gin
Pull request #2723 (2021-05-18 02:36:27)

Get client IP when using Cloudflare
When proxying an app through Cloudflare, the user's real IP is set in the CF-Connecting-IP header. This is more convenient than having to manually maintain an allow-list of trusted proxies.

Reference: https://support.cloudflare.com/hc/en-us/articles/200170986-How-does-Cloudflare-handle-HTTP-Request-headers-

codecov[bot]: # [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) Report
> Merging [#2723](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (e6cc245) into [master](https://codecov.io/gh/gin-gonic/gin/commit/4fe5f3e4b4fe62c057ec22caee4908beeef5f59c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) (4fe5f3e) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/gin-gonic/gin/pull/2723/graphs/tree.svg?width=650&height=150&src=pr&token=Qpqxhwery8&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)

```diff
@@           Coverage Diff           @@
##           master    #2723   +/-   ##
=======================================
  Coverage   98.67%   98.67%           
=======================================
  Files          41       41           
  Lines        2036     2040    +4     
=======================================
+ Hits         2009     2013    +4     
  Misses         15       15           
  Partials       12       12           
```


| [Impacted Files](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic) | Coverage Δ | |
|---|---|---|
| [gin.go](https://codecov.io/gh/gin-gonic/gin/pull/2723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-Z2luLmdv) | `99.12% <ø> (ø)` | |
| [context.go](https://codecov.io/gh/gin-gonic/gin/pull/2723/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic#diff-Y29udGV4dC5nbw==) | `97.65% <100.00%> (+0.02%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic). Last update [4fe5f3e...e6cc245](https://codecov.io/gh/gin-gonic/gin/pull/2723?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gin-gonic).

ItalyPaleAle: Thinking about it, I wonder if rather than using booleans, it might make more sense to create an enum `platform` (or something like that)?

This way, once the next platform is inevitably added, we don't add yet another boolean.

The downside is around what to do with the existing Google App Engine flag.
thinkerou: > Thinking about it, I wonder if rather than using booleans, it might make more sense to create an enum `platform` (or something like that)?
> 
> This way, once the next platform is inevitably added, we don't add yet another boolean.
> 
> The downside is around what to do with the existing Google App Engine flag.

Yes, I also like `enum`
ItalyPaleAle: I can update to use an enum-like.

What should I do for the existing Google App Engine flag? Keep that for legacy and "translate" that into the enum?
ItalyPaleAle: Wait before you merge.... I want to update it to use an enum. Before we introduce even more legacy. :) just let me know what I should do with the existing Boolean flag for Google App Engine
thinkerou: > I can update to use an enum-like.
> 
> What should I do for the existing Google App Engine flag? Keep that for legacy and "translate" that into the enum?

@ItalyPaleAle I merged the pull request at first, please commit other pull request using(refactoring) `enum`, thanks!
ItalyPaleAle: Ok. Will do it over the weekend or sooner.

What do you want me to do with the existing Boolean for GAE? I can't remove it because it would be backwards-incompatible. I can keep it as a legacy alias that then is translated to the enum?
thinkerou: @ItalyPaleAle please read https://github.com/gin-gonic/gin/blob/master/deprecated.go for reference, we should deprecated warning at first and remove.

Patch:
diff --git a/context.go b/context.go
index 12e946d..0c1fb07 100644
--- a/context.go
+++ b/context.go
@@ -731,10 +731,15 @@ func (c *Context) ShouldBindBodyWith(obj interface{}, bb binding.BindingBody) (e
 // If the headers are nots syntactically valid OR the remote IP does not correspong to a trusted proxy,
 // the remote IP (coming form Request.RemoteAddr) is returned.
 func (c *Context) ClientIP() string {
-	if c.engine.AppEngine {
+	switch {
+	case c.engine.AppEngine:
 		if addr := c.requestHeader("X-Appengine-Remote-Addr"); addr != "" {
 			return addr
 		}
+	case c.engine.CloudflareProxy:
+		if addr := c.requestHeader("CF-Connecting-IP"); addr != "" {
+			return addr
+		}
 	}
 
 	remoteIP, trusted := c.RemoteIP()
diff --git a/gin.go b/gin.go
index 56e5c76..00686e7 100644
--- a/gin.go
+++ b/gin.go
@@ -105,6 +105,10 @@ type Engine struct {
 	// 'X-AppEngine...' for better integration with that PaaS.
 	AppEngine bool
 
+	// If enabled, it will trust the CF-Connecting-IP header to determine the
+	// IP of the client.
+	CloudflareProxy bool
+
 	// If enabled, the url.RawPath will be used to find parameters.
 	UseRawPath bool
 


Test patch:
diff --git a/context_test.go b/context_test.go
index aaa358e..e0de717 100644
--- a/context_test.go
+++ b/context_test.go
@@ -1476,6 +1476,13 @@ func TestContextClientIP(t *testing.T) {
 	c.Request.Header.Del("X-Appengine-Remote-Addr")
 	assert.Equal(t, "40.40.40.40", c.ClientIP())
 
+	c.engine.AppEngine = false
+	c.engine.CloudflareProxy = true
+	assert.Equal(t, "60.60.60.60", c.ClientIP())
+
+	c.Request.Header.Del("CF-Connecting-IP")
+	assert.Equal(t, "40.40.40.40", c.ClientIP())
+
 	// no port
 	c.Request.RemoteAddr = "50.50.50.50"
 	assert.Empty(t, c.ClientIP())
@@ -1485,6 +1492,7 @@ func resetContextForClientIPTests(c *Context) {
 	c.Request.Header.Set("X-Real-IP", " 10.10.10.10  ")
 	c.Request.Header.Set("X-Forwarded-For", "  20.20.20.20, 30.30.30.30")
 	c.Request.Header.Set("X-Appengine-Remote-Addr", "50.50.50.50")
+	c.Request.Header.Set("CF-Connecting-IP", "60.60.60.60")
 	c.Request.RemoteAddr = "  40.40.40.40:42123 "
 	c.engine.AppEngine = false
 }