🛂(permissions) return 404 to users with no domain access

users having no domain access should be forbidden to know the domain is
managed in people. To this effect, 403_FORBIDDEN responses have been
replaced by 404_NOT_FOUND.
This commit is contained in:
Marie PUPO JEAMMET
2025-11-18 19:24:17 +01:00
committed by Marie
parent 15337a2226
commit 3a2b5a6428
26 changed files with 146 additions and 188 deletions

View File

@@ -8,6 +8,7 @@ and this project adheres to
## [Unreleased] ## [Unreleased]
- 🛂(permissions) return 404 to users with no access to domain #985
- ✨(aliases) can create, list and delete aliases #974 - ✨(aliases) can create, list and delete aliases #974
## [1.20.0] - 2025-10-22 ## [1.20.0] - 2025-10-22

View File

@@ -238,23 +238,10 @@ class MailDomainAccessSerializer(serializers.ModelSerializer):
"You must set a domain slug in kwargs to create a new domain access." "You must set a domain slug in kwargs to create a new domain access."
) from exc ) from exc
try: # we're sure that an access exists. otherwise it would have failed at permissions.
access = authenticated_user.mail_domain_accesses.get( access = authenticated_user.mail_domain_accesses.get(
domain__slug=domain_slug domain__slug=domain_slug
) )
except models.MailDomainAccess.DoesNotExist as exc:
raise exceptions.PermissionDenied(
"You are not allowed to manage accesses for this domain."
) from exc
# Authenticated user must be owner or admin of current domain to set new roles
if access.role not in [
enums.MailDomainRoleChoices.OWNER,
enums.MailDomainRoleChoices.ADMIN,
]:
raise exceptions.PermissionDenied(
"You are not allowed to manage accesses for this domain."
)
# only an owner can set an owner role to another user # only an owner can set an owner role to another user
if ( if (
@@ -312,10 +299,6 @@ class MailDomainInvitationSerializer(serializers.ModelSerializer):
) from exc ) from exc
domain = models.MailDomain.objects.get(slug=domain_slug) domain = models.MailDomain.objects.get(slug=domain_slug)
if not domain.get_abilities(user)["manage_accesses"]:
raise exceptions.PermissionDenied(
"You are not allowed to manage invitations for this domain."
)
attrs["domain"] = domain attrs["domain"] = domain
attrs["issuer"] = user attrs["issuer"] = user

View File

@@ -40,7 +40,10 @@ class MailDomainViewSet(
Fetch domain status and expected config from dimail. Fetch domain status and expected config from dimail.
""" """
permission_classes = [permissions.DomainResourcePermission] permission_classes = [
permissions.DomainPermission,
permissions.DomainResourcePermission,
]
serializer_class = serializers.MailDomainSerializer serializer_class = serializers.MailDomainSerializer
filter_backends = [filters.OrderingFilter] filter_backends = [filters.OrderingFilter]
ordering_fields = ["created_at", "name"] ordering_fields = ["created_at", "name"]
@@ -262,7 +265,7 @@ class MailBoxViewSet(
Send a request to partially update mailbox. Cannot modify domain or local_part. Send a request to partially update mailbox. Cannot modify domain or local_part.
""" """
permission_classes = [permissions.DomainPermission] permission_classes = [permissions.DomainResourcePermission]
serializer_class = serializers.MailboxSerializer serializer_class = serializers.MailboxSerializer
filter_backends = [filters.OrderingFilter] filter_backends = [filters.OrderingFilter]
ordering = ["-created_at"] ordering = ["-created_at"]
@@ -283,9 +286,12 @@ class MailBoxViewSet(
def get_permissions(self): def get_permissions(self):
"""Add a specific permission for domain viewers to update their own mailbox.""" """Add a specific permission for domain viewers to update their own mailbox."""
if self.action in ["update", "partial_update"]: if self.action in ["list", "retrieve"]:
permission_classes = [permissions.DomainPermission]
elif self.action in ["update", "partial_update"]:
permission_classes = [ permission_classes = [
permissions.DomainPermission | permissions.IsMailboxOwnerPermission permissions.DomainResourcePermission
| permissions.IsMailboxOwnerPermission
] ]
else: else:
return super().get_permissions() return super().get_permissions()
@@ -421,7 +427,7 @@ class AliasViewSet(
""" """
lookup_field = "local_part" lookup_field = "local_part"
permission_classes = [permissions.DomainPermission] permission_classes = [permissions.DomainResourcePermission]
serializer_class = serializers.AliasSerializer serializer_class = serializers.AliasSerializer
queryset = ( queryset = (
models.Alias.objects.all().select_related("domain").order_by("-created_at") models.Alias.objects.all().select_related("domain").order_by("-created_at")
@@ -445,14 +451,10 @@ class AliasViewSet(
).values("role") ).values("role")
queryset = ( queryset = (
# The logged-in user should be part of a domain to see its accesses queryset
queryset.filter(
domain__accesses__user=self.request.user,
)
# Abilities are computed based on logged-in user's role and # Abilities are computed based on logged-in user's role and
# the user role on each domain access # the user role on each domain access
.annotate(user_role=Subquery(user_role_query)) .annotate(user_role=Subquery(user_role_query)).distinct()
.distinct()
) )
return queryset return queryset
@@ -462,7 +464,7 @@ class AliasViewSet(
if self.action in ["destroy"]: if self.action in ["destroy"]:
permission_classes = [ permission_classes = [
permissions.DomainResourcePermission permissions.DomainResourcePermission
| permissions.IsAliasDestinationPermission | permissions.IsAliasDestinationPermission,
] ]
else: else:
return super().get_permissions() return super().get_permissions()

View File

@@ -1,13 +1,40 @@
"""Permission handlers for the People mailbox manager app.""" """Permission handlers for the People mailbox manager app."""
from django.shortcuts import get_object_or_404
from rest_framework import permissions from rest_framework import permissions
from core.api import permissions as core_permissions from core.api.permissions import IsAuthenticated
from mailbox_manager import models from mailbox_manager import models
class DomainResourcePermission(core_permissions.IsAuthenticated): class DomainPermission(IsAuthenticated):
"""Permission class to manage mailboxes and aliases for a mail domain"""
def has_permission(self, request, view):
"""Check permission based on domain."""
if not super().has_permission(request, view):
return False
if not view.kwargs.get("domain_slug"):
return True
domain = get_object_or_404(
models.MailDomain,
slug=view.kwargs.get("domain_slug", ""),
accesses__user=request.user,
)
# domain = models.MailDomain.objects.get(slug=view.kwargs.get("domain_slug", ""))
abilities = domain.get_abilities(request.user)
if request.method.lower() == "delete":
return abilities.get("manage_accesses", False)
return abilities.get(request.method.lower(), False)
class DomainResourcePermission(DomainPermission):
"""Permission class for access objects.""" """Permission class for access objects."""
def has_object_permission(self, request, view, obj): def has_object_permission(self, request, view, obj):
@@ -16,16 +43,6 @@ class DomainResourcePermission(core_permissions.IsAuthenticated):
return abilities.get(request.method.lower(), False) return abilities.get(request.method.lower(), False)
class DomainPermission(DomainResourcePermission):
"""Permission class to manage mailboxes and aliases for a mail domain"""
def has_permission(self, request, view):
"""Check permission based on domain."""
domain = models.MailDomain.objects.get(slug=view.kwargs.get("domain_slug", ""))
abilities = domain.get_abilities(request.user)
return abilities.get(request.method.lower(), False)
class IsMailboxOwnerPermission(permissions.BasePermission): class IsMailboxOwnerPermission(permissions.BasePermission):
"""Authorize update for domain viewers on their own mailbox.""" """Authorize update for domain viewers on their own mailbox."""
@@ -40,7 +57,7 @@ class IsMailboxOwnerPermission(permissions.BasePermission):
return obj.get_email() == request.user.email return obj.get_email() == request.user.email
class IsAliasDestinationPermission(core_permissions.IsAuthenticated): class IsAliasDestinationPermission(IsAuthenticated):
"""Can delete an alias if the alias points to their own email address.""" """Can delete an alias if the alias points to their own email address."""
def has_permission(self, request, view): def has_permission(self, request, view):

View File

@@ -31,8 +31,8 @@ def test_api_aliases_create__anonymous():
assert not models.Alias.objects.exists() assert not models.Alias.objects.exists()
def test_api_aliases_create__no_access_forbidden(): def test_api_aliases_create__no_access_forbidden_not_found():
"""User authenticated but not having domain permission should not create aliases.""" """Unrelated user not having domain permission should not create aliases."""
domain = factories.MailDomainEnabledFactory() domain = factories.MailDomainEnabledFactory()
client = APIClient() client = APIClient()
@@ -41,7 +41,7 @@ def test_api_aliases_create__no_access_forbidden():
f"/api/v1.0/mail-domains/{domain.slug}/aliases/", f"/api/v1.0/mail-domains/{domain.slug}/aliases/",
{"local_part": "intrusive", "destination": "intrusive@mail.com"}, {"local_part": "intrusive", "destination": "intrusive@mail.com"},
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert not models.Alias.objects.exists() assert not models.Alias.objects.exists()

View File

@@ -29,7 +29,7 @@ def test_api_aliases_delete__anonymous():
assert models.Alias.objects.count() == 1 assert models.Alias.objects.count() == 1
def test_api_aliases_delete__no_access_forbidden(): def test_api_aliases_delete__no_access_forbidden_not_found():
""" """
Authenticated users should not be allowed to delete an alias in a Authenticated users should not be allowed to delete an alias in a
mail domain to which they are not related. mail domain to which they are not related.
@@ -43,7 +43,7 @@ def test_api_aliases_delete__no_access_forbidden():
f"/api/v1.0/mail-domains/{alias.domain.slug}/aliases/{alias.local_part}/", f"/api/v1.0/mail-domains/{alias.domain.slug}/aliases/{alias.local_part}/",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert models.Alias.objects.count() == 1 assert models.Alias.objects.count() == 1

View File

@@ -25,7 +25,7 @@ def test_api_aliases_list__anonymous():
assert response.status_code == status.HTTP_401_UNAUTHORIZED assert response.status_code == status.HTTP_401_UNAUTHORIZED
def test_api_aliases_list__no_access_forbidden(): def test_api_aliases_list__no_access_forbidden_not_found():
"""User authenticated but not having domain permission should not list aliases.""" """User authenticated but not having domain permission should not list aliases."""
factories.MailDomainAccessFactory() # access to another domain factories.MailDomainAccessFactory() # access to another domain
domain = factories.MailDomainEnabledFactory() domain = factories.MailDomainEnabledFactory()
@@ -36,7 +36,7 @@ def test_api_aliases_list__no_access_forbidden():
response = client.get( response = client.get(
f"/api/v1.0/mail-domains/{domain.slug}/aliases/", f"/api/v1.0/mail-domains/{domain.slug}/aliases/",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
@pytest.mark.parametrize( @pytest.mark.parametrize(

View File

@@ -35,7 +35,7 @@ def test_api_domain_invitations__create__anonymous():
} }
def test_api_domain_invitations__create__authenticated_outsider(): def test_api_domain_invitations__create__no_access_forbidden_not_found():
"""Users should not be permitted to send domain management invitations """Users should not be permitted to send domain management invitations
for a domain they don't manage.""" for a domain they don't manage."""
user = core_factories.UserFactory() user = core_factories.UserFactory()
@@ -52,7 +52,7 @@ def test_api_domain_invitations__create__authenticated_outsider():
invitation_values, invitation_values,
format="json", format="json",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
@pytest.mark.parametrize( @pytest.mark.parametrize(
@@ -86,7 +86,7 @@ def test_api_domain_invitations__admin_should_create_invites(role):
assert email.subject == "[La Suite] Vous avez été invité(e) à rejoindre la Régie" assert email.subject == "[La Suite] Vous avez été invité(e) à rejoindre la Régie"
def test_api_domain_invitations__viewers_should_not_invite_to_manage_domains(): def test_api_domain_invitations__no_access_forbidden_not_found():
""" """
Domain viewers should not be able to invite new domain managers. Domain viewers should not be able to invite new domain managers.
""" """
@@ -110,7 +110,7 @@ def test_api_domain_invitations__viewers_should_not_invite_to_manage_domains():
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_403_FORBIDDEN
assert response.json() == { assert response.json() == {
"detail": "You are not allowed to manage invitations for this domain." "detail": "You do not have permission to perform this action."
} }

View File

@@ -26,7 +26,7 @@ def test_api_domain_invitations__anonymous_user_should_not_retrieve_invitations(
assert response.status_code == status.HTTP_401_UNAUTHORIZED assert response.status_code == status.HTTP_401_UNAUTHORIZED
def test_api_domain_invitations__unrelated_user_should_not_retrieve_invitations(): def test_api_domain_invitations__no_access_forbidden_not_found():
""" """
Authenticated unrelated users should not be able to retrieve invitations. Authenticated unrelated users should not be able to retrieve invitations.
""" """
@@ -40,8 +40,8 @@ def test_api_domain_invitations__unrelated_user_should_not_retrieve_invitations(
f"/api/v1.0/mail-domains/{invitation.domain.slug}/invitations/", f"/api/v1.0/mail-domains/{invitation.domain.slug}/invitations/",
) )
assert response.status_code == status.HTTP_200_OK assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json()["count"] == 0 assert response.json() == {"detail": "No MailDomain matches the given query."}
def test_api_domain_invitations__domain_managers_should_list_invitations(): def test_api_domain_invitations__domain_managers_should_list_invitations():

View File

@@ -41,17 +41,14 @@ def test_api_mail_domains__create_name_unique():
""" """
Creating domain should raise an error if already existing name. Creating domain should raise an error if already existing name.
""" """
factories.MailDomainFactory(name="existing_domain.com") existing_domain = factories.MailDomainFactory()
user = core_factories.UserFactory()
client = APIClient() client = APIClient()
client.force_login(user) client.force_login(core_factories.UserFactory())
response = client.post( response = client.post(
"/api/v1.0/mail-domains/", "/api/v1.0/mail-domains/",
{ {"name": existing_domain.name},
"name": "existing_domain.com",
},
) )
assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.status_code == status.HTTP_400_BAD_REQUEST

View File

@@ -2,7 +2,6 @@
Tests for MailDomains API endpoint in People's mailbox manager app. Focus on "fetch" action. Tests for MailDomains API endpoint in People's mailbox manager app. Focus on "fetch" action.
""" """
import json
import re import re
import pytest import pytest
@@ -82,7 +81,7 @@ def test_api_mail_domains__fetch_from_dimail__viewer():
], ],
) )
@responses.activate @responses.activate
def test_api_mail_domains__fetch_from_dimail(role): def test_api_mail_domains__fetch_from_dimail_admin_successful(role):
""" """
Authenticated users should be allowed to fetch a domain Authenticated users should be allowed to fetch a domain
from dimail if they are an owner or admin. from dimail if they are an owner or admin.

View File

@@ -69,7 +69,7 @@ def test_api_mail_domains__retrieve_authenticated_unrelated():
@responses.activate @responses.activate
def test_api_mail_domains__retrieve_authenticated_related(): def test_api_mail_domains__retrieve_authenticated_related_successful():
""" """
Authenticated users should be allowed to retrieve a domain Authenticated users should be allowed to retrieve a domain
to which they have access. to which they have access.

View File

@@ -35,7 +35,7 @@ def test_api_mail_domain__accesses_create_anonymous():
assert models.MailDomainAccess.objects.exists() is False assert models.MailDomainAccess.objects.exists() is False
def test_api_mail_domain__accesses_create_authenticated_unrelated(): def test_api_mail_domain__accesses_create_no_access_forbidden_not_found():
""" """
Authenticated users should not be allowed to create domain accesses for a domain to Authenticated users should not be allowed to create domain accesses for a domain to
which they are not related. which they are not related.
@@ -56,14 +56,14 @@ def test_api_mail_domain__accesses_create_authenticated_unrelated():
format="json", format="json",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json() == { assert response.json() == {
"detail": "You are not allowed to manage accesses for this domain." "detail": "No MailDomain matches the given query.",
} }
assert not models.MailDomainAccess.objects.filter(user=other_user).exists() assert not models.MailDomainAccess.objects.filter(user=other_user).exists()
def test_api_mail_domain__accesses_create_authenticated_viewer(): def test_api_mail_domain__accesses_create_viewer_forbidden():
"""Viewer of a mail domain should not be allowed to create mail domain accesses.""" """Viewer of a mail domain should not be allowed to create mail domain accesses."""
authenticated_user = core_factories.UserFactory() authenticated_user = core_factories.UserFactory()
mail_domain = factories.MailDomainFactory( mail_domain = factories.MailDomainFactory(
@@ -85,7 +85,7 @@ def test_api_mail_domain__accesses_create_authenticated_viewer():
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_403_FORBIDDEN
assert response.json() == { assert response.json() == {
"detail": "You are not allowed to manage accesses for this domain." "detail": "You do not have permission to perform this action.",
} }
assert not models.MailDomainAccess.objects.filter(user=other_user).exists() assert not models.MailDomainAccess.objects.filter(user=other_user).exists()

View File

@@ -13,7 +13,7 @@ from mailbox_manager import enums, factories, models
pytestmark = pytest.mark.django_db pytestmark = pytest.mark.django_db
def test_api_mail_domain__accesses_delete_anonymous(): def test_api_domain_accesses_delete__anonymous_forbidden():
"""Anonymous users should not be allowed to destroy a mail domain access.""" """Anonymous users should not be allowed to destroy a mail domain access."""
access = factories.MailDomainAccessFactory() access = factories.MailDomainAccessFactory()
@@ -25,7 +25,7 @@ def test_api_mail_domain__accesses_delete_anonymous():
assert models.MailDomainAccess.objects.count() == 1 assert models.MailDomainAccess.objects.count() == 1
def test_api_mail_domain__accesses_delete_authenticated(): def test_api_domain_accesses_delete__unrelated_notfound():
""" """
Authenticated users should not be allowed to delete a mail domain access for a Authenticated users should not be allowed to delete a mail domain access for a
mail domain to which they are not related. mail domain to which they are not related.
@@ -39,11 +39,11 @@ def test_api_mail_domain__accesses_delete_authenticated():
f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert models.MailDomainAccess.objects.count() == 1 assert models.MailDomainAccess.objects.count() == 1
def test_api_mail_domain__accesses_delete_viewer(): def test_api_domain_accesses_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 a mail domain access for a
mail domain in which they are a simple viewer. mail domain in which they are a simple viewer.
@@ -65,7 +65,7 @@ def test_api_mail_domain__accesses_delete_viewer():
assert models.MailDomainAccess.objects.filter(user=access.user).exists() assert models.MailDomainAccess.objects.filter(user=access.user).exists()
def test_api_mail_domain__accesses_delete_administrators(): def test_api_domain_accesses_delete__administrators_successful():
""" """
Administrators of a mail domain should be allowed to delete accesses excepted owner accesses. Administrators of a mail domain should be allowed to delete accesses excepted owner accesses.
""" """
@@ -89,7 +89,7 @@ def test_api_mail_domain__accesses_delete_administrators():
assert models.MailDomainAccess.objects.count() == 1 assert models.MailDomainAccess.objects.count() == 1
def test_api_mail_domain__accesses_delete_owners(): def test_api_domain_accesses_delete__owners_successful():
""" """
An owner should be able to delete the mail domain access of another user including An owner should be able to delete the mail domain access of another user including
a owner access. a owner access.
@@ -114,7 +114,7 @@ def test_api_mail_domain__accesses_delete_owners():
assert models.MailDomainAccess.objects.count() == 1 assert models.MailDomainAccess.objects.count() == 1
def test_api_mail_domain__accesses_delete_owners_last_owner(): def test_api_domain_accesses_delete__last_owner_forbidden():
""" """
It should not be possible to delete the last owner access from a mail domain It should not be possible to delete the last owner access from a mail domain
""" """

View File

@@ -27,8 +27,7 @@ def test_api_mail_domain__accesses_list_anonymous():
def test_api_mail_domain__accesses_list_authenticated_unrelated(): def test_api_mail_domain__accesses_list_authenticated_unrelated():
""" """
Authenticated users should not be allowed to list mail_domain accesses for a mail_domain User with no access should not be allowed to list domain accesses.
to which they are not related.
""" """
user = core_factories.UserFactory() user = core_factories.UserFactory()
mail_domain = factories.MailDomainFactory() mail_domain = factories.MailDomainFactory()
@@ -43,12 +42,9 @@ def test_api_mail_domain__accesses_list_authenticated_unrelated():
response = client.get( response = client.get(
f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/", f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/",
) )
assert response.status_code == 200 assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json() == { assert response.json() == {
"count": 0, "detail": "No MailDomain matches the given query.",
"next": None,
"previous": None,
"results": [],
} }
@@ -178,8 +174,12 @@ def test_api_mail_domain__accesses_list_authenticated_constant_numqueries(
# related users : # related users :
# - query retrieving logged-in user for user_role annotation # - query retrieving logged-in user for user_role annotation
# - count from pagination # - count from pagination
# - filter on slug and domain on permission (to return 404 instead of 403 when no access)
# - count ?
# - distinct from viewset # - distinct from viewset
with django_assert_num_queries(3): with django_assert_num_queries(5):
client.get( client.get(
f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/", f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/",
) )
@@ -189,7 +189,7 @@ def test_api_mail_domain__accesses_list_authenticated_constant_numqueries(
factories.MailDomainAccessFactory(domain=mail_domain) factories.MailDomainAccessFactory(domain=mail_domain)
# num queries should still be the same # num queries should still be the same
with django_assert_num_queries(3): with django_assert_num_queries(5):
response = client.get( response = client.get(
f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/", f"/api/v1.0/mail-domains/{mail_domain.slug}/accesses/",
) )

View File

@@ -43,7 +43,7 @@ def test_api_mail_domain__accesses_retrieve_authenticated_unrelated():
f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/", f"/api/v1.0/mail-domains/{access.domain.slug}/accesses/{access.id!s}/",
) )
assert response.status_code == status.HTTP_404_NOT_FOUND assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json() == {"detail": "No MailDomainAccess matches the given query."} assert response.json() == {"detail": "No MailDomain matches the given query."}
# Accesses related to another mail_domain should be excluded even if the user is related to it # Accesses related to another mail_domain should be excluded even if the user is related to it
for other_access in [ for other_access in [
@@ -55,9 +55,7 @@ def test_api_mail_domain__accesses_retrieve_authenticated_unrelated():
) )
assert response.status_code == status.HTTP_404_NOT_FOUND assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json() == { assert response.json() == {"detail": "No MailDomain matches the given query."}
"detail": "No MailDomainAccess matches the given query."
}
def test_api_mail_domain__accesses_retrieve_authenticated_related(): def test_api_mail_domain__accesses_retrieve_authenticated_related():

View File

@@ -27,7 +27,7 @@ def test_api_mail_domain__accesses_update_anonymous():
assert access.role == enums.MailDomainRoleChoices.VIEWER assert access.role == enums.MailDomainRoleChoices.VIEWER
def test_api_mail_domain__accesses_update_authenticated_unrelated(): def test_api_mail_domain__accesses_update_no_access_forbidden_not_found():
""" """
An authenticated user should not be allowed to update a mail domain access An authenticated user should not be allowed to update a mail domain access
for a mail_domain to which they are not related. for a mail_domain to which they are not related.
@@ -42,7 +42,7 @@ def test_api_mail_domain__accesses_update_authenticated_unrelated():
{"role": enums.MailDomainRoleChoices.ADMIN}, {"role": enums.MailDomainRoleChoices.ADMIN},
format="json", format="json",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
access.refresh_from_db() access.refresh_from_db()
assert access.role == enums.MailDomainRoleChoices.VIEWER assert access.role == enums.MailDomainRoleChoices.VIEWER

View File

@@ -30,9 +30,7 @@ def test_api_mail_domain__available_users_anonymous():
def test_api_mail_domain__available_users_forbidden(): def test_api_mail_domain__available_users_forbidden():
"""Authenticated user without accesses on maildomain should not be able to see available """Users with no access should not be able to list available users."""
users.
"""
authenticated_user = core_factories.UserFactory() authenticated_user = core_factories.UserFactory()
client = APIClient() client = APIClient()
client.force_login(authenticated_user) client.force_login(authenticated_user)
@@ -40,7 +38,7 @@ def test_api_mail_domain__available_users_forbidden():
response = client.get(f"/api/v1.0/mail-domains/{maildomain.slug}/accesses/users/") response = client.get(f"/api/v1.0/mail-domains/{maildomain.slug}/accesses/users/")
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
@pytest.mark.parametrize( @pytest.mark.parametrize(

View File

@@ -38,20 +38,23 @@ def test_api_mailboxes__create_anonymous_forbidden(mailbox_data):
assert not models.Mailbox.objects.exists() assert not models.Mailbox.objects.exists()
def test_api_mailboxes__create_authenticated_failure(mailbox_data): @responses.activate
def test_api_mailboxes__create_no_access_forbidden_not_found(mailbox_data):
"""Authenticated users should not be able to create mailbox """Authenticated users should not be able to create mailbox
without specific role on mail domain.""" without specific role on mail domain."""
mail_domain = factories.MailDomainEnabledFactory() mail_domain = factories.MailDomainEnabledFactory()
client = APIClient() client = APIClient()
client.force_login(core_factories.UserFactory()) client.force_login(core_factories.UserFactory())
# We add no simulated response in Responses
# because we expect no "outside" calls to be made
response = client.post( response = client.post(
f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/", f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/",
mailbox_data, mailbox_data,
format="json", format="json",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert not models.Mailbox.objects.exists() assert not models.Mailbox.objects.exists()
@@ -467,28 +470,6 @@ def test_api_mailboxes__existing_alias_bad_request(mailbox_data):
### REACTING TO DIMAIL-API ### REACTING TO DIMAIL-API
### We mock dimail's responses to avoid testing dimail's container too ### We mock dimail's responses to avoid testing dimail's container too
@responses.activate
def test_api_mailboxes__unrelated_user_provisioning_api_not_called(mailbox_data):
"""
Provisioning API should not be called if an user tries
to create a mailbox on a domain they have no access to.
"""
domain = factories.MailDomainEnabledFactory()
client = APIClient()
client.force_login(core_factories.UserFactory()) # user with no access
# We add no simulated response in RequestsMock
# because we expected no "outside" calls to be made
response = client.post(
f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/",
mailbox_data,
format="json",
)
# No exception raised by RequestsMock means no call was sent
# our API blocked the request before sending it
assert response.status_code == status.HTTP_403_FORBIDDEN
@responses.activate @responses.activate
def test_api_mailboxes__domain_viewer_provisioning_api_not_called(mailbox_data): def test_api_mailboxes__domain_viewer_provisioning_api_not_called(mailbox_data):
""" """
@@ -957,7 +938,6 @@ def test_api_mailboxes__send_correct_logger_infos(
assert expected_messages.issubset(actual_messages) assert expected_messages.issubset(actual_messages)
@pytest.mark.usefixtures()
@responses.activate @responses.activate
@mock.patch.object(Logger, "info") @mock.patch.object(Logger, "info")
def test_api_mailboxes__sends_new_mailbox_notification( def test_api_mailboxes__sends_new_mailbox_notification(

View File

@@ -26,7 +26,7 @@ def test_api_mailboxes__disable_anonymous_forbidden():
assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.ENABLED assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.ENABLED
def test_api_mailboxes__disable_authenticated_failure(): def test_api_mailboxes__disable_no_access_forbidden_not_found():
"""Authenticated users should not be able to disable mailbox """Authenticated users should not be able to disable mailbox
without specific role on mail domain.""" without specific role on mail domain."""
user = core_factories.UserFactory() user = core_factories.UserFactory()
@@ -39,7 +39,7 @@ def test_api_mailboxes__disable_authenticated_failure():
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/disable/", f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/disable/",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.ENABLED assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.ENABLED

View File

@@ -26,9 +26,8 @@ def test_api_mailboxes__enable_anonymous_forbidden():
assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.DISABLED assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.DISABLED
def test_api_mailboxes__enable_authenticated_failure(): def test_api_mailboxes__enable_unrelated_not_found():
"""Authenticated users should not be able to enable mailbox """Unrelated users should not be able to enable mailbox."""
without specific role on mail domain."""
user = core_factories.UserFactory() user = core_factories.UserFactory()
client = APIClient() client = APIClient()
@@ -39,7 +38,7 @@ def test_api_mailboxes__enable_authenticated_failure():
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/enable/", f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/enable/",
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.DISABLED assert models.Mailbox.objects.get().status == enums.MailboxStatusChoices.DISABLED

View File

@@ -25,8 +25,8 @@ def test_api_mailboxes__list_anonymous():
} }
def test_api_mailboxes__list_authenticated(): def test_api_mailboxes__list_unrelated_not_found():
"""Authenticated users should not be able to list mailboxes""" """Unrelated users should not be able to list mailboxes"""
user = core_factories.UserFactory() user = core_factories.UserFactory()
client = APIClient() client = APIClient()
@@ -36,10 +36,8 @@ def test_api_mailboxes__list_authenticated():
factories.MailboxFactory.create_batch(2, domain=mail_domain) factories.MailboxFactory.create_batch(2, domain=mail_domain)
response = client.get(f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/") response = client.get(f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/")
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json() == { assert response.json() == {"detail": "No MailDomain matches the given query."}
"detail": "You do not have permission to perform this action."
}
@pytest.mark.parametrize( @pytest.mark.parametrize(

View File

@@ -28,40 +28,22 @@ def test_api_mailboxes_patch__anonymous_forbidden():
assert mailbox.secondary_email == saved_secondary assert mailbox.secondary_email == saved_secondary
def test_api_mailboxes_patch__unauthorized_forbidden(): def test_api_mailboxes_patch__no_access_forbidden_not_found():
"""Authenticated but unauthoriezd users should not be able to update mailboxes.""" """Authenticated but unauthoriezd users should not be able to update mailboxes."""
client = APIClient()
client.force_login(core_factories.UserFactory())
mailbox = factories.MailboxFactory() mailbox = factories.MailboxFactory()
saved_secondary = mailbox.secondary_email saved_secondary = mailbox.secondary_email
response = client.patch(
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/",
{"secondary_email": "updated@example.com"},
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 = APIClient()
client.force_login(core_factories.UserFactory()) client.force_login(core_factories.UserFactory())
domain = factories.MailDomainEnabledFactory()
response = client.patch( response = client.patch(
f"/api/v1.0/mail-domains/{domain.slug}/mailboxes/nonexistent_mailbox_pk/", f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.local_part}/",
{"secondary_email": "updated@example.com"}, {"secondary_email": "updated@example.com"},
format="json", format="json",
) )
# permission denied at domain level # permission denied at domain level
# the existence of the mailbox is not checked assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.status_code == status.HTTP_403_FORBIDDEN mailbox.refresh_from_db()
assert mailbox.secondary_email == saved_secondary
def test_api_mailboxes_patch__viewer_forbidden(): def test_api_mailboxes_patch__viewer_forbidden():
@@ -92,6 +74,23 @@ def test_api_mailboxes_patch__viewer_forbidden():
assert mailbox.secondary_email == old_value assert mailbox.secondary_email == old_value
def test_api_mailboxes_patch__unauthorized_no_mailbox():
"""No mailbox returns a 404."""
access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.ADMIN)
client = APIClient()
client.force_login(access.user)
response = client.patch(
f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/nonexistent_mailbox_pk/",
{"secondary_email": "updated@example.com"},
format="json",
)
# permission denied at domain level
# the existence of the mailbox is not checked
assert response.status_code == status.HTTP_404_NOT_FOUND
# UPDATABLE FIELDS : SECONDARY EMAIL FIRST NAME AND LAST NAME # UPDATABLE FIELDS : SECONDARY EMAIL FIRST NAME AND LAST NAME
@pytest.mark.parametrize( @pytest.mark.parametrize(
"role", "role",

View File

@@ -35,7 +35,7 @@ def test_api_mailboxes_put__anonymous_forbidden():
assert mailbox.secondary_email == saved_secondary assert mailbox.secondary_email == saved_secondary
def test_api_mailboxes_put__unauthorized_forbidden(): def test_api_mailboxes_put__no_access_forbidden_not_found():
"""Authenticated but unauthoriezd users should not be able to update mailboxes.""" """Authenticated but unauthoriezd users should not be able to update mailboxes."""
client = APIClient() client = APIClient()
client.force_login(core_factories.UserFactory()) client.force_login(core_factories.UserFactory())
@@ -49,7 +49,7 @@ def test_api_mailboxes_put__unauthorized_forbidden():
) )
# permission denied at domain level # permission denied at domain level
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
mailbox.refresh_from_db() mailbox.refresh_from_db()
assert mailbox.secondary_email == saved_secondary assert mailbox.secondary_email == saved_secondary
@@ -68,7 +68,7 @@ def test_api_mailboxes_put__unauthorized_no_mailbox():
# permission denied at domain level # permission denied at domain level
# the existence of the mailbox is not checked # the existence of the mailbox is not checked
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
def test_api_mailboxes_put__viewer_forbidden(): def test_api_mailboxes_put__viewer_forbidden():

View File

@@ -28,7 +28,7 @@ def test_api_mailboxes__reset_password_anonymous_unauthorized():
assert response.status_code == status.HTTP_401_UNAUTHORIZED assert response.status_code == status.HTTP_401_UNAUTHORIZED
def test_api_mailboxes__reset_password_unrelated_forbidden(): def test_api_mailboxes__reset_password_no_access_forbidden_not_found():
"""Authenticated users not managing the domain """Authenticated users not managing the domain
should not be able to reset its mailboxes password.""" should not be able to reset its mailboxes password."""
user = core_factories.UserFactory() user = core_factories.UserFactory()
@@ -41,9 +41,9 @@ def test_api_mailboxes__reset_password_unrelated_forbidden():
response = client.post( response = client.post(
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/reset_password/" f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/reset_password/"
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
assert response.json() == { assert response.json() == {
"detail": "You do not have permission to perform this action." "detail": "No MailDomain matches the given query.",
} }

View File

@@ -22,9 +22,9 @@ def test_api_mailboxes__retrieve_anonymous_forbidden():
assert response.status_code == status.HTTP_401_UNAUTHORIZED assert response.status_code == status.HTTP_401_UNAUTHORIZED
def test_api_mailboxes__retrieve_unauthorized_failure(): def test_api_mailboxes__retrieve_no_access_forbidden_not_found():
"""Authenticated but unauthorized users should not be able to """Authenticated but unauthorized users should not be able to
retrieve mailbox.""" retrieve mailbox. Response should be indistinguisable from normal 404"""
client = APIClient() client = APIClient()
client.force_login(core_factories.UserFactory()) client.force_login(core_factories.UserFactory())
@@ -33,15 +33,16 @@ def test_api_mailboxes__retrieve_unauthorized_failure():
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/" f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/"
) )
assert response.status_code == status.HTTP_403_FORBIDDEN assert response.status_code == status.HTTP_404_NOT_FOUND
# 403 or 404 for confidentiality/security purposes ?
# response should be the same whether the mailbox exists or not, so that # even if they have a mailbox on this domain
# unauthorized users can't deduce mailbox existence or nonexistence mailbox = factories.MailboxFactory()
client.force_login(core_factories.UserFactory(email=str(mailbox)))
response = client.get( response = client.get(
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/thismailboxdoesntexist/" f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/"
) )
assert response.status_code == status.HTTP_403_FORBIDDEN
assert response.status_code == status.HTTP_404_NOT_FOUND
def test_api_mailboxes__retrieve_authorized_ok(): def test_api_mailboxes__retrieve_authorized_ok():
@@ -65,17 +66,3 @@ def test_api_mailboxes__retrieve_authorized_ok():
"secondary_email": mailbox.secondary_email, "secondary_email": mailbox.secondary_email,
"status": mailbox.status, "status": mailbox.status,
} }
def test_api_mailboxes__owner_not_authorized():
"""Unauthorized mailbox owner should not be able to retrieve their mailbox."""
mailbox = factories.MailboxFactory()
user = core_factories.UserFactory(email=str(mailbox))
client = APIClient()
client.force_login(user)
response = client.get(
f"/api/v1.0/mail-domains/{mailbox.domain.slug}/mailboxes/{mailbox.pk}/"
)
assert response.status_code == status.HTTP_403_FORBIDDEN