Skip to content

Commit

Permalink
✨(mailboxes) manage bad secret sent to dimail API
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
sdemagny authored and mjeammet committed Aug 9, 2024
1 parent 5ed63fc commit a7a923e
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 54 deletions.
36 changes: 36 additions & 0 deletions src/backend/mailbox_manager/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -76,3 +80,35 @@ class Meta:
)
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
2 changes: 2 additions & 0 deletions src/backend/mailbox_manager/templates/403.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<h1>403 Forbidden</h1>
{{ exception }}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()

Expand Down
74 changes: 32 additions & 42 deletions src/backend/mailbox_manager/tests/test_models_mailboxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,33 @@
"""

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)


Expand All @@ -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="[email protected]")

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")


Expand All @@ -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
Expand All @@ -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."""
Expand All @@ -107,21 +91,22 @@ 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)


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(
Expand All @@ -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(
Expand All @@ -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!
Expand All @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion src/backend/mailbox_manager/utils/dimail.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,12 @@ def send_mailbox_request(self, mailbox):
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

0 comments on commit a7a923e

Please sign in to comment.