diff --git a/CHANGELOG.md b/CHANGELOG.md index 0580fb0..be6e7ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to ## [Unreleased] +### Added + +- ✨(api) update mailboxes #934 + ### Changed - 💥(sentry) remove `DJANGO_` before Sentry DSN env variable #957 diff --git a/src/backend/mailbox_manager/api/client/serializers.py b/src/backend/mailbox_manager/api/client/serializers.py index fb3dad1..7ac92e6 100644 --- a/src/backend/mailbox_manager/api/client/serializers.py +++ b/src/backend/mailbox_manager/api/client/serializers.py @@ -29,7 +29,6 @@ class MailboxSerializer(serializers.ModelSerializer): "secondary_email", "status", ] - # everything is actually read-only as we do not allow update for now read_only_fields = ["id", "status"] def create(self, validated_data): @@ -71,6 +70,22 @@ class MailboxSerializer(serializers.ModelSerializer): return mailbox +class MailboxUpdateSerializer(MailboxSerializer): + """A more restrictive serializer when updating mailboxes""" + + class Meta: + model = models.Mailbox + fields = [ + "id", + "first_name", + "last_name", + "local_part", + "secondary_email", + "status", + ] + read_only_fields = ("id", "status", "local_part", "status") + + class MailDomainSerializer(serializers.ModelSerializer): """Serialize mail domain.""" diff --git a/src/backend/mailbox_manager/api/client/viewsets.py b/src/backend/mailbox_manager/api/client/viewsets.py index 3853937..8dc53a7 100644 --- a/src/backend/mailbox_manager/api/client/viewsets.py +++ b/src/backend/mailbox_manager/api/client/viewsets.py @@ -228,9 +228,10 @@ class MailDomainAccessViewSet( class MailBoxViewSet( + viewsets.GenericViewSet, mixins.CreateModelMixin, mixins.ListModelMixin, - viewsets.GenericViewSet, + mixins.UpdateModelMixin, ): """MailBox ViewSet @@ -252,6 +253,12 @@ class MailBoxViewSet( POST /api//mail-domains//mailboxes//reset/ Send a request to mail-provider to reset password. + + PUT /api//mail-domains//mailboxes// + Send a request to update mailbox. Cannot modify domain or local_part. + + PATCH /api//mail-domains//mailboxes// + Send a request to partially update mailbox. Cannot modify domain or local_part. """ permission_classes = [permissions.MailBoxPermission] @@ -267,6 +274,12 @@ class MailBoxViewSet( return self.queryset.filter(domain__slug=domain_slug) return self.queryset + def get_serializer_class(self): + """Chooses list or detail serializer according to the action.""" + if self.action in {"update", "partial_update"}: + return serializers.MailboxUpdateSerializer + return self.serializer_class + 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 2ed97f4..61a655d 100644 --- a/src/backend/mailbox_manager/api/permissions.py +++ b/src/backend/mailbox_manager/api/permissions.py @@ -23,6 +23,11 @@ class MailBoxPermission(core_permissions.IsAuthenticated): abilities = domain.get_abilities(request.user) return abilities.get(request.method.lower(), False) + 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) + class MailDomainAccessRolePermission(core_permissions.IsAuthenticated): """Permission class to manage mailboxes for a mail domain""" diff --git a/src/backend/mailbox_manager/models.py b/src/backend/mailbox_manager/models.py index a5c9b85..e32eac4 100644 --- a/src/backend/mailbox_manager/models.py +++ b/src/backend/mailbox_manager/models.py @@ -341,6 +341,23 @@ class Mailbox(AbstractBaseUser, BaseModel): """Return the email address of the mailbox.""" return f"{self.local_part}@{self.domain.name}" + def get_abilities(self, user): + """Compute and return abilities for a given user.""" + role = user.mail_domain_accesses.get(domain=self.domain).role + + is_owner_or_admin = role in [ + MailDomainRoleChoices.OWNER, + MailDomainRoleChoices.ADMIN, + ] + + return { + "get": bool(role), + "post": is_owner_or_admin, + "patch": is_owner_or_admin, + "put": is_owner_or_admin, + "delete": False, + } + class MailDomainInvitation(BaseInvitation): """User invitation to mail domains.""" diff --git a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_patch.py b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_patch.py new file mode 100644 index 0000000..f3d7a7d --- /dev/null +++ b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_patch.py @@ -0,0 +1,171 @@ +""" +Unit tests for the mailbox API +""" + +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_mailboxes_patch__anonymous_forbidden(): + """Anonymous users should not be able to update a mailbox.""" + mailbox = factories.MailboxFactory() + saved_secondary = mailbox.secondary_email + APIClient().get( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/" + ) + response = APIClient().patch( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {"something": "updated"}, + ) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + mailbox.refresh_from_db() + assert mailbox.secondary_email == saved_secondary + + +def test_api_mailboxes_patch__unauthorized_forbidden(): + """Authenticated but unauthoriezd users should not be able to update mailboxes.""" + client = APIClient() + client.force_login(core_factories.UserFactory()) + + mailbox = factories.MailboxFactory() + saved_secondary = mailbox.secondary_email + response = client.patch( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {"something": "updated"}, + format="json", + ) + + # permission denied at domain level + assert response.status_code == status.HTTP_403_FORBIDDEN + mailbox.refresh_from_db() + assert mailbox.secondary_email == saved_secondary + + +def test_api_mailboxes_patch__unauthorized_no_mailbox(): + """Authenticated but unauthoriezd users should not be able to update mailboxes.""" + client = APIClient() + client.force_login(core_factories.UserFactory()) + + domain = factories.MailDomainEnabledFactory() + response = client.patch( + f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/nonexistent_mailbox_pk/", + {"something": "updated"}, + format="json", + ) + + # permission denied at domain level + # the existence of the mailbox is not checked + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_api_mailboxes_patch__viewer_forbidden(): + """User having viewer access on a domain should not be able to update + anything on a mailbox that's not theirs.""" + viewer = core_factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailDomainAccessFactory( + user=viewer, + domain=mailbox.domain, + role=enums.MailDomainRoleChoices.VIEWER, + ) + + client = APIClient() + client.force_login(viewer) + + # should not be able to update any field + for field_name in [ + "secondary_email", + "first_name", + "last_name", + "domain", + "local_part", + ]: + old_value = getattr(mailbox, field_name) + response = client.patch( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {field_name: "something_else"}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + mailbox.refresh_from_db() + assert getattr(mailbox, field_name) == old_value + + +# UPDATABLE FIELDS : SECONDARY EMAIL FIRST NAME AND LAST NAME +@pytest.mark.parametrize( + "role", + ["owner", "administrator"], +) +def test_api_mailboxes_patch__admins_can_update_mailboxes(role): + """Domain owners and admins should be allowed to update secondary email on a mailbox""" + mailbox = factories.MailboxFactory() + user = core_factories.UserFactory() + factories.MailDomainAccessFactory( + user=user, + domain=mailbox.domain, + role=role, + ) + + client = APIClient() + client.force_login(user) + + valid_new_values = { + "secondary_email": "updated_mail@validformat.com", + "first_name": "Marsha", + "last_name": "Johnson", + } + for field_name in ["first_name", "last_name", "secondary_email"]: + getattr(mailbox, field_name) + response = client.patch( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {field_name: valid_new_values[field_name]}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK + mailbox.refresh_from_db() + assert getattr(mailbox, field_name) == valid_new_values[field_name] + + +# DOMAIN AND LOCAL PART +@pytest.mark.parametrize( + "role", + ["viewer", "owner", "administrator"], +) +def test_api_mailboxes_patch__no_updating_domain_or_local_part(role): + """Domain and local parts should not be updated.""" + mailbox = factories.MailboxFactory() + user = core_factories.UserFactory() + if role == "viewer": + # viewer has similar privileges as admins on own mailbox + user = core_factories.UserFactory( + email=f"{mailbox.local_part}@{mailbox.domain}" + ) + access = factories.MailDomainAccessFactory( + user=user, + domain=mailbox.domain, + role=role, + ) + + client = APIClient() + client.force_login(user) + + other_domain = factories.MailDomainEnabledFactory() + local_part = mailbox.local_part + response = client.patch( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {"local_part": "new.local.part", "domain": other_domain.name}, + format="json", + ) + assert response.status_code == status.HTTP_200_OK # a 400 would be better + mailbox.refresh_from_db() + assert mailbox.local_part == local_part + assert mailbox.domain == access.domain diff --git a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_put.py b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_put.py new file mode 100644 index 0000000..5177ccd --- /dev/null +++ b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_put.py @@ -0,0 +1,176 @@ +""" +Unit tests for the mailbox API +""" + +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_mailboxes_put__anonymous_forbidden(): + """Anonymous users should not be able to update a mailbox.""" + mailbox = factories.MailboxFactory() + saved_secondary = mailbox.secondary_email + APIClient().get( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/" + ) + response = APIClient().patch( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {"something": "updated"}, + ) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + mailbox.refresh_from_db() + assert mailbox.secondary_email == saved_secondary + + +def test_api_mailboxes_put__unauthorized_forbidden(): + """Authenticated but unauthoriezd users should not be able to update mailboxes.""" + client = APIClient() + client.force_login(core_factories.UserFactory()) + + mailbox = factories.MailboxFactory() + saved_secondary = mailbox.secondary_email + response = client.put( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {"something": "updated"}, + format="json", + ) + + # permission denied at domain level + assert response.status_code == status.HTTP_403_FORBIDDEN + mailbox.refresh_from_db() + assert mailbox.secondary_email == saved_secondary + + +def test_api_mailboxes_put__unauthorized_no_mailbox(): + """Authenticated but unauthoriezd users should not be able to update mailboxes.""" + client = APIClient() + client.force_login(core_factories.UserFactory()) + + domain = factories.MailDomainEnabledFactory() + response = client.put( + f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/nonexistent_mailbox_pk/", + {"something": "updated"}, + format="json", + ) + + # permission denied at domain level + # the existence of the mailbox is not checked + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_api_mailboxes_put__viewer_forbidden(): + """User having viewer access on a domain should not be able to update + anything on a mailbox that's not theirs.""" + viewer = core_factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailDomainAccessFactory( + user=viewer, + domain=mailbox.domain, + role=enums.MailDomainRoleChoices.VIEWER, + ) + + client = APIClient() + client.force_login(viewer) + + # should not be able to update any field + for field_name in [ + "secondary_email", + "first_name", + "last_name", + "domain", + "local_part", + ]: + old_value = getattr(mailbox, field_name) + response = client.put( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {field_name: "something_else"}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + mailbox.refresh_from_db() + assert getattr(mailbox, field_name) == old_value + + +# UPDATABLE FIELDS : SECONDARY EMAIL, FIRST NAME AND LAST NAME +@pytest.mark.parametrize( + "role", + ["owner", "administrator"], +) +def test_api_mailboxes_put__admins_can_update_mailboxes(role): + """Domain owners and admins should be allowed to update secondary email on a mailbox""" + mailbox = factories.MailboxFactory() + user = core_factories.UserFactory() + factories.MailDomainAccessFactory( + user=user, + domain=mailbox.domain, + role=role, + ) + + client = APIClient() + client.force_login(user) + + valid_new_values = { + "secondary_email": "marsha.p@social.us", + "first_name": "Marsha", + "last_name": "Johnson", + } + response = client.put( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + { + "first_name": valid_new_values["first_name"], + "last_name": valid_new_values["last_name"], + "secondary_email": valid_new_values["secondary_email"], + }, + format="json", + ) + result = response.json() + assert response.status_code == status.HTTP_200_OK + mailbox.refresh_from_db() + assert result["first_name"] == valid_new_values["first_name"] + assert result["last_name"] == valid_new_values["last_name"] + assert result["secondary_email"] == valid_new_values["secondary_email"] + + +# DOMAIN AND LOCAL PART +@pytest.mark.parametrize( + "role", + ["viewer", "owner", "administrator"], +) +def test_api_mailboxes_put__no_updating_domain_or_local_part(role): + """Domain and local parts should not be updated.""" + mailbox = factories.MailboxFactory() + user = core_factories.UserFactory() + if role == "viewer": + # viewer has similar privileges as admins on own mailbox + user = core_factories.UserFactory( + email=f"{mailbox.local_part}@{mailbox.domain}" + ) + access = factories.MailDomainAccessFactory( + user=user, + domain=mailbox.domain, + role=role, + ) + + client = APIClient() + client.force_login(user) + + other_domain = factories.MailDomainEnabledFactory() + old_local_part = mailbox.local_part + response = client.put( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/", + {"local_part": "new.local.part", "domain": other_domain.name}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + mailbox.refresh_from_db() + assert mailbox.local_part == old_local_part + assert mailbox.domain == access.domain diff --git a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_retrieve.py b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_retrieve.py new file mode 100644 index 0000000..3ca0fe6 --- /dev/null +++ b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_retrieve.py @@ -0,0 +1,37 @@ +""" +Unit tests for the mailbox API +""" + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from core import factories as core_factories + +from mailbox_manager import factories + +pytestmark = pytest.mark.django_db + + +def test_api_mailboxes__retrieve_anonymous_forbidden(): + """Anonymous users should not be able to retrieve a new mailbox via the API.""" + mailbox = factories.MailboxFactory() + response = APIClient().get( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/" + ) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +def test_api_mailboxes__retrieve_unauthorized_failure(): + """Authenticated but unauthorized users should not be able to + retrieve mailbox.""" + client = APIClient() + client.force_login(core_factories.UserFactory()) + + mailbox = factories.MailboxFactory() + response = client.get( + f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/" + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + # 403 or 404 for confidentiality/security purposes ?