From 63b8984eb2d4fc03ee904381cbdab5b4e9b63d74 Mon Sep 17 00:00:00 2001 From: Marie PUPO JEAMMET Date: Thu, 3 Jul 2025 15:44:40 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(domains)=20fix=20attemps=20to=20se?= =?UTF-8?q?nd=20invitations=20to=20existing=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users can only search other users inside their own organization. This leads to a bug where we try to invite an email already linked to an user --- CHANGELOG.md | 6 +++- src/backend/core/models.py | 6 ++-- .../core/tests/test_api_team_invitations.py | 9 +++--- .../mailbox_manager/api/client/viewsets.py | 30 +++++++++++++++++++ src/backend/mailbox_manager/exceptions.py | 18 +++++++++++ .../test_api_domain_invitations_create.py | 25 +++++++++------- 6 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 src/backend/mailbox_manager/exceptions.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a2a0d2..ea1edd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,11 @@ and this project adheres to - ✨(front) add show invitations mails domains access #1040 - ✨(invitations) can delete domain invitations -## Changed +### Fixed + +- 🐛(domains) fix attemps to send invitations to existing users #953 + +### Changed - 🏗️(core) migrate from pip to uv - ✨(front) add show invitations mails domains access #1040 diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 4c14c2f..cf8ccf1 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -36,6 +36,8 @@ from core.plugins.registry import registry as plugin_hooks_registry from core.utils.webhooks import webhooks_synchronizer from core.validators import get_field_validators_from_setting +from mailbox_manager.exceptions import EmailAlreadyKnownException + logger = getLogger(__name__) current_dir = os.path.dirname(os.path.abspath(__file__)) @@ -997,9 +999,7 @@ class BaseInvitation(BaseModel): # Check if a user already exists for the provided email if User.objects.filter(email=self.email).exists(): - raise exceptions.ValidationError( - {"email": _("This email is already associated to a registered user.")} - ) + raise EmailAlreadyKnownException @property def is_expired(self): diff --git a/src/backend/core/tests/test_api_team_invitations.py b/src/backend/core/tests/test_api_team_invitations.py index a63c5ba..87df937 100644 --- a/src/backend/core/tests/test_api_team_invitations.py +++ b/src/backend/core/tests/test_api_team_invitations.py @@ -171,10 +171,11 @@ def test_api_team_invitations__create__cannot_invite_existing_users(): format="json", ) - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json()["email"] == [ - "This email is already associated to a registered user." - ] + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["detail"] + == "Email already known. Invitation not sent but access created instead." + ) def test_api_team_invitations__list__anonymous_user(): diff --git a/src/backend/mailbox_manager/api/client/viewsets.py b/src/backend/mailbox_manager/api/client/viewsets.py index 63a8735..7cdc00d 100644 --- a/src/backend/mailbox_manager/api/client/viewsets.py +++ b/src/backend/mailbox_manager/api/client/viewsets.py @@ -13,6 +13,7 @@ from core.api.client.serializers import UserSerializer from mailbox_manager import enums, models from mailbox_manager.api import permissions from mailbox_manager.api.client import serializers +from mailbox_manager.exceptions import EmailAlreadyKnownException from mailbox_manager.utils.dimail import DimailAPIClient @@ -363,6 +364,7 @@ class MailDomainInvitationViewset( - issuer : User, automatically added from user making query, if allowed - domain : Domain, automatically added from requested URI Return a newly created invitation + or an access if email is already linked to an existing user PUT / PATCH : Not permitted. Instead of updating your invitation, delete and create a new one. @@ -410,6 +412,34 @@ class MailDomainInvitationViewset( return queryset + def perform_create(self, serializer): + """Lookup for existing user before inviting.""" + if email := serializer.validated_data["email"]: + existing_user = models.User.objects.filter(email=email) + if existing_user.exists(): + return models.MailDomainAccess.objects.create( + user=existing_user[0], + domain=serializer.validated_data["domain"], + role=serializer.validated_data["role"], + ) + + return super().perform_create(serializer) + + + def create(self, request, *args, **kwargs): + """Attempt to create invitation. If user is already registered, + they don't need an invitation but an access, which we create here.""" + try: + return super().create(request, *args, **kwargs) + except EmailAlreadyKnownException as exc: + user = models.User.objects.get(email=email) + + models.MailDomainAccess.objects.create( + user=user, + domain=models.MailDomain.objects.get(slug=kwargs["domain_slug"]), + role=request.data["role"], + ) + raise exc class AliasViewSet( mixins.CreateModelMixin, diff --git a/src/backend/mailbox_manager/exceptions.py b/src/backend/mailbox_manager/exceptions.py new file mode 100644 index 0000000..b57dd78 --- /dev/null +++ b/src/backend/mailbox_manager/exceptions.py @@ -0,0 +1,18 @@ +""" +Custom exceptions for mailbox manager app +""" + +from django.utils.translation import gettext_lazy as _ + +from rest_framework import status +from rest_framework.exceptions import APIException + + +class EmailAlreadyKnownException(APIException): + """Exception raised when trying to create a user with an already existing email address.""" + + status_code = status.HTTP_201_CREATED + default_detail = _( + "Email already known. Invitation not sent but access created instead." + ) + default_code = "email already known" diff --git a/src/backend/mailbox_manager/tests/api/invitations/test_api_domain_invitations_create.py b/src/backend/mailbox_manager/tests/api/invitations/test_api_domain_invitations_create.py index 6503bb8..4565419 100644 --- a/src/backend/mailbox_manager/tests/api/invitations/test_api_domain_invitations_create.py +++ b/src/backend/mailbox_manager/tests/api/invitations/test_api_domain_invitations_create.py @@ -11,7 +11,7 @@ from rest_framework.test import APIClient from core import factories as core_factories -from mailbox_manager import enums, factories +from mailbox_manager import enums, factories, models from mailbox_manager.api.client import serializers pytestmark = pytest.mark.django_db @@ -141,9 +141,10 @@ def test_api_domain_invitations__should_not_create_duplicate_invitations(): assert response.json()["__all__"] == [ "Mail domain invitation with this Email address and Domain already exists." ] + assert models.MailDomainInvitation.objects.count() == 1 # and specifically, not 2 -def test_api_domain_invitations__should_not_invite_when_user_already_exists(): +def test_api_domain_invitations__should_create_access_when_user_already_exists(): """Already existing users should not be invited but given access directly.""" existing_user = core_factories.UserFactory() @@ -152,15 +153,19 @@ def test_api_domain_invitations__should_not_invite_when_user_already_exists(): client = APIClient() client.force_login(access.user) - invitation_values = serializers.MailDomainInvitationSerializer( - factories.MailDomainInvitationFactory.build(email=existing_user.email) - ).data response = client.post( f"/api/v1.0/mail-domains/{access.domain.slug}/invitations/", - invitation_values, + { + "email": existing_user.email, + "role": "owner", + }, format="json", ) - assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json()["email"] == [ - "This email is already associated to a registered user." - ] + assert response.status_code == status.HTTP_201_CREATED + assert ( + response.json()["detail"] + == "Email already known. Invitation not sent but access created instead." + ) + + assert not models.MailDomainInvitation.objects.exists() + assert models.MailDomainAccess.objects.filter(user=existing_user).exists()