From 1e35740c6c7cc28457767b13f065f1dcfc4b81fc Mon Sep 17 00:00:00 2001 From: Julian Date: Tue, 3 Oct 2023 13:07:56 -0600 Subject: [PATCH 1/7] Separated contents of login_as_user to django_su::utils.py::su_in to allow for suing in from python --- django_su/utils.py | 36 ++++++++++++++++++++++++++++++++++++ django_su/views.py | 26 ++------------------------ 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/django_su/utils.py b/django_su/utils.py index 160993b..532832b 100644 --- a/django_su/utils.py +++ b/django_su/utils.py @@ -5,7 +5,43 @@ from django.conf import settings from django.utils.module_loading import import_string +from django.contrib.auth import ( + BACKEND_SESSION_KEY, + SESSION_KEY, + authenticate, + login, +) +def su_in(request, user_id): + ''' + Returns: a User Object or None + ''' + if not request.user.has_perm("auth.change_user"): + return None + + userobj = authenticate(request=request, su=True, user_id=user_id) + if not userobj: + return None + + exit_users_pk = request.session.get("exit_users_pk", default=[]) + exit_users_pk.append( + (request.session[SESSION_KEY], request.session[BACKEND_SESSION_KEY]) + ) + + maintain_last_login = hasattr(userobj, "last_login") + if maintain_last_login: + last_login = userobj.last_login + + try: + if not custom_login_action(request, userobj): + login(request, userobj) + request.session["exit_users_pk"] = exit_users_pk + finally: + if maintain_last_login: + userobj.last_login = last_login + userobj.save(update_fields=["last_login"]) + + return userobj def su_login_callback(user): if hasattr(settings, "SU_LOGIN"): diff --git a/django_su/views.py b/django_su/views.py index 1293232..89f2884 100644 --- a/django_su/views.py +++ b/django_su/views.py @@ -4,9 +4,6 @@ from django.conf import settings from django.contrib.auth import ( - BACKEND_SESSION_KEY, - SESSION_KEY, - authenticate, get_user_model, login, ) @@ -17,7 +14,7 @@ from django.views.decorators.http import require_http_methods from .forms import UserSuForm -from .utils import custom_login_action, su_login_callback +from .utils import custom_login_action, su_login_callback, su_in User = get_user_model() @@ -27,28 +24,9 @@ @require_http_methods(["POST"]) @user_passes_test(su_login_callback) def login_as_user(request, user_id): - userobj = authenticate(request=request, su=True, user_id=user_id) - if not userobj: + if not su_in(request, user_id): raise Http404("User not found") - exit_users_pk = request.session.get("exit_users_pk", default=[]) - exit_users_pk.append( - (request.session[SESSION_KEY], request.session[BACKEND_SESSION_KEY]) - ) - - maintain_last_login = hasattr(userobj, "last_login") - if maintain_last_login: - last_login = userobj.last_login - - try: - if not custom_login_action(request, userobj): - login(request, userobj) - request.session["exit_users_pk"] = exit_users_pk - finally: - if maintain_last_login: - userobj.last_login = last_login - userobj.save(update_fields=["last_login"]) - if hasattr(settings, "SU_REDIRECT_LOGIN"): warnings.warn( "SU_REDIRECT_LOGIN is deprecated, use SU_LOGIN_REDIRECT_URL", From 9a3b699f08ad6c3730e9ff2b01639bd94d25d27f Mon Sep 17 00:00:00 2001 From: Julian Date: Fri, 6 Oct 2023 13:54:20 -0600 Subject: [PATCH 2/7] Adjusted whitespace to pass linter --- django_su/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/django_su/utils.py b/django_su/utils.py index 532832b..45425b4 100644 --- a/django_su/utils.py +++ b/django_su/utils.py @@ -12,6 +12,7 @@ login, ) + def su_in(request, user_id): ''' Returns: a User Object or None @@ -43,6 +44,7 @@ def su_in(request, user_id): return userobj + def su_login_callback(user): if hasattr(settings, "SU_LOGIN"): warnings.warn( From 4f3b97de5dff7dc47f27313917cfbb1440822843 Mon Sep 17 00:00:00 2001 From: Julian Date: Tue, 17 Oct 2023 16:32:47 -0600 Subject: [PATCH 3/7] Moved django pip install command above usage of pypa's setup tools as it installs an incompatible version of typing-extensions for python 3.7 --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 25be3ff..59de437 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -69,8 +69,8 @@ jobs: run: sudo apt-get install -y gettext - name: Install run: | - python setup.py develop pip install Django==${{ matrix.DJANGO_VERSION }} + python setup.py develop pip install -e . pip install -r requirements.test.txt pip install coveralls From fc259b0b60c88d2844efa95ceef30424dfc25660 Mon Sep 17 00:00:00 2001 From: Julian Date: Tue, 17 Oct 2023 16:41:47 -0600 Subject: [PATCH 4/7] Sorted imports --- django_su/utils.py | 2 +- django_su/views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/django_su/utils.py b/django_su/utils.py index 45425b4..63269af 100644 --- a/django_su/utils.py +++ b/django_su/utils.py @@ -4,13 +4,13 @@ from collections.abc import Callable from django.conf import settings -from django.utils.module_loading import import_string from django.contrib.auth import ( BACKEND_SESSION_KEY, SESSION_KEY, authenticate, login, ) +from django.utils.module_loading import import_string def su_in(request, user_id): diff --git a/django_su/views.py b/django_su/views.py index 89f2884..aaaf9a1 100644 --- a/django_su/views.py +++ b/django_su/views.py @@ -14,7 +14,7 @@ from django.views.decorators.http import require_http_methods from .forms import UserSuForm -from .utils import custom_login_action, su_login_callback, su_in +from .utils import custom_login_action, su_in, su_login_callback User = get_user_model() From 46882fa77c5859c1f1983847a88a94b022e16e67 Mon Sep 17 00:00:00 2001 From: Julian Date: Tue, 17 Oct 2023 17:02:00 -0600 Subject: [PATCH 5/7] Sorted imports and swapped ''' for """ in docstring --- django_su/utils.py | 11 +++-------- django_su/views.py | 5 +---- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/django_su/utils.py b/django_su/utils.py index 63269af..5968bf6 100644 --- a/django_su/utils.py +++ b/django_su/utils.py @@ -4,19 +4,14 @@ from collections.abc import Callable from django.conf import settings -from django.contrib.auth import ( - BACKEND_SESSION_KEY, - SESSION_KEY, - authenticate, - login, -) +from django.contrib.auth import BACKEND_SESSION_KEY, SESSION_KEY, authenticate, login from django.utils.module_loading import import_string def su_in(request, user_id): - ''' + """ Returns: a User Object or None - ''' + """ if not request.user.has_perm("auth.change_user"): return None diff --git a/django_su/views.py b/django_su/views.py index aaaf9a1..ebdd3cc 100644 --- a/django_su/views.py +++ b/django_su/views.py @@ -3,10 +3,7 @@ import warnings from django.conf import settings -from django.contrib.auth import ( - get_user_model, - login, -) +from django.contrib.auth import get_user_model, login from django.contrib.auth.decorators import user_passes_test from django.http import Http404, HttpResponseBadRequest, HttpResponseRedirect from django.shortcuts import get_object_or_404, render From d78083c51e743faa0ba315f981195920a00e41c9 Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 25 Oct 2023 18:30:36 -0600 Subject: [PATCH 6/7] Improved Test Coverage: - Made test_login_user_id_invalid actually check an invalid id - added test_login_success_without_custom_login_action - added test for auth.change_user permission check when calling su_in from user code --- django_su/tests/test_views.py | 50 ++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/django_su/tests/test_views.py b/django_su/tests/test_views.py index 75294b8..f2d948b 100644 --- a/django_su/tests/test_views.py +++ b/django_su/tests/test_views.py @@ -7,6 +7,7 @@ from django.test import Client, TestCase from django.urls import reverse from django.utils.datetime_safe import datetime +from django.http import HttpRequest User = get_user_model() @@ -60,10 +61,33 @@ def test_login_success(self): self.assertEqual(str(pk), str(self.authorized_user.pk)) self.assertEqual(backend, "django.contrib.auth.backends.ModelBackend") + def test_login_success_without_custom_login_action(self): + """Ensure login works for a valid user when SU_CUSTOM_LOGIN_ACTION is None""" + self.client.login(username="authorized", password="pass") + with self.settings(SU_CUSTOM_LOGIN_ACTION=None): + response = self.client.post( + reverse("login_as_user", args=[self.destination_user.id]) + ) + self.assertEqual(response.status_code, 302) + # Check the user is logged in in the session + self.assertIn(auth.SESSION_KEY, self.client.session) + self.assertEqual( + str(self.client.session[auth.SESSION_KEY]), str(self.destination_user.id) + ) + # Check the 'exit_users_pk' is set so we know which user to change back to + self.assertIn("exit_users_pk", self.client.session) + pk, backend = self.client.session["exit_users_pk"][0] + self.assertEqual(str(pk), str(self.authorized_user.pk)) + self.assertEqual(backend, "django.contrib.auth.backends.ModelBackend") + def test_login_user_id_invalid(self): """Ensure login fails with an invalid user id""" self.client.login(username="authorized", password="pass") - response = self.client.post("/su/abc/") + user_ids = User.objects.all().values_list('id', flat=True) + invalid_user_id = max(user_ids) + 1 + response = self.client.post( + reverse("login_as_user", args=[invalid_user_id]) + ) self.assertEqual(response.status_code, 404) # User should still be logged in, but as the original user self.assertIn(auth.SESSION_KEY, self.client.session) @@ -149,6 +173,30 @@ def error_action(request, user): self.assertTrue(connections) +class SuInUtilsTestCase(SuViewsBaseTestCase): + def test_su_in_no_perms(self): + """Ensure user must still have the auth.change_user permission when calling + su_in from user code""" + no_perms_user = self.user("noperms") + self.client.login(username="noperms", password="pass") + + from ..utils import su_in + + request = HttpRequest() + request.user = no_perms_user + + result = su_in(request, no_perms_user) + + self.assertEqual(result, None) + # User should still be logged in, but as the original user + self.assertIn(auth.SESSION_KEY, self.client.session) + self.assertEqual( + str(self.client.session[auth.SESSION_KEY]), str(no_perms_user.id) + ) + # Exit user should never get set + self.assertNotIn("exit_users_pk", self.client.session) + + class LoginViewTestCase(SuViewsBaseTestCase): def test_get_authorised(self): """Load the login page as an authorised user""" From 89ad91d3e686b484fd85ce3eba0fd34b80e6de1b Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 25 Oct 2023 19:01:09 -0600 Subject: [PATCH 7/7] Fixed linting errors --- django_su/tests/test_views.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/django_su/tests/test_views.py b/django_su/tests/test_views.py index f2d948b..12c7678 100644 --- a/django_su/tests/test_views.py +++ b/django_su/tests/test_views.py @@ -4,10 +4,10 @@ from django.contrib import auth from django.contrib.auth import get_user_model from django.contrib.sessions.backends import cached_db +from django.http import HttpRequest from django.test import Client, TestCase from django.urls import reverse from django.utils.datetime_safe import datetime -from django.http import HttpRequest User = get_user_model() @@ -83,11 +83,9 @@ def test_login_success_without_custom_login_action(self): def test_login_user_id_invalid(self): """Ensure login fails with an invalid user id""" self.client.login(username="authorized", password="pass") - user_ids = User.objects.all().values_list('id', flat=True) + user_ids = User.objects.all().values_list("id", flat=True) invalid_user_id = max(user_ids) + 1 - response = self.client.post( - reverse("login_as_user", args=[invalid_user_id]) - ) + response = self.client.post(reverse("login_as_user", args=[invalid_user_id])) self.assertEqual(response.status_code, 404) # User should still be logged in, but as the original user self.assertIn(auth.SESSION_KEY, self.client.session)