From e3aad24b7a443e59b824a40bb17cd49593e3b3d3 Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Mon, 27 Feb 2017 18:58:48 -0800 Subject: 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. --- tests/contrib/django/tests.py | 40 ++++++++++++++++++++++++++++++++++++++-- tests/contrib/flask/tests.py | 4 ++-- tests/utils/wsgi/tests.py | 12 +++++++++++- 3 files changed, 51 insertions(+), 5 deletions(-) (limited to 'tests') 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') -- cgit v1.2.1