Merge pull request #2884 from modoboa/better-dkim

Added an alarm on dkim write error, reworked alarms

modoboa/admin/api/v2/serializers.py

@@ -226,6 +226,10 @@ class AdminGlobalParametersSerializer(serializers.Serializer):
                 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(
@@ -621,3 +625,14 @@ class AlarmSerializer(serializers.ModelSerializer):
         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

modoboa/admin/api/v2/tests.py

@@ -6,7 +6,7 @@ from django.utils.encoding import force_text
 
 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 @@ class AlarmViewSetTestCase(ModoAPITestCase):
         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()

modoboa/admin/api/v2/viewsets.py

@@ -20,6 +20,7 @@ from modoboa.lib.throttle import GetThrottleViewsetMixin
 
 from ... import lib
 from ... import models
+from ... import constants
 from . import serializers
 
 
@@ -325,10 +326,30 @@ class UserAccountViewSet(GetThrottleViewsetMixin, viewsets.ViewSet):
         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 @@ class AlarmViewSet(GetThrottleViewsetMixin, viewsets.ReadOnlyModelViewSet):
             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)

modoboa/admin/constants.py

@@ -35,6 +35,8 @@ DKIM_KEY_LENGTHS = [
     (4096, "4096"),
 ]
 
+DKIM_WRITE_ERROR = "DKIM path error"
+
 ALARM_OPENED = 1
 ALARM_CLOSED = 2
 

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

@@ -4,12 +4,14 @@ import os
 
 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 @@ 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)
+            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)

modoboa/admin/models/alarm.py

@@ -51,5 +51,11 @@ class Alarm(models.Model):
         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)

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"):

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)

modoboa/admin/tests/test_domain.py

@@ -20,9 +20,10 @@ from modoboa.core.tests.test_views import SETTINGS_SAMPLE
 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 @@ class DKIMTestCase(ModoTestCase):
         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)

modoboa/parameters/api/v2/tests.py

@@ -75,6 +75,18 @@ class ParametersAPITestCase(ModoAPITestCase):
             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,