From 4262f469d6681cbf17af557c5fdedb334d5f23b7 Mon Sep 17 00:00:00 2001 From: Marie PUPO JEAMMET Date: Fri, 12 Sep 2025 12:09:23 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(tests)=20refacto=20dimail=20?= =?UTF-8?q?tests=20with=20fixtures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_api_mailboxes_create exceeded 1000 lines. By using fixtures, we can at least factorize dimail response when token is ok or mailbox_data sent to our API when creating a mailbox. --- .../mailbox_manager/api/client/serializers.py | 15 +- src/backend/mailbox_manager/models.py | 3 + .../mailboxes/test_api_mailboxes_create.py | 745 +++++++----------- src/backend/mailbox_manager/tests/conftest.py | 37 + .../mailbox_manager/tests/fixtures/dimail.py | 10 +- 5 files changed, 337 insertions(+), 473 deletions(-) create mode 100644 src/backend/mailbox_manager/tests/conftest.py diff --git a/src/backend/mailbox_manager/api/client/serializers.py b/src/backend/mailbox_manager/api/client/serializers.py index c5fc8ac..c6449b0 100644 --- a/src/backend/mailbox_manager/api/client/serializers.py +++ b/src/backend/mailbox_manager/api/client/serializers.py @@ -3,6 +3,7 @@ from logging import getLogger from django.contrib.auth.hashers import make_password +from django.core import exceptions as django_exceptions from requests.exceptions import HTTPError from rest_framework import exceptions, serializers @@ -44,10 +45,17 @@ class MailboxSerializer(serializers.ModelSerializer): "password": make_password(None), # generate an unusable password } ) - if mailbox.domain.status == enums.MailDomainStatusChoices.ENABLED: - client = DimailAPIClient() + + if validated_data["domain"].status == enums.MailDomainStatusChoices.ENABLED: # send new mailbox request to dimail - response = client.create_mailbox(mailbox, self.context["request"].user.sub) + client = DimailAPIClient() + try: + response = client.create_mailbox( + mailbox, self.context["request"].user.sub + ) + except django_exceptions.ValidationError as exc: + mailbox.delete() + raise exc mailbox.status = enums.MailDomainStatusChoices.ENABLED mailbox_data = response.json() @@ -68,7 +76,6 @@ class MailboxSerializer(serializers.ModelSerializer): mailbox, ) - # actually save mailbox on our database return mailbox diff --git a/src/backend/mailbox_manager/models.py b/src/backend/mailbox_manager/models.py index c83e0fd..3ba754e 100644 --- a/src/backend/mailbox_manager/models.py +++ b/src/backend/mailbox_manager/models.py @@ -311,6 +311,9 @@ class Mailbox(AbstractBaseUser, BaseModel): fields=["first_name", "last_name", "domain"], name="unique_ox_display_name", ), + # Display name in OpenXChange must be unique + # To avoid sending failing requests to dimail, + # we impose uniqueness here too ] ordering = ["-created_at"] 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 185e395..f2f419f 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 @@ -1,6 +1,7 @@ """ Unit tests for the mailbox API """ +# pylint: disable=W0613 import json import re @@ -20,42 +21,33 @@ from core import factories as core_factories from mailbox_manager import enums, factories, models from mailbox_manager.api.client import serializers from mailbox_manager.tests.fixtures.dimail import ( - TOKEN_OK, response_mailbox_created, ) pytestmark = pytest.mark.django_db -def test_api_mailboxes__create_anonymous_forbidden(): +def test_api_mailboxes__create_anonymous_forbidden(mailbox_data): """Anonymous users should not be able to create a new mailbox via the API.""" mail_domain = factories.MailDomainEnabledFactory() - mailbox_values = serializers.MailboxSerializer( - factories.MailboxFactory.build() - ).data response = APIClient().post( f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, + mailbox_data, ) assert response.status_code == status.HTTP_401_UNAUTHORIZED assert not models.Mailbox.objects.exists() -def test_api_mailboxes__create_authenticated_failure(): +def test_api_mailboxes__create_authenticated_failure(mailbox_data): """Authenticated users should not be able to create mailbox without specific role on mail domain.""" - user = core_factories.UserFactory() + mail_domain = factories.MailDomainEnabledFactory() client = APIClient() - client.force_login(user) - - mailbox_values = serializers.MailboxSerializer( - factories.MailboxFactory.build() - ).data - mail_domain = factories.MailDomainEnabledFactory() + client.force_login(core_factories.UserFactory()) response = client.post( f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, + mailbox_data, format="json", ) @@ -63,7 +55,7 @@ def test_api_mailboxes__create_authenticated_failure(): assert not models.Mailbox.objects.exists() -def test_api_mailboxes__create_viewer_failure(): +def test_api_mailboxes__create_viewer_failure(mailbox_data): """Users with viewer role should not be able to create mailbox on the mail domain.""" mail_domain = factories.MailDomainEnabledFactory() access = factories.MailDomainAccessFactory( @@ -72,13 +64,9 @@ def test_api_mailboxes__create_viewer_failure(): client = APIClient() client.force_login(access.user) - - mailbox_values = serializers.MailboxSerializer( - factories.MailboxFactory.build() - ).data response = client.post( f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, + mailbox_data, format="json", ) @@ -116,7 +104,9 @@ def test_api_mailboxes__create_display_name_must_be_unique(): @responses.activate -def test_api_mailboxes__create_display_name_no_constraint_on_different_domains(): +def test_api_mailboxes__create_display_name_no_constraint_on_different_domains( + dimail_token_ok, +): """Should still be allowed to user same display name on another domain""" existing_mailbox = factories.MailboxFactory() @@ -128,13 +118,7 @@ def test_api_mailboxes__create_display_name_no_constraint_on_different_domains() } # ensure response - responses.add( - responses.GET, - re.compile(r".*/token/"), - body=TOKEN_OK, - status=status.HTTP_200_OK, - content_type="application/json", - ) + # token response in fixtures responses.add( responses.POST, re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), @@ -159,50 +143,40 @@ def test_api_mailboxes__create_display_name_no_constraint_on_different_domains() assert models.Mailbox.objects.count() == 2 +@responses.activate @pytest.mark.parametrize( "role", [enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN], ) -def test_api_mailboxes__create_roles_success(role): +def test_api_mailboxes__create_roles_success(role, dimail_token_ok, mailbox_data): """Users with owner or admin role should be able to create mailbox on the mail domain.""" mail_domain = factories.MailDomainEnabledFactory() access = factories.MailDomainAccessFactory(role=role, domain=mail_domain) client = APIClient() client.force_login(access.user) - - mailbox_values = serializers.MailboxSerializer( - factories.MailboxFactory.build() - ).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/{mail_domain.name}/mailboxes/"), - body=response_mailbox_created( - f"{mailbox_values['local_part']}@{mail_domain.name}" - ), - 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", - ) + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile(rf".*/domains/{mail_domain.name}/mailboxes/"), + body=response_mailbox_created( + f"{mailbox_data['local_part']}@{mail_domain.name}" + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + response = client.post( + f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) assert response.status_code == status.HTTP_201_CREATED mailbox = models.Mailbox.objects.get() - assert mailbox.local_part == mailbox_values["local_part"] - assert mailbox.secondary_email == mailbox_values["secondary_email"] + assert mailbox.local_part == mailbox_data["local_part"] + assert mailbox.secondary_email == mailbox_data["secondary_email"] assert response.json() == { "id": str(mailbox.id), "first_name": str(mailbox.first_name), @@ -213,11 +187,12 @@ def test_api_mailboxes__create_roles_success(role): } +@responses.activate @pytest.mark.parametrize( "role", [enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN], ) -def test_api_mailboxes__create_with_accent_success(role): +def test_api_mailboxes__create_with_accent_success(role, dimail_token_ok): """Users with proper abilities should be able to create mailbox on the mail domain with a first_name accentuated.""" mail_domain = factories.MailDomainEnabledFactory() @@ -229,29 +204,23 @@ def test_api_mailboxes__create_with_accent_success(role): mailbox_values = serializers.MailboxSerializer( factories.MailboxFactory.build(first_name="Aimé") ).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/{mail_domain.name}/mailboxes/"), - body=response_mailbox_created( - f"{mailbox_values['local_part']}@{mail_domain.name}" - ), - 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", - ) + + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile(rf".*/domains/{mail_domain.name}/mailboxes/"), + body=response_mailbox_created( + f"{mailbox_values['local_part']}@{mail_domain.name}" + ), + 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() @@ -298,7 +267,7 @@ def test_api_mailboxes__create_administrator_missing_fields(): "role", [enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN], ) -def test_api_mailboxes__create_without_secondary_email(role, caplog): +def test_api_mailboxes__create_without_secondary_email(role, caplog, dimail_token_ok): """ Creating a new mailbox should not require a secondary email. We should be able to create a mailbox but not send any email notification. @@ -312,13 +281,7 @@ def test_api_mailboxes__create_without_secondary_email(role, caplog): ).data del mailbox_values["secondary_email"] - responses.add( - responses.GET, - re.compile(r".*/token/"), - body=TOKEN_OK, - status=status.HTTP_200_OK, - content_type="application/json", - ) + # token response in fixtures responses.add( responses.POST, re.compile(rf".*/domains/{mail_domain.name}/mailboxes/"), @@ -349,7 +312,7 @@ def test_api_mailboxes__create_without_secondary_email(role, caplog): enums.MailDomainRoleChoices.ADMIN, ], ) -def test_api_mailboxes__cannot_create_on_disabled_domain(role): +def test_api_mailboxes__cannot_create_on_disabled_domain(role, mailbox_data): """Admin and owner users should not be able to create mailboxes for a disabled domain""" mail_domain = factories.MailDomainFactory( status=enums.MailDomainStatusChoices.DISABLED @@ -358,13 +321,9 @@ def test_api_mailboxes__cannot_create_on_disabled_domain(role): client = APIClient() client.force_login(access.user) - - mailbox_values = serializers.MailboxSerializer( - factories.MailboxFactory.build() - ).data response = client.post( f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, + mailbox_data, format="json", ) assert response.status_code == status.HTTP_400_BAD_REQUEST @@ -374,6 +333,7 @@ def test_api_mailboxes__cannot_create_on_disabled_domain(role): ] +@responses.activate def test_api_mailboxes__no_dimail_call_if_mailbox_creation_failed(): """Duplication case fails on our side at creation step thanks to unique_together on Mailbox model and no dimail call should be made.""" @@ -388,27 +348,8 @@ def test_api_mailboxes__no_dimail_call_if_mailbox_creation_failed(): # now we try to make the same mailbox mailbox_data = serializers.MailboxSerializer(mailbox).data - with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: - 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=str( - { - "email": f"{mailbox_data['local_part']}@{access.domain.name}", - "password": "newpass", - "uuid": "uuid", - } - ), - status=status.HTTP_201_CREATED, - content_type="application/json", - ) + with responses.RequestsMock() as rsps: + # no call expected response = client.post( f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", @@ -423,7 +364,8 @@ def test_api_mailboxes__no_dimail_call_if_mailbox_creation_failed(): assert len(rsps.calls) == 0 -def test_api_mailboxes__same_local_part_on_different_domains(): +@responses.activate +def test_api_mailboxes__same_local_part_on_different_domains(dimail_token_ok): """A domain admin should be able to create a mailbox with the same local part of another mailbox, on different domain.""" # a mailbox exists on another domain @@ -441,28 +383,20 @@ def test_api_mailboxes__same_local_part_on_different_domains(): factories.MailboxFactory.build(local_part=existing_mailbox.local_part) ).data - with responses.RequestsMock() as rsps: - 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=response_mailbox_created( - f"{mailbox_values['local_part']}@{access.domain.name}" - ), - status=status.HTTP_201_CREATED, - content_type="application/json", - ) - response = client.post( - f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", - mailbox_values, - format="json", - ) + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body=response_mailbox_created( + f"{mailbox_values['local_part']}@{access.domain.name}" + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_values, + format="json", + ) assert response.status_code == status.HTTP_201_CREATED assert ( @@ -477,7 +411,7 @@ def test_api_mailboxes__same_local_part_on_different_domains(): enums.MailDomainStatusChoices.FAILED, ], ) -def test_api_mailboxes__create_pending_mailboxes(domain_status): +def test_api_mailboxes__create_pending_mailboxes(domain_status, mailbox_data): """ Admin and owner users should be able to create mailboxes, including on pending and failed domains. @@ -490,18 +424,13 @@ def test_api_mailboxes__create_pending_mailboxes(domain_status): client = APIClient() client.force_login(access.user) - - mailbox_values = serializers.MailboxSerializer( - factories.MailboxFactory.build() - ).data - with responses.RequestsMock(): - # We add no response in RequestsMock - # because we expect no outside calls to be made - response = client.post( - f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", - mailbox_values, - format="json", - ) + # We add no response in RequestsMock + # because we expect no outside calls to be made + response = client.post( + f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) assert response.status_code == status.HTTP_201_CREATED mailbox = models.Mailbox.objects.get() assert mailbox.status == "pending" @@ -509,9 +438,8 @@ def test_api_mailboxes__create_pending_mailboxes(domain_status): ### REACTING TO DIMAIL-API ### We mock dimail's responses to avoid testing dimail's container too - - -def test_api_mailboxes__unrelated_user_provisioning_api_not_called(): +@responses.activate +def test_api_mailboxes__unrelated_user_provisioning_api_not_called(mailbox_data): """ Provisioning API should not be called if an user tries to create a mailbox on a domain they have no access to. @@ -520,23 +448,20 @@ def test_api_mailboxes__unrelated_user_provisioning_api_not_called(): client = APIClient() client.force_login(core_factories.UserFactory()) # user with no access - body_values = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=domain) - ).data - with responses.RequestsMock(): - # We add no simulated response in RequestsMock - # because we expected no "outside" calls to be made - response = client.post( - f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/", - body_values, - format="json", - ) - # No exception raised by RequestsMock means no call was sent - # our API blocked the request before sending it - assert response.status_code == status.HTTP_403_FORBIDDEN + # We add no simulated response in RequestsMock + # because we expected no "outside" calls to be made + response = client.post( + f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) + # No exception raised by RequestsMock means no call was sent + # our API blocked the request before sending it + assert response.status_code == status.HTTP_403_FORBIDDEN -def test_api_mailboxes__domain_viewer_provisioning_api_not_called(): +@responses.activate +def test_api_mailboxes__domain_viewer_provisioning_api_not_called(mailbox_data): """ Provisioning API should not be called if a domain viewer tries to create a mailbox on a domain they are not owner/admin of. @@ -549,22 +474,23 @@ def test_api_mailboxes__domain_viewer_provisioning_api_not_called(): client = APIClient() client.force_login(access.user) - body_values = serializers.MailboxSerializer(factories.MailboxFactory.build()).data - with responses.RequestsMock(): - # We add no simulated response in RequestsMock - # because we expected no "outside" calls to be made - response = client.post( - f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", - body_values, - format="json", - ) - # No exception raised by RequestsMock means no call was sent - # our API blocked the request before sending it - assert response.status_code == status.HTTP_403_FORBIDDEN + # We add no simulated response in RequestsMock + # because we expected no "outside" calls to be made + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) + # No exception raised by RequestsMock means no call was sent + # our API blocked the request before sending it + assert response.status_code == status.HTTP_403_FORBIDDEN +@responses.activate @mock.patch.object(Logger, "error") -def test_api_mailboxes__async_dimail_unauthorized(mock_error): +def test_api_mailboxes__async_dimail_unauthorized( + mock_error, dimail_token_ok, mailbox_data +): """ Dimail should raise an error if token has been successfully granted but mailbox creation request returns a 403. @@ -577,34 +503,23 @@ def test_api_mailboxes__async_dimail_unauthorized(mock_error): client = APIClient() client.force_login(access.user) - mailbox_data = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=access.domain) - ).data + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile( + rf".*/domains/{access.domain.name}/mailboxes/{mailbox_data['local_part']}" + ), + status=status.HTTP_403_FORBIDDEN, + content_type="application/json", + ) - 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, # user is in dimail-api - content_type="application/json", - ) - rsps.add( - rsps.POST, - re.compile( - rf".*/domains/{access.domain.name}/mailboxes/{mailbox_data['local_part']}" - ), - 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 + 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 mock_error.call_count == 1 assert mock_error.call_args_list[0][0] == ( @@ -613,64 +528,51 @@ def test_api_mailboxes__async_dimail_unauthorized(mock_error): ) +@responses.activate @pytest.mark.parametrize( "role", [enums.MailDomainRoleChoices.ADMIN, enums.MailDomainRoleChoices.OWNER], ) def test_api_mailboxes__domain_owner_or_admin_successful_creation_and_provisioning( - role, + dimail_token_ok, role, mailbox_data ): """ Domain owner/admin should be able to create mailboxes. Provisioning API should be called when owner/admin makes a call. successful 201 response from dimail should trigger mailbox creation on our side. """ - # creating all needed objects access = factories.MailDomainAccessFactory(role=role) client = APIClient() client.force_login(access.user) - mailbox_data = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=access.domain) - ).data + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body=response_mailbox_created( + f"{mailbox_data['local_part']}@{access.domain.name}" + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) - 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", - ) - rsp = rsps.add( - rsps.POST, - re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), - body=response_mailbox_created( - f"{mailbox_data['local_part']}@{access.domain.name}" - ), - 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", + ) - response = client.post( - f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", - mailbox_data, - format="json", - ) - - # Checks payload sent to email-provisioning API - payload = json.loads(rsps.calls[1].request.body) - assert payload == { - "displayName": f"{mailbox_data['first_name']} {mailbox_data['last_name']}", - "givenName": mailbox_data["first_name"], - "surName": mailbox_data["last_name"], - } - - # Checks response - assert response.status_code == status.HTTP_201_CREATED - assert rsp.call_count == 1 + # Checks payload sent to email-provisioning API + payload = json.loads(responses.calls[1].request.body) + assert payload == { + "displayName": f"{mailbox_data['first_name']} {mailbox_data['last_name']}", + "givenName": mailbox_data["first_name"], + "surName": mailbox_data["last_name"], + } + # Checks response + assert response.status_code == status.HTTP_201_CREATED mailbox = models.Mailbox.objects.get() assert response.json() == { "id": str(mailbox.id), @@ -686,12 +588,13 @@ def test_api_mailboxes__domain_owner_or_admin_successful_creation_and_provisioni assert mailbox.secondary_email == mailbox_data["secondary_email"] +@responses.activate @pytest.mark.parametrize( "role", [enums.MailDomainRoleChoices.ADMIN, enums.MailDomainRoleChoices.OWNER], ) def test_api_mailboxes__domain_owner_or_admin_successful_creation_sets_password( - role, + role, mailbox_data, dimail_token_ok ): """ Upon successful creation, mailbox password should be synced to Dimail's. @@ -701,53 +604,40 @@ def test_api_mailboxes__domain_owner_or_admin_successful_creation_sets_password( 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.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body=response_mailbox_created( + f"{mailbox_data['local_part']}@{access.domain.name}" + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) - 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", - ) - rsp = rsps.add( - rsps.POST, - re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), - body=response_mailbox_created( - f"{mailbox_data['local_part']}@{access.domain.name}" - ), - 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", + ) - response = client.post( - f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", - mailbox_data, - format="json", - ) - - # Checks payload sent to email-provisioning API - payload = json.loads(rsps.calls[1].request.body) - assert payload == { - "displayName": f"{mailbox_data['first_name']} {mailbox_data['last_name']}", - "givenName": mailbox_data["first_name"], - "surName": mailbox_data["last_name"], - } - - # Checks response - assert response.status_code == status.HTTP_201_CREATED - assert rsp.call_count == 1 + # Checks payload sent to email-provisioning API + payload = json.loads(responses.calls[1].request.body) + assert payload == { + "displayName": f"{mailbox_data['first_name']} {mailbox_data['last_name']}", + "givenName": mailbox_data["first_name"], + "surName": mailbox_data["last_name"], + } + # Checks response + assert response.status_code == status.HTTP_201_CREATED mailbox = models.Mailbox.objects.get() assert mailbox.check_password("password") +@responses.activate @override_settings(MAIL_PROVISIONING_API_CREDENTIALS="wrongCredentials") -def test_api_mailboxes__dimail_token_permission_denied(caplog): +def test_api_mailboxes__dimail_token_permission_denied(caplog, mailbox_data): """ API should raise a clear "permission denied" error when receiving a permission denied from dimail upon requesting token. @@ -757,44 +647,40 @@ def test_api_mailboxes__dimail_token_permission_denied(caplog): 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='{"details": "Permission denied"}', + status=status.HTTP_403_FORBIDDEN, + content_type="application/json", + ) - 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", + ) - 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": "Token denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." + } + # mailbox was created in our side only and in pending status + mailbox = models.Mailbox.objects.get() + assert mailbox.status == enums.MailboxStatusChoices.PENDING - assert response.status_code == status.HTTP_403_FORBIDDEN - assert response.json() == { - "detail": "Token denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." - } - # 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 - log_messages = [msg.message for msg in caplog.records] - assert ( - "[DIMAIL] 403 Forbidden: Could not retrieve a token,\ + # Check error logger was called + log_messages = [msg.message for msg in caplog.records] + assert ( + "[DIMAIL] 403 Forbidden: Could not retrieve a token,\ please check 'MAIL_PROVISIONING_API_CREDENTIALS' setting." - in log_messages - ) + in log_messages + ) -def test_api_mailboxes__user_unrelated_to_domain(): +@responses.activate +def test_api_mailboxes__user_unrelated_to_domain(dimail_token_ok, mailbox_data): """ API should raise a clear "permission denied" when dimail returns a permission denied on mailbox creation. This means token was granted for this user @@ -805,66 +691,44 @@ def test_api_mailboxes__user_unrelated_to_domain(): client = APIClient() client.force_login(access.user) - mailbox_data = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=access.domain) - ).data + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body='{"details": "Permission denied"}', + status=status.HTTP_403_FORBIDDEN, + content_type="application/json", + ) - 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='{"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", + ) - 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": "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." - } - # mailbox was created in our side only and in pending status - mailbox = models.Mailbox.objects.get() - assert mailbox.status == enums.MailboxStatusChoices.PENDING + assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.json() == { + "detail": "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." + } + # mailbox was created in our side only and in pending status + mailbox = models.Mailbox.objects.get() + assert mailbox.status == enums.MailboxStatusChoices.PENDING @responses.activate -def test_api_mailboxes__duplicate_display_name(): +def test_api_mailboxes__duplicate_display_name(dimail_token_ok, mailbox_data): """ In OpenXchange, the display name (first + last name) must be unique. In absence of a specific response 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) 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", - ) + # token response in fixtures responses.add( responses.POST, re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), @@ -910,7 +774,9 @@ def test_api_mailboxes__duplicate_display_name(): @responses.activate -def test_api_mailboxes__handling_dimail_unexpected_error(caplog): +def test_api_mailboxes__handling_dimail_unexpected_error( + dimail_token_ok, caplog, mailbox_data +): """ API should raise a clear error when dimail returns an unexpected response. """ @@ -918,18 +784,8 @@ def test_api_mailboxes__handling_dimail_unexpected_error(caplog): 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", - ) + # token response in fixtures responses.add( responses.POST, re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), @@ -971,7 +827,7 @@ def test_api_mailboxes__handling_dimail_unexpected_error(caplog): @responses.activate -def test_api_mailboxes__display_name_duplicate_error(): +def test_api_mailboxes__display_name_duplicate_error(dimail_token_ok, mailbox_data): """ API should raise a clear error when display_name is already used on context. """ @@ -979,18 +835,8 @@ def test_api_mailboxes__display_name_duplicate_error(): 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", - ) + # token response in fixtures responses.add( responses.POST, re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), @@ -1032,15 +878,15 @@ def test_api_mailboxes__display_name_duplicate_error(): 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 + assert not models.Mailbox.objects.exists() # failing mailbox was not created +@responses.activate @mock.patch.object(Logger, "error") @mock.patch.object(Logger, "info") -def test_api_mailboxes__send_correct_logger_infos(mock_info, mock_error): +def test_api_mailboxes__send_correct_logger_infos( + mock_info, mock_error, dimail_token_ok, mailbox_data +): """ Upon requesting mailbox creation, logs should report request user. """ @@ -1048,35 +894,24 @@ def test_api_mailboxes__send_correct_logger_infos(mock_info, mock_error): client = APIClient() client.force_login(access.user) - mailbox_data = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=access.domain) - ).data + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body=response_mailbox_created( + f"{mailbox_data['local_part']}@{access.domain.name}" + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) - 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=response_mailbox_created( - f"{mailbox_data['local_part']}@{access.domain.name}" - ), - 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 + 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 @@ -1093,8 +928,12 @@ def test_api_mailboxes__send_correct_logger_infos(mock_info, mock_error): assert expected_messages.issubset(actual_messages) +@pytest.mark.usefixtures() +@responses.activate @mock.patch.object(Logger, "info") -def test_api_mailboxes__sends_new_mailbox_notification(mock_info): +def test_api_mailboxes__sends_new_mailbox_notification( + mock_info, dimail_token_ok, mailbox_data +): """ Creating a new mailbox should send confirmation email to secondary email. @@ -1107,41 +946,25 @@ def test_api_mailboxes__sends_new_mailbox_notification(mock_info): client = APIClient() client.force_login(user) - mailbox_data = serializers.MailboxSerializer( - factories.MailboxFactory.build(domain=access.domain) - ).data + # Ensure successful response using "responses": + # token response in fixtures + responses.add( + responses.POST, + re.compile(rf".*/domains/{access.domain.name}/mailboxes/"), + body=response_mailbox_created(f"{mailbox_data['local_part']}@{access.domain}"), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + with mock.patch("django.core.mail.send_mail") as mock_send: + client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", + mailbox_data, + format="json", + ) - 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=response_mailbox_created( - f"{mailbox_data['local_part']}@{access.domain}" - ), - status=status.HTTP_201_CREATED, - content_type="application/json", - ) - with mock.patch("django.core.mail.send_mail") as mock_send: - client.post( - f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/", - mailbox_data, - format="json", - ) - - assert mock_send.call_count == 1 - assert ( - "Informations sur votre nouvelle boîte mail" - in mock_send.mock_calls[0][1][1] - ) - assert mock_send.mock_calls[0][1][3][0] == mailbox_data["secondary_email"] + assert mock_send.call_count == 1 + assert "Informations sur votre nouvelle boîte mail" in mock_send.mock_calls[0][1][1] + assert mock_send.mock_calls[0][1][3][0] == mailbox_data["secondary_email"] expected_messages = { ( diff --git a/src/backend/mailbox_manager/tests/conftest.py b/src/backend/mailbox_manager/tests/conftest.py new file mode 100644 index 0000000..e6dcc29 --- /dev/null +++ b/src/backend/mailbox_manager/tests/conftest.py @@ -0,0 +1,37 @@ +""" +Fixtures for mailbox manager tests +""" + +import re + +import pytest +import responses +from rest_framework import status + +from mailbox_manager import factories +from mailbox_manager.tests.fixtures.dimail import TOKEN_OK + + +## DIMAIL RESPONSES +@pytest.fixture(name="dimail_token_ok") +def fixture_dimail_token_ok(): + """Mock dimail response when /token/ endpoit is given valid credentials.""" + responses.add( + responses.GET, + re.compile(r".*/token/"), + body=TOKEN_OK, + status=status.HTTP_200_OK, + content_type="application/json", + ) + + +@pytest.fixture(name="mailbox_data") +def fixture_mailbox_data(): + """Provides valid mailbox data for tests.""" + example = factories.MailboxFactory.build() + return { + "first_name": example.first_name, + "last_name": example.last_name, + "local_part": example.local_part, + "secondary_email": example.secondary_email, + } diff --git a/src/backend/mailbox_manager/tests/fixtures/dimail.py b/src/backend/mailbox_manager/tests/fixtures/dimail.py index d980df9..9c95d6d 100644 --- a/src/backend/mailbox_manager/tests/fixtures/dimail.py +++ b/src/backend/mailbox_manager/tests/fixtures/dimail.py @@ -3,9 +3,8 @@ import json + ## USERS - - def response_user_created(user_sub): """mimic dimail response upon successful user creation.""" return json.dumps( @@ -19,7 +18,6 @@ def response_user_created(user_sub): ## DOMAINS - CHECK_DOMAIN_BROKEN = { "name": "example.fr", "state": "broken", @@ -290,12 +288,10 @@ DOMAIN_SPEC = [ ## TOKEN - TOKEN_OK = json.dumps({"access_token": "token", "token_type": "bearer"}) + ## ALLOWS - - def response_allows_created(user_name, domain_name): """mimic dimail response upon successful allows creation. Dimail expects a name but our names are ProConnect's uuids.""" @@ -303,8 +299,6 @@ def response_allows_created(user_name, domain_name): ## MAILBOXES - - def response_mailbox_created(email_address): """mimic dimail response upon successful mailbox creation.""" return json.dumps({"email": email_address, "password": "password"})