From 151f030582dcfb483e4921d1f679245e79b974cb Mon Sep 17 00:00:00 2001 From: Sabrina Demagny Date: Thu, 26 Dec 2024 19:15:36 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20fix=20dimail=20call=20d?= =?UTF-8?q?espite=20mailbox=20creation=20failure=20on=20our=20side?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we try to create a duplicate email, a request to dimail is sent despite a reject on our side. To solve this issue, we call mailbox creation first to benefit from validation of our Django model. Mailbox creation try was called too late after dimail call. So in attempt to create a duplicated email a request to dimail was sent despite a failure in our side. --- CHANGELOG.md | 1 + .../mailbox_manager/api/client/serializers.py | 9 ++- .../mailboxes/test_api_mailboxes_create.py | 62 ++++++++++++++++++- 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9664bd..693c9f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to ### Fixed +- 🐛(backend) fix dimail call despite mailbox creation failure on our side - 🧑‍💻(user) fix the User.language infinite migration #611 ## [1.9.1] - 2024-12-18 diff --git a/src/backend/mailbox_manager/api/client/serializers.py b/src/backend/mailbox_manager/api/client/serializers.py index 1637daa..9de7620 100644 --- a/src/backend/mailbox_manager/api/client/serializers.py +++ b/src/backend/mailbox_manager/api/client/serializers.py @@ -35,8 +35,7 @@ class MailboxSerializer(serializers.ModelSerializer): """ Override create function to fire a request on mailbox creation. """ - mailbox_status = enums.MailDomainStatusChoices.PENDING - + mailbox = super().create(validated_data) if validated_data["domain"].status == enums.MailDomainStatusChoices.ENABLED: client = DimailAPIClient() # send new mailbox request to dimail @@ -48,8 +47,8 @@ class MailboxSerializer(serializers.ModelSerializer): mailbox_data = json.loads( response.content.decode("utf-8").replace("'", '"') ) - - mailbox_status = enums.MailDomainStatusChoices.ENABLED + mailbox.status = enums.MailDomainStatusChoices.ENABLED + mailbox.save() # send confirmation email client.notify_mailbox_creation( @@ -57,7 +56,7 @@ class MailboxSerializer(serializers.ModelSerializer): ) # actually save mailbox on our database - return models.Mailbox.objects.create(**validated_data, status=mailbox_status) + return mailbox class MailDomainSerializer(serializers.ModelSerializer): 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 a9d1a8d..5653f45 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 @@ -269,6 +269,55 @@ def test_api_mailboxes__cannot_create_on_disabled_domain(role): ] +def test_api_mailboxes__no_dimail_call_if_mailbox_creation_failed(): + """Duplication case fails on our side at creation step thanks to unique_together + on Mailbox model and no dimail call should be made.""" + mail_domain = factories.MailDomainEnabledFactory() + mailbox = factories.MailboxFactory(domain=mail_domain) + access = factories.MailDomainAccessFactory( + role=enums.MailDomainRoleChoices.ADMIN, domain=mail_domain + ) + + client = APIClient() + client.force_login(access.user) + + # now we try to make the same mailbox + mailbox_data = serializers.MailboxSerializer(mailbox).data + with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: + rsps.add( + rsps.GET, + re.compile(r".*/token/"), + body='{"access_token": "domain_owner_token"}', + status=status.HTTP_200_OK, + content_type="application/json", + ) + rsps.add( + rsps.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body=str( + { + "email": f"{mailbox_data['local_part']}@{access.domain.name}", + "password": "newpass", + "uuid": "uuid", + } + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) + # try to create duplicate mailbox but django validation does not agree + assert response.status_code == status.HTTP_400_BAD_REQUEST + # no new mailbox should be created + assert models.Mailbox.objects.count() == 1 + # no dimail call should be made + assert len(rsps.calls) == 0 + + @pytest.mark.parametrize( "domain_status", [ @@ -524,7 +573,9 @@ def test_api_mailboxes__dimail_token_permission_denied(): assert response.json() == { "detail": "Token denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." } - assert not models.Mailbox.objects.exists() + # mailbox was created in our side only and in pending status + mailbox = models.Mailbox.objects.get() + assert mailbox.status == enums.MailboxStatusChoices.PENDING def test_api_mailboxes__user_unrelated_to_domain(): @@ -569,7 +620,9 @@ def test_api_mailboxes__user_unrelated_to_domain(): assert response.json() == { "detail": "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." } - assert not models.Mailbox.objects.exists() + # mailbox was created in our side only and in pending status + mailbox = models.Mailbox.objects.get() + assert mailbox.status == enums.MailboxStatusChoices.PENDING def test_api_mailboxes__handling_dimail_unexpected_error(caplog): @@ -612,7 +665,10 @@ def test_api_mailboxes__handling_dimail_unexpected_error(caplog): assert response.json() == { "detail": "Unexpected response from dimail: {'details': 'Internal server error'}" } - assert not models.Mailbox.objects.exists() + + # mailbox was created in our side only and in pending status + mailbox = models.Mailbox.objects.get() + assert mailbox.status == enums.MailboxStatusChoices.PENDING # Check error logger was called assert caplog.records[0].levelname == "ERROR"