Always supply a user.ip_address value

This is explicitly choosing to also parse the X-Forwarded-For header to
yank out this value. Otherwise, the Sentry server relies only on the
REMOTE_ADDR value which will always be wrong when when someone is behind
a reverse proxy.

This logic already exists in some other clients, and this has been
brought up a number of times with users via tickets and support.

Worth noting that it's potentially possible for this value to now be
forged from a user, but the ramification of doing so is so low, it's not
worth putting this behavior behind a feature flag IMO. The worst someone
could do is make some data inside Sentry inaccurate and possibly
confusing. No worse than the current state of the world where the data
is completely inaccurate.

3 files changed, 51 insertions, 5 deletions

diff --git a/tests/contrib/django/tests.py b/tests/contrib/django/tests.py
index 14b68a1..e76f1f5 100644
--- a/tests/contrib/django/tests.py
+++ b/tests/contrib/django/tests.py
@@ -250,6 +250,7 @@ class DjangoClientTest(TestCase):
     @pytest.mark.skipif(not DJANGO_15, reason='< Django 1.5')
     def test_get_user_info_abstract_user(self):
         from django.db import models
+        from django.http import HttpRequest
         from django.contrib.auth.models import AbstractBaseUser
 
         class MyUser(AbstractBaseUser):
@@ -263,8 +264,25 @@ class DjangoClientTest(TestCase):
             email='admin@example.com',
             id=1,
         )
-        user_info = self.raven.get_user_info(user)
+
+        request = HttpRequest()
+        request.META['REMOTE_ADDR'] = '127.0.0.1'
+        request.user = user
+        user_info = self.raven.get_user_info(request)
+        assert user_info == {
+            'ip_address': '127.0.0.1',
+            'username': user.username,
+            'id': user.id,
+            'email': user.email,
+        }
+
+        request = HttpRequest()
+        request.META['REMOTE_ADDR'] = '127.0.0.1'
+        request.META['HTTP_X_FORWARDED_FOR'] = '1.1.1.1, 2.2.2.2'
+        request.user = user
+        user_info = self.raven.get_user_info(request)
         assert user_info == {
+            'ip_address': '1.1.1.1',
             'username': user.username,
             'id': user.id,
             'email': user.email,
@@ -273,6 +291,7 @@ class DjangoClientTest(TestCase):
     @pytest.mark.skipif(not DJANGO_110, reason='< Django 1.10')
     def test_get_user_info_is_authenticated_property(self):
         from django.db import models
+        from django.http import HttpRequest
         from django.contrib.auth.models import AbstractBaseUser
 
         class MyUser(AbstractBaseUser):
@@ -289,8 +308,25 @@ class DjangoClientTest(TestCase):
             email='admin@example.com',
             id=1,
         )
-        user_info = self.raven.get_user_info(user)
+
+        request = HttpRequest()
+        request.META['REMOTE_ADDR'] = '127.0.0.1'
+        request.user = user
+        user_info = self.raven.get_user_info(request)
+        assert user_info == {
+            'ip_address': '127.0.0.1',
+            'username': user.username,
+            'id': user.id,
+            'email': user.email,
+        }
+
+        request = HttpRequest()
+        request.META['REMOTE_ADDR'] = '127.0.0.1'
+        request.META['HTTP_X_FORWARDED_FOR'] = '1.1.1.1, 2.2.2.2'
+        request.user = user
+        user_info = self.raven.get_user_info(request)
         assert user_info == {
+            'ip_address': '1.1.1.1',
             'username': user.username,
             'id': user.id,
             'email': user.email,
diff --git a/tests/contrib/flask/tests.py b/tests/contrib/flask/tests.py
index ab7d803..e90a70d 100644
--- a/tests/contrib/flask/tests.py
+++ b/tests/contrib/flask/tests.py
@@ -59,7 +59,7 @@ def create_app(ignore_exceptions=None, debug=False, **config):
     def capture_exception():
         try:
             raise ValueError('Boom')
-        except:
+        except Exception:
             current_app.extensions['sentry'].captureException()
         return 'Hello'
 
@@ -318,4 +318,4 @@ class FlaskLoginTest(BaseTest):
         assert event['message'] == 'ValueError: hello world'
         assert 'request' in event
         assert 'user' in event
-        self.assertDictEqual(event['user'], User().to_dict())
+        self.assertDictEqual(event['user'], dict({'ip_address': '127.0.0.1'}, **User().to_dict()))
diff --git a/tests/utils/wsgi/tests.py b/tests/utils/wsgi/tests.py
index 5b2ffdc..79022ae 100644
--- a/tests/utils/wsgi/tests.py
+++ b/tests/utils/wsgi/tests.py
@@ -1,5 +1,5 @@
 from raven.utils.testutils import TestCase
-from raven.utils.wsgi import get_headers, get_host, get_environ
+from raven.utils.wsgi import get_headers, get_host, get_environ, get_client_ip
 
 
 class GetHeadersTest(TestCase):
@@ -84,3 +84,13 @@ class GetHostTest(TestCase):
             'SERVER_PORT': '81',
         })
         self.assertEquals(result, 'example.com:81')
+
+
+class GetClientIpTest(TestCase):
+    def test_has_remote_addr(self):
+        result = get_client_ip({'REMOTE_ADDR': '127.0.0.1'})
+        self.assertEquals(result, '127.0.0.1')
+
+    def test_xff(self):
+        result = get_client_ip({'HTTP_X_FORWARDED_FOR': '1.1.1.1, 127.0.0.1'})
+        self.assertEquals(result, '1.1.1.1')