Repository: modoboa/modoboa (language: python)

Issue:
Handle password schemes deprecation
Weak password schemes might disappear at some point and that's for example the case with CRYPT.

The underlying python module (crypt) has be removed starting with version 3.13 and so we need to way to prevent any issue and warn admins that they must upgrade the configured scheme ASAP.

A possible plan:
1. Mark crypt scheme as deprecated for the next patch version (2.3.3) and notify admins using checks and notifications to indicate that they must update their configuration before next minor version
2. Remove support of this scheme in next minor version (2.4.0)


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/modoboa.yml b/.github/workflows/modoboa.yml
index eea0bdbab..893468a62 100644
--- a/.github/workflows/modoboa.yml
+++ b/.github/workflows/modoboa.yml
@@ -72,7 +72,7 @@ jobs:
     strategy:
       matrix:
         database: ['postgres', 'mysql']
-        python-version: [3.8, 3.9, '3.10', '3.11']
+        python-version: [3.9, '3.10', '3.11', '3.12']
       fail-fast: false
 
     steps:
@@ -109,7 +109,7 @@ jobs:
           pip install -r mysql-requirements.txt
           echo "DB=mysql" >> $GITHUB_ENV
       - name: Test with pytest
-        if: ${{ matrix.python-version != '3.11' || matrix.database != 'postgres' }}
+        if: ${{ matrix.python-version != '3.12' || matrix.database != 'postgres' }}
         run: |
           python ./tests.py
           cd test_project
@@ -127,7 +127,7 @@ jobs:
           OIDC_RSA_PRIVATE_KEY: ${{ vars.OIDC_RSA_PRIVATE_KEY }}
 
       - name: Test with pytest and coverage
-        if: ${{ matrix.python-version == '3.11' && matrix.database == 'postgres' }}
+        if: ${{ matrix.python-version == '3.12' && matrix.database == 'postgres' }}
         run: |
           python ./tests.py
           cd test_project
@@ -148,7 +148,7 @@ jobs:
           OIDC_RSA_PRIVATE_KEY: ${{ vars.OIDC_RSA_PRIVATE_KEY }}
 
       - name: Upload coverage result
-        if: ${{ matrix.python-version == '3.11' && matrix.database == 'postgres' }}
+        if: ${{ matrix.python-version == '3.12' && matrix.database == 'postgres' }}
         uses: actions/upload-artifact@v4
         with:
           name: coverage-results
@@ -182,10 +182,10 @@ jobs:
       - uses: actions/checkout@v4
         with:
           fetch-depth: 0
-      - name: Set up Python 3.11
+      - name: Set up Python 3.12
         uses: actions/setup-python@v5
         with:
-          python-version: '3.11'
+          python-version: '3.12'
       - name: Build frontend
         shell: bash -l {0}
         run: |
diff --git a/docker/Dockerfile.dev b/docker/Dockerfile.dev
index cd965d33d..536ff01ec 100644
--- a/docker/Dockerfile.dev
+++ b/docker/Dockerfile.dev
@@ -1,10 +1,10 @@
-FROM python:3.12-alpine as base
+FROM python:3.12-alpine AS base
 
-MAINTAINER Antoine Nguyen <tonio@ngyn.org>
+LABEL org.opencontainers.image.authors="Antoine Nguyen <tonio@ngyn.org>"
 ENV VIRTUAL_ENV=/opt/venv
 ENV PATH="$VIRTUAL_ENV/bin:$PATH"
 
-FROM base as build
+FROM base AS build
 
 RUN apk add --update openssl librrd python3-dev libffi-dev gcc g++ musl-dev libxml2-dev libxslt-dev \
     libressl-dev jpeg-dev rrdtool-dev file make gettext freetype-dev cairo-dev cargo \
@@ -16,7 +16,8 @@ COPY test-requirements.txt /tmp
 COPY dev-requirements.txt /tmp
 RUN python -m pip install -U pip
 RUN pip install -r requirements.txt -r test-requirements.txt -r dev-requirements.txt
-FROM base as run
+
+FROM base AS run
 COPY docker/doveadm /usr/local/bin
 RUN chmod +x /usr/local/bin/doveadm
 COPY --from=build $VIRTUAL_ENV $VIRTUAL_ENV
diff --git a/modoboa/core/checks.py b/modoboa/core/checks.py
index fb445a9e7..a8977339f 100644
--- a/modoboa/core/checks.py
+++ b/modoboa/core/checks.py
@@ -3,7 +3,8 @@
 from django.utils.translation import gettext as _
 
 from modoboa.core.utils import generate_rsa_private_key
-
+from modoboa.parameters.tools import get_global_parameter
+from modoboa.core.password_hashers import get_password_hasher
 
 W001 = Warning(
     _(
@@ -14,6 +15,11 @@
     hint=_("Set `USE_TZ = True` in settings.py"),
     id="modoboa.W001",
 )
+W002 = Warning(
+    _("The password scheme you are using is deprecated."),
+    hint=_("Upgrade your password scheme to a stronger one"),
+    id="modoboa.W002",
+)
 
 
 @register(deploy=True)
@@ -38,3 +44,13 @@ def check_rsa_private_key_exists(app_configs, **kwargs):
     if generate_rsa_private_key(settings.BASE_DIR):
         msgs.append(Info("An RSA private key has been generated for OIDC."))
     return msgs
+
+
+@register()
+def check_password_hasher(app_configs, **kwargs):
+    msgs = []
+    scheme_in_use = get_global_parameter("password_scheme", app="core")
+    hasher = get_password_hasher(scheme_in_use)
+    if hasher.deprecated:
+        msgs.append(W002)
+    return msgs
diff --git a/modoboa/core/models.py b/modoboa/core/models.py
index 25d3d4e0f..a9595c719 100644
--- a/modoboa/core/models.py
+++ b/modoboa/core/models.py
@@ -2,6 +2,7 @@
 
 import re
 from email.header import Header
+from typing import Optional
 
 from django.core.serializers.json import DjangoJSONEncoder
 from django.conf import settings
@@ -96,7 +97,7 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.parameters = param_tools.Manager("user", self._parameters)
 
-    def _crypt_password(self, raw_value):
+    def _crypt_password(self, raw_value: str) -> str:
         """Crypt the local password using the appropriate scheme.
 
         In case we don't find the scheme (for example when the
@@ -117,7 +118,7 @@ def _crypt_password(self, raw_value):
         raw_value = smart_bytes(raw_value)
         return get_password_hasher(scheme.upper())().encrypt(raw_value)
 
-    def set_password(self, raw_value, curvalue=None):
+    def set_password(self, raw_value: str, curvalue: Optional[str] = None) -> None:
         """Password update.
 
         Update the current mailbox's password with the given clear
diff --git a/modoboa/core/password_hashers/__init__.py b/modoboa/core/password_hashers/__init__.py
index 5bc8bd086..560c43ef4 100644
--- a/modoboa/core/password_hashers/__init__.py
+++ b/modoboa/core/password_hashers/__init__.py
@@ -19,7 +19,7 @@
 )
 
 
-def get_password_hasher(scheme):
+def get_password_hasher(scheme: str) -> PasswordHasher:
     """Retrieve the hasher corresponding to :keyword:`scheme`.
 
     If no class is found, `PLAINHasher` is returned.
diff --git a/modoboa/core/password_hashers/base.py b/modoboa/core/password_hashers/base.py
index 0d355ac63..f6346ca73 100644
--- a/modoboa/core/password_hashers/base.py
+++ b/modoboa/core/password_hashers/base.py
@@ -5,10 +5,12 @@
 """
 
 import base64
+
 import crypt
 import hashlib
-import string
 from random import Random
+import string
+from typing import Optional
 
 from django.utils.crypto import constant_time_compare
 from django.utils.encoding import force_bytes, force_str
@@ -28,7 +30,15 @@ def name(cls):
     @property
     def label(cls):
         """Returns the label of the hasher"""
-        return cls.name if not cls._weak else "{} (weak)".format(cls.name)
+        result = cls.name
+        tags = []
+        if cls._weak:
+            tags.append("weak")
+        if cls.deprecated:
+            tags.append("deprecated")
+        if tags:
+            result += f" ({', '.join(tags)})"
+        return result
 
 
 class PasswordHasher(metaclass=MetaHasher):
@@ -36,12 +46,14 @@ class PasswordHasher(metaclass=MetaHasher):
     Base class of all hashers.
     """
 
-    _weak = False
+    _weak: bool = False
+    deprecated: bool = False
+    scheme: str
 
     def __init__(self, target="local"):
         self._target = target
 
-    def _encrypt(self, clearvalue, salt=None):
+    def _encrypt(self, clearvalue: str, salt: Optional[str] = None) -> str:
         raise NotImplementedError
 
     def _b64encode(self, pwhash):
@@ -54,7 +66,7 @@ def _b64encode(self, pwhash):
             return base64.b64encode(pwhash)
         return pwhash
 
-    def encrypt(self, clearvalue):
+    def encrypt(self, clearvalue: str) -> str:
         """Encrypt a password.
 
         The format used to store passwords is the same than dovecot's one::
@@ -94,7 +106,7 @@ def needs_rehash(self, hashed_value):
     @classmethod
     def get_password_hashers(cls):
         """Return all the PasswordHasher supported by Modoboa"""
-        return cls.__subclasses__()
+        return [hasher for hasher in cls.__subclasses__()]
 
 
 class PLAINHasher(PasswordHasher):
@@ -120,12 +132,13 @@ class CRYPTHasher(PasswordHasher):
     """
 
     _weak = True
+    deprecated = True
 
     @property
     def scheme(self):
         return "{CRYPT}"
 
-    def _encrypt(self, clearvalue, salt=None):
+    def _encrypt(self, clearvalue: str, salt: Optional[str] = None) -> str:
         if salt is None:
             salt = "".join(Random().sample(string.ascii_letters + string.digits, 2))
         return crypt.crypt(clearvalue, salt)
diff --git a/modoboa/lib/email_utils.py b/modoboa/lib/email_utils.py
index 793304d8b..b216336bc 100644
--- a/modoboa/lib/email_utils.py
+++ b/modoboa/lib/email_utils.py
@@ -9,11 +9,11 @@
 import chardet
 import lxml.html
 from lxml.html import defs
-from lxml.html.clean import Cleaner
+from lxml_html_clean import Cleaner
 
 from django.core.mail import EmailMessage
 from django.template.loader import render_to_string
-from django.utils.encoding import smart_str, smart_str
+from django.utils.encoding import smart_str
 from django.utils.html import conditional_escape, escape
 from django.utils.translation import gettext as _
 
@@ -26,7 +26,7 @@
 _RE_CID = re.compile(r".*[ ]*cid=\"([^\"]*)\".*", re.I)
 
 
-class EmailAddress(object):
+class EmailAddress:
 
     def __init__(self, address):
         self.name, self.address = u2u_decode.decode_address(address)
@@ -39,7 +39,7 @@ def __str__(self):
         return self.fulladdress
 
 
-class Email(object):
+class Email:
 
     _basic_headers = (
         "From",
diff --git a/requirements.txt b/requirements.txt
index e6d86cba9..c314b0d8c 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -30,7 +30,7 @@ python-dateutil
 cryptography
 pytz
 requests
-lxml==4.9.4  # FIXME: issue with version 5+
+lxml[html_clean]
 chardet
 ovh
 oath


Test patch:
diff --git a/modoboa/core/tests/test_authentication.py b/modoboa/core/tests/test_authentication.py
index 9b0e4e306..58acb51d6 100644
--- a/modoboa/core/tests/test_authentication.py
+++ b/modoboa/core/tests/test_authentication.py
@@ -252,6 +252,10 @@ def test_dovecot_default_schemes(self):
         supported_schemes = get_dovecot_schemes()[0]
         self.assertEqual(supported_schemes, ["{MD5-CRYPT}", "{PLAIN}"])
 
+    def test_deprecated_password_scheme(self):
+        hasher = get_password_hasher("crypt")
+        self.assertEqual(hasher.label, "crypt (weak, deprecated)")
+
     def test_fido_auth_begin(self):
         url = reverse("core:fido_auth_begin")
         resp = self.client.post(url)
diff --git a/modoboa/core/tests/test_checks.py b/modoboa/core/tests/test_checks.py
index b3beda5dd..0c538b865 100644
--- a/modoboa/core/tests/test_checks.py
+++ b/modoboa/core/tests/test_checks.py
@@ -2,13 +2,14 @@
 import shutil
 import tempfile
 
-from django.test import SimpleTestCase
 from django.test.utils import override_settings
 
 from modoboa.core import checks
+from modoboa.lib.tests import SimpleModoTestCase
+from modoboa.core.password_hashers.base import CRYPTHasher
 
 
-class CheckSessionCookieSecureTest(SimpleTestCase):
+class CheckSessionCookieSecureTest(SimpleModoTestCase):
 
     def setUp(self):
         self.workdir = tempfile.mkdtemp()
@@ -35,3 +36,10 @@ def test_rsa_private_key_exists(self):
             os.environ["OIDC_RSA_PRIVATE_KEY"] = "KEY"
             msgs = checks.check_rsa_private_key_exists(None)
             self.assertEqual(len(msgs), 0)
+
+    def test_password_hasher_checks(self):
+        msgs = checks.check_password_hasher(None)
+        self.assertEqual(msgs, [])
+        self.set_global_parameter("password_scheme", "crypt")
+        msgs = checks.check_password_hasher(None)
+        self.assertEqual(msgs, [checks.W002])
diff --git a/modoboa/lib/tests/__init__.py b/modoboa/lib/tests/__init__.py
index 1c5322920..23425ef51 100644
--- a/modoboa/lib/tests/__init__.py
+++ b/modoboa/lib/tests/__init__.py
@@ -30,7 +30,7 @@
     NO_LDAP = True
 
 
-class ParametersMixin(object):
+class ParametersMixin:
     """Add tools to manage parameters."""
 
     @classmethod
diff --git a/modoboa/lib/tests/sample_messages/multipart-input.txt b/modoboa/lib/tests/sample_messages/multipart-input.txt
index ee0b51430..9b29082fd 100644
--- a/modoboa/lib/tests/sample_messages/multipart-input.txt
+++ b/modoboa/lib/tests/sample_messages/multipart-input.txt
@@ -57,7 +57,7 @@ This is an *html* /test /_message._
 :-)
 
 
--- 
+--
 Someone
 someone@example.net
 
@@ -83,12 +83,11 @@ Content-Transfer-Encoding: 7bit
     <img moz-do-not-send="false"
       src="cid:part1.D56823CE.7959DBD4@example.net" alt="" height="10"
       width="10"><br>
-    <pre class="moz-signature" cols="72">-- 
+    <pre class="moz-signature" cols="72">--
 someone
 <a class="moz-txt-link-abbreviated" href="mailto:someone@example.net">someone@example.net</a></pre>
   </body>
 </html>
-
 --------------D585B241EEF60F0F44EC20E5
 Content-Type: image/png;
  name="image.png"
diff --git a/modoboa/lib/tests/sample_messages/multipart-output-html_links.txt b/modoboa/lib/tests/sample_messages/multipart-output-html_links.txt
index 92c507ec5..67177a723 100644
--- a/modoboa/lib/tests/sample_messages/multipart-output-html_links.txt
+++ b/modoboa/lib/tests/sample_messages/multipart-output-html_links.txt
@@ -8,13 +8,8 @@
     <span class="moz-smiley-s1"><span>:-)</span></span><br>
     <br>
     <img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAIAAAACUFjqAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAF0lEQVQY02NgYGD4//8/ThKfHASMRN0AfaqVa6MQxIQAAAAASUVORK5CYII=" alt="" height="10" width="10" target="_blank"><br>
-    <pre class="moz-signature" cols="72">-- 
+    <pre class="moz-signature" cols="72">--
 someone
 <a class="moz-txt-link-abbreviated" href="mailto:someone@example.net" target="_blank" rel="nofollow">someone@example.net</a></pre>
   </body>
-<span><b>Some <i>HTML</i> text</b>, a <a href="http://example.net" target="_blank" rel="nofollow">link</a>, an
-embedded image and an external image.<br>
-<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAIAAAACUFjqAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAF0lEQVQY02NgYGD4//8/ThKfHASMRN0AfaqVa6MQxIQAAAAASUVORK5CYII=" target="_blank"><br>
-<img src="http://example.net/image.png" target="_blank"><br>
-Nifty!</span>
-</div>
\ No newline at end of file
+</div>
diff --git a/modoboa/lib/tests/sample_messages/multipart-output-html_nolinks.txt b/modoboa/lib/tests/sample_messages/multipart-output-html_nolinks.txt
index 56d847fb8..00328ac8d 100644
--- a/modoboa/lib/tests/sample_messages/multipart-output-html_nolinks.txt
+++ b/modoboa/lib/tests/sample_messages/multipart-output-html_nolinks.txt
@@ -8,13 +8,8 @@
     <span class="moz-smiley-s1"><span>:-)</span></span><br>
     <br>
     <img alt="" height="10" width="10"><br>
-    <pre class="moz-signature" cols="72">-- 
+    <pre class="moz-signature" cols="72">--
 someone
 <a class="moz-txt-link-abbreviated">someone@example.net</a></pre>
   </body>
-<span><b>Some <i>HTML</i> text</b>, a <a target="_blank" rel="nofollow">link</a>, an
-embedded image and an external image.<br>
-<img target="_blank"><br>
-<img target="_blank"><br>
-Nifty!</span>
-</div>
\ No newline at end of file
+</div>
diff --git a/modoboa/lib/tests/sample_messages/multipart-output-plain_nolinks.txt b/modoboa/lib/tests/sample_messages/multipart-output-plain_nolinks.txt
index 8795edadc..855fa5ca7 100644
--- a/modoboa/lib/tests/sample_messages/multipart-output-plain_nolinks.txt
+++ b/modoboa/lib/tests/sample_messages/multipart-output-plain_nolinks.txt
@@ -2,6 +2,6 @@
 
 :-)
 
--- 
+--
 Someone
-someone@example.net</pre>
\ No newline at end of file
+someone@example.net</pre>
diff --git a/modoboa/lib/tests/test_email_utils.py b/modoboa/lib/tests/test_email_utils.py
index 963ed319a..1c6844ccc 100644
--- a/modoboa/lib/tests/test_email_utils.py
+++ b/modoboa/lib/tests/test_email_utils.py
@@ -34,10 +34,10 @@ class EmailTests(SimpleTestCase):
     output: {message_id}-output-{dformat}_{no,}links.txt
     """
 
-    def _get_expected_output(self, message_id, **kwargs):
+    def _get_expected_output(self, message_id: str, **kwargs) -> str:
         ext = kwargs["dformat"] if "dformat" in kwargs else "plain"
         ext += "_links" if "links" in kwargs and kwargs["links"] else "_nolinks"
-        message_path = os.path.join(SAMPLES_DIR, "%s-output-%s.txt" % (message_id, ext))
+        message_path = os.path.join(SAMPLES_DIR, f"{message_id}-output-{ext}.txt")
         assert os.path.isfile(message_path), "%s does not exist." % message_path
 
         with open(message_path, "rb") as fp:
@@ -46,11 +46,11 @@ def _get_expected_output(self, message_id, **kwargs):
 
         return mail_text
 
-    def _test_email(self, message_id, **kwargs):
+    def _test_email(self, message_id: str, **kwargs) -> None:
         """Boiler plate code for testing e-mails."""
         expected_output = self._get_expected_output(message_id, **kwargs)
         output = EmailTestImplementation(message_id, **kwargs).body
-        self.assertEqual(output, expected_output)
+        self.assertEqual(output.strip("\n"), expected_output.strip("\n"))
 
     def test_invalid_links_value(self):
         """modoboa-amavis set links = "0"; in python "0" == True, fixed in PR 79