Repository: modoboa/modoboa (language: python)

Issue:
dkim_keys_storage_dir	[ "Directory non-writable" ]
# Impacted versions

* OS Type: 			Debian
* OS Version: 		12
* Database Type: 	postgres
* Database version:	15.6
* Modoboa:  		2.2.4
* installer used: 		yes
* Webserver: 		nginx
# Steps to reproduce

1. install this, don't change anything from default.
2. login into a new-admin
3. go to /new-admin/parameters/admin (go to settings > administration)
4. ctrl+shift+k (open debug console in your browser)
5. no need to change anything, just click on the green floppy disk icon in bottom right corner and then see response from the server indicating failure

# Current behavior
this was installed yesterday (found after #3234), nothing was changed in filesystem - everything is either default debian or was changed by installation script.
<!--

  Explain the behavior you're seeing that you think is a bug, and explain how you
  think things should behave instead.

-->


## Response
```XHRPUT
XHRPUT
https://mail.perfugium.net/api/v2/parameters/admin/
[HTTP/2 400  109ms]

	
dkim_keys_storage_dir	[ "Directory non-writable" ]
0	"Directory non-writable"
```
this can be fixed by a chmod/chown command, but maybe should be set properly? (which user should have write access to this directory apart from the opendkim guy?)
```bash
ls -l /var/lib/dkim
total 8
drwxr-xr-x  2 opendkim opendkim 4096 Apr 10 11:34 .
drwxr-xr-x 36 root     root     4096 Apr 10 11:41 ..
```
# Expected behavior
status 200
# Video/Screenshot link (optional)

![Screenshot_20240411_142447](https://github.com/modoboa/modoboa/assets/120217643/5613c22a-55f3-431f-ae89-c9b1819240c2)





Discussion / hints:
@usernamehyphen This folder should belong to opendkim:opendkim after a successful installation. (see https://github.com/modoboa/modoboa-installer/blob/master/modoboa_installer/scripts/opendkim.py#L35)
Did you notice anything else?

Fix patch:
diff --git a/README.rst b/README.rst
index e4f717745..9555fbe14 100644
--- a/README.rst
+++ b/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
diff --git a/modoboa/admin/api/v2/serializers.py b/modoboa/admin/api/v2/serializers.py
index 081af930f..c51e68086 100644
--- a/modoboa/admin/api/v2/serializers.py
+++ b/modoboa/admin/api/v2/serializers.py
@@ -228,10 +228,6 @@ def validate_default_mailbox_quota(self, value):
     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(
diff --git a/modoboa/admin/constants.py b/modoboa/admin/constants.py
index aaa2d2bc6..39a95023a 100644
--- a/modoboa/admin/constants.py
+++ b/modoboa/admin/constants.py
@@ -29,6 +29,7 @@
 ]
 
 DKIM_WRITE_ERROR = "DKIM path error"
+DKIM_ERROR = "general DKIM generation error"
 
 ALARM_OPENED = 1
 ALARM_CLOSED = 2
diff --git a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py
index 7f5a61275..9cac1a2b8 100644
--- a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py
+++ b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py
@@ -10,8 +10,7 @@
 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 @@ def create_new_dkim_key(self, domain):
         """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 @@ def create_new_dkim_key(self, domain):
                     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 @@ def create_new_dkim_key(self, domain):
                     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("-----"):


Test patch:
diff --git a/modoboa/parameters/api/v2/tests.py b/modoboa/parameters/api/v2/tests.py
index 24494fa72..6cf783698 100644
--- a/modoboa/parameters/api/v2/tests.py
+++ b/modoboa/parameters/api/v2/tests.py
@@ -6,7 +6,6 @@
 
 
 class ParametersAPITestCase(ModoAPITestCase):
-
     def test_applications(self):
         url = reverse("v2:parameter-applications")
         resp = self.client.get(url)
@@ -69,15 +68,6 @@ def test_update_admin_settings(self):
         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,