[PATCH django-impersonate] add option for auto-expiring impersonate sesions, implements #45
Export this patch
# HG changeset patch
# User Leonhard Kuboschek <leo@jacobs-alumni.de>
# Date 1606156336 -3600
# Mon Nov 23 19:32:16 2020 +0100
# Node ID 5ae6838bdebd6fa362f7fd730476a045c6f2772d
# Parent 1e3192bbb6762d1756fd203d28714620ef43da15
add option for auto-expiring impersonate sesions, implements #45
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -395,6 +395,13 @@
Default is `True`
+ MAX_DURATION+ + A number specifying the maximum allowed duration of impersonation + sessions in **seconds**.+ + Default is `None`+ Admin
=====
diff --git a/README.rst b/README.rst
--- a/README.rst
+++ b/README.rst
@@ -464,6 +464,15 @@
Default is ``True``
+ ::+ + MAX_DURATION+ + A number specifying the maximum allowed duration of impersonation + sessions in **seconds**.+ + Default is `None`+ Admin
=====
diff --git a/impersonate/middleware.py b/impersonate/middleware.py
--- a/impersonate/middleware.py
+++ b/impersonate/middleware.py
@@ -1,4 +1,6 @@
# -*- coding: utf-8 -*-
+ from datetime import datetime, timedelta
I wonder if it's not better to use the `django.utils.timezone` module
here? Also use UTC aware timestamps.
+ from django.http import HttpResponseNotAllowed
from django.utils.deprecation import MiddlewareMixin
@@ -12,6 +14,17 @@
request.impersonator = None
if request.user.is_authenticated and '_impersonate' in request.session:
+ if settings.MAX_DURATION:+ if '_impersonate_start' not in request.session:+ return+ + start_time = datetime.fromtimestamp(request.session['_impersonate_start'])+ if datetime.now() - start_time > timedelta(seconds=settings.MAX_DURATION):
Use `datetime.utcfromtimestamp()` and `timezone.now()` which returns a
UTC aware copy of datetime.now(). We'd have to also use
`timezone.make_aware()` on the `start_time` variable when subtracting.
`if timezone.now() - timezone.make_aware(start_time) > ...`
I could be overthinking it but I'm imagining a crazy bug where an app is
deployed across multiple time zones, and systems are not setup to use
UTC, and then we end up invalidating sessions prematurely.
What do you think?
+ del request.session['_impersonate']+ del request.session['_impersonate_start']+ + return+ new_user_id = request.session['_impersonate']
if isinstance(new_user_id, User):
# Edge case for issue 15
diff --git a/impersonate/settings.py b/impersonate/settings.py
--- a/impersonate/settings.py
+++ b/impersonate/settings.py
@@ -23,6 +23,7 @@
'ADMIN_DELETE_PERMISSION': False,
'ADMIN_ADD_PERMISSION': False,
'ADMIN_READ_ONLY': True,
+ 'MAX_DURATION': None, }
diff --git a/impersonate/tests.py b/impersonate/tests.py
--- a/impersonate/tests.py
+++ b/impersonate/tests.py
@@ -22,6 +22,7 @@
from distutils.version import LooseVersion
from unittest.mock import PropertyMock, patch
from urllib.parse import urlencode, urlsplit
+ from datetime import datetime, timedelta
import django
from django.urls import include, path
@@ -110,11 +111,12 @@
return None
self.middleware = ImpersonateMiddleware(dummy_get_response)
- def _impersonated_request(self, use_id=True):+ def _impersonated_request(self, use_id=True, _impersonate_start=None): request = self.factory.get('/')
request.user = self.superuser
request.session = {
'_impersonate': self.user.pk if use_id else self.user,
+ '_impersonate_start': _impersonate_start }
self.middleware.process_request(request)
@@ -133,6 +135,45 @@
'''
self._impersonated_request(use_id=False)
+ @override_settings(IMPERSONATE={'MAX_DURATION': 3600})+ def test_impersonated_request_with_max_duration(self):+ self._impersonated_request(_impersonate_start=datetime.now().timestamp())+ + @override_settings(IMPERSONATE={'MAX_DURATION': 3600})+ def test_reject_without_start_time(self):+ ''' Test to ensure that requests without a start time+ are rejected when MAX_DURATION is set+ '''+ request = self.factory.get('/')+ request.user = self.superuser+ request.session = {+ '_impersonate': self.user.pk,+ }+ self.middleware.process_request(request)+ + self.assertEqual(request.user, self.superuser)+ self.assertFalse(request.user.is_impersonate)+ + + @override_settings(IMPERSONATE={'MAX_DURATION': 3600})+ def test_reject_expired_impersonation(self):+ ''' Test to ensure that requests with a start time before (now - MAX_DURATION)+ are rejected+ '''+ request = self.factory.get('/')+ request.user = self.superuser+ request.session = {+ '_impersonate': self.user.pk,+ '_impersonate_start': (datetime.now() - timedelta(seconds=3601)).timestamp()+ }+ self.middleware.process_request(request)+ + self.assertEqual(request.user, self.superuser)+ self.assertFalse(request.user.is_impersonate)+ self.assertNotIn('_impersonate', request.session)+ self.assertNotIn('_impersonate_start', request.session)+ + def test_not_impersonated_request(self, use_id=True):
"""Check the real_user request attr is set correctly when **not** impersonating."""
request = self.factory.get('/')
diff --git a/impersonate/views.py b/impersonate/views.py
--- a/impersonate/views.py
+++ b/impersonate/views.py
@@ -1,6 +1,8 @@
# -*- coding: utf-8 -*-
import logging
+ from datetime import datetime+ from django.db.models import Q
from django.http import Http404
from django.shortcuts import get_object_or_404, redirect, render
@@ -36,6 +38,7 @@
raise Http404('Invalid value given.')
if check_allow_for_user(request, new_user):
request.session['_impersonate'] = new_user.pk
+ request.session['_impersonate_start'] = datetime.now().timestamp()
If we make the change, this obviously would also have to move to
`timezone.now()`
prev_path = request.META.get('HTTP_REFERER')
if prev_path:
request.session[