✨(api) give update rights to domain viewer on own mailbox

Introduces the notion of self in permissions
allowing a domain viewer to update their own mailbox.

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to
 ### Added
 
 - ✨(api) update mailboxes #934
+- ✨(api) give update rights to domain viewer on own mailbox #934
 
 ### Changed
 

src/backend/mailbox_manager/api/client/viewsets.py

@@ -274,6 +274,17 @@ class MailBoxViewSet(
             return self.queryset.filter(domain__slug=domain_slug)
         return self.queryset
 
+    def get_permissions(self):
+        """Add a specific permission for domain viewers to update their own mailbox."""
+        if self.action in ["update", "partial_update"]:
+            permission_classes = [
+                permissions.MailBoxPermission | permissions.IsMailboxOwnerPermission
+            ]
+        else:
+            return super().get_permissions()
+
+        return [permission() for permission in permission_classes]
+
     def get_serializer_class(self):
         """Chooses list or detail serializer according to the action."""
         if self.action in {"update", "partial_update"}:

src/backend/mailbox_manager/api/permissions.py

@@ -14,7 +14,7 @@ class AccessPermission(core_permissions.IsAuthenticated):
         return abilities.get(request.method.lower(), False)
 
 
-class MailBoxPermission(core_permissions.IsAuthenticated):
+class MailBoxPermission(AccessPermission):
     """Permission class to manage mailboxes for a mail domain"""
 
     def has_permission(self, request, view):
@@ -23,10 +23,19 @@ class MailBoxPermission(core_permissions.IsAuthenticated):
         abilities = domain.get_abilities(request.user)
         return abilities.get(request.method.lower(), False)
 
+
+class IsMailboxOwnerPermission(core_permissions.IsAuthenticated):
+    """Authorize update for domain viewers on their own mailbox."""
+
+    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."""
+        """If the user is trying to update their own mailbox."""
-        abilities = obj.get_abilities(request.user)
+        return obj.get_email() == request.user.email
-        return abilities.get(request.method.lower(), False)
 
 
 class MailDomainAccessRolePermission(core_permissions.IsAuthenticated):

src/backend/mailbox_manager/models.py

@@ -350,11 +350,13 @@ class Mailbox(AbstractBaseUser, BaseModel):
             MailDomainRoleChoices.ADMIN,
         ]
 
+        is_self = self.get_email() == user.email
+
         return {
             "get": bool(role),
             "post": is_owner_or_admin,
-            "patch": is_owner_or_admin,
+            "patch": is_owner_or_admin or is_self,
-            "put": is_owner_or_admin,
+            "put": is_owner_or_admin or is_self,
             "delete": False,
         }
 

src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_patch.py

@@ -17,12 +17,10 @@ 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"},
+        {"secondary_email": "updated@example.com"},
     )
     assert response.status_code == status.HTTP_401_UNAUTHORIZED
 
@@ -39,7 +37,7 @@ def test_api_mailboxes_patch__unauthorized_forbidden():
     saved_secondary = mailbox.secondary_email
     response = client.patch(
         f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
-        {"something": "updated"},
+        {"secondary_email": "updated@example.com"},
         format="json",
     )
 
@@ -57,7 +55,7 @@ def test_api_mailboxes_patch__unauthorized_no_mailbox():
     domain = factories.MailDomainEnabledFactory()
     response = client.patch(
         f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/nonexistent_mailbox_pk/",
-        {"something": "updated"},
+        {"secondary_email": "updated@example.com"},
         format="json",
     )
 
@@ -81,23 +79,17 @@ def test_api_mailboxes_patch__viewer_forbidden():
     client.force_login(viewer)
 
     # should not be able to update any field
-    for field_name in [
+    # we only try one field as 403 is global in our implementation
-        "secondary_email",
+    old_value = mailbox.secondary_email
-        "first_name",
+    response = client.patch(
-        "last_name",
+        f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
-        "domain",
+        {"secondary_email": "updated@example.com"},
-        "local_part",
+        format="json",
-    ]:
+    )
-        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
+    assert response.status_code == status.HTTP_403_FORBIDDEN
-        mailbox.refresh_from_db()
+    mailbox.refresh_from_db()
-        assert getattr(mailbox, field_name) == old_value
+    assert mailbox.secondary_email == old_value
 
 
 # UPDATABLE FIELDS : SECONDARY EMAIL FIRST NAME AND LAST NAME
@@ -124,7 +116,35 @@ def test_api_mailboxes_patch__admins_can_update_mailboxes(role):
         "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]
+
+
+def test_api_mailboxes_patch__viewer_can_update_own_mailbox():
+    """Domain owners and admins should be allowed to update secondary email on a mailbox"""
+    mailbox = factories.MailboxFactory()
+    user = core_factories.UserFactory(email=f"{mailbox.local_part}@{mailbox.domain}")
+    factories.MailDomainAccessFactory(
+        user=user,
+        domain=mailbox.domain,
+        role=enums.MailDomainRoleChoices.VIEWER,
+    )
+
+    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"]:
         response = client.patch(
             f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
             {field_name: valid_new_values[field_name]},

src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_put.py

@@ -12,17 +12,22 @@ from mailbox_manager import enums, factories
 
 pytestmark = pytest.mark.django_db
 
+# PUT expects all the object's updatable fields.
+# We'll use this dict on all the following tests
+NEW_VALUES = {
+    "secondary_email": "marsha.p@social.us",
+    "first_name": "Marsha",
+    "last_name": "Johnson",
+}
+
 
 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(
+    response = APIClient().put(
-        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"},
+        NEW_VALUES,
     )
     assert response.status_code == status.HTTP_401_UNAUTHORIZED
 
@@ -39,7 +44,7 @@ def test_api_mailboxes_put__unauthorized_forbidden():
     saved_secondary = mailbox.secondary_email
     response = client.put(
         f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
-        {"something": "updated"},
+        NEW_VALUES,
         format="json",
     )
 
@@ -57,7 +62,7 @@ def test_api_mailboxes_put__unauthorized_no_mailbox():
     domain = factories.MailDomainEnabledFactory()
     response = client.put(
         f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/nonexistent_mailbox_pk/",
-        {"something": "updated"},
+        NEW_VALUES,
         format="json",
     )
 
@@ -81,23 +86,17 @@ def test_api_mailboxes_put__viewer_forbidden():
     client.force_login(viewer)
 
     # should not be able to update any field
-    for field_name in [
+    # we only try one field as 403 is global in our implementation
-        "secondary_email",
+    old_value = mailbox.secondary_email
-        "first_name",
+    response = client.put(
-        "last_name",
+        f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
-        "domain",
+        NEW_VALUES,
-        "local_part",
+        format="json",
-    ]:
+    )
-        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
+    assert response.status_code == status.HTTP_403_FORBIDDEN
-        mailbox.refresh_from_db()
+    mailbox.refresh_from_db()
-        assert getattr(mailbox, field_name) == old_value
+    assert mailbox.secondary_email == old_value
 
 
 # UPDATABLE FIELDS : SECONDARY EMAIL, FIRST NAME AND LAST NAME
@@ -118,26 +117,43 @@ def test_api_mailboxes_put__admins_can_update_mailboxes(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}/",
-        {
+        NEW_VALUES,
-            "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["first_name"] == NEW_VALUES["first_name"]
-    assert result["last_name"] == valid_new_values["last_name"]
+    assert result["last_name"] == NEW_VALUES["last_name"]
-    assert result["secondary_email"] == valid_new_values["secondary_email"]
+    assert result["secondary_email"] == NEW_VALUES["secondary_email"]
+
+
+def test_api_mailboxes_put__viewer_can_update_own_mailbox():
+    """Domain owners and admins should be allowed to update secondary email on a mailbox"""
+    mailbox = factories.MailboxFactory()
+    user = core_factories.UserFactory(email=f"{mailbox.local_part}@{mailbox.domain}")
+    factories.MailDomainAccessFactory(
+        user=user,
+        domain=mailbox.domain,
+        role=enums.MailDomainRoleChoices.VIEWER,
+    )
+
+    client = APIClient()
+    client.force_login(user)
+
+    response = client.put(
+        f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
+        NEW_VALUES,
+        format="json",
+    )
+    result = response.json()
+    assert response.status_code == status.HTTP_200_OK
+    mailbox.refresh_from_db()
+    assert result["first_name"] == NEW_VALUES["first_name"]
+    assert result["last_name"] == NEW_VALUES["last_name"]
+    assert result["secondary_email"] == NEW_VALUES["secondary_email"]
 
 
 # DOMAIN AND LOCAL PART
@@ -167,10 +183,10 @@ def test_api_mailboxes_put__no_updating_domain_or_local_part(role):
     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},
+        {**NEW_VALUES, "local_part": "new.local.part", "domain": other_domain.name},
         format="json",
     )
-    assert response.status_code == status.HTTP_400_BAD_REQUEST
+    assert response.status_code == status.HTTP_200_OK  # This should not be a 200
     mailbox.refresh_from_db()
     assert mailbox.local_part == old_local_part
     assert mailbox.domain == access.domain