Merge pull request #3272 from modoboa/fix/dkim-non-writable

Removed serializer check for dkim dir, added alarms on gen

README.rst

@@ -32,9 +32,9 @@ Main features
 * `Webmail <https://github.com/modoboa/modoboa-webmail>`_
 * `Calendar <https://github.com/modoboa/modoboa-radicale>`_
 * `Address book <https://github.com/modoboa/modoboa-contacts>`_
-* `Per-user Sieve filters <https://github.com/modoboa/modoboa-sievefilters>`_
-* `Autoreply messages for Postfix <https://github.com/modoboa/modoboa-postfix-autoreply>`_
-* `Graphical statistics about email traffic <https://github.com/modoboa/modoboa-stats>`_
+* Per-user Sieve filters
+* Autoreply messages for Postfix
+* Graphical statistics about email traffic
 
 ************
 Installation

modoboa/admin/api/v2/serializers.py

@@ -228,10 +228,6 @@ class AdminGlobalParametersSerializer(serializers.Serializer):
     def validate_dkim_keys_storage_dir(self, value):
         """Check that directory exists."""
         if value:
-            if not os.path.isdir(value):
-                raise serializers.ValidationError(_("Directory not found."))
-            if not os.access(value, os.W_OK):
-                raise serializers.ValidationError(_("Directory non-writable"))
             code, output = exec_cmd("which openssl")
             if code:
                 raise serializers.ValidationError(

modoboa/admin/constants.py

@@ -29,6 +29,7 @@ DKIM_KEY_LENGTHS = [
 ]
 
 DKIM_WRITE_ERROR = "DKIM path error"
+DKIM_ERROR = "general DKIM generation error"
 
 ALARM_OPENED = 1
 ALARM_CLOSED = 2

modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py

@@ -10,8 +10,7 @@ from modoboa.lib import sysutils
 from modoboa.parameters import tools as param_tools
 
 from .... import models
-from .... import signals
-from ....constants import DKIM_WRITE_ERROR, ALARM_OPENED
+from ....constants import DKIM_WRITE_ERROR, ALARM_OPENED, DKIM_ERROR
 
 
 class ManageDKIMKeys(BaseCommand):
@@ -21,11 +20,16 @@ class ManageDKIMKeys(BaseCommand):
         """Create a new DKIM key."""
         storage_dir = param_tools.get_global_parameter("dkim_keys_storage_dir")
         pkey_path = os.path.join(storage_dir, "{}.pem".format(domain.name))
+
         alarm_qset = domain.alarms.filter(internal_name=DKIM_WRITE_ERROR)
         if not os.access(storage_dir, os.W_OK):
             if not alarm_qset.exists():
                 domain.alarms.create(
-                    title=_("DKIM path non-writable"), internal_name=DKIM_WRITE_ERROR
+                    title=_(
+                        "DKIM path non-writable "
+                        "(either a permission issue or the directory does not exist)"
+                    ),
+                    internal_name=DKIM_WRITE_ERROR,
                 )
             else:
                 alarm = alarm_qset.first()
@@ -48,6 +52,10 @@ class ManageDKIMKeys(BaseCommand):
                     domain.name, smart_str(output)
                 )
             )
+            domain.alarms.create(
+                title=_("Failed to generate DKIM private key"), internal_name=DKIM_ERROR
+            )
+            return
         domain.dkim_private_key_path = pkey_path
         code, output = sysutils.exec_cmd("openssl rsa -in {} -pubout".format(pkey_path))
         if code:
@@ -56,6 +64,10 @@ class ManageDKIMKeys(BaseCommand):
                     domain.name, smart_str(output)
                 )
             )
+            domain.alarms.create(
+                title=_("Failed to generate DKIM public key"), internal_name=DKIM_ERROR
+            )
+            return
         public_key = ""
         for cpt, line in enumerate(smart_str(output).splitlines()):
             if cpt == 0 or line.startswith("-----"):

modoboa/parameters/api/v2/tests.py

@@ -6,7 +6,6 @@ from modoboa.lib.tests import ModoAPITestCase
 
 
 class ParametersAPITestCase(ModoAPITestCase):
-
     def test_applications(self):
         url = reverse("v2:parameter-applications")
         resp = self.client.get(url)
@@ -69,15 +68,6 @@ class ParametersAPITestCase(ModoAPITestCase):
         self.assertEqual(
             errors["default_mailbox_quota"], ["Must be a positive integer"]
         )
-        self.assertEqual(errors["dkim_keys_storage_dir"], ["Directory not found."])
-
-        # Try to set dkim storage dir to a non-writable directory.
-        data = {"dkim_keys_storage_dir": "/root"}
-        url = reverse("v2:parameter-detail", args=["admin"])
-        resp = self.client.put(url, data, format="json")
-        self.assertEqual(resp.status_code, 400)
-        errors = resp.json()
-        self.assertEqual(errors["dkim_keys_storage_dir"], ["Directory non-writable"])
 
         data = {
             "enable_mx_checks": True,