Subject: [PATCH 1 of 2 django-impersonate] Allow OPTIONS requests when READ_ONLY is True - Refs #69

From: Sarah Boyce <[email-masked]>
Date: Sun, 19 Feb 2023 21:59:23 +0100

# HG changeset patch
# User [email-masked]
# Date 1676370721 -3600
#      Tue Feb 14 11:32:01 2023 +0100
# Node ID 77c3932f8751d8457a92596acb8b6a8ba2f73dbb
# Parent  89fffb32473e64276ca1a114bd2291a08e078227
Allow OPTIONS requests when READ_ONLY is True - Refs #69

diff --git a/README.rst b/README.rst
--- a/README.rst
+++ b/README.rst
@@ -272,9 +272,9 @@
    READ_ONLY
 
 A boolean that if set to ``True`` any requests that are not either
-``GET`` or ``HEAD`` will result in a "Bad Request" response (status code
-405). Use this if you want to limit your impersonating users to read
-only impersonation sessions.
+``GET`` or ``HEAD`` or ``OPTIONS`` will result in a "Bad Request"
+response (status code 405). Use this if you want to limit your
+impersonating users to read only impersonation sessions.
 
 Value should be a boolean, defaults to ``False``
 
diff --git a/impersonate/admin.py b/impersonate/admin.py
--- a/impersonate/admin.py
+++ b/impersonate/admin.py
@@ -176,7 +176,7 @@
     # `return False` hides impersonates module in admin page
     def has_change_permission(self, request, obj=None):
         if settings.ADMIN_READ_ONLY:
-            return request.method in ['GET', 'HEAD']
+            return request.method in ['GET', 'HEAD', 'OPTIONS']
         return True

diff --git a/impersonate/middleware.py b/impersonate/middleware.py
--- a/impersonate/middleware.py
+++ b/impersonate/middleware.py
@@ -50,8 +50,8 @@
             except User.DoesNotExist:
                 return
 
-            if settings.READ_ONLY and request.method not in ['GET', 'HEAD']:
-                return HttpResponseNotAllowed(['GET', 'HEAD'])
+            if settings.READ_ONLY and request.method not in ['GET', 'HEAD', 'OPTIONS']:
+                return HttpResponseNotAllowed(['GET', 'HEAD', 'OPTIONS'])
 
             if check_allow_for_user(request, new_user) and check_allow_for_uri(
                 request.path
diff --git a/impersonate/tests.py b/impersonate/tests.py
--- a/impersonate/tests.py
+++ b/impersonate/tests.py
@@ -828,6 +828,8 @@
         self.assertTrue(model_admin.has_change_permission(request))
         request.method = 'HEAD'
         self.assertTrue(model_admin.has_change_permission(request))
+        request.method = 'OPTIONS'
+        self.assertTrue(model_admin.has_change_permission(request))
         request.method = 'POST'
         self.assertFalse(model_admin.has_change_permission(request))
 
@@ -842,5 +844,11 @@
     @override_settings(IMPERSONATE={'READ_ONLY': True})
     def test_impersonate_read_only(self):
         self._impersonate_helper('user1', 'foobar', 4)
-        resp = self.client.post('/not/real/url/')
+        resp = self.client.post(reverse('impersonate-test'))
         self.assertEqual(resp.status_code, 405)
+        resp = self.client.get(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.head(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.options(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)

---

From: Sarah Boyce <[email-masked]>
Date: Sun, 19 Feb 2023 21:59:24 +0100

# HG changeset patch
# User Sarah Boyce <[email-masked]>
# Date 1676839415 -3600
#      Sun Feb 19 21:43:35 2023 +0100
# Node ID 49802f32cb83c01c77abf49bd0743055f8331306
# Parent  77c3932f8751d8457a92596acb8b6a8ba2f73dbb
Added CUSTOM_READ_ONLY setting to customise when to restrict to read only access - Refs #70

diff --git a/CHANGELOG b/CHANGELOG
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,12 @@
 Changes
 -------
 
+1.9.0 (unreleased)
+
+- Prevent redirect loop when MAX_DURATION is used. Refs ~petersanchez/django-impersonate#67
+- Allow OPTIONS requests when READ_ONLY is True. Refs ~petersanchez/django-impersonate#69
+- Added CUSTOM_READ_ONLY to customise when to restrict to read only access. Refs ~petersanchez/django-impersonate#70
+
 1.8.1 (2022-02-17)
 
 - Patch version bump for README updates. I know... I'm shameful.
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -241,11 +241,24 @@
     READ_ONLY
 
 A boolean that if set to `True` any requests that are not either `GET` or
-`HEAD` will result in a "Bad Request" response (status code 405). Use this if
-you want to limit your impersonating users to read only impersonation sessions.
+`HEAD` or `OPTIONS` will result in a "Bad Request" response (status code 405). 
+Use this if you want to limit your impersonating users to read only 
+impersonation sessions.
 
 Value should be a boolean, defaults to `False`
 
+If the `CUSTOM_READ_ONLY` is set, then that custom function is used, and this
+setting is ignored.
+
+    CUSTOM_READ_ONLY
+
+A string that represents a function (e.g. `module.submodule.mod.function_name`) 
+that allows more fine grained control over who has read only access. It takes 
+one argument, the request object, and should return True to restrict the user 
+to only allow `GET`, `HEAD` and `OPTIONS` requests.
+
+It is optional, and if it is not present, `READ_ONLY` setting value applies.
+
     USE_HTTP_REFERER
 
 If this is set to `True`, then the app will attempt to be redirect you to
diff --git a/impersonate/helpers.py b/impersonate/helpers.py
--- a/impersonate/helpers.py
+++ b/impersonate/helpers.py
@@ -128,3 +128,14 @@
         if re.search(exclusion, uri):
             return False
     return True
+
+
+def check_read_only(request):
+    ''' Returns True if can only do read only requests.
+        Uses the CUSTOM_READ_ONLY function if required, else
+        looks at the READ_ONLY setting.
+    '''
+    if settings.CUSTOM_READ_ONLY is not None:
+        custom_read_only_func = import_func_from_string(settings.CUSTOM_READ_ONLY)
+        return custom_read_only_func(request)
+    return settings.READ_ONLY
diff --git a/impersonate/middleware.py b/impersonate/middleware.py
--- a/impersonate/middleware.py
+++ b/impersonate/middleware.py
@@ -7,7 +7,7 @@
 from django.utils.deprecation import MiddlewareMixin
 from django.utils.functional import SimpleLazyObject
 
-from .helpers import User, check_allow_for_uri, check_allow_for_user
+from .helpers import User, check_allow_for_uri, check_allow_for_user, check_read_only
 from .settings import settings

@@ -50,7 +50,7 @@
 
-            if settings.READ_ONLY and request.method not in ['GET', 'HEAD', 'OPTIONS']:
+            if check_read_only(request) and request.method not in ['GET', 'HEAD', 'OPTIONS']:
                 return HttpResponseNotAllowed(['GET', 'HEAD', 'OPTIONS'])
 
             if check_allow_for_user(request, new_user) and check_allow_for_uri(
diff --git a/impersonate/settings.py b/impersonate/settings.py
--- a/impersonate/settings.py
+++ b/impersonate/settings.py
@@ -20,6 +20,7 @@
     'SEARCH_FIELDS': [username_field, 'first_name', 'last_name', 'email'],
     'REDIRECT_URL': getattr(django_settings, 'LOGIN_REDIRECT_URL', u'/'),
     'READ_ONLY': False,
+    'CUSTOM_READ_ONLY': None,
     'ADMIN_DELETE_PERMISSION': False,
     'ADMIN_ADD_PERMISSION': False,
     'ADMIN_READ_ONLY': True,
diff --git a/impersonate/tests.py b/impersonate/tests.py
--- a/impersonate/tests.py
+++ b/impersonate/tests.py
@@ -89,6 +89,13 @@
     return User.objects.all().order_by('pk')

+def test_allow_read_only(request):
+    ''' Used via the IMPERSONATE['CUSTOM_READ_ONLY'] setting.
+        Simple check that the user is not a superuser.
+    '''
+    return not request.user.is_superuser
+
+
 class UserFactory(object):
     @staticmethod
     def create(**kwargs):
@@ -852,3 +859,29 @@
         self.assertEqual(resp.status_code, 200)
         resp = self.client.options(reverse('impersonate-test'))
         self.assertEqual(resp.status_code, 200)
+
+    @override_settings(IMPERSONATE={'CUSTOM_READ_ONLY': 'impersonate.tests.test_allow_read_only'})
+    def test_impersonate_custom_read_only(self):
+        # superuser is able to do all requests
+        self._impersonate_helper('user1', 'foobar', 4)
+        resp = self.client.post(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.get(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.head(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.options(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        self.client.logout()
+
+        # staff user is only able to do read only requests
+        self._impersonate_helper('user3', 'foobar', 4)
+        resp = self.client.post(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 405)
+        resp = self.client.get(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.head(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        resp = self.client.options(reverse('impersonate-test'))
+        self.assertEqual(resp.status_code, 200)
+        self.client.logout()