Correctly retrieve originating client IP in more cases

The current implementation would have returned the IP of the client sending/forwarding the request to the Django server which would not be accurate whenever Django would be running behind a proxy which would almost be the case in production. Used the standard HTTP_X_FORWARDED_FOR header to figure out the actual IP of the originating client that initiated the request.

djstripe/models/webhooks.py

@@ -51,6 +51,32 @@ def _get_version():
     return __version__
 
 
+def get_remote_ip(request):
+    """Given the HTTPRequest object return the IP Address of the client
+
+    :param request: client request
+    :type request: HTTPRequest
+
+    :Returns: the client ip address
+    """
+
+    # HTTP_X_FORWARDED_FOR is relevant for django running behind a proxy
+    x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR")
+    if x_forwarded_for:
+        ip = x_forwarded_for.split(",")[0]
+    else:
+        ip = request.META.get("REMOTE_ADDR")
+
+    if not ip:
+        warnings.warn(
+            "Could not determine remote IP (missing REMOTE_ADDR). "
+            "This is likely an issue with your wsgi/server setup."
+        )
+        ip = "0.0.0.0"
+
+    return ip
+
+
 class WebhookEventTrigger(models.Model):
     """
     An instance of a request that reached the server endpoint for Stripe webhooks.
@@ -122,13 +148,7 @@ def from_request(cls, request):
         except Exception:
             body = "(error decoding body)"
 
-        ip = request.META.get("REMOTE_ADDR")
-        if not ip:
-            warnings.warn(
-                "Could not determine remote IP (missing REMOTE_ADDR). "
-                "This is likely an issue with your wsgi/server setup."
-            )
-            ip = "0.0.0.0"
+        ip = get_remote_ip(request)
 
         try:
             data = json.loads(body)

tests/test_webhooks.py

@@ -7,12 +7,14 @@
 from copy import deepcopy
 from unittest.mock import Mock, PropertyMock, call, patch
 
+import pytest
 from django.test import TestCase, override_settings
 from django.test.client import Client
 from django.urls import reverse
 
 from djstripe import webhooks
 from djstripe.models import Event, Transfer, WebhookEventTrigger
+from djstripe.models.webhooks import get_remote_ip
 from djstripe.settings import djstripe_settings
 from djstripe.webhooks import TEST_EVENT_ID, call_handlers, handler, handler_all
 
@@ -25,6 +27,8 @@
     IS_STATICMETHOD_AUTOSPEC_SUPPORTED,
 )
 
+pytestmark = pytest.mark.django_db
+
 
 def mock_webhook_handler(webhook_event_trigger):
     webhook_event_trigger.process()
@@ -533,3 +537,54 @@ def _call_handlers(event_spec, data):
         type(event).verb = PropertyMock(return_value=verb)
         call_handlers(event=event)
         return event
+
+
+class TestGetRemoteIp:
+    class RequestClass:
+        def __init__(self, data):
+            self.data = data
+
+        @property
+        def META(self):
+            return self.data
+
+    @pytest.mark.parametrize(
+        "data",
+        [
+            {"HTTP_X_FORWARDED_FOR": "127.0.0.1,345.5.5.3,451.1.1.2"},
+            {
+                "REMOTE_ADDR": "422.0.0.1",
+                "HTTP_X_FORWARDED_FOR": "127.0.0.1,345.5.5.3,451.1.1.2",
+            },
+            {
+                "REMOTE_ADDR": "127.0.0.1",
+            },
+        ],
+    )
+    def test_get_remote_ip(self, data):
+        request = self.RequestClass(data)
+        assert get_remote_ip(request) == "127.0.0.1"
+
+    @pytest.mark.parametrize(
+        "data",
+        [
+            {
+                "REMOTE_ADDR": "",
+            },
+            {
+                "pqwwe": "127.0.0.1",
+            },
+        ],
+    )
+    def test_get_remote_ip_remote_addr_is_none(self, data):
+        request = self.RequestClass(data)
+
+        # ensure warning is raised
+        with pytest.warns(None) as recorded_warning:
+            assert get_remote_ip(request) == "0.0.0.0"
+
+        assert len(recorded_warning) == 1
+        assert (
+            "Could not determine remote IP (missing REMOTE_ADDR)."
+            in recorded_warning[0].message.args[0]
+        )