don't append default headers to ip_headers, drop X-Client-Id (#64)

bartes · web-flow · commit accaa55fd82a · 2020-03-31T11:43:31.000+02:00
* dont append default headers, drop X-Client-Id

* add pr number to changelog
diff --git a/HISTORY.md b/HISTORY.md
@@ -1,5 +1,7 @@
 ## master
 
+- [#64](https://github.com/castle/castle-python/pull/64)dropped X-Client-Id from calculation of ip, drop appending default ip headers to the ip_header list config when config is provided (in that case default headers have to explicitly provided)
+
 ## 3.1.0 (2020-02-27)
 
 - [#61](https://github.com/castle/castle-python/pull/61) improve headers and ip extractions, improve ip_headers config, add trusted proxies config, added more events to events list
diff --git a/README.rst b/README.rst
@@ -44,11 +44,11 @@ import and configure the library with your Castle API secret.
     configuration.blacklisted = ['HTTP-X-header']
 
     # Castle needs the original IP of the client, not the IP of your proxy or load balancer.
-    # we try to fetch proper ip based on X-Forwarded-For, X-Client-Id or Remote-Addr headers in that order
+    # we try to fetch proper ip based on X-Forwarded-For or Remote-Addr headers in that order
     # but sometimes proper ip may be stored in different header or order could be different.
     # SDK can extract ip automatically for you, but you must configure which ip_headers you would like to use
     configuration.ip_headers = []
-    # Additionally to make X-Forwarded-For or X-Client-Id work better discovering client ip address,
+    # Additionally to make X-Forwarded-For and other headers work better discovering client ip address,
     # and not the address of a reverse proxy server, you can define trusted proxies
     # which will help to fetch proper ip from those headers
     configuration.trusted_proxies = []
diff --git a/castle/extractors/ip.py b/castle/extractors/ip.py
@@ -3,31 +3,34 @@
 
 
 # ordered list of ip headers for ip extraction
-DEFAULT = ['X-Forwarded-For', 'Client-Ip', 'Remote-Addr']
-# default header fallback when ip is not found
-FALLBACK = 'Remote-Addr'
+DEFAULT = ['X-Forwarded-For', 'Remote-Addr']
 
 
 class ExtractorsIp(object):
     def __init__(self, headers):
         self.headers = headers
-        self.ip_headers = configuration.ip_headers + DEFAULT
+        if len(configuration.ip_headers) > 0:
+            self.ip_headers = configuration.ip_headers
+        else:
+            self.ip_headers = DEFAULT
         self.proxies = configuration.trusted_proxies + TRUSTED_PROXIES
 
     def call(self):
+        all_ips = []
+
         for ip_header in self.ip_headers:
-            ip_value = self._calculate_ip(ip_header)
-            if ip_value:
-                return ip_value
+            ips = self._ips_from(ip_header)
+            filtered_ips = self._remove_proxies(ips)
+
+            if len(filtered_ips) > 0:
+                return filtered_ips[-1]
 
-        return self.headers.get(FALLBACK, None)
+            all_ips = all_ips + ips
 
-    def _calculate_ip(self, header):
-        ips = self._ips_from(header)
-        filtered_ips = self._remove_proxies(ips)
+        # fallback to first whatever ip
+        if len(all_ips) > 0:
+            return all_ips[0]
 
-        if len(filtered_ips) > 0:
-            return filtered_ips[0]
         return None
 
     def _remove_proxies(self, ips):
@@ -51,4 +54,4 @@ def _ips_from(self, header):
         if not value:
             return []
 
-        return re.split(r'[,\s]+', value.strip())[::-1]
+        return re.split(r'[,\s]+', value.strip())
diff --git a/castle/test/extractors/ip_test.py b/castle/test/extractors/ip_test.py
@@ -14,15 +14,15 @@ def test_extract_ip(self):
 
     def test_extract_ip_when_second_header(self):
         headers = {'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5'}
-        configuration.ip_headers = ["HTTP_CF_CONNECTING_IP"]
+        configuration.ip_headers = ["HTTP_CF_CONNECTING_IP", "X-Forwarded-For"]
         self.assertEqual(
             ExtractorsIp(headers).call(),
             '1.2.3.4'
         )
 
     def test_extract_ip_when_second_header_with_different_setting(self):
         headers = {'Cf-Connecting-Ip': '1.2.3.4', 'X-Forwarded-For': '1.1.1.1, 1.2.2.2, 1.2.3.5'}
-        configuration.ip_headers = ["CF-CONNECTING-IP"]
+        configuration.ip_headers = ["CF-CONNECTING-IP", "X-Forwarded-For"]
         self.assertEqual(
             ExtractorsIp(headers).call(),
             '1.2.3.4'
