diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d23030..27373fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,13 @@ and this project adheres to ### Added -- ✨(domains) add endpoint to list and retrieve domain accesses @sdemagny #404 -- 🍱(dev) embark dimail-api as container by @mjeammet #366 +- ✨(domains) add endpoint to list and retrieve domain accesses #404 +- 🍱(dev) embark dimail-api as container #366 + + +### Changed + +- ♻️(serializers) move business logic to serializers #414 ## [1.1.0] - 2024-09-10 diff --git a/src/backend/mailbox_manager/api/serializers.py b/src/backend/mailbox_manager/api/serializers.py index 5b8686e..60372df 100644 --- a/src/backend/mailbox_manager/api/serializers.py +++ b/src/backend/mailbox_manager/api/serializers.py @@ -5,6 +5,7 @@ from rest_framework import serializers from core.api.serializers import UserSerializer from mailbox_manager import enums, models +from mailbox_manager.utils.dimail import DimailAPIClient class MailboxSerializer(serializers.ModelSerializer): @@ -16,6 +17,14 @@ class MailboxSerializer(serializers.ModelSerializer): # everything is actually read-only as we do not allow update for now read_only_fields = ["id"] + def create(self, validated_data): + """ + Override create function to fire a request on mailbox creation. + """ + client = DimailAPIClient() + client.send_mailbox_request(validated_data) + return models.Mailbox.objects.create(**validated_data) + class MailDomainSerializer(serializers.ModelSerializer): """Serialize mail domain.""" diff --git a/src/backend/mailbox_manager/factories.py b/src/backend/mailbox_manager/factories.py index a866d14..48ababb 100644 --- a/src/backend/mailbox_manager/factories.py +++ b/src/backend/mailbox_manager/factories.py @@ -2,14 +2,10 @@ 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 @@ -79,37 +75,3 @@ 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/{kwargs['local_part']}" - ), - 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/models.py b/src/backend/mailbox_manager/models.py index 882244c..20f6959 100644 --- a/src/backend/mailbox_manager/models.py +++ b/src/backend/mailbox_manager/models.py @@ -4,14 +4,13 @@ Declare and configure the models for the People additional application : mailbox from django.conf import settings from django.core import exceptions, validators -from django.db import models, transaction +from django.db import models from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ from core.models import BaseModel from mailbox_manager.enums import MailDomainRoleChoices, MailDomainStatusChoices -from mailbox_manager.utils.dimail import DimailAPIClient class MailDomain(BaseModel): @@ -155,16 +154,12 @@ class Mailbox(BaseModel): def save(self, *args, **kwargs): """ - Override save function to fire a request on mailbox creation. Modification is forbidden for now. """ self.full_clean() if self._state.adding: - with transaction.atomic(): - client = DimailAPIClient() - client.send_mailbox_request(self) - return super().save(*args, **kwargs) + return super().save(*args, **kwargs) # Update is not implemented for now raise NotImplementedError() 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 c31da87..d4d932d 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 @@ -4,6 +4,8 @@ Unit tests for the mailbox API import json import re +from logging import Logger +from unittest import mock from django.test.utils import override_settings @@ -488,3 +490,59 @@ def test_api_mailboxes__handling_dimail_unexpected_error(): "detail": "Unexpected response from dimail: {'details': 'Internal server error'}" } assert not models.Mailbox.objects.exists() + + +@mock.patch.object(Logger, "error") +@mock.patch.object(Logger, "info") +def test_api_mailboxes__send_correct_logger_infos(mock_info, mock_error): + """ + Upon requesting mailbox creation, things are correctly logged + """ + 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=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", + ) + assert response.status_code == status.HTTP_201_CREATED + + # Logger + assert not mock_error.called + assert mock_info.call_count == 3 + assert mock_info.call_args_list[0][0] == ( + "Token succesfully granted by mail-provisioning API.", + ) + assert mock_info.call_args_list[1][0] == ( + "Mailbox successfully created on domain %s", + access.domain.name, + ) diff --git a/src/backend/mailbox_manager/tests/test_models_mailboxes.py b/src/backend/mailbox_manager/tests/test_models_mailboxes.py index c1527d1..91ece88 100644 --- a/src/backend/mailbox_manager/tests/test_models_mailboxes.py +++ b/src/backend/mailbox_manager/tests/test_models_mailboxes.py @@ -2,20 +2,12 @@ Unit tests for the mailbox model """ -import json -import re -from logging import Logger -from unittest import mock - from django.core import exceptions from django.test.utils import override_settings import pytest -import responses -from rest_framework import status from mailbox_manager import enums, factories, models -from mailbox_manager.api import serializers pytestmark = pytest.mark.django_db @@ -158,111 +150,3 @@ def test_models_mailboxes__dimail_no_credentials(): match="Please configure MAIL_PROVISIONING_API_CREDENTIALS before creating any mailbox.", ): factories.MailboxFactory(domain=domain) - - -@override_settings(MAIL_PROVISIONING_API_CREDENTIALS="wrongCredentials") -def test_models_mailboxes__dimail_token_permissions_denied(): - """ - Our API should raise a clear "Permission denied" error - if dimail returns a permission denied on /token/ endpoint. - """ - - domain = factories.MailDomainEnabledFactory() - - with responses.RequestsMock() as rsps: - # Ensure successful response by scim provider using "responses": - rsps.add( - rsps.GET, - re.compile(r".*/token/"), - body='{"detail": "Permission denied"}', - status=status.HTTP_403_FORBIDDEN, - content_type="application/json", - ) - - with pytest.raises( - exceptions.PermissionDenied, - match="Token denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS.", - ): - 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}", - "givenName": mailbox.first_name, - "surName": mailbox.last_name, - } - - -@mock.patch.object(Logger, "error") -@mock.patch.object(Logger, "info") -def test_models_mailboxes__create_mailbox_success(mock_info, mock_error): - """Creating a mailbox sends the expected information and get expected response before saving.""" - domain = factories.MailDomainEnabledFactory() - - # generate mailbox data before mailbox, to mock responses - mailbox_data = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=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/{domain.name}/mailboxes/"), - body=str( - { - "email": f"{mailbox_data['local_part']}@{domain.name}", - "password": "newpass", - "uuid": "uuid", - } - ), - status=status.HTTP_201_CREATED, - content_type="application/json", - ) - - mailbox = factories.MailboxFactory( - use_mock=False, local_part=mailbox_data["local_part"], domain=domain - ) - - # Check headers - headers = rsps.calls[1].request.headers - assert headers["Content-Type"] == "application/json" - - # Payload sent to mailbox provider - payload = json.loads(rsps.calls[1].request.body) - assert payload == { - "displayName": f"{mailbox.first_name} {mailbox.last_name}", - "givenName": mailbox.first_name, - "surName": mailbox.last_name, - } - - # Logger - assert not mock_error.called - assert mock_info.call_count == 2 - assert mock_info.call_args_list[0][0] == ( - "Token succesfully granted by mail-provisioning API.", - ) - assert mock_info.call_args_list[1][0] == ( - "Mailbox successfully created on domain %s", - domain.name, - ) - assert mock_info.call_args_list[1][1] == ( - { - "extra": { - "response": str( - { - "email": f"{mailbox.local_part}@{domain.name}", - "password": "newpass", - "uuid": "uuid", - } - ) - } - } - ) diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index a1e88b7..60d260c 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -64,15 +64,15 @@ class DimailAPIClient: """Send a CREATE mailbox request to mail provisioning API.""" payload = { - "givenName": mailbox.first_name, - "surName": mailbox.last_name, - "displayName": f"{mailbox.first_name} {mailbox.last_name}", + "givenName": mailbox["first_name"], + "surName": mailbox["last_name"], + "displayName": f"{mailbox['first_name']} {mailbox['last_name']}", } headers = self.get_headers() try: response = session.post( - f"{self.API_URL}/domains/{mailbox.domain}/mailboxes/{mailbox.local_part}/", + f"{self.API_URL}/domains/{mailbox['domain']}/mailboxes/{mailbox['local_part']}/", json=payload, headers=headers, verify=True, @@ -93,7 +93,7 @@ class DimailAPIClient: # In the meantime, we log mailbox info (including password !) logger.info( "Mailbox successfully created on domain %s", - mailbox.domain.name, + str(mailbox["domain"]), extra=extra, ) return response