Merge pull request #1976 from peterbe/bug-989055-django-rate-limiting…

…-should-honor-x-forwarded-for-header-2

fixes bug 989055 - django rate limiting should honor x-forwarded-for

webapp-django/crashstats/api/tests/test_views.py

@@ -4,6 +4,7 @@
 
 from django.contrib.auth.models import User, Permission
 from django.core.urlresolvers import reverse
+from django.conf import settings
 
 import mock
 from nose.tools import eq_, ok_
@@ -68,6 +69,17 @@ def setUpClass():
     def tearDownClass():
         TestViews.switch.delete()
 
+    def setUp(self):
+        super(TestViews, self).setUp()
+        self._middleware_classes = settings.MIDDLEWARE_CLASSES
+        settings.MIDDLEWARE_CLASSES += (
+            'crashstats.crashstats.middleware.SetRemoteAddrFromForwardedFor',
+        )
+
+    def tearDown(self):
+        super(TestViews, self).tearDown()
+        settings.MIDDLEWARE_CLASSES = self._middleware_classes
+
     def test_invalid_url(self):
         url = reverse('api:model_wrapper', args=('BlaBLabla',))
         response = self.client.get(url)
@@ -1829,6 +1841,10 @@ def mocked_get(url, params, **options):
             response = self.client.get(url)
         eq_(response.status_code, 429)
 
+        # but it'll work if you use a different X-Forwarded-For IP
+        response = self.client.get(url, HTTP_X_FORWARDED_FOR='11.11.11.11')
+        eq_(response.status_code, 200)
+
         user = User.objects.create(username='test')
         token = Token.objects.create(
             user=user,

webapp-django/crashstats/crashstats/middleware.py

@@ -0,0 +1,23 @@
+class SetRemoteAddrFromForwardedFor(object):
+    """
+    Middleware that sets REMOTE_ADDR based on HTTP_X_FORWARDED_FOR, if the
+    latter is set. This is useful if you're sitting behind a reverse proxy that
+    causes each request's REMOTE_ADDR to be set to 127.0.0.1.
+
+    Note that this does NOT validate HTTP_X_FORWARDED_FOR. If you're not behind
+    a reverse proxy that sets HTTP_X_FORWARDED_FOR automatically, do not use
+    this middleware. Anybody can spoof the value of HTTP_X_FORWARDED_FOR, and
+    because this sets REMOTE_ADDR based on HTTP_X_FORWARDED_FOR, that means
+    anybody can "fake" their IP address. Only use this when you can absolutely
+    trust the value of HTTP_X_FORWARDED_FOR.
+    """
+    def process_request(self, request):
+        try:
+            real_ip = request.META['HTTP_X_FORWARDED_FOR']
+        except KeyError:
+            return None
+        else:
+            # HTTP_X_FORWARDED_FOR can be a comma-separated list of IPs. The
+            # client's IP will be the first one.
+            real_ip = real_ip.split(",")[0].strip()
+            request.META['REMOTE_ADDR'] = real_ip

webapp-django/crashstats/settings/local.py-dist

@@ -143,3 +143,13 @@ SECRET_KEY = 'you must change this'
 
 # Set to True enable analysis of all model fetches
 #ANALYZE_MODEL_FETCHES = True
+
+# If you crashstats behind a load balancer, your `REMOTE_ADDR` header
+# will be that of the load balancer instead of the actual user.
+# The solution is to instead rely on the `X-Forwarded-For` header.
+# You ONLY want this if you know you can trust `X-Forwarded-For`.
+# (Note! Make sure you uncomment the line `from . import base` at
+# the top of this file first)
+#base.MIDDLEWARE_CLASSES += (
+#    'crashstats.crashstats.middleware.SetRemoteAddrFromForwardedFor',
+#)