From e4936ef3aeb61649b9d4109e7860dc6686053ffc Mon Sep 17 00:00:00 2001 From: Sabrina Demagny Date: Mon, 16 Dec 2024 22:28:42 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(maildomain)=20fix=20domain=20acces?= =?UTF-8?q?s=20creation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During a new domain creation, a call to dimail is made to create user/allow on dimail side before owner role creation on our side. So when user/allow creation on dimain side fails, the owner role is not created on our side. Therefore the domain is created but invisible on the user interface. The user will probably try to create the same domain again and see the error message 'this domain already exists'. To avoid this we make sure to create owner role on our side despite dimail failure and set domain to failed status to retry later dimail access creation. --- CHANGELOG.md | 4 ++ .../mailbox_manager/api/client/serializers.py | 18 +++-- .../test_api_mail_domains_create.py | 71 +++++++++++++++++++ src/backend/mailbox_manager/utils/dimail.py | 2 +- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9a22afb1..abb872fed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to ## [Unreleased] +### Fixed + +- 🐛(backend) fix manage roles on domain admin view + ### Added - ✨(organizations) add siret to name conversion #584 diff --git a/src/backend/mailbox_manager/api/client/serializers.py b/src/backend/mailbox_manager/api/client/serializers.py index ceca66dec..81f248ae4 100644 --- a/src/backend/mailbox_manager/api/client/serializers.py +++ b/src/backend/mailbox_manager/api/client/serializers.py @@ -1,7 +1,9 @@ """Client serializers for People's mailbox manager app.""" import json +from logging import getLogger +from requests.exceptions import HTTPError from rest_framework import exceptions, serializers from core.api.client.serializers import UserSerializer @@ -10,6 +12,8 @@ from mailbox_manager import enums, models from mailbox_manager.utils.dimail import DimailAPIClient +logger = getLogger(__name__) + class MailboxSerializer(serializers.ModelSerializer): """Serialize mailbox.""" @@ -190,14 +194,20 @@ def create(self, validated_data): """ dimail = DimailAPIClient() + user = validated_data["user"] + domain = validated_data['domain'] + if validated_data["role"] in [ enums.MailDomainRoleChoices.ADMIN, enums.MailDomainRoleChoices.OWNER, ]: - dimail.create_user(validated_data["user"].sub) - dimail.create_allow( - validated_data["user"].sub, validated_data["domain"].name - ) + try: + dimail.create_user(user.sub) + dimail.create_allow(user.sub, domain.name) + except HTTPError: + logger.exception("[DIMAIL] access creation failed %s") + domain.status = enums.MailDomainStatusChoices.FAILED + domain.save() return super().create(validated_data) diff --git a/src/backend/mailbox_manager/tests/api/mail_domain/test_api_mail_domains_create.py b/src/backend/mailbox_manager/tests/api/mail_domain/test_api_mail_domains_create.py index 56aacaada..b915069dc 100644 --- a/src/backend/mailbox_manager/tests/api/mail_domain/test_api_mail_domains_create.py +++ b/src/backend/mailbox_manager/tests/api/mail_domain/test_api_mail_domains_create.py @@ -122,6 +122,77 @@ def test_api_mail_domains__create_authenticated(): assert domain.accesses.filter(role="owner", user=user).exists() +def test_api_mail_domains__create_authenticated__dimail_failure(): + """ + Despite a dimail failure for user and/or allow creation, + an authenticated user should be able to create a mail domain + and should automatically be added as owner of the newly created domain. + """ + user = core_factories.UserFactory() + + client = APIClient() + client.force_login(user) + + domain_name = "test.domain.fr" + + with responses.RequestsMock() as rsps: + rsps.add( + rsps.POST, + re.compile(r".*/domains/"), + body=str( + { + "name": domain_name, + } + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + rsps.add( + rsps.POST, + re.compile(r".*/users/"), + body=str( + { + "name": "request-user-sub", + "is_admin": "false", + "uuid": "user-uuid-on-dimail", + "perms": [], + } + ), + status=status.HTTP_201_CREATED, + content_type="application/json", + ) + rsps.add( + rsps.POST, + re.compile(r".*/allows/"), + body=str({"user": "request-user-sub", "domain": str(domain_name)}), + status=status.HTTP_403_FORBIDDEN, + content_type="application/json", + ) + response = client.post( + "/api/v1.0/mail-domains/", + {"name": domain_name, "context": "null", "features": ["webmail"]}, + format="json", + ) + assert response.status_code == status.HTTP_201_CREATED + domain = models.MailDomain.objects.get() + + # response is as expected + assert response.json() == { + "id": str(domain.id), + "name": domain.name, + "slug": domain.slug, + "status": enums.MailDomainStatusChoices.FAILED, + "created_at": domain.created_at.isoformat().replace("+00:00", "Z"), + "updated_at": domain.updated_at.isoformat().replace("+00:00", "Z"), + "abilities": domain.get_abilities(user), + } + + # a new domain with status "failed" is created and authenticated user is the owner + assert domain.status == enums.MailDomainStatusChoices.FAILED + assert domain.name == domain_name + assert domain.accesses.filter(role="owner", user=user).exists() + + ## SYNC TO DIMAIL def test_api_mail_domains__create_dimail_domain(caplog): """ diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index e8d8b41cc..b00515611 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -245,7 +245,7 @@ def raise_exception_for_unexpected_response(self, response): except json.decoder.JSONDecodeError: error_content = response.content.decode(response.encoding) - logger.error( + logger.exception( "[DIMAIL] unexpected error : %s %s", response.status_code, error_content ) raise requests.exceptions.HTTPError(