Repository: modoboa/modoboa (language: python)

Issue:
DKIM key refresh issues
OS: Debian 11 Bullseye (stable)
Manual old (various) upgrades of Modoboa. Currently on version 2.0.3
Database: MariaDB

DKIM keys generated using Modoboa. I expected a path in the database like in (screenshot):
https://github.com/modoboa/modoboa/issues/2208#issuecomment-817108156
However only the name of the PEM file shows. No paths. Keys are stored in: /srv/modoboa/env/instance
A bit messy. Expected location on Debian would have been: /etc/dkimkeys

DKIM keys have been added to DNS.

DKIM signing does not work properly:
- mail sent from commandline is delivered but not signed
- mail sent from webmail (roundcube) errors: OpenDKIM is configured using TCP sockets, but problem and log entries are like: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=944513 (although TCP sockets are claimed to be working there)

The Debian bug  report suggested that the cause might be a corrupt key so I regenerated for 1 domain a DKIM key using the old(!) admin interface. I can let the admin panel show the key, so it seems ok, but ...
I decided to restart the opendkim service which failed. It turned out to be that:
- the PEM file in /srv/modoboa/env/instance was not updated
- afaik the PEM file was not stored in an other location
- the database field holding the PEM-file (and path) is empty! This is the reason why opendkim does not restart



Discussion / hints:
manually running: python manage.py modo manage_dkim_keys
fixed the empty field and refreshed/renewed the file in /srv/modoboa/env/instance
However I still think this is troublesome as in the meantime opendkim can not be restarted (or the server).
Also remains that the PEM files have no path in the DB, the storage location of the files is weird and the signing issues are still there.
The GUI says: SMTP Error: [451] 4.7.0 resource unavailable
But the logfile is more interesting and descriptive.
@olaf7 You should check why the command you ran manually was not called earlier. It is supposed to be run periodically as a cron task (look at /etc/cron.d/modoboa file)
And about keys location, that's something you can configure through the web interface.
@tonioo , my comment on the restart failure issue is about that there is a time gap between key modification  in the DB and the execution of the cron job. In that gap restart will fail. A solution would be if a mutation in the admin panel would result in immediately calling the code from the panel instead of waiting for the cronjob.

It turns out I have multiple issues getting DKIM to work.  Here is my feedback on the issue addressed in this ticket:
In my Modoboa installation the DKIM key path was not defined. This results in storage of PEM files in the root of the Modoboa installation (in my case a virtual env owned by user Modoboa) owned by the user running Modoboa (username modoboa in my case). But also in a registry in the database of the filename without a path. This is troublesome by default: postfix will have no idea where to find the PEM files and produces SMTP errors. Maybe Modoboa would warn/error on not valid values for the key-path and incomplete path/filename field entries?

Related to this is that the desired location should be writable by the systemuser running Modoboa where as OpenDKIM wants more restrictive rights and ownership. I do not see how to work around that without manual intervention as sudo will not do the trick:
- the path in the panel can not be set to the desired location (in my/Debian's case: /etc/dkimkeys) as it is not validated. It gives a warning, makes/keeps the setting empty and allows the user/admin to continue resulting in the situation described above.
- the dkim cronjob script uses the openssl binary but not using sudo, so adding something to /etc/sudoers will not help as that would only do the trick when strictly using binaries and sudo should be called. Currently not the case afaik.

I manually intervened and copied files to /etc/dkimkeys, did set rights and ownership correctly. Next I added full path to the PEM filename in the database. 
Result:
- no more errors when sending email
- getting a perfect score of 10 when using mail-tester.com, both from cli and when using roundcube webmail
- unfortunately mail does not get signed and does not arrive at gmail and yahoo. According to the sent box it does not get signed. Hard to debug as there are no errors etc in /var/log/mail.* or syslog. But that is probably an entirely different issue.

Fix patch:
diff --git a/frontend/src/App.vue b/frontend/src/App.vue
index 640da0f5f..a2603c62b 100644
--- a/frontend/src/App.vue
+++ b/frontend/src/App.vue
@@ -35,6 +35,7 @@
 
 <script>
 import { mapGetters } from 'vuex'
+import alarms from '@/api/alarms'
 import { bus } from '@/main'
 import Navbar from '@/components/layout/Navbar'
 
@@ -51,12 +52,23 @@ export default {
   created () {
     // this.$store.dispatch('auth/initialize')
     bus.$on('notification', this.showNotification)
+    bus.$on('loggedIn', this.checkAlarms)
+  },
+  mounted () {
+    this.checkAlarms()
+  },
+  beforeDestroy () {
+    clearInterval(this.alarmChecker)
   },
   data: () => ({
     snackbar: false,
     notification: '',
     notificationColor: 'success',
-    notificationTimeout: 2000
+    notificationTimeout: 2000,
+    alarmNotified: [],
+    alarmChecker: null,
+    lastAlarmDate: '',
+    alarmSleepTime: 10000
   }),
   methods: {
     showNotification (options) {
@@ -65,6 +77,33 @@ export default {
         this.notificationColor = (options.type) ? options.type : 'success'
         this.snackbar = true
       }
+    },
+    checkAlarms () {
+      if (this.isAuthenticated) {
+        this.alarmChecker = window.setInterval(() => {
+          const params = {}
+          if (this.lastAlarmDate instanceof Date) {
+            params.min_date = this.lastAlarmDate
+          }
+          alarms.getAll(params).then(resp => {
+            let count = 0
+            for (const alarm of resp.data) {
+              if (alarm.status === 1 && !this.alarmNotified.includes(alarm.id)) {
+                if (this.lastAlarmDate instanceof Date && new Date(alarm.created) > this.lastAlarmDate) {
+                  this.lastAlarmDate = new Date(alarm.created)
+                } else if (!(this.lastAlarmDate instanceof Date)) {
+                  this.lastAlarmDate = new Date(alarm.created)
+                }
+                count++
+                this.alarmNotified.push(alarm.id)
+              }
+            }
+            if (count !== 0) {
+              bus.$emit('notification', { msg: this.$gettext('You have one or more opened alarms'), type: 'error' })
+            }
+          })
+        }, this.alarmSleepTime)
+      }
     }
   }
 }
diff --git a/frontend/src/api/alarms.js b/frontend/src/api/alarms.js
index aa655bc31..535b7cb34 100644
--- a/frontend/src/api/alarms.js
+++ b/frontend/src/api/alarms.js
@@ -5,5 +5,14 @@ const resource = 'alarms'
 export default {
   getAll (params) {
     return repository.get(`${resource}/`, { params })
+  },
+  switchStatus (alarmId, data) {
+    return repository.patch(`${resource}/${alarmId}/switch/`, data)
+  },
+  delete (alarmId) {
+    return repository.delete(`${resource}/${alarmId}/`)
+  },
+  bulkDelete (alarmsIds) {
+    return repository.delete(`${resource}/bulk_delete/`, { params: { ids: alarmsIds } })
   }
 }
diff --git a/frontend/src/components/alarms/AlarmList.vue b/frontend/src/components/alarms/AlarmList.vue
index b7c6891b2..eaabeff12 100644
--- a/frontend/src/components/alarms/AlarmList.vue
+++ b/frontend/src/components/alarms/AlarmList.vue
@@ -8,6 +8,7 @@
         </v-btn>
       </template>
       <v-list dense>
+        <menu-items :items="getActionMenuItems()"/>
       </v-list>
     </v-menu>
     <v-spacer></v-spacer>
@@ -26,7 +27,7 @@
     :headers="headers"
     :items="alarms"
     :search="search"
-    item-key="pk"
+    item-key="id"
     :options.sync="options"
     :server-items-length="totalAlarms"
     :loading="loading"
@@ -53,18 +54,40 @@
       <template v-if="item.mailbox">
         {{ item.mailbox.address }}@{{ item.domain.name }}
       </template>
+      <template v-else>
+        /
+      </template>
     </template>
     <template v-slot:item.created="{ item }">
       {{ item.created|date }}
     </template>
+    <template v-slot:item.actions="{ item }">
+      <div class="text-right">
+        <v-menu offset-y>
+          <template v-slot:activator="{ on, attrs }">
+            <v-btn icon v-bind="attrs" v-on="on">
+              <v-icon>mdi-dots-horizontal</v-icon>
+            </v-btn>
+          </template>
+          <menu-items :items="getMenuItems(item)" :object="item" />
+        </v-menu>
+      </div>
+    </template>
   </v-data-table>
 </v-card>
 </template>
 
 <script>
+import { bus } from '@/main'
 import alarms from '@/api/alarms'
 import debounce from 'debounce'
+import MenuItems from '@/components/tools/MenuItems'
+import constants from '@/constants.json'
+
 export default {
+  components: {
+    MenuItems
+  },
   data () {
     return {
       alarms: [],
@@ -73,7 +96,8 @@ export default {
         { text: this.$gettext('Status'), value: 'status' },
         { text: this.$gettext('Domain'), value: 'domain.name', sortable: false },
         { text: this.$gettext('Mailbox'), value: 'mailbox', sortable: false },
-        { text: this.$gettext('Message'), value: 'title' }
+        { text: this.$gettext('Message'), value: 'title' },
+        { text: this.$gettext('Actions'), value: 'actions', sortable: false, align: 'right' }
       ],
       loading: true,
       options: {},
@@ -99,6 +123,66 @@ export default {
         this.totalAlarms = resp.data.count
         this.loading = false
       })
+    },
+    async deleteAlarm (alarm, single = true) {
+      alarms.delete(alarm.id).then(() => {
+        if (single) {
+          bus.$emit('notification', { msg: this.$gettext('Alarm deleted') })
+          this.fetchAlarms()
+        }
+      })
+    },
+    async closeAlarm (alarm) {
+      alarms.switchStatus(alarm.id, { status: constants.ALARM_CLOSED }).then(() => {
+        bus.$emit('notification', { msg: this.$gettext('Alarm closed') })
+        this.fetchAlarms()
+      })
+    },
+    async openAlarm (alarm) {
+      alarms.switchStatus(alarm.id, { status: constants.ALARM_OPENED }).then(() => {
+        bus.$emit('notification', { msg: this.$gettext('Alarm re-opened') })
+        this.fetchAlarms()
+      })
+    },
+    async deleteAlarms () {
+      await alarms.bulkDelete(this.selected.map(alarm => alarm.id))
+      bus.$emit('notification', { msg: this.$gettext('Alarms deleted') })
+      this.selected = []
+      this.fetchAlarms()
+    },
+    getMenuItems (item) {
+      const result = []
+      result.push({
+        label: this.$gettext('Delete'),
+        icon: 'mdi-delete-outline',
+        onClick: this.deleteAlarm,
+        color: 'red'
+      })
+      if (item.status === constants.ALARM_OPENED) {
+        result.push({
+          label: this.$gettext('Close alarm'),
+          icon: 'mdi-check',
+          onClick: this.closeAlarm,
+          color: 'green'
+        })
+      } else {
+        result.push({
+          label: this.$gettext('Reopen alarm'),
+          icon: 'mdi-alert-circle-outline',
+          onClick:
+          this.openAlarm,
+          color: 'orange'
+        })
+      }
+      return result
+    },
+    getActionMenuItems () {
+      const result = []
+      if (this.selected.length > 0) {
+        result.push({ label: this.$gettext('Delete'), icon: 'mdi-delete-outline', onClick: this.deleteAlarms, color: 'red' })
+      }
+      result.push({ label: this.$gettext('Reload'), icon: 'mdi-reload', onClick: this.fetchAlarms })
+      return result
     }
   },
   mounted () {
diff --git a/frontend/src/constants.json b/frontend/src/constants.json
new file mode 100644
index 000000000..b67288602
--- /dev/null
+++ b/frontend/src/constants.json
@@ -0,0 +1,4 @@
+{
+    "ALARM_OPENED": 1,
+    "ALARM_CLOSED": 2
+}
diff --git a/frontend/src/views/Login.vue b/frontend/src/views/Login.vue
index 4141c18eb..77a927dc0 100644
--- a/frontend/src/views/Login.vue
+++ b/frontend/src/views/Login.vue
@@ -58,6 +58,7 @@
 </template>
 
 <script>
+import { bus } from '@/main'
 import Cookies from 'js-cookie'
 import auth from '@/api/auth'
 
@@ -86,6 +87,7 @@ export default {
         Cookies.set('token', resp.data.access, { sameSite: 'strict' })
         Cookies.set('refreshToken', resp.data.refresh, { sameSite: 'strict' })
         this.$store.dispatch('auth/initialize').then(() => {
+          bus.$emit('loggedIn')
           this.$router.push({ name: 'DomainList' })
         })
       }).catch(err => {
diff --git a/frontend/vue.config.js b/frontend/vue.config.js
index 3ceebaec0..c0e75e1bc 100644
--- a/frontend/vue.config.js
+++ b/frontend/vue.config.js
@@ -5,7 +5,7 @@ module.exports = {
         target:
           process.env.DOCKER == "yes"
             ? "http://api:8000/"
-            : "http://localhost:8000",
+            : "http://127.0.0.1:8000",
       },
     },
   },
diff --git a/modoboa/admin/api/v2/serializers.py b/modoboa/admin/api/v2/serializers.py
index 84bccec49..1f4205535 100644
--- a/modoboa/admin/api/v2/serializers.py
+++ b/modoboa/admin/api/v2/serializers.py
@@ -225,6 +225,10 @@ def validate_dkim_keys_storage_dir(self, 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(
@@ -588,3 +592,14 @@ class Meta:
         depth = 1
         fields = "__all__"
         model = models.Alarm
+
+
+class AlarmSwitchStatusSerializer(serializers.Serializer):
+    """Serializer to switch the status of an Alarm."""
+
+    status = serializers.IntegerField()
+
+    def validate(self, data):
+        if data["status"] not in [constants.ALARM_OPENED, constants.ALARM_CLOSED]:
+            raise ValidationError(_("Unknown status"))
+        return data
diff --git a/modoboa/admin/api/v2/viewsets.py b/modoboa/admin/api/v2/viewsets.py
index f21348c84..54d42b3f2 100644
--- a/modoboa/admin/api/v2/viewsets.py
+++ b/modoboa/admin/api/v2/viewsets.py
@@ -20,6 +20,7 @@
 
 from ... import lib
 from ... import models
+from ... import constants
 from . import serializers
 
 
@@ -325,10 +326,30 @@ def forward(self, request, **kwargs):
         return response.Response(serializer.validated_data)
 
 
-class AlarmViewSet(GetThrottleViewsetMixin, viewsets.ReadOnlyModelViewSet):
+class AlarmFilterSet(dj_filters.FilterSet):
+    """Custom FilterSet for Alarms."""
+
+    min_date = dj_filters.DateTimeFilter(method="filter_date")
+
+    class Meta:
+        model = models.Alarm
+        fields = ["created"]
+
+    def filter_date(self, queryset, name, value):
+        return queryset.filter(created__gt=value)
+
+
+class AlarmViewSet(GetThrottleViewsetMixin,
+                   mixins.DestroyModelMixin,
+                   mixins.ListModelMixin,
+                   mixins.RetrieveModelMixin,
+                   viewsets.GenericViewSet):
     """Viewset for Alarm."""
 
-    filter_backends = (filters.OrderingFilter, filters.SearchFilter, )
+    filter_backends = (filters.OrderingFilter,
+                       filters.SearchFilter,
+                       dj_filters.DjangoFilterBackend)
+    filterset_class = AlarmFilterSet
     ordering_fields = ["created", "status", "title"]
     pagination_class = pagination.PageNumberPagination
     permission_classes = (
@@ -342,3 +363,29 @@ def get_queryset(self):
             domain__in=models.Domain.objects.get_for_admin(
                 self.request.user)
         ).order_by("-created")
+
+    @action(methods=['patch'], detail=True)
+    def switch(self, request, **kwargs):
+        """Custom update method that switch status of an alarm."""
+        alarm = self.get_object()
+        serializer = serializers.AlarmSwitchStatusSerializer(data=request.data)
+        serializer.is_valid(raise_exception=True)
+        if serializer.data["status"] == constants.ALARM_CLOSED:
+            alarm.close()
+        else:
+            alarm.reopen()
+        return response.Response(status=status.HTTP_204_NO_CONTENT)
+
+    @action(methods=['delete'], detail=False)
+    def bulk_delete(self, request, **kwargs):
+        """Delete multiple alarms at the same time."""
+        ids = request.query_params.getlist("ids[]")
+        if not ids:
+            return response.Response(_("No alarm ID provided"), status=400)
+        try:
+            ids = [int(alarm_id) for alarm_id in ids]
+        except ValueError:
+            return response.Response(
+                _("Received invalid alarm id(s)"), status=400)
+        models.Alarm.objects.filter(pk__in=ids).delete()
+        return response.Response(status=204)
diff --git a/modoboa/admin/constants.py b/modoboa/admin/constants.py
index 18480ab29..c1272c052 100644
--- a/modoboa/admin/constants.py
+++ b/modoboa/admin/constants.py
@@ -35,6 +35,8 @@
     (4096, "4096"),
 ]
 
+DKIM_WRITE_ERROR = "DKIM path 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 0bc9e6f4e..1d125ed43 100644
--- a/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py
+++ b/modoboa/admin/management/commands/subcommands/_manage_dkim_keys.py
@@ -4,12 +4,14 @@
 
 from django.core.management.base import BaseCommand
 from django.utils.encoding import smart_text
+from django.utils.translation import ugettext as _
 
 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
 
 
 class ManageDKIMKeys(BaseCommand):
@@ -19,6 +21,19 @@ 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)
+            else:
+                alarm = alarm_qset.first()
+                if alarm.status != ALARM_OPENED:
+                    alarm.reopen()
+            return
+        elif alarm_qset.exists():
+            alarm_qset.first().close()
         key_size = (
             domain.dkim_key_length if domain.dkim_key_length
             else self.default_key_length)
diff --git a/modoboa/admin/models/alarm.py b/modoboa/admin/models/alarm.py
index 8168fe51d..03509f32a 100644
--- a/modoboa/admin/models/alarm.py
+++ b/modoboa/admin/models/alarm.py
@@ -51,5 +51,11 @@ def close(self):
         self.closed = timezone.now()
         self.save()
 
+    def reopen(self):
+        """Reopen this alarm."""
+        self.status = constants.ALARM_OPENED
+        self.closed = None
+        self.save()
+
 
 reversion.register(Alarm)
diff --git a/modoboa/admin/models/mxrecord.py b/modoboa/admin/models/mxrecord.py
index c03ae3e4f..c0653ffbe 100644
--- a/modoboa/admin/models/mxrecord.py
+++ b/modoboa/admin/models/mxrecord.py
@@ -60,7 +60,7 @@ class MXRecord(models.Model):
     managed = models.BooleanField(default=False)
     updated = models.DateTimeField()
 
-    objects = MXRecordManager.from_queryset(MXRecordQuerySet)()
+    objects: MXRecordManager = MXRecordManager.from_queryset(MXRecordQuerySet)()
 
     def is_managed(self):
         if not param_tools.get_global_parameter("enable_mx_checks"):


Test patch:
diff --git a/modoboa/admin/api/v2/tests.py b/modoboa/admin/api/v2/tests.py
index b4235ad95..347d4be99 100644
--- a/modoboa/admin/api/v2/tests.py
+++ b/modoboa/admin/api/v2/tests.py
@@ -6,7 +6,7 @@
 
 from rest_framework.authtoken.models import Token
 
-from modoboa.admin import factories, models
+from modoboa.admin import factories, models, constants
 from modoboa.core import models as core_models
 from modoboa.lib.tests import ModoAPITestCase
 
@@ -479,9 +479,84 @@ def setUpTestData(cls):  # NOQA:N802
         factories.populate_database()
         factories.AlarmFactory(
             domain__name="test.com", mailbox=None, title="Test alarm")
+        cls.da_token = Token.objects.create(
+            user=core_models.User.objects.get(username="admin@test.com"))
 
     def test_list(self):
         url = reverse("v2:alarm-list")
         resp = self.client.get(url)
         self.assertEqual(resp.status_code, 200)
         self.assertEqual(len(resp.json()), 1)
+
+    def test_update_alarm(self):
+        """Try updating alarm status and delete it afterward."""
+
+        domain = models.Domain.objects.get(name="test.com")
+
+        # Try performing action on restricted domains
+        self.client.credentials(
+            HTTP_AUTHORIZATION="Token " + self.da_token.key)
+        domain = models.Domain.objects.get(name="test2.com")
+        alarm_restricted = models.Alarm.objects.create(
+            domain=domain, mailbox=None, title="Test alarm 2")
+        alarm_restricted.save()
+        url = reverse("v2:alarm-switch", args=[alarm_restricted.pk])
+        resp = self.client.post(url)
+        self.assertEqual(resp.status_code, 405)
+        url = reverse("v2:alarm-detail", args=[alarm_restricted.pk])
+        resp = self.client.delete(url)
+        self.assertEqual(resp.status_code, 404)
+
+        # Perform actions as SuperAdmin
+        self.client.credentials(HTTP_AUTHORIZATION="Token " + self.token.key)
+
+        alarm = models.Alarm.objects.create(
+            domain=domain, mailbox=None, title="Test alarm 3")
+        alarm.save()
+
+        # Switch status of the alarm to close
+        url = reverse("v2:alarm-switch", args=[alarm.pk])
+        resp = self.client.patch(url, {"status": constants.ALARM_CLOSED})
+        self.assertEqual(resp.status_code, 204)
+
+        # Check actual status
+        url = reverse("v2:alarm-detail", args=[alarm.pk])
+        resp = self.client.get(url)
+        self.assertEqual(resp.json()["status"], constants.ALARM_CLOSED)
+
+        # Switch status back to open
+        url = reverse("v2:alarm-switch", args=[alarm.pk])
+        resp = self.client.patch(url, {"status": constants.ALARM_OPENED})
+        self.assertEqual(resp.status_code, 204)
+
+        # Check actual status
+        url = reverse("v2:alarm-detail", args=[alarm.pk])
+        resp = self.client.get(url)
+        self.assertEqual(resp.json()["status"], constants.ALARM_OPENED)
+
+        # Try to set an non-existant status
+        url = reverse("v2:alarm-switch", args=[alarm.pk])
+        resp = self.client.patch(url, {"status": 10})
+        self.assertEqual(resp.status_code, 400)
+
+        # Delete the alarm
+        url = reverse("v2:alarm-detail", args=[alarm.pk])
+        resp = self.client.delete(url)
+        self.assertEqual(resp.status_code, 204)
+
+    def test_bulk_delete(self):
+        url = reverse("v2:alarm-bulk-delete")
+        resp = self.client.delete(url)
+        self.assertEqual(resp.status_code, 400)
+        resp = self.client.delete(f"{url}?ids[]=toto")
+        self.assertEqual(resp.status_code, 400)
+        alarm1 = factories.AlarmFactory(
+            domain__name="test.com", mailbox=None, title="Test alarm")
+        alarm2 = factories.AlarmFactory(
+            domain__name="test.com", mailbox=None, title="Test alarm")
+        resp = self.client.delete(f"{url}?ids[]={alarm1.pk}&ids[]={alarm2.pk}")
+        self.assertEqual(resp.status_code, 204)
+        with self.assertRaises(models.Alarm.DoesNotExist):
+            alarm1.refresh_from_db()
+        with self.assertRaises(models.Alarm.DoesNotExist):
+            alarm2.refresh_from_db()
diff --git a/modoboa/admin/tests/test_alarms.py b/modoboa/admin/tests/test_alarms.py
new file mode 100644
index 000000000..560dde817
--- /dev/null
+++ b/modoboa/admin/tests/test_alarms.py
@@ -0,0 +1,26 @@
+"""Alarm related tests."""
+
+from django.urls import reverse
+
+from modoboa.lib.tests import ModoTestCase
+from ..factories import populate_database
+from ..models import Alarm, Domain
+
+
+class AlarmsTestCase(ModoTestCase):
+    """Alarm open/close test cases."""
+
+    def setUp(self):
+        super().setUp()
+        populate_database()
+
+    def test_set_forward(self):
+        domain = Domain.objects.all().first()
+        alarm = Alarm.objects.create(
+            domain=domain, mailbox=None, title="Test alarm 3")
+        alarm.save()
+        self.assertEqual(alarm.status, 1)
+        alarm.close()
+        self.assertEqual(alarm.status, 2)
+        alarm.reopen()
+        self.assertEqual(alarm.status, 1)
diff --git a/modoboa/admin/tests/test_domain.py b/modoboa/admin/tests/test_domain.py
index 4b3793b0c..6fd4a8b7a 100644
--- a/modoboa/admin/tests/test_domain.py
+++ b/modoboa/admin/tests/test_domain.py
@@ -20,9 +20,10 @@
 from modoboa.lib.tests import ModoTestCase
 from modoboa.maillog import factories as ml_factories
 
+from ..import constants
 from . import utils
 from .. import factories
-from ..models import Alias, Domain
+from ..models import Alarm, Alias, Domain
 
 
 class DomainTestCase(ModoTestCase):
@@ -445,10 +446,30 @@ def test_dkim_key_creation(self):
         values["dkim_key_selector"] = "default"
         self.ajax_post(reverse("admin:domain_add"), values)
 
+        # Try generating a key to a protected directory
+        self.set_global_parameter("dkim_keys_storage_dir", "/I_DONT_EXIST")
         call_command("modo", "manage_dkim_keys")
+        domain = Domain.objects.get(name=values["name"])
+
+        # Closing the alarm and retry generating dkim keys
+        Alarm.objects.get(domain=domain,
+                          internal_name=constants.DKIM_WRITE_ERROR
+                          ).close()
+        call_command("modo", "manage_dkim_keys")
+        self.assertEqual(
+            Alarm.objects.get(domain=domain,
+                              internal_name=constants.DKIM_WRITE_ERROR).status,
+            constants.ALARM_OPENED)
+
+        # Generate normally
+        self.set_global_parameter("dkim_keys_storage_dir", self.workdir)
+        call_command("modo", "manage_dkim_keys")
+        self.assertEqual(
+            Alarm.objects.get(domain=domain,
+                              internal_name=constants.DKIM_WRITE_ERROR).status,
+            constants.ALARM_CLOSED)
         key_path = os.path.join(self.workdir, "{}.pem".format(values["name"]))
         self.assertTrue(os.path.exists(key_path))
-
         domain = Domain.objects.get(name=values["name"])
         url = reverse("admin:domain_detail", args=[domain.pk])
         response = self.client.get(url)
diff --git a/modoboa/parameters/api/v2/tests.py b/modoboa/parameters/api/v2/tests.py
index 0803b650d..dc5e21c59 100644
--- a/modoboa/parameters/api/v2/tests.py
+++ b/modoboa/parameters/api/v2/tests.py
@@ -75,6 +75,18 @@ def test_update_admin_settings(self):
             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,
             "domains_must_have_authorized_mx": True,