Merge pull request #35 from mikemanger/feature/json-session-serializer

Feature/json session serializer and Django 5 support

Author: mpasternak

.github/workflows/main.yml

@@ -19,6 +19,7 @@ jobs:
         - '3.9'
         - '3.10'
         - '3.11'
+        - '3.12'
 
     steps:
     - uses: actions/checkout@v4

docs/topics/setup.rst

@@ -195,10 +195,10 @@ file::
 Serializer
 ============================
 
-For now this app uses the PickleSerializer. This needs to be set up in the Django settings
-file::
+This app is tested with both PickleSerializer and JsonSerializer.
 
-    SESSION_SERIALIZER='django.contrib.sessions.serializers.PickleSerializer'
+Django recommends to change from old pickle serializer to json because
+possible remote code execution vulnerability.
 
 .. _setup-create-db-tables:
 

password_policies/conf/settings.py

@@ -184,3 +184,8 @@
 
 
 PASSWORD_RESET_TIMEOUT_DAYS = 1
+
+PASSWORD_POLICIES_LAST_CHECKED_SESSION_KEY = "_password_policies_last_checked"
+PASSWORD_POLICIES_EXPIRED_SESSION_KEY = "_password_policies_expired"
+PASSWORD_POLICIES_LAST_CHANGED_SESSION_KEY = "_password_policies_last_changed"
+PASSWORD_POLICIES_CHANGE_REQUIRED_SESSION_KEY = "_password_policies_change_required"

password_policies/context_processors.py

@@ -1,3 +1,4 @@
+from password_policies.conf import settings
 from password_policies.models import PasswordHistory
 
 
@@ -29,9 +30,9 @@ def password_status(request):
         auth = auth()
 
     if auth:
-        if '_password_policies_change_required' not in request.session:
+        if settings.PASSWORD_POLICIES_CHANGE_REQUIRED_SESSION_KEY not in request.session:
             r = PasswordHistory.objects.change_required(request.user)
         else:
-            r = request.session['_password_policies_change_required']
+            r = request.session[settings.PASSWORD_POLICIES_CHANGE_REQUIRED_SESSION_KEY]
         d['password_change_required'] = r
     return d

password_policies/middleware.py

@@ -20,8 +20,7 @@
 
 from password_policies.conf import settings
 from password_policies.models import PasswordChangeRequired, PasswordHistory
-from password_policies.utils import PasswordCheck
-
+from password_policies.utils import PasswordCheck, string_to_datetime, datetime_to_string
 
 class PasswordChangeMiddleware(MiddlewareMixin):
     """
@@ -70,22 +69,24 @@ class PasswordChangeMiddleware(MiddlewareMixin):
         This middleware does not try to redirect using the HTTPS
         protocol."""
 
-    checked = "_password_policies_last_checked"
-    expired = "_password_policies_expired"
-    last = "_password_policies_last_changed"
-    required = "_password_policies_change_required"
+    checked = settings.PASSWORD_POLICIES_LAST_CHECKED_SESSION_KEY
+    expired = settings.PASSWORD_POLICIES_EXPIRED_SESSION_KEY
+    last = settings.PASSWORD_POLICIES_LAST_CHANGED_SESSION_KEY
+    required = settings.PASSWORD_POLICIES_CHANGE_REQUIRED_SESSION_KEY
     td = timedelta(seconds=settings.PASSWORD_DURATION_SECONDS)
 
     def _check_history(self, request):
         if not request.session.get(self.last, None):
             newest = PasswordHistory.objects.get_newest(request.user)
             if newest:
-                request.session[self.last] = newest.created
+                request.session[self.last] = datetime_to_string(newest.created)
             else:
                 # TODO: This relies on request.user.date_joined which might not
                 # be available!!!
-                request.session[self.last] = request.user.date_joined
-        if request.session[self.last] < self.expiry_datetime:
+                request.session[self.last] = datetime_to_string(request.user.date_joined)
+
+        date_last = string_to_datetime(request.session[self.last])
+        if date_last < self.expiry_datetime:
             request.session[self.required] = True
             if not PasswordChangeRequired.objects.filter(user=request.user).count():
                 PasswordChangeRequired.objects.create(user=request.user)
@@ -95,27 +96,30 @@ def _check_history(self, request):
     def _check_necessary(self, request):
 
         if not request.session.get(self.checked, None):
-            request.session[self.checked] = self.now
+            request.session[self.checked] = datetime_to_string(self.now)
 
             #  If the PASSWORD_CHECK_ONLY_AT_LOGIN is set, then only check at the beginning of session, which we can
             #  tell by self.now time having just been set.
         if (
             not settings.PASSWORD_CHECK_ONLY_AT_LOGIN
-            or request.session.get(self.checked, None) == self.now
+            or request.session.get(self.checked, None) == datetime_to_string(self.now)
         ):
             # If a password change is enforced we won't check
             # the user's password history, thus reducing DB hits...
             if PasswordChangeRequired.objects.filter(user=request.user).count():
                 request.session[self.required] = True
                 return
-            if request.session[self.checked] < self.expiry_datetime:
+
+            date_checked = string_to_datetime(request.session[self.checked])
+            if date_checked < self.expiry_datetime:
                 try:
                     del request.session[self.last]
                     del request.session[self.checked]
                     del request.session[self.required]
                     del request.session[self.expired]
                 except KeyError:
                     pass
+
             if settings.PASSWORD_USE_HISTORY:
                 self._check_history(request)
         else:

password_policies/tests/test_middleware.py

@@ -10,6 +10,7 @@
 except ImportError:
     from django.urls.base import reverse
 
+from django.test.utils import override_settings
 from django.utils import timezone
 
 from password_policies.conf import settings
@@ -71,3 +72,48 @@ def test_password_change_required_enforced_redirect(self):
         self.assertEqual(get_response_location(response["Location"]), self.redirect_url)
         self.client.logout()
         p.delete()
+
+
+class PasswordPoliciesMiddlewareJsonSerializerTest(TestCase):
+    def setUp(self):
+        self.user = create_user()
+        self.redirect_url = "http://testserver/password/change/?next=/"
+
+    def test_password_middleware_without_history(self):
+        seconds = settings.PASSWORD_DURATION_SECONDS - 60
+        self.user.date_joined = get_datetime_from_delta(timezone.now(), seconds)
+        self.user.last_login = get_datetime_from_delta(timezone.now(), seconds)
+        self.user.save()
+        self.client.login(username="alice", password=passwords[-1])
+        response = self.client.get(reverse("home"))
+        self.assertEqual(response.status_code, 200)
+        self.client.logout()
+
+    def test_password_middleware_with_history(self):
+        create_password_history(self.user)
+        self.client.login(username="alice", password=passwords[-1])
+        response = self.client.get(reverse("home"), follow=False)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(get_response_location(response["Location"]), self.redirect_url)
+        self.client.logout()
+        PasswordHistory.objects.filter(user=self.user).delete()
+
+    def test_password_middleware_enforced_redirect(self):
+        self.client.login(username="alice", password=passwords[-1])
+        response = self.client.get(reverse("home"), follow=False)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(get_response_location(response["Location"]), self.redirect_url)
+        self.client.logout()
+
+    def test_password_change_required_enforced_redirect(self):
+        seconds = settings.PASSWORD_DURATION_SECONDS - 60
+        self.user.date_joined = get_datetime_from_delta(timezone.now(), seconds)
+        self.user.last_login = get_datetime_from_delta(timezone.now(), seconds)
+        self.user.save()
+        p = PasswordChangeRequired.objects.create(user=self.user)
+        self.client.login(username="alice", password=passwords[-1])
+        response = self.client.get(reverse("home"), follow=False)
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(get_response_location(response["Location"]), self.redirect_url)
+        self.client.logout()
+        p.delete()

password_policies/tests/test_models.py

@@ -23,4 +23,4 @@ def test_password_history_expiration(self):
         self.assertEqual(count, settings.PASSWORD_HISTORY_COUNT)
 
     def test_password_history_recent_passwords(self):
-        self.failIf(PasswordHistory.objects.check_password(self.user, passwords[-1]))
+        self.assertFalse(PasswordHistory.objects.check_password(self.user, passwords[-1]))

password_policies/tests/test_settings.py

@@ -1,9 +1,4 @@
 import os
-from distutils.version import LooseVersion
-
-from django import get_version
-
-django_version = get_version()
 
 DEBUG = False
 
@@ -42,36 +37,24 @@
 
 SITE_ID = 1
 
-# This is to maintain compatibility with Django 1.7
-if LooseVersion(django_version) < LooseVersion("1.8.0"):
-    TEMPLATE_DIRS = (os.path.join(os.path.dirname(__file__), "templates"),)
-    TEMPLATE_CONTEXT_PROCESSORS = (
-        "django.contrib.auth.context_processors.auth",
-        "django.core.context_processors.debug",
-        "django.core.context_processors.i18n",
-        "django.contrib.messages.context_processors.messages",
-        "password_policies.context_processors.password_status",
-    )
-
-else:
-    TEMPLATES = [
-        {
-            "BACKEND": "django.template.backends.django.DjangoTemplates",
-            "DIRS": [
-                os.path.join(os.path.dirname(__file__), "templates"),
+TEMPLATES = [
+    {
+        "BACKEND": "django.template.backends.django.DjangoTemplates",
+        "DIRS": [
+            os.path.join(os.path.dirname(__file__), "templates"),
+        ],
+        "APP_DIRS": True,
+        "OPTIONS": {
+            "context_processors": [
+                "django.contrib.auth.context_processors.auth",
+                "django.template.context_processors.debug",
+                "django.template.context_processors.i18n",
+                "django.contrib.messages.context_processors.messages",
+                "password_policies.context_processors.password_status",
             ],
-            "APP_DIRS": True,
-            "OPTIONS": {
-                "context_processors": [
-                    "django.contrib.auth.context_processors.auth",
-                    "django.template.context_processors.debug",
-                    "django.template.context_processors.i18n",
-                    "django.contrib.messages.context_processors.messages",
-                    "password_policies.context_processors.password_status",
-                ],
-            },
         },
-    ]
+    },
+]
 
 MIDDLEWARE = (
     "django.middleware.common.CommonMiddleware",
@@ -82,7 +65,8 @@
     "django.contrib.messages.middleware.MessageMiddleware",
 )
 
-SESSION_SERIALIZER = "django.contrib.sessions.serializers.PickleSerializer"
+SESSION_SERIALIZER = "django.contrib.sessions.serializers.JSONSerializer"
+
 
 MEDIA_URL = "/media/somewhere/"