From a7a923e79031310917f6511df3b5a87192e72de0 Mon Sep 17 00:00:00 2001 From: Sabrina Demagny Date: Thu, 8 Aug 2024 18:28:01 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(mailboxes)=20manage=20bad=20secret=20?= =?UTF-8?q?sent=20to=20dimail=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - manage 403 returned by dimail API when mail domain secret is not valid - improve some tests - improve MailboxFactory to mock success for dimail API POST call - override 403.html to return a nice failing error in django admin - an error message is displayed on mailbox creation form of frontend --- src/backend/mailbox_manager/factories.py | 36 +++++++++ .../mailbox_manager/templates/403.html | 2 + .../mailboxes/test_api_mailboxes_create.py | 65 +++++++++++++--- .../tests/test_models_mailboxes.py | 74 ++++++++----------- src/backend/mailbox_manager/utils/dimail.py | 9 ++- 5 files changed, 132 insertions(+), 54 deletions(-) create mode 100644 src/backend/mailbox_manager/templates/403.html diff --git a/src/backend/mailbox_manager/factories.py b/src/backend/mailbox_manager/factories.py index e01f9dc..0a4458c 100644 --- a/src/backend/mailbox_manager/factories.py +++ b/src/backend/mailbox_manager/factories.py @@ -2,10 +2,14 @@ Mailbox manager application factories """ +import re + from django.utils.text import slugify import factory.fuzzy +import responses from faker import Faker +from rest_framework import status from core import factories as core_factories from core import models as core_models @@ -76,3 +80,35 @@ class MailboxFactory(factory.django.DjangoModelFactory): ) domain = factory.SubFactory(MailDomainEnabledFactory) secondary_email = factory.Faker("email") + + @classmethod + def _create(cls, model_class, *args, use_mock=True, **kwargs): + domain = kwargs["domain"] + if use_mock and isinstance(domain, models.MailDomain): + 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/{domain.name}/mailboxes/"), + body=str( + { + "email": f"{kwargs['local_part']}@{domain.name}", + "password": "newpass", + "uuid": "uuid", + } + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + + result = super()._create(model_class, *args, **kwargs) + else: + result = super()._create(model_class, *args, **kwargs) + return result diff --git a/src/backend/mailbox_manager/templates/403.html b/src/backend/mailbox_manager/templates/403.html new file mode 100644 index 0000000..ef1fbab --- /dev/null +++ b/src/backend/mailbox_manager/templates/403.html @@ -0,0 +1,2 @@ +

403 Forbidden

+{{ exception }} 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 4adf91b..45d4513 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 @@ -95,11 +95,33 @@ def test_api_mailboxes__create_roles_success(role): mailbox_values = serializers.MailboxSerializer( factories.MailboxFactory.build() ).data - response = client.post( - f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, - format="json", - ) + 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/{mail_domain.name}/mailboxes/"), + body=str( + { + "email": f"{mailbox_values['local_part']}@{mail_domain.name}", + "password": "newpass", + "uuid": "uuid", + } + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + response = client.post( + f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", + mailbox_values, + format="json", + ) assert response.status_code == status.HTTP_201_CREATED mailbox = models.Mailbox.objects.get() @@ -134,12 +156,33 @@ def test_api_mailboxes__create_with_accent_success(role): mailbox_values = serializers.MailboxSerializer( factories.MailboxFactory.build(first_name="Aimé") ).data - - response = client.post( - f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, - format="json", - ) + 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/{mail_domain.name}/mailboxes/"), + body=str( + { + "email": f"{mailbox_values['local_part']}@{mail_domain.name}", + "password": "newpass", + "uuid": "uuid", + } + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + response = client.post( + f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", + mailbox_values, + format="json", + ) assert response.status_code == status.HTTP_201_CREATED mailbox = models.Mailbox.objects.get() diff --git a/src/backend/mailbox_manager/tests/test_models_mailboxes.py b/src/backend/mailbox_manager/tests/test_models_mailboxes.py index 9ed3b49..8cab9e4 100644 --- a/src/backend/mailbox_manager/tests/test_models_mailboxes.py +++ b/src/backend/mailbox_manager/tests/test_models_mailboxes.py @@ -3,47 +3,33 @@ Unit tests for the mailbox model """ import json -import logging import re from logging import Logger from unittest import mock -from django.core.exceptions import ValidationError +from django.core import exceptions import pytest -import requests import responses from rest_framework import status -from urllib3.util import Retry from mailbox_manager import enums, factories, models from mailbox_manager.api import serializers pytestmark = pytest.mark.django_db -logger = logging.getLogger(__name__) - -adapter = requests.adapters.HTTPAdapter( - max_retries=Retry( - total=4, - backoff_factor=0.1, - status_forcelist=[500, 502], - allowed_methods=["PATCH"], - ) -) - # LOCAL PART FIELD def test_models_mailboxes__local_part_cannot_be_empty(): """The "local_part" field should not be empty.""" - with pytest.raises(ValidationError, match="This field cannot be blank"): + with pytest.raises(exceptions.ValidationError, match="This field cannot be blank"): factories.MailboxFactory(local_part="") def test_models_mailboxes__local_part_cannot_be_null(): """The "local_part" field should not be null.""" - with pytest.raises(ValidationError, match="This field cannot be null"): + with pytest.raises(exceptions.ValidationError, match="This field cannot be null"): factories.MailboxFactory(local_part=None) @@ -55,11 +41,11 @@ def test_models_mailboxes__local_part_matches_expected_format(): factories.MailboxFactory(local_part="Marie-Jose.Perec+JO_2024") # other special characters (such as "@" or "!") should raise a validation error - with pytest.raises(ValidationError, match="Enter a valid value"): + with pytest.raises(exceptions.ValidationError, match="Enter a valid value"): factories.MailboxFactory(local_part="mariejo@unnecessarydomain.com") for character in ["!", "$", "%"]: - with pytest.raises(ValidationError, match="Enter a valid value"): + with pytest.raises(exceptions.ValidationError, match="Enter a valid value"): factories.MailboxFactory(local_part=f"marie{character}jo") @@ -73,7 +59,8 @@ def test_models_mailboxes__local_part_unique_per_domain(): # same local part on the same domain should not be possible with pytest.raises( - ValidationError, match="Mailbox with this Local_part and Domain already exists." + exceptions.ValidationError, + match="Mailbox with this Local_part and Domain already exists.", ): factories.MailboxFactory( local_part=existing_mailbox.local_part, domain=existing_mailbox.domain @@ -82,9 +69,6 @@ def test_models_mailboxes__local_part_unique_per_domain(): # DOMAIN FIELD -session = requests.Session() -session.mount("http://", adapter) - def test_models_mailboxes__domain_must_be_a_maildomain_instance(): """The "domain" field should be an instance of MailDomain.""" @@ -107,13 +91,13 @@ def test_models_mailboxes__domain_cannot_be_null(): def test_models_mailboxes__secondary_email_cannot_be_empty(): """The "secondary_email" field should not be empty.""" - with pytest.raises(ValidationError, match="This field cannot be blank"): + with pytest.raises(exceptions.ValidationError, match="This field cannot be blank"): factories.MailboxFactory(secondary_email="") def test_models_mailboxes__secondary_email_cannot_be_null(): """The "secondary_email" field should not be null.""" - with pytest.raises(ValidationError, match="This field cannot be null"): + with pytest.raises(exceptions.ValidationError, match="This field cannot be null"): factories.MailboxFactory(secondary_email=None) @@ -121,7 +105,8 @@ def test_models_mailboxes__cannot_be_created_for_disabled_maildomain(): """Mailbox creation is allowed only for a domain enabled. A disabled status for the mail domain raises an error.""" with pytest.raises( - ValidationError, match="You can create mailbox only for a domain enabled" + exceptions.ValidationError, + match="You can create mailbox only for a domain enabled", ): factories.MailboxFactory( domain=factories.MailDomainFactory( @@ -134,7 +119,8 @@ def test_models_mailboxes__cannot_be_created_for_failed_maildomain(): """Mailbox creation is allowed only for a domain enabled. A failed status for the mail domain raises an error.""" with pytest.raises( - ValidationError, match="You can create mailbox only for a domain enabled" + exceptions.ValidationError, + match="You can create mailbox only for a domain enabled", ): factories.MailboxFactory( domain=factories.MailDomainFactory( @@ -147,7 +133,8 @@ def test_models_mailboxes__cannot_be_created_for_pending_maildomain(): """Mailbox creation is allowed only for a domain enabled. A pending status for the mail domain raises an error.""" with pytest.raises( - ValidationError, match="You can create mailbox only for a domain enabled" + exceptions.ValidationError, + match="You can create mailbox only for a domain enabled", ): # MailDomainFactory initializes a mail domain with default values, # so mail domain status is pending! @@ -162,7 +149,7 @@ def test_models_mailboxes__no_secret(): domain = factories.MailDomainEnabledFactory(secret=None) with pytest.raises( - ValidationError, + exceptions.ValidationError, match="Please configure your domain's secret before creating any mailbox.", ): factories.MailboxFactory(domain=domain) @@ -179,27 +166,30 @@ def test_models_mailboxes__wrong_secret(): rsps.GET, re.compile(r".*/token/"), body='{"detail": "Permission denied"}', - status=status.HTTP_401_UNAUTHORIZED, + 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_401_UNAUTHORIZED, + status=status.HTTP_403_FORBIDDEN, content_type="application/json", ) - mailbox = factories.MailboxFactory(domain=domain) - - # Payload sent to mailbox provider - payload = json.loads(rsps.calls[1].request.body) - assert payload == { - "displayName": f"{mailbox.first_name} {mailbox.last_name}", - "email": f"{mailbox.local_part}@{domain.name}", - "givenName": mailbox.first_name, - "surName": mailbox.last_name, - } + with pytest.raises( + exceptions.PermissionDenied, + match=f"Please check secret of the mail domain {domain.name}", + ): + mailbox = factories.MailboxFactory(use_mock=False, domain=domain) + # Payload sent to mailbox provider + payload = json.loads(rsps.calls[1].request.body) + assert payload == { + "displayName": f"{mailbox.first_name} {mailbox.last_name}", + "email": f"{mailbox.local_part}@{domain.name}", + "givenName": mailbox.first_name, + "surName": mailbox.last_name, + } @mock.patch.object(Logger, "error") @@ -237,7 +227,7 @@ def test_models_mailboxes__create_mailbox_success(mock_info, mock_error): ) mailbox = factories.MailboxFactory( - local_part=mailbox_data["local_part"], domain=domain + use_mock=False, local_part=mailbox_data["local_part"], domain=domain ) # Check headers diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index 44bde44..1a21aad 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -84,5 +84,12 @@ 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, + ) + raise exceptions.PermissionDenied( + f"Please check secret of the mail domain {mailbox.domain.name}" + ) return response