fizista / django-password-validators

Additional libraries for validating passwords in Django.
BSD 3-Clause "New" or "Revised" License
50 stars 31 forks source link

Django 4.0 Compat #25

Closed dwasyl closed 2 years ago

dwasyl commented 2 years ago

Like most apps, Django 4 has a couple of issues, namely it depreciates ugettext_lazy in favour of gettext_lazy. Looks like only the models.py file is impacted.

/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:17: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  _('When created salt'),
/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:22: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  verbose_name=_('Salt for the user'),
/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:27: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  _('The number of iterations for hasher'),
/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:35: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  verbose_name = _('Configuration')
/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:36: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  verbose_name_plural = _('Configurations')
/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:75: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  _('Password hash'),
/home/lib/python3.7/site-packages/django_password_validators/password_history/models.py:80: RemovedInDjango40Warning: django.utils.translation.ugettext_lazy() is deprecated in favor of django.utils.translation.gettext_lazy().
  _('Date'),
fizista commented 2 years ago

Unfortunately, the tests still don't pass. I currently don't have time to look for the problem. If you could fix this it would be great.

sbruno commented 2 years ago

Hi @fizista , I found this problem today migrating a project to Django 4.0.3, and the problem went away when I installed the fork linked in this issue. Which are the tests that do not pass? I checked out that fork and run make test, and the tests for the latest interpreters that I have installed passed. I couldn't run them with python 3.5 or 3.6 interpreters.

fizista commented 2 years ago

The current test file does not include Django 4.x and 3.2. And unfortunately on these versions of Django, the tests do not pass.

sbruno commented 2 years ago

@fizista , in case it helps, I've been trying to find the test problems. I found two.

One could be fixed with a change in the test urlpatterns, but the other is caused because password_changed is not called on user creation. So the unique password validator will not count the initial password and tests always expect an additional password history item. Not sure if this could be a regression in Django or expected, but password_changed is called on User's save only when self._password is set, and that happens when set_password is called: https://github.com/django/django/blob/7119f40c9881666b6f9b5cf7df09ee1d21cc8344/django/contrib/auth/base_user.py#L57

So here I made some changes, calling set_password on user creation. It makes the tests pass, but I'm not really sure if it is safe to assume that behavior... I tried manually in the Django Admin, changing users password and seems to work, but maybe there are other scenarios where this could not be correct.

diff --git a/tests/test_project/tests/base.py b/tests/test_project/tests/base.py
index 8e694dc..9a3a418 100644
--- a/tests/test_project/tests/base.py
+++ b/tests/test_project/tests/base.py
@@ -13,11 +13,13 @@ class PasswordsTestCase(TestCase):
     PASSWORD_TEMPLATE = 'ABCDEFGHIJKLMNOPRSTUWXYZ_%d'

     def create_user(self, number=1):
-        return self.UserModel.objects.create_user(
+        user = self.UserModel.objects.create_user(
             'test%d' % number,
-            email='test%d@example.com' % number,
-            password=self.PASSWORD_TEMPLATE % 1
+            email='test%d@example.com' % number
         )
+        user.set_password(self.PASSWORD_TEMPLATE % 1)
+        user.save()
+        return user

     def user_change_password(self, user_number, password_number):
         user = self.UserModel.objects.get(username='test%d' % user_number)
diff --git a/tests/test_project/urls.py b/tests/test_project/urls.py
index b03753d..eeb20a1 100644
--- a/tests/test_project/urls.py
+++ b/tests/test_project/urls.py
@@ -13,9 +13,9 @@ Including another URLconf
     1. Import the include() function: from django.conf.urls import url, include
     2. Add a URL to urlpatterns:  url(r'^blog/', include('blog.urls'))
 """
-from django.conf.urls import url
 from django.contrib import admin
+from django.urls import re_path

 urlpatterns = [
-    url(r'^admin/', admin.site.urls),
+    re_path(r'^admin/', admin.site.urls),
 ]
diff --git a/tox.ini b/tox.ini
index caf622e..3158de0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -3,6 +3,8 @@ envlist =
     {py35,py36,py37,py38}-django22,
     {py36,py37,py38,py39}-django30
     {py36,py37,py38,py39}-django31
+    {py36,py37,py38,py39}-django32
+    {py36,py37,py38,py39}-django40

 setenv =
     PYTHONPATH = {toxinidir}:{toxinidir}/tests/
@@ -13,6 +15,7 @@ deps =
     django30: Django >= 3.0, < 3.1
     django31: Django >= 3.1, < 3.2
     django32: Django >= 3.2, < 3.3
+    django40: Django >= 4.0, < 4.1

 commands =
     python -V
fizista commented 2 years ago

Thank you for your patches. I've included them in the code, and now the library works properly with Django 4.x.