diff --git a/CHANGELOG.md b/CHANGELOG.md index c3cea92..9666e40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ and this project adheres to - ✨(api) update mailboxes #934 - ✨(api) give update rights to domain viewer on own mailbox #934 +### Fixed + +- 🐛(dimail) grab duplicate displayname error #961 + ### Changed - 💥(sentry) remove `DJANGO_` before Sentry DSN env variable #957 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 4fc4ef7..fb8c457 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 @@ -768,9 +768,12 @@ def test_api_mailboxes__user_unrelated_to_domain(): assert mailbox.status == enums.MailboxStatusChoices.PENDING -def test_api_mailboxes__handling_dimail_unexpected_error(caplog): +@responses.activate +def test_api_mailboxes__duplicate_display_name(): """ - API should raise a clear error when dimail returns an unexpected response. + In OpenXchange, the display name (first + last name) must be unique. + In absence of clear duplicate message from dimail (yet), we catch errors 500 and + check if it's not due to the display name already existing in the context """ # creating all needed objects access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.OWNER) @@ -781,44 +784,184 @@ def test_api_mailboxes__handling_dimail_unexpected_error(caplog): factories.MailboxFactory.build(domain=access.domain) ).data - with responses.RequestsMock() as rsps: - # Ensure successful response using "responses": - rsps.add( - rsps.GET, - re.compile(r".*/token/"), - body=TOKEN_OK, - status=status.HTTP_200_OK, - content_type="application/json", - ) - rsps.add( - rsps.POST, - re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), - body='{"detail": "Internal server error"}', - status=status.HTTP_500_INTERNAL_SERVER_ERROR, - content_type="application/json", - ) - - with pytest.raises(HTTPError): - response = client.post( - f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", - mailbox_data, - format="json", - ) - assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR - assert response.json() == { - "detail": "Unexpected response from dimail: {'details': 'Internal server error'}" + # Ensure successful response using "responses": + responses.add( + responses.GET, + re.compile(r".*/token/"), + body=TOKEN_OK, + status=status.HTTP_200_OK, + content_type="application/json", + ) + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body='{"detail": "Internal server error"}', + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + content_type="application/json", + ) + responses.add( + responses.GET, + re.compile( + rf".*/domains/{access.domain.name}/address/{mailbox_data['local_part']}" + ), + body=json.dumps( + { + "domain_name": str(access.domain), + "user_name": mailbox_data["local_part"], + "is_alias": "false", + "is_mailbox": "false", + "is_ox": "true", + "ox_primary_email": f"{mailbox_data['local_part']}@some_other_domain.com", + "ox_senders": [ + "some-alias@un-domaine.fr", + f"{mailbox_data['local_part']}@some_third_domain.com", + ], } + ), + status=status.HTTP_200_OK, + content_type="application/json", + ) - # mailbox was created in our side only and in pending status - mailbox = models.Mailbox.objects.get() - assert mailbox.status == enums.MailboxStatusChoices.PENDING + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "NON_FIELD_ERRORS": [ + f"First name + last name combination already in use in this context : \ +{mailbox_data['local_part']}@some_other_domain.com." + ] + } - # Check error logger was called - assert caplog.records[0].levelname == "ERROR" - assert ( - caplog.records[0].message - == "[DIMAIL] unexpected error: 500 {'detail': 'Internal server error'}" + +@responses.activate +def test_api_mailboxes__handling_dimail_unexpected_error(caplog): + """ + API should raise a clear error when dimail returns an unexpected response. + """ + access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.OWNER) + + client = APIClient() + client.force_login(access.user) + mailbox_data = serializers.MailboxSerializer( + factories.MailboxFactory.build(domain=access.domain) + ).data + + # Ensure successful response using "responses": + responses.add( + responses.GET, + re.compile(r".*/token/"), + body=TOKEN_OK, + status=status.HTTP_200_OK, + content_type="application/json", + ) + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body='{"detail": "Internal server error"}', + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + content_type="application/json", + ) + responses.add( + responses.GET, + re.compile( + rf".*/domains/{access.domain.name}/address/{mailbox_data['local_part']}/" + ), + body='{"detail": "Domain not found"}', + status=status.HTTP_404_NOT_FOUND, + content_type="application/json", + ) + + with pytest.raises(HTTPError): + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_data, + format="json", ) + assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert response.json() == { + "detail": "Unexpected response from dimail: {'details': 'Internal server error'}" + } + + # 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" + assert ( + caplog.records[0].message + == "[DIMAIL] unexpected error: 500 {'detail': 'Internal server error'}" + ) + + +@responses.activate +def test_api_mailboxes__display_name_duplicate_error(caplog): + """ + API should raise a clear error when display_name is already used on context. + """ + access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.OWNER) + + client = APIClient() + client.force_login(access.user) + mailbox_data = serializers.MailboxSerializer( + factories.MailboxFactory.build(domain=access.domain) + ).data + + # Ensure successful response using "responses": + responses.add( + responses.GET, + re.compile(r".*/token/"), + body=TOKEN_OK, + status=status.HTTP_200_OK, + content_type="application/json", + ) + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body='{"detail": "Internal server error"}', + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + content_type="application/json", + ) + responses.add( + responses.GET, + re.compile( + rf".*/domains/{access.domain.name}/address/{mailbox_data['local_part']}/" + ), + body=json.dumps( + { + "domain_name": access.domain.name, + "user_name": mailbox_data["local_part"], + "is_alias": False, + "is_mailbox": False, + "is_ox": True, + "ox_primary_email": f"{mailbox_data['local_part']}@primary.domain.com", + "ox_senders": [ + f"{mailbox_data['local_part']}@secondary.domain.com", + ], + } + ), + status=status.HTTP_200_OK, + content_type="application/json", + ) + + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "NON_FIELD_ERRORS": [ + f"First name + last name combination already in use in this context : {mailbox_data['local_part']}@primary.domain.com." + ] + } + + # mailbox was created in our side only and in pending status + mailbox = models.Mailbox.objects.get() + assert mailbox.status == enums.MailboxStatusChoices.PENDING @mock.patch.object(Logger, "error") diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index 1dfdfb9..48105b5 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -165,6 +165,35 @@ class DimailAPIClient: "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." ) + # Dimail doesn't return a clear error for duplicate yet + # but a 500 internal error + if response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR: + try: + address = session.get( + f"{self.API_URL}/domains/{mailbox.domain.name}/address/{mailbox.local_part}/", + json=payload, + 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 address.status_code == status.HTTP_200_OK: + primary = address.json()["ox_primary_email"] + raise exceptions.ValidationError( + { + "NON_FIELD_ERRORS": [ + f"First name + last name combination already in use in this context : {primary}." + ] + } + ) + return self.raise_exception_for_unexpected_response(response) def create_user(self, user_id):