From 7913567fbae35092aa20fa287685fb52bd104368 Mon Sep 17 00:00:00 2001 From: Terence Honles Date: Thu, 1 Jul 2021 14:34:53 -0700 Subject: [PATCH 1/5] fix #373 breaking collecting users information on *not* starlette The change fixes the check to match the previous code: `if not StarletteRequest and hasattr(request, 'user'):` --- rollbar/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 5101ea35..19d035f3 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -935,7 +935,7 @@ def _build_person_data(request): if StarletteRequest: from rollbar.contrib.starlette.requests import hasuser else: - hasuser = lambda request: False + hasuser = lambda request: True if hasuser(request) and hasattr(request, 'user'): user_prop = request.user From 1575465afb9f4c75e0fa9725e925341ce3c8e71b Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 19:47:31 +0200 Subject: [PATCH 2/5] Test person data building for Django --- rollbar/test/test_rollbar.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index 938c76d0..1d355d72 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -327,6 +327,33 @@ def test_fastapi_request_data_empty_values(self): self.assertEqual(data['user_ip'], scope['client'][0]) self.assertEqual(data['method'], scope['method']) + def test_django_build_person_data(self): + try: + import django + from django.conf import settings + except ImportError: + self.skipTest('Requires Django to be installed') + else: + settings.configure( + INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes'] + ) + django.setup() + + from django.contrib.auth.models import User + from django.http.request import HttpRequest + + request = HttpRequest() + request.user = User() + request.user.id = 123 + request.user.username = 'admin' + request.user.email = 'admin@example.org' + + data = rollbar._build_person_data(request) + + self.assertDictEqual( + data, {'id': '123', 'username': 'admin', 'email': 'admin@example.org'} + ) + @unittest.skipUnless(sys.version_info >= (3, 6), 'Python3.6+ required') def test_get_request_starlette_middleware(self): try: From fd2888432e70f3216a96942f2a2a08068a7e263b Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 19:52:25 +0200 Subject: [PATCH 3/5] Test person data building for Starlette --- rollbar/test/test_rollbar.py | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index 1d355d72..e0882bc9 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -354,6 +354,48 @@ def test_django_build_person_data(self): data, {'id': '123', 'username': 'admin', 'email': 'admin@example.org'} ) + def test_starlette_build_person_data_if_user_authenticated(self): + try: + from starlette.authentication import SimpleUser + from starlette.requests import Request + except ImportError: + self.skipTest('Requires Starlette to be installed') + + # Implement interface with the id attribute + class User(SimpleUser): + counter = 0 + + def __init__(self, username, email): + super().__init__(username) + self.email = email + + User.counter += 1 + self.id = User.counter + + scope = {'type': 'http'} + request = Request(scope) + # Make the user authenticated + request.scope['user'] = User('admin', 'admin@example.org') + + data = rollbar._build_person_data(request) + + self.assertDictEqual( + data, {'id': '1', 'username': 'admin', 'email': 'admin@example.org'} + ) + + def test_starlette_failsafe_build_person_data_if_user_not_authenticated(self): + try: + from starlette.requests import Request + except ImportError: + self.skipTest('Requires Starlette to be installed') + + scope = {'type': 'http'} + request = Request(scope) + + data = rollbar._build_person_data(request) + + self.assertIsNone(data) + @unittest.skipUnless(sys.version_info >= (3, 6), 'Python3.6+ required') def test_get_request_starlette_middleware(self): try: From 119dc59892307c7b4e1dbe13b29693d95bf61a02 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sat, 18 Sep 2021 20:01:18 +0200 Subject: [PATCH 4/5] Enrich the traceback with the defined function --- rollbar/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 19d035f3..215d0dbc 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -935,7 +935,7 @@ def _build_person_data(request): if StarletteRequest: from rollbar.contrib.starlette.requests import hasuser else: - hasuser = lambda request: True + def hasuser(request): return True if hasuser(request) and hasattr(request, 'user'): user_prop = request.user From c4f7e3e47f87dc503b40bd493d39ed37138acb38 Mon Sep 17 00:00:00 2001 From: Bart Skowron Date: Sun, 19 Sep 2021 01:39:02 +0200 Subject: [PATCH 5/5] Call `django.setup()` for Django 1.7+ only --- rollbar/test/test_rollbar.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rollbar/test/test_rollbar.py b/rollbar/test/test_rollbar.py index e0882bc9..ed797883 100644 --- a/rollbar/test/test_rollbar.py +++ b/rollbar/test/test_rollbar.py @@ -337,7 +337,8 @@ def test_django_build_person_data(self): settings.configure( INSTALLED_APPS=['django.contrib.auth', 'django.contrib.contenttypes'] ) - django.setup() + if django.VERSION >= (1, 7): + django.setup() from django.contrib.auth.models import User from django.http.request import HttpRequest