diff --git a/CHANGELOG.md b/CHANGELOG.md index e0468db7c..f236cca1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to - 🥅(frontend) improve add & update group forms error handling #387 - ✨(frontend) allow group members filtering #363 - ✨(mailbox) send new mailbox confirmation email #397 +- ✨(domains) domain accesses update API #423 ### Fixed diff --git a/src/backend/mailbox_manager/api/serializers.py b/src/backend/mailbox_manager/api/serializers.py index 0a4dcb960..76a784b3e 100644 --- a/src/backend/mailbox_manager/api/serializers.py +++ b/src/backend/mailbox_manager/api/serializers.py @@ -2,11 +2,11 @@ import json -from rest_framework import serializers +from rest_framework import exceptions, serializers from core.api.serializers import UserSerializer -from mailbox_manager import enums, models +from mailbox_manager import models from mailbox_manager.utils.dimail import DimailAPIClient @@ -94,23 +94,29 @@ class Meta: read_only_fields = ["id", "user", "can_set_role_to"] def get_can_set_role_to(self, access): - """Return roles available to set""" - roles = list(enums.MailDomainRoleChoices) - # get role of authenticated user - authenticated_user_role = access.user_role - if authenticated_user_role != enums.MailDomainRoleChoices.OWNER: - roles.remove(enums.MailDomainRoleChoices.OWNER) - # if the user authenticated is a viewer, they can't modify role - # and only an owner can change role of an owner - if authenticated_user_role == enums.MailDomainRoleChoices.VIEWER or ( - authenticated_user_role != enums.MailDomainRoleChoices.OWNER - and access.role == enums.MailDomainRoleChoices.OWNER - ): - return [] - # we only want to return other roles available to change, - # so we remove the current role of current access. - roles.remove(access.role) - return sorted(roles) + """Return roles available to set for the authenticated user""" + return access.get_can_set_role_to(self.context.get("request").user) + + def validate(self, attrs): + """ + Check access rights specific to writing (update) + """ + request = self.context.get("request") + authenticated_user = getattr(request, "user", None) + role = attrs.get("role") + + # Update + if self.instance: + can_set_role_to = self.instance.get_can_set_role_to(authenticated_user) + + if role and role not in can_set_role_to: + message = ( + f"You are only allowed to set role to {', '.join(can_set_role_to)}" + if can_set_role_to + else "You are not allowed to modify role for this user." + ) + raise exceptions.PermissionDenied(message) + return attrs class MailDomainAccessReadOnlySerializer(MailDomainAccessSerializer): diff --git a/src/backend/mailbox_manager/api/viewsets.py b/src/backend/mailbox_manager/api/viewsets.py index c7a9ad366..37399a886 100644 --- a/src/backend/mailbox_manager/api/viewsets.py +++ b/src/backend/mailbox_manager/api/viewsets.py @@ -2,12 +2,12 @@ from django.db.models import Subquery -from rest_framework import filters, mixins, viewsets +from rest_framework import exceptions, filters, mixins, viewsets from rest_framework import permissions as drf_permissions from core import models as core_models -from mailbox_manager import models +from mailbox_manager import enums, models from mailbox_manager.api import permissions, serializers @@ -58,6 +58,7 @@ def perform_create(self, serializer): class MailDomainAccessViewSet( viewsets.GenericViewSet, mixins.ListModelMixin, + mixins.UpdateModelMixin, mixins.RetrieveModelMixin, ): """ @@ -66,6 +67,14 @@ class MailDomainAccessViewSet( GET /api/v1.0/mail-domains//accesses/: Return list of all domain accesses related to the logged-in user and one domain access if an id is provided. + + PUT /api/v1.0/mail-domains//accesses// with expected data: + - role: str [owner|admin|viewer] + Return updated domain access + + PATCH /api/v1.0/mail-domains//accesses// with expected data: + - role: str [owner|admin|viewer] + Return partially updated domain access """ permission_classes = [drf_permissions.IsAuthenticated] @@ -90,6 +99,7 @@ def get_serializer_context(self): """Extra context provided to the serializer class.""" context = super().get_serializer_context() context["domain_slug"] = self.kwargs["domain_slug"] + context["authenticated_user"] = self.request.user return context def get_queryset(self): @@ -118,6 +128,28 @@ def get_queryset(self): ) return queryset + def perform_update(self, serializer): + """Check that we don't change the role if it leads to losing the last owner.""" + instance = serializer.instance + + # Check if the role is being updated and the new role is not "owner" + if ( + "role" in self.request.data + and self.request.data["role"] != enums.MailDomainRoleChoices.OWNER + ): + domain = instance.domain + # Check if the access being updated is the last owner access for the domain + if ( + instance.role == enums.MailDomainRoleChoices.OWNER + and domain.accesses.filter( + role=enums.MailDomainRoleChoices.OWNER + ).count() + == 1 + ): + message = "Cannot change the role to a non-owner role for the last owner access." + raise exceptions.PermissionDenied({"role": message}) + serializer.save() + class MailBoxViewSet( mixins.CreateModelMixin, diff --git a/src/backend/mailbox_manager/models.py b/src/backend/mailbox_manager/models.py index 20f695905..d9af148ec 100644 --- a/src/backend/mailbox_manager/models.py +++ b/src/backend/mailbox_manager/models.py @@ -47,7 +47,6 @@ def get_abilities(self, user): """ Compute and return abilities for a given user on the domain. """ - is_owner_or_admin = False role = None if user.is_authenticated: @@ -103,6 +102,40 @@ class Meta: def __str__(self): return f"Access of user {self.user} on domain {self.domain}." + def get_can_set_role_to(self, user): + """Return roles available to set""" + if not user.is_authenticated: + return [] + roles = list(MailDomainRoleChoices) + authenticated_user_role = None + + # get role of authenticated user + if hasattr(self, "user_role"): + authenticated_user_role = self.user_role + else: + try: + authenticated_user_role = user.mail_domain_accesses.get( + domain=self.domain + ).role + except (MailDomainAccess.DoesNotExist, IndexError): + return [] + + # only an owner can set an owner role + if authenticated_user_role != MailDomainRoleChoices.OWNER: + roles.remove(MailDomainRoleChoices.OWNER) + + # if the user authenticated is a viewer, they can't modify role + # and only an owner can change role of an owner + if authenticated_user_role == MailDomainRoleChoices.VIEWER or ( + authenticated_user_role != MailDomainRoleChoices.OWNER + and self.role == MailDomainRoleChoices.OWNER + ): + return [] + # we only want to return other roles available to change, + # so we remove the current role of current access. + roles.remove(self.role) + return sorted(roles) + class Mailbox(BaseModel): """Mailboxes for users from mail domain.""" diff --git a/src/backend/mailbox_manager/tests/api/mail_domain_accesses/test_api_mail_domain_accesses_update.py b/src/backend/mailbox_manager/tests/api/mail_domain_accesses/test_api_mail_domain_accesses_update.py new file mode 100644 index 000000000..a2d2d6eae --- /dev/null +++ b/src/backend/mailbox_manager/tests/api/mail_domain_accesses/test_api_mail_domain_accesses_update.py @@ -0,0 +1,295 @@ +""" +Test for mail_domain accesses API endpoints in People's core app : update +""" + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from core import factories as core_factories + +from mailbox_manager import enums, factories + +pytestmark = pytest.mark.django_db + + +def test_api_mail_domain__accesses_update_anonymous(): + """An anonymous users should not be allowed to update a mail domain access.""" + access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.VIEWER) + + response = APIClient().put( + f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", + data={"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + access.refresh_from_db() + assert access.role == enums.MailDomainRoleChoices.VIEWER + + +def test_api_mail_domain__accesses_update_authenticated_unrelated(): + """ + An authenticated user should not be allowed to update a mail domain access + for a mail_domain to which they are not related. + """ + authenticated_user = core_factories.UserFactory() + access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.VIEWER) + + client = APIClient() + client.force_login(authenticated_user) + response = client.put( + f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", + {"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + access.refresh_from_db() + assert access.role == enums.MailDomainRoleChoices.VIEWER + + +def test_api_mail_domain__accesses_update_authenticated_viewer(): + """A viewer of a mail domain should not be allowed to update accesses.""" + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.VIEWER)] + ) + access = factories.MailDomainAccessFactory( + domain=mail_domain, + role=enums.MailDomainRoleChoices.VIEWER, + ) + client = APIClient() + client.force_login(authenticated_user) + response = client.put( + f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", + {"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + access.refresh_from_db() + assert access.role == enums.MailDomainRoleChoices.VIEWER + + +def test_api_mail_domain__accesses_update_authenticated_viewer_themself(): + """A viewer of a mail domain should not be allowed to update its accesses.""" + authenticated_user = core_factories.UserFactory() + access = factories.MailDomainAccessFactory( + user=authenticated_user, + role=enums.MailDomainRoleChoices.VIEWER, + ) + client = APIClient() + client.force_login(authenticated_user) + response = client.put( + f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", + {"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + access.refresh_from_db() + assert access.role == enums.MailDomainRoleChoices.VIEWER + + +def test_api_mail_domain__accesses_update_administrator_except_owner(): + """ + An administrator of a mail domain should be allowed to update a user + access for this mail domain, as long as they don't try to set the role to owner. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + admin_access = factories.MailDomainAccessFactory( + domain=mail_domain, role=enums.MailDomainRoleChoices.ADMIN + ) + viewer_access = factories.MailDomainAccessFactory( + domain=mail_domain, role=enums.MailDomainRoleChoices.VIEWER + ) + + client = APIClient() + client.force_login(authenticated_user) + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{admin_access.id!s}/", + data={"role": enums.MailDomainRoleChoices.OWNER}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + admin_access.refresh_from_db() + assert admin_access.role == enums.MailDomainRoleChoices.ADMIN + + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{admin_access.id!s}/", + data={"role": enums.MailDomainRoleChoices.VIEWER}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + admin_access.refresh_from_db() + assert admin_access.role == enums.MailDomainRoleChoices.VIEWER + + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{viewer_access.id!s}/", + data={"role": enums.MailDomainRoleChoices.OWNER}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + viewer_access.refresh_from_db() + assert viewer_access.role == enums.MailDomainRoleChoices.VIEWER + + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{viewer_access.id!s}/", + data={"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + viewer_access.refresh_from_db() + assert viewer_access.role == enums.MailDomainRoleChoices.ADMIN + + +def test_api_mail_domain__accesses_update_administrator_from_owner(): + """ + An administrator for a mail domain, should not be allowed to update + the user access of an "owner" for this mail domain. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + owner = core_factories.UserFactory() + owner_access = factories.MailDomainAccessFactory( + domain=mail_domain, user=owner, role=enums.MailDomainRoleChoices.OWNER + ) + + client = APIClient() + client.force_login(authenticated_user) + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{owner_access.id!s}/", + data={"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + owner_access.refresh_from_db() + assert owner_access.role == enums.MailDomainRoleChoices.OWNER + + +def test_api_mail_domain__accesses_update_owner(): + """ + An owner of a mail domain should be allowed to update + a user access for this domain. + """ + owner_authenticated = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(owner_authenticated, enums.MailDomainRoleChoices.OWNER)] + ) + user_access1 = factories.MailDomainAccessFactory( + domain=mail_domain, role=enums.MailDomainRoleChoices.ADMIN + ) + user_access2 = factories.MailDomainAccessFactory( + domain=mail_domain, role=enums.MailDomainRoleChoices.VIEWER + ) + + client = APIClient() + client.force_login(owner_authenticated) + # turn admin in viewer + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{user_access1.id!s}/", + data={"role": enums.MailDomainRoleChoices.VIEWER}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + user_access1.refresh_from_db() + assert user_access1.role == enums.MailDomainRoleChoices.VIEWER + + # turn viewer in owner + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{user_access1.id!s}/", + data={"role": enums.MailDomainRoleChoices.OWNER}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + user_access1.refresh_from_db() + assert user_access1.role == enums.MailDomainRoleChoices.OWNER + + # turn viewer in admin + response = client.put( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{user_access2.id!s}/", + data={"role": enums.MailDomainRoleChoices.ADMIN}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + user_access2.refresh_from_db() + assert user_access2.role == enums.MailDomainRoleChoices.ADMIN + + +def test_api_mail_domain__accesses_update_owner_for_owners(): + """ + An owner of a mail domain should be allowed to update + an existing owner access for this mail domain. + """ + owner_authenticated = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(owner_authenticated, enums.MailDomainRoleChoices.OWNER)] + ) + other_owner_access = factories.MailDomainAccessFactory( + domain=mail_domain, role=enums.MailDomainRoleChoices.OWNER + ) + + client = APIClient() + client.force_login(owner_authenticated) + for new_role in [ + enums.MailDomainRoleChoices.ADMIN, + enums.MailDomainRoleChoices.VIEWER, + ]: + response = client.patch( + f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/{other_owner_access.id!s}/", + data={"role": new_role}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + other_owner_access.refresh_from_db() + assert other_owner_access.role == new_role + + +def test_api_mail_domain__accesses_update_owner_self(): + """ + An owner of a mail domain should be allowed to update + their own user access provided there are other owners in the mail domain. + """ + owner_authenticated = core_factories.UserFactory() + access = factories.MailDomainAccessFactory( + user=owner_authenticated, role=enums.MailDomainRoleChoices.OWNER + ) + + client = APIClient() + client.force_login(owner_authenticated) + for new_role in [ + enums.MailDomainRoleChoices.ADMIN, + enums.MailDomainRoleChoices.VIEWER, + ]: + response = client.patch( + f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", + data={"role": new_role}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert ( + response.json()["role"] + == "Cannot change the role to a non-owner role for the last owner access." + ) + access.refresh_from_db() + assert access.role == enums.MailDomainRoleChoices.OWNER + + # Add another owner and it should now work + factories.MailDomainAccessFactory( + domain=access.domain, role=enums.MailDomainRoleChoices.OWNER + ) + for new_role in [ + enums.MailDomainRoleChoices.ADMIN, + enums.MailDomainRoleChoices.VIEWER, + ]: + response = client.patch( + f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", + data={"role": new_role}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + access.refresh_from_db() + assert access.role == new_role