# 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
+
 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):
+                    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()
         prev_path = request.META.get('HTTP_REFERER')
         if prev_path:
             request.session[

On 11/23, Leonhard Kuboschek wrote:
>+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.

>+                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?

>     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()`