🐛(dimail) improve handling of dimail errors on failed mailbox creation

dimail is called twice when creating a mailbox (once for the token,
and once for the post on mailbox endpoint). we want to clarify
the status_codes and messages of each error to inform user and ease debug

CHANGELOG.md

@@ -8,6 +8,10 @@ and this project adheres to
 
 ## [Unreleased]
 
+### Fixed
+
+- 🐛(dimail) improve handling of dimail errors on failed mailbox creation #377 
+
 ## [1.0.2] - 2024-08-30
 
 ### Added

src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py

@@ -359,3 +359,128 @@ def test_api_mailboxes__domain_owner_or_admin_successful_creation_and_provisioni
     assert mailbox.last_name == mailbox_data["last_name"]
     assert mailbox.local_part == mailbox_data["local_part"]
     assert mailbox.secondary_email == mailbox_data["secondary_email"]
+
+
+def test_api_mailboxes__wrong_secret_no_token_error():
+    """
+    API should raise a clear "permission denied" error
+    when receiving a 403_forbidden from dimail.
+    """
+    # creating all needed objects
+    access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.OWNER)
+    access.domain.secret = "nottherealsecret"
+
+    client = APIClient()
+    client.force_login(access.user)
+    mailbox_data = serializers.MailboxSerializer(
+        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='{"details": "Permission denied"}',
+            status=status.HTTP_403_FORBIDDEN,
+            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_403_FORBIDDEN
+        assert response.json() == {
+            "detail": f"Token denied - Wrong secret on mail domain {access.domain.name}"
+        }
+        assert not models.Mailbox.objects.exists()
+
+
+def test_api_mailboxes__secret_unrelated_to_domain():
+    """
+    API should raise a clear "permission denied"
+    when secret allows for a token but is not linked to queried domain on dimail-api.
+    """
+    # creating all needed objects
+    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
+
+    with responses.RequestsMock() as rsps:
+        # Ensure successful response using "responses":
+        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='{"details": "Permission denied"}',
+            status=status.HTTP_403_FORBIDDEN,
+            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_403_FORBIDDEN
+        assert response.json() == {
+            "detail": f"Secret not valid for this domain {access.domain.name}"
+        }
+        assert not models.Mailbox.objects.exists()
+
+
+def test_api_mailboxes__handling_dimail_unexpected_error():
+    """
+    API should raise a clear error when dimail gives an unexpected response.
+    """
+    # creating all needed objects
+    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
+
+    with responses.RequestsMock() as rsps:
+        # Ensure successful response using "responses":
+        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='{"details": "Internal server error"}',
+            status=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            content_type="application/json",
+        )
+
+        with pytest.raises(SystemError):
+            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'}"
+            }
+        assert not models.Mailbox.objects.exists()

src/backend/mailbox_manager/tests/test_models_mailboxes.py

@@ -168,17 +168,10 @@ def test_models_mailboxes__wrong_secret():
             status=status.HTTP_403_FORBIDDEN,
             content_type="application/json",
         )
-        rsps.add(
-            rsps.POST,
-            re.compile(rf".*/domains/{domain.name}/mailboxes/"),
-            body='{"detail": "Permission denied"}',
-            status=status.HTTP_403_FORBIDDEN,
-            content_type="application/json",
-        )
 
         with pytest.raises(
             exceptions.PermissionDenied,
-            match=f"Please check secret of the mail domain {domain.name}",
+            match=f"Token denied - Wrong secret on mail domain {domain.name}",
         ):
             mailbox = factories.MailboxFactory(use_mock=False, domain=domain)
             # Payload sent to mailbox provider

src/backend/mailbox_manager/utils/dimail.py

@@ -38,13 +38,15 @@ class DimailAPIClient:
         response = requests.get(
             f"{self.API_URL}/token/",
             headers={"Authorization": f"Basic {domain.secret}"},
-            timeout=status.HTTP_200_OK,
+            timeout=20,
         )
 
-        if response.json() == "{'detail': 'Permission denied'}":
-            raise exceptions.PermissionDenied(
-                "This secret does not allow for a new token."
+        if response.status_code == status.HTTP_403_FORBIDDEN:
+            logger.error(
+                "[DIMAIL] 403 Forbidden: please check the mail domain secret of %s",
+                domain.name,
             )
+            raise exceptions.PermissionDenied
 
         if "access_token" in response.json():
             headers["Authorization"] = f"Bearer {response.json()['access_token']}"
@@ -76,6 +78,10 @@ class DimailAPIClient:
                 exc_info=error,
             )
             raise error
+        except exceptions.PermissionDenied as error:
+            raise exceptions.PermissionDenied(
+                f"Token denied - Wrong secret on mail domain {mailbox.domain.name}"
+            ) from error
 
         if response.status_code == status.HTTP_201_CREATED:
             extra = {"response": response.content.decode("utf-8")}
@@ -87,18 +93,18 @@ class DimailAPIClient:
                 mailbox.domain.name,
                 extra=extra,
             )
-        elif response.status_code == status.HTTP_403_FORBIDDEN:
-            logger.error(
-                "[DIMAIL] 403 Forbidden: please check the mail domain secret of %s",
-                mailbox.domain.name,
-            )
+            return response
+
+        if response.status_code == status.HTTP_403_FORBIDDEN:
             raise exceptions.PermissionDenied(
-                f"Please check secret of the mail domain {mailbox.domain.name}"
-            )
-        else:
-            logger.error(
-                "Unexpected response: %s",
-                response.content.decode("utf-8"),
+                f"Secret not valid for this domain {mailbox.domain.name}"
             )
 
-        return response
+        # All other errors are considered 'unexpected'
+        logger.error(
+            "Unexpected response: %s",
+            response.content.decode("utf-8"),
+        )
+        raise SystemError(
+            f"Unexpected response from dimail: {response.content.decode('utf-8')}"
+        )