✨(mailboxes) cannot create mailbox with same local part as alias
should not be able to create a mailbox having the same local part as an alias
This commit is contained in:
committed by
Marie
parent
23561cd0e0
commit
0f7e312eb6
@@ -78,6 +78,17 @@ class MailboxSerializer(serializers.ModelSerializer):
|
|||||||
|
|
||||||
return mailbox
|
return mailbox
|
||||||
|
|
||||||
|
def validate_local_part(self, value):
|
||||||
|
"""Validate this local part does not match a mailbox."""
|
||||||
|
if models.Alias.objects.filter(
|
||||||
|
local_part=value, domain__slug=self.context["domain_slug"]
|
||||||
|
):
|
||||||
|
raise exceptions.ValidationError(
|
||||||
|
f'Local part "{value}" already used by an alias.'
|
||||||
|
)
|
||||||
|
|
||||||
|
return value
|
||||||
|
|
||||||
|
|
||||||
class MailboxUpdateSerializer(MailboxSerializer):
|
class MailboxUpdateSerializer(MailboxSerializer):
|
||||||
"""A more restrictive serializer when updating mailboxes"""
|
"""A more restrictive serializer when updating mailboxes"""
|
||||||
@@ -343,7 +354,7 @@ class AliasSerializer(serializers.ModelSerializer):
|
|||||||
local_part=value, domain__slug=self.context["domain_slug"]
|
local_part=value, domain__slug=self.context["domain_slug"]
|
||||||
).exists():
|
).exists():
|
||||||
raise exceptions.ValidationError(
|
raise exceptions.ValidationError(
|
||||||
f'Local part "{value}" already used for a mailbox.'
|
f'Local part "{value}" already used by a mailbox.'
|
||||||
)
|
)
|
||||||
|
|
||||||
return value
|
return value
|
||||||
|
|||||||
@@ -275,6 +275,12 @@ class MailBoxViewSet(
|
|||||||
return self.queryset.filter(domain__slug=domain_slug)
|
return self.queryset.filter(domain__slug=domain_slug)
|
||||||
return self.queryset
|
return self.queryset
|
||||||
|
|
||||||
|
def get_serializer_context(self):
|
||||||
|
"""Extra context provided to the serializer class."""
|
||||||
|
context = super().get_serializer_context()
|
||||||
|
context["domain_slug"] = self.kwargs["domain_slug"]
|
||||||
|
return context
|
||||||
|
|
||||||
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 ["update", "partial_update"]:
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ from django.db import migrations, models
|
|||||||
class Migration(migrations.Migration):
|
class Migration(migrations.Migration):
|
||||||
|
|
||||||
dependencies = [
|
dependencies = [
|
||||||
('mailbox_manager', '0026_alter_mailbox_unique_together_and_more'),
|
('mailbox_manager', '0027_remove_mailbox_unique_ox_display_name_and_more'),
|
||||||
]
|
]
|
||||||
|
|
||||||
operations = [
|
operations = [
|
||||||
@@ -80,7 +80,7 @@ def test_api_aliases_create__duplicate_forbidden():
|
|||||||
assert models.Alias.objects.filter(domain=access.domain).count() == 1
|
assert models.Alias.objects.filter(domain=access.domain).count() == 1
|
||||||
|
|
||||||
|
|
||||||
def test_api_aliases_create__existing_alias_bad_request():
|
def test_api_aliases_create__existing_mailbox_bad_request():
|
||||||
"""Cannot create alias if local_part is already used by a mailbox."""
|
"""Cannot create alias if local_part is already used by a mailbox."""
|
||||||
access = factories.MailDomainAccessFactory(
|
access = factories.MailDomainAccessFactory(
|
||||||
role="owner", domain=factories.MailDomainEnabledFactory()
|
role="owner", domain=factories.MailDomainEnabledFactory()
|
||||||
@@ -95,7 +95,7 @@ def test_api_aliases_create__existing_alias_bad_request():
|
|||||||
)
|
)
|
||||||
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||||
assert response.json() == {
|
assert response.json() == {
|
||||||
"local_part": [f'Local part "{mailbox.local_part}" already used for a mailbox.']
|
"local_part": [f'Local part "{mailbox.local_part}" already used by a mailbox.']
|
||||||
}
|
}
|
||||||
assert not models.Alias.objects.exists()
|
assert not models.Alias.objects.exists()
|
||||||
|
|
||||||
|
|||||||
@@ -3,7 +3,10 @@ Tests for aliases API endpoint in People's app mailbox_manager.
|
|||||||
Focus on "list" action.
|
Focus on "list" action.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
import re
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
import responses
|
||||||
from rest_framework import status
|
from rest_framework import status
|
||||||
from rest_framework.test import APIClient
|
from rest_framework.test import APIClient
|
||||||
|
|
||||||
@@ -87,6 +90,7 @@ def test_api_aliases_delete__viewer_can_delete_self_alias():
|
|||||||
assert not models.Alias.objects.exists()
|
assert not models.Alias.objects.exists()
|
||||||
|
|
||||||
|
|
||||||
|
@responses.activate
|
||||||
def test_api_aliases_delete__administrators_allowed():
|
def test_api_aliases_delete__administrators_allowed():
|
||||||
"""
|
"""
|
||||||
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.
|
||||||
@@ -97,6 +101,14 @@ def test_api_aliases_delete__administrators_allowed():
|
|||||||
)
|
)
|
||||||
alias = factories.AliasFactory(domain=mail_domain)
|
alias = factories.AliasFactory(domain=mail_domain)
|
||||||
|
|
||||||
|
# Mock dimail response
|
||||||
|
responses.add(
|
||||||
|
responses.POST,
|
||||||
|
re.compile(r".*/aliases/"),
|
||||||
|
status=status.HTTP_204_NO_CONTENT,
|
||||||
|
content_type="application/json",
|
||||||
|
)
|
||||||
|
|
||||||
client = APIClient()
|
client = APIClient()
|
||||||
client.force_login(authenticated_user)
|
client.force_login(authenticated_user)
|
||||||
response = client.delete(
|
response = client.delete(
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
"""
|
"""
|
||||||
Unit tests for the mailbox API
|
Unit tests for the mailbox API
|
||||||
"""
|
"""
|
||||||
# pylint: disable=W0613
|
# pylint: disable=W0613, C0302
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import re
|
import re
|
||||||
@@ -436,6 +436,35 @@ def test_api_mailboxes__create_pending_mailboxes(domain_status, mailbox_data):
|
|||||||
assert mailbox.status == "pending"
|
assert mailbox.status == "pending"
|
||||||
|
|
||||||
|
|
||||||
|
def test_api_mailboxes__existing_alias_bad_request(mailbox_data):
|
||||||
|
"""
|
||||||
|
Cannot create mailbox if local_part is already used by an alias.
|
||||||
|
"""
|
||||||
|
alias = factories.AliasFactory()
|
||||||
|
access = factories.MailDomainAccessFactory(
|
||||||
|
role=enums.MailDomainRoleChoices.ADMIN, domain=alias.domain
|
||||||
|
)
|
||||||
|
|
||||||
|
client = APIClient()
|
||||||
|
client.force_login(access.user)
|
||||||
|
# No response because we expect no outside calls to be made
|
||||||
|
response = client.post(
|
||||||
|
f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/",
|
||||||
|
{
|
||||||
|
"local_part": alias.local_part,
|
||||||
|
"first_name": mailbox_data["first_name"],
|
||||||
|
"last_name": mailbox_data["last_name"],
|
||||||
|
"secondary_email": mailbox_data["secondary_email"],
|
||||||
|
},
|
||||||
|
format="json",
|
||||||
|
)
|
||||||
|
assert response.status_code == status.HTTP_400_BAD_REQUEST
|
||||||
|
assert response.json() == {
|
||||||
|
"local_part": [f'Local part "{alias.local_part}" already used by an alias.']
|
||||||
|
}
|
||||||
|
assert not models.Mailbox.objects.exists()
|
||||||
|
|
||||||
|
|
||||||
### 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
|
@responses.activate
|
||||||
|
|||||||
@@ -725,3 +725,44 @@ class DimailAPIClient:
|
|||||||
)
|
)
|
||||||
|
|
||||||
return self.raise_exception_for_unexpected_response(response)
|
return self.raise_exception_for_unexpected_response(response)
|
||||||
|
|
||||||
|
def delete_alias(self, alias, request_user=None):
|
||||||
|
"""Send a Delete alias request to mail provisioning API."""
|
||||||
|
|
||||||
|
headers = self.get_headers()
|
||||||
|
|
||||||
|
try:
|
||||||
|
response = session.delete(
|
||||||
|
f"{self.API_URL}/domains/{alias.domain.name}/aliases/{alias.local_part}/{alias.destination}",
|
||||||
|
json={},
|
||||||
|
headers=headers,
|
||||||
|
verify=True,
|
||||||
|
timeout=self.API_TIMEOUT,
|
||||||
|
)
|
||||||
|
except requests.exceptions.ConnectionError as error:
|
||||||
|
logger.error(
|
||||||
|
"Connection error while trying to reach %s.",
|
||||||
|
self.API_URL,
|
||||||
|
exc_info=error,
|
||||||
|
)
|
||||||
|
raise error
|
||||||
|
|
||||||
|
if response.status_code == status.HTTP_201_CREATED:
|
||||||
|
logger.info(
|
||||||
|
"User %s removed destination %s from alias %s.",
|
||||||
|
request_user,
|
||||||
|
alias.destination,
|
||||||
|
f"{alias.local_part}@{alias.domain}",
|
||||||
|
)
|
||||||
|
return response
|
||||||
|
|
||||||
|
if response.status_code == status.HTTP_403_FORBIDDEN:
|
||||||
|
logger.error(
|
||||||
|
"[DIMAIL] 403 Forbidden: you cannot access domain %s",
|
||||||
|
str(alias.domain),
|
||||||
|
)
|
||||||
|
raise exceptions.PermissionDenied(
|
||||||
|
"Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS."
|
||||||
|
)
|
||||||
|
|
||||||
|
return self.raise_exception_for_unexpected_response(response)
|
||||||
|
|||||||
Reference in New Issue
Block a user