Harden StaffSession handling

src/pretix/base/models/auth.py

@@ -647,25 +647,22 @@ class User(AbstractBaseUser, PermissionsMixin, LoggingMixin):
             id__in=self.teams.filter(TeamQuerySet.organizer_permission_q(permission)).values_list('organizer', flat=True)
         )
 
-    def has_active_staff_session(self, session_key=None):
+    def has_active_staff_session(self, session_key):
         """
         Returns whether or not a user has an active staff session (formerly known as superuser session)
         with the given session key.
         """
         return self.get_active_staff_session(session_key) is not None
 
-    def get_active_staff_session(self, session_key=None):
-        if not self.is_staff:
+    def get_active_staff_session(self, session_key):
+        if not self.is_staff or not session_key:
             return None
         if not hasattr(self, '_staff_session_cache'):
             self._staff_session_cache = {}
         if session_key not in self._staff_session_cache:
-            qs = StaffSession.objects.filter(
-                user=self, date_end__isnull=True
-            )
-            if session_key:
-                qs = qs.filter(session_key=session_key)
-            sess = qs.first()
+            sess = StaffSession.objects.filter(
+                user=self, date_end__isnull=True, session_key=session_key
+            ).first()
             if sess:
                 if sess.date_start < now() - timedelta(seconds=settings.PRETIX_SESSION_TIMEOUT_ABSOLUTE):
                     sess.date_end = now()

src/pretix/control/middleware.py

@@ -36,6 +36,7 @@ from urllib.parse import quote, urljoin, urlparse
 
 from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME, logout
+from django.contrib.auth.views import redirect_to_login
 from django.http import Http404
 from django.shortcuts import get_object_or_404, resolve_url
 from django.template.response import TemplateResponse
@@ -214,12 +215,15 @@ class AuditLogMiddleware:
                 hijack_history = request.session.get('hijack_history', False)
                 hijacker = get_object_or_404(User, pk=hijack_history[0]["user"])
                 ss = hijacker.get_active_staff_session(request.session.get('hijacker_session'))
-                if ss:
-                    ss.logs.create(
-                        url=request.path,
-                        method=request.method,
-                        impersonating=request.user
-                    )
+                if not ss:
+                    # Staff session expired or not found
+                    logout(request)
+                    return redirect_to_login(request.get_full_path())
+                ss.logs.create(
+                    url=request.path,
+                    method=request.method,
+                    impersonating=request.user
+                )
             else:
                 ss = request.user.get_active_staff_session(request.session.session_key)
                 if ss:

src/pretix/control/views/users.py

@@ -31,6 +31,7 @@ from django.contrib.auth import (
 )
 from django.contrib.auth.mixins import LoginRequiredMixin
 from django.contrib.auth.views import redirect_to_login
+from django.core.exceptions import PermissionDenied
 from django.db import transaction
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse
@@ -221,11 +222,13 @@ class UserImpersonateView(AdministratorPermissionRequiredMixin, RecentAuthentica
 
     def post(self, request, *args, **kwargs):
         self.object = get_object_or_404(User, pk=self.kwargs.get("id"))
+        staff_session = request.user.get_active_staff_session(request.session.session_key)
         self.request.user.log_action('pretix.control.auth.user.impersonated',
                                      user=request.user,
                                      data={
                                          'other': self.kwargs.get("id"),
-                                         'other_email': self.object.email
+                                         'other_email': self.object.email,
+                                         'staff_session': staff_session.pk,
                                      })
         oldkey = request.session.session_key
 
@@ -249,6 +252,12 @@ class UserImpersonateView(AdministratorPermissionRequiredMixin, RecentAuthentica
         with signals.no_update_last_login(), keep_session_age(request.session):
             login(request, hijacked, backend=backend)
 
+        request.session.save()
+        staff_session.logs.create(
+            method='(NOTE)',
+            url=f'Begin impersonating user #{hijacked.pk} (request session {oldkey[:8]} -> {request.session.session_key[:8]})',
+        )
+
         request.session["hijack_history"] = hijack_history
 
         signals.hijack_started.send(
@@ -265,13 +274,15 @@ class UserImpersonateView(AdministratorPermissionRequiredMixin, RecentAuthentica
 class UserImpersonateStopView(LoginRequiredMixin, View):
 
     def post(self, request, *args, **kwargs):
-        impersonated = request.user
-
-        hijs = request.session['hijacker_session']
+        staff_session_key = request.session['hijacker_session']
+        prev_session_key = request.session.session_key
         hijack_history = request.session.get("hijack_history", [])
         hijacked = request.user
         prev_session = hijack_history.pop()
         hijacker = get_object_or_404(get_user_model(), pk=prev_session["user"])
+        staff_session = hijacker.get_active_staff_session(staff_session_key)
+        if not staff_session:
+            raise PermissionDenied
 
         expected_hash = salted_hmac(
             key_salt=b"hijack-history-hash",
@@ -299,17 +310,21 @@ class UserImpersonateStopView(LoginRequiredMixin, View):
             hijacked=hijacked,
         )
 
-        ss = request.user.get_active_staff_session(hijs)
-        if ss:
-            request.session.save()
-            ss.session_key = request.session.session_key
-            ss.save()
+        request.session.save()
+        staff_session.session_key = request.session.session_key
+        staff_session.save()
+
+        staff_session.logs.create(
+            method='(NOTE)',
+            url=f'Stop impersonating user #{hijacked.pk} (request session {prev_session_key[:8]}, staff session {staff_session_key[:8]} -> {request.session.session_key[:8]})',
+        )
 
         request.user.log_action('pretix.control.auth.user.impersonate_stopped',
                                 user=request.user,
                                 data={
-                                    'other': impersonated.pk,
-                                    'other_email': impersonated.email
+                                    'other': hijacked.pk,
+                                    'other_email': hijacked.email,
+                                    'staff_session': staff_session.pk,
                                 })
         return redirect(reverse('control:index'))