✨(aliases) warn that domain is out of sync when unexpected 404
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
This commit is contained in:
committed by
Marie
parent
0f7e312eb6
commit
15337a2226
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user