core: move impersonation to core, add tests, add better permission checks

passbook/admin/middleware.py

@@ -1,26 +0,0 @@
-"""passbook admin Middleware to impersonate users"""
-
-from passbook.core.models import User
-
-
-def impersonate(get_response):
-    """Middleware to impersonate users"""
-
-    def middleware(request):
-        """Middleware to impersonate users"""
-
-        # User is superuser and has __impersonate ID set
-        if request.user.is_superuser and "__impersonate" in request.GET:
-            request.session["impersonate_id"] = request.GET["__impersonate"]
-        # user wants to stop impersonation
-        elif "__unimpersonate" in request.GET and "impersonate_id" in request.session:
-            del request.session["impersonate_id"]
-
-        # Actually impersonate user
-        if request.user.is_superuser and "impersonate_id" in request.session:
-            request.user = User.objects.get(pk=request.session["impersonate_id"])
-
-        response = get_response(request)
-        return response
-
-    return middleware

passbook/admin/settings.py

@@ -1,5 +0,0 @@
-"""passbook admin settings"""
-
-MIDDLEWARE = [
-    "passbook.admin.middleware.impersonate",
-]

passbook/admin/templates/administration/user/list.html

@@ -55,7 +55,7 @@
                         <a class="pf-c-button pf-m-secondary" href="{% url 'passbook_admin:user-update' pk=user.pk %}?back={{ request.get_full_path }}">{% trans 'Edit' %}</a>
                         <a class="pf-c-button pf-m-danger" href="{% url 'passbook_admin:user-delete' pk=user.pk %}?back={{ request.get_full_path }}">{% trans 'Delete' %}</a>
                         <a class="pf-c-button pf-m-tertiary" href="{% url 'passbook_admin:user-password-reset' pk=user.pk %}?back={{ request.get_full_path }}">{% trans 'Reset Password' %}</a>
-                        <a class="pf-c-button pf-m-tertiary" href="{% url 'passbook_core:overview' %}?__impersonate={{ user.pk }}">{% trans 'Impersonate' %}</a>
+                        <a class="pf-c-button pf-m-tertiary" href="{% url 'passbook_core:impersonate-init' user_id=user.pk %}">{% trans 'Impersonate' %}</a>
                     </td>
                 </tr>
                 {% endfor %}

passbook/core/middleware.py

@@ -0,0 +1,26 @@
+"""passbook admin Middleware to impersonate users"""
+
+from typing import Callable
+
+from django.http import HttpRequest, HttpResponse
+
+SESSION_IMPERSONATE_USER = "passbook_impersonate_user"
+SESSION_IMPERSONATE_ORIGINAL_USER = "passbook_impersonate_original_user"
+
+
+class ImpersonateMiddleware:
+    """Middleware to impersonate users"""
+
+    get_response: Callable[[HttpRequest], HttpResponse]
+
+    def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]):
+        self.get_response = get_response
+
+    def __call__(self, request: HttpRequest) -> HttpResponse:
+        # No permission checks are done here, they need to be checked before
+        # SESSION_IMPERSONATE_USER is set.
+
+        if SESSION_IMPERSONATE_USER in request.session:
+            request.user = request.session[SESSION_IMPERSONATE_USER]
+
+        return self.get_response(request)

passbook/core/migrations/0010_auto_20200917_1021.py

@@ -0,0 +1,24 @@
+# Generated by Django 3.1.1 on 2020-09-17 10:21
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("passbook_core", "0009_group_is_superuser"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="user",
+            options={
+                "permissions": (
+                    ("reset_user_password", "Reset Password"),
+                    ("impersonate", "Can impersonate other users"),
+                ),
+                "verbose_name": "User",
+                "verbose_name_plural": "Users",
+            },
+        ),
+    ]

passbook/core/models.py

@@ -98,7 +98,10 @@ class User(GuardianUserMixin, AbstractUser):
 
     class Meta:
 
-        permissions = (("reset_user_password", "Reset Password"),)
+        permissions = (
+            ("reset_user_password", "Reset Password"),
+            ("impersonate", "Can impersonate other users"),
+        )
         verbose_name = _("User")
         verbose_name_plural = _("Users")
 

passbook/core/templates/base/skeleton.html

@@ -21,13 +21,13 @@
         {% endblock %}
     </head>
     <body>
-        {% if 'impersonate_id' in request.session %}
+        {% if 'passbook_impersonate_user' in request.session %}
         <div class="pf-c-banner pf-m-warning pf-c-alert pf-m-sticky">
             <div class="pf-l-flex pf-m-justify-content-center pf-m-justify-content-space-between-on-lg pf-m-nowrap" style="height: 100%;">
                 <div class=""></div>
                 <div class="pf-u-display-none pf-u-display-block-on-lg">
                     {% blocktrans with user=user %}You're currently impersonating {{ user }}.{% endblocktrans %}
-                    <a href="?__unimpersonate=True" id="acceptMessage">{% trans 'Stop impersonation' %}</a>
+                    <a href="{% url 'passbook_core:impersonate-end' %}?back={{ request.get_full_path }}" id="acceptMessage">{% trans 'Stop impersonation' %}</a>
                 </div>
                 <div class=""></div>
             </div>

passbook/core/tests/test_impersonation.py

@@ -0,0 +1,55 @@
+"""impersonation tests"""
+from django.shortcuts import reverse
+from django.test.testcases import TestCase
+
+from passbook.core.models import User
+
+
+class TestImpersonation(TestCase):
+    """impersonation tests"""
+
+    def setUp(self) -> None:
+        super().setUp()
+        self.other_user = User.objects.create(username="to-impersonate")
+        self.pbadmin = User.objects.get(username="pbadmin")
+
+    def test_impersonate_simple(self):
+        """test simple impersonation and un-impersonation"""
+        self.client.force_login(self.pbadmin)
+
+        self.client.get(
+            reverse(
+                "passbook_core:impersonate-init", kwargs={"user_id": self.other_user.pk}
+            )
+        )
+
+        response = self.client.get(reverse("passbook_core:overview"))
+        self.assertIn(self.other_user.username, response.content.decode())
+        self.assertNotIn(self.pbadmin.username, response.content.decode())
+
+        self.client.get(reverse("passbook_core:impersonate-end"))
+
+        response = self.client.get(reverse("passbook_core:overview"))
+        self.assertNotIn(self.other_user.username, response.content.decode())
+        self.assertIn(self.pbadmin.username, response.content.decode())
+
+    def test_impersonate_denied(self):
+        """test impersonation without permissions"""
+        self.client.force_login(self.other_user)
+
+        self.client.get(
+            reverse(
+                "passbook_core:impersonate-init", kwargs={"user_id": self.pbadmin.pk}
+            )
+        )
+
+        response = self.client.get(reverse("passbook_core:overview"))
+        self.assertIn(self.other_user.username, response.content.decode())
+        self.assertNotIn(self.pbadmin.username, response.content.decode())
+
+    def test_un_impersonate_empty(self):
+        """test un-impersonation without impersonating first"""
+        self.client.force_login(self.other_user)
+
+        response = self.client.get(reverse("passbook_core:impersonate-end"))
+        self.assertRedirects(response, reverse("passbook_core:overview"))

passbook/core/urls.py

@@ -1,11 +1,22 @@
 """passbook URL Configuration"""
 from django.urls import path
 
-from passbook.core.views import overview, user
+from passbook.core.views import impersonate, overview, user
 
 urlpatterns = [
     # User views
     path("-/user/", user.UserSettingsView.as_view(), name="user-settings"),
     # Overview
     path("", overview.OverviewView.as_view(), name="overview"),
+    # Impersonation
+    path(
+        "-/impersonation/<int:user_id>/",
+        impersonate.ImpersonateInitView.as_view(),
+        name="impersonate-init",
+    ),
+    path(
+        "-/impersonation/end/",
+        impersonate.ImpersonateEndView.as_view(),
+        name="impersonate-end",
+    ),
 ]

passbook/core/views/impersonate.py

@@ -0,0 +1,55 @@
+"""passbook impersonation views"""
+
+from django.http import HttpRequest, HttpResponse
+from django.shortcuts import get_object_or_404, redirect
+from django.views import View
+from structlog import get_logger
+
+from passbook.core.middleware import (
+    SESSION_IMPERSONATE_ORIGINAL_USER,
+    SESSION_IMPERSONATE_USER,
+)
+from passbook.core.models import User
+
+LOGGER = get_logger()
+
+
+class ImpersonateInitView(View):
+    """Initiate Impersonation"""
+
+    def get(self, request: HttpRequest, user_id: int) -> HttpResponse:
+        """Impersonation handler, checks permissions"""
+        if not request.user.has_perm("impersonate"):
+            LOGGER.debug(
+                "User attempted to impersonate without permissions", user=request.user
+            )
+            return HttpResponse("Unauthorized", status=401)
+
+        user_to_be = get_object_or_404(User, pk=user_id)
+
+        request.session[SESSION_IMPERSONATE_ORIGINAL_USER] = request.user
+        request.session[SESSION_IMPERSONATE_USER] = user_to_be
+
+        # TODO Audit log entry
+
+        return redirect("passbook_core:overview")
+
+
+class ImpersonateEndView(View):
+    """End User impersonation"""
+
+    def get(self, request: HttpRequest) -> HttpResponse:
+        """End Impersonation handler"""
+        if (
+            SESSION_IMPERSONATE_USER not in request.session
+            or SESSION_IMPERSONATE_ORIGINAL_USER not in request.session
+        ):
+            LOGGER.debug("Can't end impersonation", user=request.user)
+            return redirect("passbook_core:overview")
+
+        del request.session[SESSION_IMPERSONATE_USER]
+        del request.session[SESSION_IMPERSONATE_ORIGINAL_USER]
+
+        # TODO: Audit log entry
+
+        return redirect("passbook_core:overview")

passbook/root/settings.py

@@ -179,6 +179,7 @@ MIDDLEWARE = [
     "django.middleware.csrf.CsrfViewMiddleware",
     "django.contrib.messages.middleware.MessageMiddleware",
     "django.middleware.clickjacking.XFrameOptionsMiddleware",
+    "passbook.core.middleware.ImpersonateMiddleware",
     "django_prometheus.middleware.PrometheusAfterMiddleware",
 ]