From ea41ccd44e090e18398b8727ff159a655e4d1b9b Mon Sep 17 00:00:00 2001
From: Antoine Nguyen <tonio@ngyn.org>
Date: Thu, 5 May 2022 15:19:28 +0200
Subject: [PATCH] Valid MXs field must contain only IPs.

see #1664
---
 modoboa/admin/api/v2/serializers.py | 16 +++++++++++
 modoboa/admin/app_settings.py       | 21 ++++++++++++++-
 modoboa/core/api/v2/views.py        |  2 +-
 modoboa/parameters/api/v2/tests.py  | 42 ++++++++++++++++++++++++++++-
 4 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/modoboa/admin/api/v2/serializers.py b/modoboa/admin/api/v2/serializers.py
index 8ffcf3807..62fe5fb51 100644
--- a/modoboa/admin/api/v2/serializers.py
+++ b/modoboa/admin/api/v2/serializers.py
@@ -1,5 +1,6 @@
 """Admin API v2 serializers."""
 
+import ipaddress
 import os
 
 from django.conf import settings
@@ -230,6 +231,21 @@ class AdminGlobalParametersSerializer(serializers.Serializer):
                 )
         return value
 
+    def validate_valid_mxs(self, value):
+        """Make sure it only contains IP addresses."""
+        if not value:
+            return value
+        try:
+            ip_addresses = [
+                ipaddress.ip_network(v.strip())
+                for v in value.split() if v.strip()
+            ]
+        except ValueError:
+            raise serializers.ValidationError(
+                _("This field only allows valid IP addresses (or networks)")
+            )
+        return value
+
     def validate(self, data):
         """Check MX options."""
         condition = (
diff --git a/modoboa/admin/app_settings.py b/modoboa/admin/app_settings.py
index 184e3b5e2..1b763c0ad 100644
--- a/modoboa/admin/app_settings.py
+++ b/modoboa/admin/app_settings.py
@@ -1,6 +1,7 @@
 """Modoboa admin settings."""
 
 import collections
+import ipaddress
 import os
 
 from django import forms
@@ -196,7 +197,7 @@ class AdminParametersForm(param_forms.AdminParametersForm):
     }
 
     def __init__(self, *args, **kwargs):
-        super(AdminParametersForm, self).__init__(*args, **kwargs)
+        super().__init__(*args, **kwargs)
         self.field_widths = {
             "default_domain_quota": 2,
             "default_mailbox_quota": 2
@@ -260,6 +261,24 @@ class AdminParametersForm(param_forms.AdminParametersForm):
                 )
         return storage_dir
 
+    def clean_valid_mxs(self):
+        """Make sure it only contains IP addresses."""
+        value = self.cleaned_data.get("valid_mxs")
+        if not value:
+            return value
+        try:
+            ip_addresses = [
+                ipaddress.ip_network(v.strip())
+                for v in value.split() if v.strip()
+            ]
+        except ValueError:
+            raise forms.ValidationError(
+                ugettext_lazy(
+                    "This field only allows valid IP addresses (or networks)"
+                )
+            )
+        return value
+
     def clean(self):
         """Check MX options."""
         cleaned_data = super(AdminParametersForm, self).clean()
diff --git a/modoboa/core/api/v2/views.py b/modoboa/core/api/v2/views.py
index 214affd20..c6ca853ff 100644
--- a/modoboa/core/api/v2/views.py
+++ b/modoboa/core/api/v2/views.py
@@ -54,7 +54,7 @@ class TokenObtainPairView(jwt_views.TokenObtainPairView):
                         _("Password scheme mismatch. Updating %s password"),
                         user.username
                     )
-                    user.set_password(serializer.data["password"])
+                    user.set_password(request.data["password"])
                     user.save()
                 if pwhash.needs_rehash(user.password):
                     logger.info(
diff --git a/modoboa/parameters/api/v2/tests.py b/modoboa/parameters/api/v2/tests.py
index 0d3dbba7f..0803b650d 100644
--- a/modoboa/parameters/api/v2/tests.py
+++ b/modoboa/parameters/api/v2/tests.py
@@ -46,5 +46,45 @@ class ParametersAPITestCase(ModoAPITestCase):
             "deflt_domain_mailbox_aliases_limit": 0
         }
         url = reverse("v2:parameter-detail", args=["limits"])
-        resp = self.client.put(url, json=data)
+        resp = self.client.put(url, data, format="json")
         self.assertEqual(resp.status_code, 200)
+
+    def test_update_admin_settings(self):
+        data = {
+            "default_domain_quota": -1,
+            "default_mailbox_quota": -1,
+            "valid_mxs": "mail.test.com",
+            "custom_dns_server": "",
+            "dkim_keys_storage_dir": "/wrong"
+        }
+        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["valid_mxs"],
+            ["This field only allows valid IP addresses (or networks)"]
+        )
+        self.assertEqual(
+            errors["default_domain_quota"], ["Must be a positive integer"]
+        )
+        self.assertEqual(
+            errors["default_mailbox_quota"], ["Must be a positive integer"]
+        )
+        self.assertEqual(
+            errors["dkim_keys_storage_dir"], ["Directory not found."]
+        )
+
+        data = {
+            "enable_mx_checks": True,
+            "domains_must_have_authorized_mx": True,
+            "valid_mxs": "",
+            "custom_dns_server": "",
+            "dkim_keys_storage_dir": ""
+        }
+        resp = self.client.put(url, data, format="json")
+        self.assertEqual(resp.status_code, 400)
+        self.assertEqual(
+            resp.json()["valid_mxs"],
+            ["Define at least one authorized network / address"]
+        )