diff --git a/src/backend/mailbox_manager/api/client/serializers.py b/src/backend/mailbox_manager/api/client/serializers.py index 8c56756..ce393a4 100644 --- a/src/backend/mailbox_manager/api/client/serializers.py +++ b/src/backend/mailbox_manager/api/client/serializers.py @@ -78,6 +78,17 @@ class MailboxSerializer(serializers.ModelSerializer): return mailbox + def validate_local_part(self, value): + """Validate this local part does not match a mailbox.""" + if models.Alias.objects.filter( + local_part=value, domain__slug=self.context["domain_slug"] + ): + raise exceptions.ValidationError( + f'Local part "{value}" already used by an alias.' + ) + + return value + class MailboxUpdateSerializer(MailboxSerializer): """A more restrictive serializer when updating mailboxes""" @@ -343,7 +354,7 @@ class AliasSerializer(serializers.ModelSerializer): local_part=value, domain__slug=self.context["domain_slug"] ).exists(): raise exceptions.ValidationError( - f'Local part "{value}" already used for a mailbox.' + f'Local part "{value}" already used by a mailbox.' ) return value diff --git a/src/backend/mailbox_manager/api/client/viewsets.py b/src/backend/mailbox_manager/api/client/viewsets.py index 5761b75..d8ab1ee 100644 --- a/src/backend/mailbox_manager/api/client/viewsets.py +++ b/src/backend/mailbox_manager/api/client/viewsets.py @@ -275,6 +275,12 @@ class MailBoxViewSet( return self.queryset.filter(domain__slug=domain_slug) return self.queryset + def get_serializer_context(self): + """Extra context provided to the serializer class.""" + context = super().get_serializer_context() + context["domain_slug"] = self.kwargs["domain_slug"] + return context + def get_permissions(self): """Add a specific permission for domain viewers to update their own mailbox.""" if self.action in ["update", "partial_update"]: diff --git a/src/backend/mailbox_manager/migrations/0027_alias.py b/src/backend/mailbox_manager/migrations/0028_alias.py similarity index 94% rename from src/backend/mailbox_manager/migrations/0027_alias.py rename to src/backend/mailbox_manager/migrations/0028_alias.py index 5ac2dad..cf1ab8d 100644 --- a/src/backend/mailbox_manager/migrations/0027_alias.py +++ b/src/backend/mailbox_manager/migrations/0028_alias.py @@ -8,7 +8,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('mailbox_manager', '0026_alter_mailbox_unique_together_and_more'), + ('mailbox_manager', '0027_remove_mailbox_unique_ox_display_name_and_more'), ] operations = [ diff --git a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py index a174416..2a6795a 100644 --- a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py +++ b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py @@ -80,7 +80,7 @@ def test_api_aliases_create__duplicate_forbidden(): assert models.Alias.objects.filter(domain=access.domain).count() == 1 -def test_api_aliases_create__existing_alias_bad_request(): +def test_api_aliases_create__existing_mailbox_bad_request(): """Cannot create alias if local_part is already used by a mailbox.""" access = factories.MailDomainAccessFactory( role="owner", domain=factories.MailDomainEnabledFactory() @@ -95,7 +95,7 @@ def test_api_aliases_create__existing_alias_bad_request(): ) assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json() == { - "local_part": [f'Local part "{mailbox.local_part}" already used for a mailbox.'] + "local_part": [f'Local part "{mailbox.local_part}" already used by a mailbox.'] } assert not models.Alias.objects.exists() 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 c0af6e2..8f5df93 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 @@ -3,7 +3,10 @@ Tests for aliases API endpoint in People's app mailbox_manager. Focus on "list" action. """ +import re + import pytest +import responses from rest_framework import status from rest_framework.test import APIClient @@ -87,6 +90,7 @@ def test_api_aliases_delete__viewer_can_delete_self_alias(): 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. @@ -97,6 +101,14 @@ def test_api_aliases_delete__administrators_allowed(): ) alias = factories.AliasFactory(domain=mail_domain) + # Mock dimail response + responses.add( + responses.POST, + re.compile(r".*/aliases/"), + status=status.HTTP_204_NO_CONTENT, + content_type="application/json", + ) + client = APIClient() client.force_login(authenticated_user) response = client.delete( diff --git a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py index e9bbc80..7cb21e4 100644 --- a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py +++ b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py @@ -1,7 +1,7 @@ """ Unit tests for the mailbox API """ -# pylint: disable=W0613 +# pylint: disable=W0613, C0302 import json import re @@ -436,6 +436,35 @@ def test_api_mailboxes__create_pending_mailboxes(domain_status, mailbox_data): assert mailbox.status == "pending" +def test_api_mailboxes__existing_alias_bad_request(mailbox_data): + """ + Cannot create mailbox if local_part is already used by an alias. + """ + alias = factories.AliasFactory() + access = factories.MailDomainAccessFactory( + role=enums.MailDomainRoleChoices.ADMIN, domain=alias.domain + ) + + client = APIClient() + client.force_login(access.user) + # No response because we expect no outside calls to be made + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + { + "local_part": alias.local_part, + "first_name": mailbox_data["first_name"], + "last_name": mailbox_data["last_name"], + "secondary_email": mailbox_data["secondary_email"], + }, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "local_part": [f'Local part "{alias.local_part}" already used by an alias.'] + } + assert not models.Mailbox.objects.exists() + + ### REACTING TO DIMAIL-API ### We mock dimail's responses to avoid testing dimail's container too @responses.activate diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index cfd3a95..f9ba34a 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -725,3 +725,44 @@ class DimailAPIClient: ) return self.raise_exception_for_unexpected_response(response) + + def delete_alias(self, alias, request_user=None): + """Send a Delete alias request to mail provisioning API.""" + + headers = self.get_headers() + + try: + response = session.delete( + f"{self.API_URL}/domains/{alias.domain.name}/aliases/{alias.local_part}/{alias.destination}", + json={}, + headers=headers, + verify=True, + timeout=self.API_TIMEOUT, + ) + except requests.exceptions.ConnectionError as error: + logger.error( + "Connection error while trying to reach %s.", + self.API_URL, + exc_info=error, + ) + raise error + + if response.status_code == status.HTTP_201_CREATED: + logger.info( + "User %s removed destination %s from alias %s.", + request_user, + alias.destination, + f"{alias.local_part}@{alias.domain}", + ) + return response + + if response.status_code == status.HTTP_403_FORBIDDEN: + logger.error( + "[DIMAIL] 403 Forbidden: you cannot access domain %s", + str(alias.domain), + ) + raise exceptions.PermissionDenied( + "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." + ) + + return self.raise_exception_for_unexpected_response(response)