From 8407a49e26c2eaf4eb986159319e92c3e79f3fdf Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Thu, 21 Dec 2023 17:47:06 +0100 Subject: [PATCH 1/4] Prevent creation of same alias addresses using different case fix #3134 --- modoboa/admin/api/v1/serializers.py | 6 +++--- modoboa/admin/api/v1/tests.py | 11 +++++++++++ modoboa/admin/models/alias.py | 2 +- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/modoboa/admin/api/v1/serializers.py b/modoboa/admin/api/v1/serializers.py index c40d89c06..f4d8651f8 100644 --- a/modoboa/admin/api/v1/serializers.py +++ b/modoboa/admin/api/v1/serializers.py @@ -469,7 +469,7 @@ class AliasSerializer(serializers.ModelSerializer): """Check domain.""" local_part, self.domain = admin_models.validate_alias_address( value, self.context["request"].user, instance=self.instance) - return value + return value.lower() def create(self, validated_data): """Create appropriate objects.""" @@ -498,13 +498,13 @@ class SenderAddressSerializer(serializers.ModelSerializer): def validate_address(self, value): """Check domain.""" - local_part, domain = email_utils.split_mailbox(value) + local_part, domain = email_utils.split_mailbox(value.lower()) domain = admin_models.Domain.objects.filter(name=domain).first() user = self.context["request"].user if domain and not user.can_access(domain): raise serializers.ValidationError( _("You don't have access to this domain.")) - return value + return value.lower() def validate_mailbox(self, value): """Check permission.""" diff --git a/modoboa/admin/api/v1/tests.py b/modoboa/admin/api/v1/tests.py index fd48783c5..7922e8c29 100644 --- a/modoboa/admin/api/v1/tests.py +++ b/modoboa/admin/api/v1/tests.py @@ -678,6 +678,17 @@ class AliasAPITestCase(ModoAPITestCase): response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 201) + def test_create_duplicate_alias(self): + url = reverse("v1:alias-list") + response = self.client.post(url, self.ALIAS_DATA, format="json") + self.assertEqual(response.status_code, 201) + + # Try with same address but different case + data = copy.deepcopy(self.ALIAS_DATA) + data["address"] = "ALIAS_fromapi@test.com" + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 400) + def test_create_alias_as_domadmin(self): """As DomainAdmin, try to create a new alias.""" self.client.credentials( diff --git a/modoboa/admin/models/alias.py b/modoboa/admin/models/alias.py index 2e7f32683..28f48eef8 100644 --- a/modoboa/admin/models/alias.py +++ b/modoboa/admin/models/alias.py @@ -28,7 +28,7 @@ def validate_alias_address( address, creator, internal=False, instance=None, ignore_existing=False ): """Check if the given alias address can be created by creator.""" - local_part, domain = split_mailbox(address) + local_part, domain = split_mailbox(address.lower()) domain = Domain.objects.filter(name=domain).first() if domain is None: raise ValidationError(_("Domain not found.")) From d3398cc51f916b833975e5c37f65d4b17c0364a2 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Thu, 21 Dec 2023 18:03:23 +0100 Subject: [PATCH 2/4] Better error handling fix #3134 --- modoboa/admin/api/v1/serializers.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/modoboa/admin/api/v1/serializers.py b/modoboa/admin/api/v1/serializers.py index f4d8651f8..b1769ccc8 100644 --- a/modoboa/admin/api/v1/serializers.py +++ b/modoboa/admin/api/v1/serializers.py @@ -17,6 +17,7 @@ from modoboa.admin import models as admin_models from modoboa.core import ( constants as core_constants, models as core_models, signals as core_signals ) +from modoboa.lib.exceptions import AliasExists from modoboa.lib import ( email_utils, exceptions as lib_exceptions, fields as lib_fields, permissions, validators, web_utils @@ -467,8 +468,11 @@ class AliasSerializer(serializers.ModelSerializer): def validate_address(self, value): """Check domain.""" - local_part, self.domain = admin_models.validate_alias_address( - value, self.context["request"].user, instance=self.instance) + try: + local_part, self.domain = admin_models.validate_alias_address( + value, self.context["request"].user, instance=self.instance) + except (AliasExists, ValidationError) as err: + raise serializers.ValidationError(err) return value.lower() def create(self, validated_data): From 0d5e8b26071f8f3feefbfb20beb82c01b7a556cc Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Thu, 21 Dec 2023 18:17:54 +0100 Subject: [PATCH 3/4] Fixed wrong exception handling --- modoboa/admin/api/v1/serializers.py | 4 +++- modoboa/admin/api/v2/tests.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modoboa/admin/api/v1/serializers.py b/modoboa/admin/api/v1/serializers.py index b1769ccc8..0ba2c6b3c 100644 --- a/modoboa/admin/api/v1/serializers.py +++ b/modoboa/admin/api/v1/serializers.py @@ -471,8 +471,10 @@ class AliasSerializer(serializers.ModelSerializer): try: local_part, self.domain = admin_models.validate_alias_address( value, self.context["request"].user, instance=self.instance) - except (AliasExists, ValidationError) as err: + except ValidationError as err: raise serializers.ValidationError(err) + except AliasExists: + raise serializers.ValidationError(_("This alias already exists")) return value.lower() def create(self, validated_data): diff --git a/modoboa/admin/api/v2/tests.py b/modoboa/admin/api/v2/tests.py index 0f09ae935..db5881cbb 100644 --- a/modoboa/admin/api/v2/tests.py +++ b/modoboa/admin/api/v2/tests.py @@ -535,7 +535,7 @@ class AliasViewSetTestCase(ModoAPITestCase): data = {"address": "alias@test.com"} resp = self.client.post(url, data, format="json") - self.assertEqual(resp.status_code, 409) + self.assertEqual(resp.status_code, 400) al_id = models.Alias.objects.get(address="alias@test.com").pk self.assertEqual(resp.json()["id"], al_id) From 2f5e4a58d1fd712ebd7f4dee4b5d2b32db2251c9 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Fri, 22 Dec 2023 08:54:07 +0100 Subject: [PATCH 4/4] Fixed test --- modoboa/admin/api/v2/tests.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/modoboa/admin/api/v2/tests.py b/modoboa/admin/api/v2/tests.py index db5881cbb..472c87e33 100644 --- a/modoboa/admin/api/v2/tests.py +++ b/modoboa/admin/api/v2/tests.py @@ -536,9 +536,10 @@ class AliasViewSetTestCase(ModoAPITestCase): data = {"address": "alias@test.com"} resp = self.client.post(url, data, format="json") self.assertEqual(resp.status_code, 400) - - al_id = models.Alias.objects.get(address="alias@test.com").pk - self.assertEqual(resp.json()["id"], al_id) + self.assertEqual( + resp.json()["address"][0], + "This alias already exists" + ) data = {"address": "alias2@test.com"} resp = self.client.post(url, data, format="json")