Get IP from "standard" HTTP headers #3287

internal/httputil/rate_limiting.go

@@ -88,11 +88,7 @@ func (l *RateLimits) Limit(req *http.Request, device *userapi.Device) *util.JSON
 			caller = device.UserID + device.ID
 		}
 	} else {
-		if forwardedFor := req.Header.Get("X-Forwarded-For"); forwardedFor != "" {
-			caller = forwardedFor
-		} else {
-			caller = req.RemoteAddr
-		}
+		caller = ip.GetRemoteHeader(req, "")
 	}
 
 	// Look up the caller's channel, if they have one.

ip/remote.go

@@ -0,0 +1,40 @@
+package ip
+
+import (
+	"net"
+	"net/http"
+	"strings"
+)
+
+// Gets real IP of the user. Order:
+//   - 'X-Forwarded-For' - de facto standard, which is supported by majority of reverse proxies
+//   - a custom header defined in the params
+//   - req.RemoteAddr
+//
+// Returns single IP address
+func GetRemoteHeader(req *http.Request, customHeaderName string) string {
+	header := req.RemoteAddr
+
+	// TODO: to discuss the order of precedence
+	possibleIPHeaders := []string{
+		req.Header.Get("X-Forwarded-For"),
+		req.Header.Get(customHeaderName),
+		req.RemoteAddr,
+	}
+
+	// pick first with meaningful data
+	for _, v := range possibleIPHeaders {
+		if v != "" {
+			header = v
+			break
+		}
+	}
+
+	// sometimes you get multiple addresses
+	addresses := strings.Split(header, ",")
+	if ip := net.ParseIP(addresses[0]); ip != nil {
+		header = addresses[0]
+	}
+
+	return header
+}

syncapi/sync/requestpool.go

@@ -23,6 +23,7 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/element-hq/dendrite/internal/sqlutil"
+	"github.com/element-hq/dendrite/ip"
 	roomserverAPI "github.com/element-hq/dendrite/roomserver/api"
 	"github.com/element-hq/dendrite/setup/config"
 	"github.com/element-hq/dendrite/syncapi/internal"
@@ -229,23 +230,10 @@ func (rp *RequestPool) updateLastSeen(req *http.Request, device *userapi.Device)
 		return
 	}
 
-	remoteAddr := req.RemoteAddr
-	if rp.cfg.RealIPHeader != "" {
-		if header := req.Header.Get(rp.cfg.RealIPHeader); header != "" {
-			// TODO: Maybe this isn't great but it will satisfy both X-Real-IP
-			// and X-Forwarded-For (which can be a list where the real client
-			// address is the first listed address). Make more intelligent?
-			addresses := strings.Split(header, ",")
-			if ip := net.ParseIP(addresses[0]); ip != nil {
-				remoteAddr = addresses[0]
-			}
-		}
-	}
-
 	lsreq := &userapi.PerformLastSeenUpdateRequest{
 		UserID:     device.UserID,
 		DeviceID:   device.ID,
-		RemoteAddr: remoteAddr,
+		RemoteAddr: ip.GetRemoteHeader(req, rp.cfg.RealIPHeader),
 		UserAgent:  req.UserAgent(),
 	}
 	lsres := &userapi.PerformLastSeenUpdateResponse{}