internal/log: return full X-Forwarded-For

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>

docs/docs/CHANGELOG.md

@@ -22,6 +22,7 @@
 
 - The healthcheck endpoints (`/ping`) now returns the http status `405` StatusMethodNotAllowed for non-`GET` requests. [GH-319](https://github.com/pomerium/pomerium/issues/319)
 - Authenticate service no longer uses gRPC.
+- The global request logger now captures the full array of proxies from `X-Forwarded-For`, in addition to just the client IP.
 
 ### Removed
 

internal/log/middleware.go

@@ -172,22 +172,19 @@ func AccessHandler(f func(r *http.Request, status, size int, duration time.Durat
 	}
 }
 
-// ForwardedAddrHandler returns the client IP address from a request. If present, the
-// X-Forwarded-For header is assumed to be set by a load balancer, and its
-// rightmost entry (the client IP that connected to the LB) is returned.
+// ForwardedAddrHandler returns the client IP address from a request. If a
+// request goes through multiple proxies, the IP addresses of each successive
+// proxy is listed. This means, the right-most IP address is the IP address of
+// the most recent proxy and the left-most IP address is the IP address of the
+// originating client.
+// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For
 func ForwardedAddrHandler(fieldKey string) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			addr := r.RemoteAddr
 			if ra := r.Header.Get("X-Forwarded-For"); ra != "" {
-				forwardedList := strings.Split(ra, ",")
-				forwardedAddr := strings.TrimSpace(forwardedList[len(forwardedList)-1])
-				if forwardedAddr != "" {
-					addr = forwardedAddr
-				}
 				log := zerolog.Ctx(r.Context())
 				log.UpdateContext(func(c zerolog.Context) zerolog.Context {
-					return c.Str(fieldKey, addr)
+					return c.Strs(fieldKey, strings.Split(ra, ","))
 				})
 			}
 			next.ServeHTTP(w, r)

internal/log/middleware_test.go

@@ -253,18 +253,18 @@ func BenchmarkDataRace(b *testing.B) {
 
 func TestForwardedAddrHandler(t *testing.T) {
 	out := &bytes.Buffer{}
-	r := &http.Request{
-		Header: http.Header{
-			"X-Forwarded-For": []string{"client", "proxy1", "proxy2"},
-		},
-	}
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+
+	r.Header.Set("X-Forwarded-For", "proxy1,proxy2,proxy3")
+
 	h := ForwardedAddrHandler("fwd_ip")(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		l := FromRequest(r)
 		l.Log().Msg("")
 	}))
 	h = NewHandler(zerolog.New(out))(h)
 	h.ServeHTTP(nil, r)
-	if want, got := `{"fwd_ip":"client"}`+"\n", decodeIfBinary(out); want != got {
+	if want, got := `{"fwd_ip":["proxy1","proxy2","proxy3"]}`+"\n", decodeIfBinary(out); want != got {
 		t.Errorf("Invalid log output, got: %s, want: %s", got, want)
 	}
 }