From 15337a22269a3258b71cc6faa7ec57c0e62c023a Mon Sep 17 00:00:00 2001 From: Marie PUPO JEAMMET Date: Fri, 31 Oct 2025 16:52:13 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(aliases)=20warn=20that=20domain=20is?= =?UTF-8?q?=20out=20of=20sync=20when=20unexpected=20404?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deleting an alias should trigger a request to dimail. if dimail's response if a 404, it means people and dimail are out of sync with each other. log error and warn user --- .../mailbox_manager/api/client/viewsets.py | 18 +++- .../api/aliases/test_api_aliases_delete.py | 88 ++++++++++++++----- src/backend/mailbox_manager/utils/dimail.py | 12 ++- 3 files changed, 92 insertions(+), 26 deletions(-) diff --git a/src/backend/mailbox_manager/api/client/viewsets.py b/src/backend/mailbox_manager/api/client/viewsets.py index d8ab1ee..545bd94 100644 --- a/src/backend/mailbox_manager/api/client/viewsets.py +++ b/src/backend/mailbox_manager/api/client/viewsets.py @@ -2,7 +2,7 @@ from django.db.models import Q, Subquery -from rest_framework import exceptions, filters, mixins, viewsets +from rest_framework import exceptions, filters, mixins, status, viewsets from rest_framework.decorators import action from rest_framework.response import Response @@ -477,3 +477,19 @@ class AliasViewSet( 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.""" + 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.", + status=status.HTTP_200_OK, + ) + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py index 8f5df93..14bade2 100644 --- a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py +++ b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py @@ -2,6 +2,7 @@ Tests for aliases API endpoint in People's app mailbox_manager. Focus on "list" action. """ +# pylint: disable=W0613 import re @@ -67,7 +68,8 @@ def test_api_aliases_delete__viewer_forbidden(): assert models.Alias.objects.count() == 1 -def test_api_aliases_delete__viewer_can_delete_self_alias(): +@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. @@ -80,30 +82,8 @@ def test_api_aliases_delete__viewer_can_delete_self_alias(): domain=mail_domain, destination=authenticated_user.email ) - 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(): - """ - Administrators of a mail domain should be allowed to delete accesses excepted owner accesses. - """ - authenticated_user = core_factories.UserFactory() - mail_domain = factories.MailDomainFactory( - users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] - ) - alias = factories.AliasFactory(domain=mail_domain) - # Mock dimail response - responses.add( - responses.POST, + responses.delete( re.compile(r".*/aliases/"), status=status.HTTP_204_NO_CONTENT, content_type="application/json", @@ -117,3 +97,63 @@ def test_api_aliases_delete__administrators_allowed(): 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. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + alias = factories.AliasFactory(domain=mail_domain) + + # 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__404_out_of_sync(dimail_token_ok): + """ + Should return a clear message when dimail returns 404 not found. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + alias = factories.AliasFactory(domain=mail_domain) + + # Mock dimail response + responses.delete( + re.compile(r".*/aliases/"), + json={"detail": "Not found"}, + status=status.HTTP_404_NOT_FOUND, + 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_200_OK + assert ( + response.json() + == "Alias already deleted. Domain out of sync, please contact our support." + ) + assert not models.Alias.objects.exists() diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index f9ba34a..f392933 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -747,7 +747,7 @@ class DimailAPIClient: ) raise error - if response.status_code == status.HTTP_201_CREATED: + if response.status_code == status.HTTP_204_NO_CONTENT: logger.info( "User %s removed destination %s from alias %s.", request_user, @@ -765,4 +765,14 @@ class DimailAPIClient: "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." ) + if response.status_code == status.HTTP_404_NOT_FOUND: + logger.error( + "[DIMAIL] 404, alias %s not found. Domain out of sync with dimail. Admin, please import aliases for domain %s", + str(alias.local_part), + str(alias.domain), + ) + # we don't raise error because we actually want this alias to be deleted + # to match dimail's states + return response + return self.raise_exception_for_unexpected_response(response)