✨(aliases) delete aliases

delete a single alias using its pk

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to
 
 ## [Unreleased]
 
+- ✨(aliases) fix deleting single aliases #1002
 - 🔥(plugins) remove CommuneCreation plugin
 
 ## [1.21.0] - 2025-12-05

src/backend/mailbox_manager/api/client/serializers.py

@@ -4,6 +4,7 @@ from logging import getLogger
 
 from django.contrib.auth.hashers import make_password
 from django.core import exceptions as django_exceptions
+from django.shortcuts import get_object_or_404
 
 from requests.exceptions import HTTPError
 from rest_framework import exceptions, serializers
@@ -306,7 +307,9 @@ class MailDomainInvitationSerializer(serializers.ModelSerializer):
 
 
 class AliasSerializer(serializers.ModelSerializer):
-    """Serialize mailbox."""
+    """Serialize aliases."""
+
+    domain = MailDomainSerializer(default="")
 
     class Meta:
         model = models.Alias
@@ -314,8 +317,13 @@ class AliasSerializer(serializers.ModelSerializer):
             "id",
             "local_part",
             "destination",
+            "domain",
         ]
-        read_only_fields = ["id"]
+        read_only_fields = ["id", "domain"]
+
+    def validate_domain(self, value):  # pylint: disable=unused-argument
+        """Forcefully set domain field to url domain."""
+        return get_object_or_404(models.MailDomain, slug=self.context["domain_slug"])
 
     def create(self, validated_data):
         """

src/backend/mailbox_manager/api/client/viewsets.py

@@ -1,6 +1,7 @@
 """API endpoints"""
 
 from django.db.models import Q, Subquery
+from django.http import Http404
 
 from rest_framework import exceptions, filters, mixins, status, viewsets
 from rest_framework.decorators import action
@@ -422,11 +423,11 @@ class AliasViewSet(
         - destination: str
         Return a newly created alias
 
-    DELETE /api/<version>/mail-domains/<domain_slug>/accesses/<alias-local-part>/
+    DELETE /api/<version>/mail-domains/<domain_slug>/aliases/<alias_pk>/
         Delete targeted alias
     """
 
-    lookup_field = "local_part"
+    lookup_field = "pk"
     permission_classes = [permissions.DomainResourcePermission]
     serializer_class = serializers.AliasSerializer
     queryset = (
@@ -459,38 +460,19 @@ class AliasViewSet(
 
         return queryset
 
-    def get_permissions(self):
-        """Add a specific permission for domain viewers to delete their aliases."""
-        if self.action in ["destroy"]:
-            permission_classes = [
-                permissions.DomainResourcePermission
-                | permissions.IsAliasDestinationPermission,
-            ]
-        else:
-            return super().get_permissions()
-
-        return [permission() for permission in permission_classes]
-
-    def perform_create(self, serializer):
-        """Create new mailbox."""
-        domain_slug = self.kwargs.get("domain_slug", "")
-        if domain_slug:
-            serializer.validated_data["domain"] = models.MailDomain.objects.get(
-                slug=domain_slug
-            )
-        super().perform_create(serializer)
-
     def destroy(self, request, *args, **kwargs):
-        """Override destroy method to send a delete request to dimail
-        and return clear message if domain out of sync."""
+        """
+        Override destroy method to delete specific alias and send request to dimail.
+        """
         instance = self.get_object()
         self.perform_destroy(instance)
 
         client = DimailAPIClient()
         dimail_response = client.delete_alias(instance)
+
         if dimail_response.status_code == status.HTTP_404_NOT_FOUND:
             return Response(
-                "Alias already deleted. Domain out of sync, please contact our support.",
+                "Domain out of sync with mailbox provider, please contact our support.",
                 status=status.HTTP_200_OK,
             )
 

src/backend/mailbox_manager/api/permissions.py

@@ -55,17 +55,3 @@ class IsMailboxOwnerPermission(permissions.BasePermission):
     def has_object_permission(self, request, view, obj):
         """If the user is trying to update their own mailbox."""
         return obj.get_email() == request.user.email
-
-
-class IsAliasDestinationPermission(IsAuthenticated):
-    """Can delete an alias if the alias points to their own email address."""
-
-    def has_permission(self, request, view):
-        """This permission is specifically about updates"""
-        domain = models.MailDomain.objects.get(slug=view.kwargs.get("domain_slug", ""))
-        abilities = domain.get_abilities(request.user)
-        return abilities["get"]
-
-    def has_object_permission(self, request, view, obj):
-        """If the user is trying to update their own mailbox."""
-        return obj.destination == request.user.email

src/backend/mailbox_manager/migrations/0029_alter_alias_unique_together_alias_no_duplicate.py

@@ -0,0 +1,21 @@
+# Generated by Django 5.2.9 on 2025-12-17 17:01
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('mailbox_manager', '0028_alias'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='alias',
+            unique_together=set(),
+        ),
+        migrations.AddConstraint(
+            model_name='alias',
+            constraint=models.UniqueConstraint(fields=('domain', 'local_part', 'destination'), name='no_duplicate'),
+        ),
+    ]

src/backend/mailbox_manager/models.py

@@ -89,6 +89,8 @@ class MailDomain(BaseModel):
     def __str__(self):
         return self.name
 
+    objects = models.Manager()
+
     def save(self, *args, **kwargs):
         """Override save function to compute the slug."""
         self.slug = self.get_slug()
@@ -471,8 +473,12 @@ class Alias(BaseModel):
         db_table = "people_aliases"
         verbose_name = _("Alias")
         verbose_name_plural = _("Aliases")
-        unique_together = ("local_part", "destination")
         ordering = ["-created_at"]
+        constraints = [
+            models.UniqueConstraint(
+                fields=["domain", "local_part", "destination"], name="no_duplicate"
+            )
+        ]
 
     def __str__(self):
         return f"{self.local_part} to {self.destination}"

src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py

@@ -60,8 +60,9 @@ def test_api_aliases_create__viewer_forbidden():
     assert not models.Alias.objects.exists()
 
 
+@responses.activate
 def test_api_aliases_create__duplicate_forbidden():
-    """Cannot create alias if same local part + destination."""
+    """Cannot create alias if existing alias same domain + local part + destination."""
     access = factories.MailDomainAccessFactory(
         role="owner", domain=factories.MailDomainEnabledFactory()
     )

src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py

@@ -1,6 +1,6 @@
 """
 Tests for aliases API endpoint in People's app mailbox_manager.
-Focus on "list" action.
+Focus on "delete" action.
 """
 # pylint: disable=W0613
 
@@ -20,11 +20,12 @@ pytestmark = pytest.mark.django_db
 
 def test_api_aliases_delete__anonymous():
     """Anonymous user should not be able to delete aliases."""
-    alias = factories.AliasFactory()
+    alias_ = factories.AliasFactory()
 
     response = APIClient().delete(
-        f"/api/v1.0/mail-domains/{alias.domain.slug}/aliases/{alias.local_part}/",
+        f"/api/v1.0/mail-domains/{alias_.domain.slug}/aliases/{alias_.pk}/"
     )
+
     assert response.status_code == status.HTTP_401_UNAUTHORIZED
     assert models.Alias.objects.count() == 1
 
@@ -35,12 +36,12 @@ def test_api_aliases_delete__no_access_forbidden_not_found():
     mail domain to which they are not related.
     """
     authenticated_user = core_factories.UserFactory()
-    alias = factories.AliasFactory()
+    alias_ = factories.AliasFactory()
 
     client = APIClient()
     client.force_login(authenticated_user)
     response = client.delete(
-        f"/api/v1.0/mail-domains/{alias.domain.slug}/aliases/{alias.local_part}/",
+        f"/api/v1.0/mail-domains/{alias_.domain.slug}/aliases/{alias_.pk}/"
     )
 
     assert response.status_code == status.HTTP_404_NOT_FOUND
@@ -56,59 +57,40 @@ def test_api_aliases_delete__viewer_forbidden():
     mail_domain = factories.MailDomainFactory(
         users=[(authenticated_user, enums.MailDomainRoleChoices.VIEWER)]
     )
-    alias = factories.AliasFactory(domain=mail_domain)
+    alias_ = factories.AliasFactory(domain=mail_domain)
 
     client = APIClient()
     client.force_login(authenticated_user)
     response = client.delete(
-        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/",
+        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias_.pk}/"
     )
 
     assert response.status_code == status.HTTP_403_FORBIDDEN
     assert models.Alias.objects.count() == 1
 
 
-@responses.activate
-def test_api_aliases_delete__viewer_can_delete_self_alias(dimail_token_ok):
-    """
-    Authenticated users should be allowed to delete aliases when linking
-    to their own mailbox.
-    """
-    authenticated_user = core_factories.UserFactory()
-    mail_domain = factories.MailDomainFactory(
-        users=[(authenticated_user, enums.MailDomainRoleChoices.VIEWER)]
-    )
-    alias = factories.AliasFactory(
-        domain=mail_domain, destination=authenticated_user.email
-    )
-
-    # Mock dimail response
-    responses.delete(
-        re.compile(r".*/aliases/"),
-        status=status.HTTP_204_NO_CONTENT,
-        content_type="application/json",
-    )
-
-    client = APIClient()
-    client.force_login(authenticated_user)
-    response = client.delete(
-        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/",
-    )
-
-    assert response.status_code == status.HTTP_204_NO_CONTENT
-    assert not models.Alias.objects.exists()
-
-
 @responses.activate
 def test_api_aliases_delete__administrators_allowed(dimail_token_ok):
     """
-    Administrators of a mail domain should be allowed to delete accesses excepted owner accesses.
+    Administrators of a mail domain should be allowed to delete aliases.
     """
     authenticated_user = core_factories.UserFactory()
     mail_domain = factories.MailDomainFactory(
         users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)]
     )
-    alias = factories.AliasFactory(domain=mail_domain)
+    alias_ = factories.AliasFactory(domain=mail_domain)
+    factories.AliasFactory.create_batch(
+        2, domain=mail_domain, local_part=alias_.local_part
+    )
+
+    # additional aliases that shouldn't be affected
+    factories.AliasFactory.create_batch(
+        2, domain=mail_domain, destination=alias_.destination
+    )
+    factories.AliasFactory(
+        local_part=alias_.local_part,
+        destination=alias_.destination,
+    )
 
     # Mock dimail response
     responses.delete(
@@ -120,11 +102,10 @@ def test_api_aliases_delete__administrators_allowed(dimail_token_ok):
     client = APIClient()
     client.force_login(authenticated_user)
     response = client.delete(
-        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/",
+        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias_.pk}/"
     )
-
     assert response.status_code == status.HTTP_204_NO_CONTENT
-    assert not models.Alias.objects.exists()
+    assert models.Alias.objects.count() == 5
 
 
 @responses.activate
@@ -136,8 +117,7 @@ def test_api_aliases_delete__404_out_of_sync(dimail_token_ok):
     mail_domain = factories.MailDomainFactory(
         users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)]
     )
-    alias = factories.AliasFactory(domain=mail_domain)
-
+    alias_ = factories.AliasFactory(domain=mail_domain)
     # Mock dimail response
     responses.delete(
         re.compile(r".*/aliases/"),
@@ -149,11 +129,11 @@ def test_api_aliases_delete__404_out_of_sync(dimail_token_ok):
     client = APIClient()
     client.force_login(authenticated_user)
     response = client.delete(
-        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/",
+        f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias_.pk}/"
     )
     assert response.status_code == status.HTTP_200_OK
     assert (
         response.json()
-        == "Alias already deleted. Domain out of sync, please contact our support."
+        == "Domain out of sync with mailbox provider, please contact our support."
     )
     assert not models.Alias.objects.exists()