From 23561cd0e031ee7aae8f5eb177dfa1b43b8c81b0 Mon Sep 17 00:00:00 2001 From: Marie PUPO JEAMMET Date: Tue, 21 Oct 2025 18:23:29 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5(sops)=20remove=20obsolete=20sops?= =?UTF-8?q?=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit remove obsolete sops file --- .sops.yaml | 11 ---- CHANGELOG.md | 5 +- .../mailbox_manager/api/client/serializers.py | 15 ++--- .../mailbox_manager/api/client/viewsets.py | 25 +++++++-- .../mailbox_manager/api/permissions.py | 19 ++++--- .../{0026_alias.py => 0027_alias.py} | 2 +- src/backend/mailbox_manager/models.py | 23 ++++++++ .../api/aliases/test_api_aliases_create.py | 56 +++++++++++++++---- .../api/aliases/test_api_aliases_delete.py | 52 +++++++++++++++-- src/backend/mailbox_manager/utils/dimail.py | 13 +++++ 10 files changed, 171 insertions(+), 50 deletions(-) delete mode 100644 .sops.yaml rename src/backend/mailbox_manager/migrations/{0026_alias.py => 0027_alias.py} (94%) diff --git a/.sops.yaml b/.sops.yaml deleted file mode 100644 index f9ff015..0000000 --- a/.sops.yaml +++ /dev/null @@ -1,11 +0,0 @@ -creation_rules: - - path_regex: ./* - key_groups: - - age: - - age15fyxdwmg5mvldtqqus87xspuws2u0cpvwheehrtvkexj4tnsqqysw6re2x # jacques - - age16hnlml8yv4ynwy0seer57g8qww075crd0g7nsundz3pj4wk7m3vqftszg7 # github-repo - - age1plkp8td6zzfcavjusmsfrlk54t9vn8jjxm8zaz7cmnr7kzl2nfnsd54hwg # Anthony Le-Courric - - age12g6f5fse25tgrwweleh4jls3qs52hey2edh759smulwmk5lnzadslu2cp3 # Antoine Lebaud - - age1hnhuzj96ktkhpyygvmz0x9h8mfvssz7ss6emmukags644mdhf4msajk93r # Samuel Paccoud - - age1tl80n23wq6zxegupwn70ew0yp225ua5v4dk800x7g2w6pvlxz46qk592pa # Marie Pupo Jeammet - - age1rjchule5sncn8r8gfph07muee6vzx4wqfrtldt5jjzke4vlfxy2qqplfvc # Quentin Bey diff --git a/CHANGELOG.md b/CHANGELOG.md index 666cd75..6f3a34b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,12 @@ and this project adheres to ## [Unreleased] +- ✨(aliases) can create, list and delete aliases #974 + ## [1.20.0] - 2025-10-22 -- 🐛(mailbox) fix case-sensitive duplicates on display names +- ✨(models) impose uniqueness on display name, to match ox's constraint +- 🐛(dimail) catch duplicate displayname error - ✨(mailbox) synchronize password of newly created mailbox with Dimail's ## [1.19.1] - 2025-09-19 diff --git a/src/backend/mailbox_manager/api/client/serializers.py b/src/backend/mailbox_manager/api/client/serializers.py index ebf9fb9..8c56756 100644 --- a/src/backend/mailbox_manager/api/client/serializers.py +++ b/src/backend/mailbox_manager/api/client/serializers.py @@ -327,18 +327,15 @@ class AliasSerializer(serializers.ModelSerializer): """ Override create function to fire a request to dimail on alias creation. """ - alias = super().create(validated_data) + if validated_data["domain"].status == enums.MailDomainStatusChoices.ENABLED: + alias = models.Alias(**validated_data) - if alias.domain.status == enums.MailDomainStatusChoices.ENABLED: - client = DimailAPIClient() # send new alias request to dimail - try: - client.create_alias(alias, self.context["request"].user.sub) - except django_exceptions.ValidationError as exc: - alias.delete() - raise exc + client = DimailAPIClient() + client.create_alias(alias, self.context["request"].user.sub) + return super().create(validated_data) - return alias + return None def validate_local_part(self, value): """Validate this local part does not match a mailbox.""" diff --git a/src/backend/mailbox_manager/api/client/viewsets.py b/src/backend/mailbox_manager/api/client/viewsets.py index 4db8d69..5761b75 100644 --- a/src/backend/mailbox_manager/api/client/viewsets.py +++ b/src/backend/mailbox_manager/api/client/viewsets.py @@ -40,7 +40,7 @@ class MailDomainViewSet( Fetch domain status and expected config from dimail. """ - permission_classes = [permissions.AccessPermission] + permission_classes = [permissions.DomainResourcePermission] serializer_class = serializers.MailDomainSerializer filter_backends = [filters.OrderingFilter] ordering_fields = ["created_at", "name"] @@ -112,7 +112,7 @@ class MailDomainAccessViewSet( Delete targeted domain access """ - permission_classes = [permissions.MailDomainAccessRolePermission] + permission_classes = [permissions.DomainResourcePermission] serializer_class = serializers.MailDomainAccessSerializer filter_backends = [filters.OrderingFilter] ordering_fields = ["role", "user__email", "user__name"] @@ -355,7 +355,7 @@ class MailDomainInvitationViewset( """ lookup_field = "id" - permission_classes = [permissions.AccessPermission] + permission_classes = [permissions.DomainResourcePermission] queryset = ( models.MailDomainInvitation.objects.all() .select_related("domain") @@ -395,13 +395,16 @@ class MailDomainInvitationViewset( class AliasViewSet( - viewsets.GenericViewSet, mixins.CreateModelMixin, mixins.ListModelMixin, mixins.DestroyModelMixin, + viewsets.GenericViewSet, ): """API ViewSet for aliases. + GET /api//mail-domains//aliases/ + Return list of aliases related to that domain + POST /api//mail-domains//aliases/ with expected data: - local_part: str - destination: str @@ -411,7 +414,7 @@ class AliasViewSet( Delete targeted alias """ - lookup_field = "id" + lookup_field = "local_part" permission_classes = [permissions.DomainPermission] serializer_class = serializers.AliasSerializer queryset = ( @@ -448,6 +451,18 @@ class AliasViewSet( return queryset + def get_permissions(self): + """Add a specific permission for domain viewers to delete their aliases.""" + if self.action in ["destroy"]: + permission_classes = [ + permissions.DomainResourcePermission + | permissions.IsAliasDestinationPermission + ] + else: + return super().get_permissions() + + return [permission() for permission in permission_classes] + def perform_create(self, serializer): """Create new mailbox.""" domain_slug = self.kwargs.get("domain_slug", "") diff --git a/src/backend/mailbox_manager/api/permissions.py b/src/backend/mailbox_manager/api/permissions.py index 348542d..f1d5f88 100644 --- a/src/backend/mailbox_manager/api/permissions.py +++ b/src/backend/mailbox_manager/api/permissions.py @@ -7,7 +7,7 @@ from core.api import permissions as core_permissions from mailbox_manager import models -class AccessPermission(core_permissions.IsAuthenticated): +class DomainResourcePermission(core_permissions.IsAuthenticated): """Permission class for access objects.""" def has_object_permission(self, request, view, obj): @@ -16,7 +16,7 @@ class AccessPermission(core_permissions.IsAuthenticated): return abilities.get(request.method.lower(), False) -class DomainPermission(AccessPermission): +class DomainPermission(DomainResourcePermission): """Permission class to manage mailboxes and aliases for a mail domain""" def has_permission(self, request, view): @@ -40,10 +40,15 @@ class IsMailboxOwnerPermission(permissions.BasePermission): return obj.get_email() == request.user.email -class MailDomainAccessRolePermission(core_permissions.IsAuthenticated): - """Permission class to manage mailboxes for a mail domain""" +class IsAliasDestinationPermission(core_permissions.IsAuthenticated): + """Can delete an alias if the alias points to their own email address.""" + + def has_permission(self, request, view): + """This permission is specifically about updates""" + domain = models.MailDomain.objects.get(slug=view.kwargs.get("domain_slug", "")) + abilities = domain.get_abilities(request.user) + return abilities["get"] def has_object_permission(self, request, view, obj): - """Check permission for a given object.""" - abilities = obj.get_abilities(request.user) - return abilities.get(request.method.lower(), False) + """If the user is trying to update their own mailbox.""" + return obj.destination == request.user.email diff --git a/src/backend/mailbox_manager/migrations/0026_alias.py b/src/backend/mailbox_manager/migrations/0027_alias.py similarity index 94% rename from src/backend/mailbox_manager/migrations/0026_alias.py rename to src/backend/mailbox_manager/migrations/0027_alias.py index e506bd0..5ac2dad 100644 --- a/src/backend/mailbox_manager/migrations/0026_alias.py +++ b/src/backend/mailbox_manager/migrations/0027_alias.py @@ -8,7 +8,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('mailbox_manager', '0025_alter_mailbox_secondary_email'), + ('mailbox_manager', '0026_alter_mailbox_unique_together_and_more'), ] operations = [ diff --git a/src/backend/mailbox_manager/models.py b/src/backend/mailbox_manager/models.py index ab8375c..d033549 100644 --- a/src/backend/mailbox_manager/models.py +++ b/src/backend/mailbox_manager/models.py @@ -476,3 +476,26 @@ class Alias(BaseModel): def __str__(self): return f"{self.local_part} to {self.destination}" + + def get_abilities(self, user): + """Compute and return abilities for a given user. Admin and owners can + edit aliases, but also viewer if the alias points to their email.""" + try: + role = user.mail_domain_accesses.get(domain=self.domain).role + except (MailDomainAccess.DoesNotExist, IndexError): + role = None + + is_owner_or_admin = role in [ + MailDomainRoleChoices.OWNER, + MailDomainRoleChoices.ADMIN, + ] + + is_self = self.destination == user.email + + return { + "get": bool(role), + "post": is_owner_or_admin, + "patch": False, + "put": False, + "delete": is_owner_or_admin or is_self, + } diff --git a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py index 69e56a4..a174416 100644 --- a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py +++ b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_create.py @@ -1,7 +1,8 @@ """ -Tests for mailbox Aliases API endpoint in People's app mailbox_manager. +Tests for aliases API endpoint. Focus on "create" action. """ +# pylint: disable=W0613 import json import re @@ -14,7 +15,6 @@ from rest_framework.test import APIClient from core import factories as core_factories from mailbox_manager import enums, factories, models -from mailbox_manager.tests.fixtures.dimail import TOKEN_OK pytestmark = pytest.mark.django_db @@ -80,7 +80,7 @@ def test_api_aliases_create__duplicate_forbidden(): assert models.Alias.objects.filter(domain=access.domain).count() == 1 -def test_api_aliases_create__existing_mailbox_bad_request(): +def test_api_aliases_create__existing_alias_bad_request(): """Cannot create alias if local_part is already used by a mailbox.""" access = factories.MailDomainAccessFactory( role="owner", domain=factories.MailDomainEnabledFactory() @@ -94,6 +94,46 @@ def test_api_aliases_create__existing_mailbox_bad_request(): {"local_part": mailbox.local_part, "destination": "someone@outsidedomain.com"}, ) assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "local_part": [f'Local part "{mailbox.local_part}" already used for a mailbox.'] + } + assert not models.Alias.objects.exists() + + +@responses.activate +def test_api_aliases_create__async_alias_bad_request(dimail_token_ok): + """ + If People fall out of sync with dimail, return a clear error if alias cannot be created + because it already exists on dimail. + """ + access = factories.MailDomainAccessFactory( + role="owner", domain=factories.MailDomainEnabledFactory() + ) + + client = APIClient() + client.force_login(access.user) + # Mock dimail response + responses.add( + responses.POST, + re.compile(r".*/aliases/"), + body=json.dumps({"detail": "Alias already exists"}), + status=status.HTTP_409_CONFLICT, + content_type="application/json", + ) + + response = client.post( + f"/api/v1.0/mail-domains/{access.domain.slug}/aliases/", + { + "local_part": "already_existing_alias", + "destination": "someone@outsidedomain.com", + }, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == { + "NON_FIELD_ERRORS": [ + "Alias already exists. Your domain is out of sync, please contact our support." + ] + } assert not models.Alias.objects.exists() @@ -102,20 +142,14 @@ def test_api_aliases_create__existing_mailbox_bad_request(): "role", [enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN], ) -def test_api_aliases_create__admins_ok(role): +def test_api_aliases_create__admins_ok(role, dimail_token_ok): """Domain admins should be able to create aliases.""" access = factories.MailDomainAccessFactory(role=role) client = APIClient() client.force_login(access.user) # Prepare 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}/aliases/"), diff --git a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py index 8b4a0f5..c0af6e2 100644 --- a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py +++ b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py @@ -45,21 +45,63 @@ def test_api_aliases_delete__no_access_forbidden(): def test_api_aliases_delete__viewer_forbidden(): """ - Authenticated users should not be allowed to delete a mail domain access for a + Authenticated users should not be allowed to delete aliases for a mail domain in which they are a simple viewer. """ authenticated_user = core_factories.UserFactory() mail_domain = factories.MailDomainFactory( users=[(authenticated_user, enums.MailDomainRoleChoices.VIEWER)] ) - access = factories.MailDomainAccessFactory(domain=mail_domain) + alias = factories.AliasFactory(domain=mail_domain) client = APIClient() client.force_login(authenticated_user) response = client.delete( - f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{access.id!s}/", + f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/", ) assert response.status_code == status.HTTP_403_FORBIDDEN - assert models.MailDomainAccess.objects.count() == 2 - assert models.MailDomainAccess.objects.filter(user=access.user).exists() + assert models.Alias.objects.count() == 1 + + +def test_api_aliases_delete__viewer_can_delete_self_alias(): + """ + Authenticated users should be allowed to delete aliases when linking + to their own mailbox. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.VIEWER)] + ) + alias = factories.AliasFactory( + domain=mail_domain, destination=authenticated_user.email + ) + + client = APIClient() + client.force_login(authenticated_user) + response = client.delete( + f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/", + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not models.Alias.objects.exists() + + +def test_api_aliases_delete__administrators_allowed(): + """ + Administrators of a mail domain should be allowed to delete accesses excepted owner accesses. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + alias = factories.AliasFactory(domain=mail_domain) + + client = APIClient() + client.force_login(authenticated_user) + response = client.delete( + f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/", + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not models.Alias.objects.exists() diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index bcb5750..cfd3a95 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -711,4 +711,17 @@ class DimailAPIClient: "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." ) + if response.status_code == status.HTTP_409_CONFLICT: + logger.error( + "[DIMAIL] Out of sync with dimail. Admin, please import aliases for domain %s", + str(alias.domain), + ) + raise exceptions.ValidationError( + { + "NON_FIELD_ERRORS": [ + "Alias already exists. Your domain is out of sync, please contact our support." + ] + } + ) + return self.raise_exception_for_unexpected_response(response)