(mailbox) remove secondary email as required field

The secondary email address is no longer required for all creation
processes and we should not force the user to provide and store an
insecure email address.
This commit is contained in:
Sabrina Demagny
2025-04-08 22:14:08 +02:00
parent 99d7b23dc9
commit 7a1fc6b626
8 changed files with 89 additions and 32 deletions

View File

@@ -8,6 +8,10 @@ and this project adheres to
## [Unreleased]
### Changed
- ✨(mailbox) remove secondary email as required field
## [1.15.0] - 2025-04-04
### Added

View File

@@ -53,12 +53,19 @@ class MailboxSerializer(serializers.ModelSerializer):
mailbox.status = enums.MailDomainStatusChoices.ENABLED
mailbox.save()
if mailbox.secondary_email:
# send confirmation email
client.notify_mailbox_creation(
recipient=mailbox.secondary_email,
mailbox_data=response.json(),
issuer=self.context["request"].user,
)
else:
logger.warning(
"Email notification for %s creation not sent "
"because no secondary email found",
mailbox,
)
# actually save mailbox on our database
return mailbox

View File

@@ -241,7 +241,7 @@ class MailBoxViewSet(
- first_name: str
- last_name: str
- local_part: str
- secondary_email: str
- secondary_email: str (optional)
Sends request to email provisioning API and returns newly created mailbox
POST /api/<version>/mail-domains/<domain_slug>/mailboxes/<mailbox_id>/disable/

View File

@@ -0,0 +1,18 @@
# Generated by Django 5.1.8 on 2025-04-08 20:07
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('mailbox_manager', '0024_domaininvitation'),
]
operations = [
migrations.AlterField(
model_name='mailbox',
name='secondary_email',
field=models.EmailField(blank=True, max_length=254, null=True, verbose_name='secondary email address'),
),
]

View File

@@ -284,7 +284,7 @@ class Mailbox(AbstractBaseUser, BaseModel):
blank=False,
)
secondary_email = models.EmailField(
_("secondary email address"), null=False, blank=False
_("secondary email address"), null=True, blank=True
)
status = models.CharField(
max_length=20,

View File

@@ -197,7 +197,7 @@ def test_api_mailboxes__create_with_accent_success(role):
def test_api_mailboxes__create_administrator_missing_fields():
"""
Administrator users should not be able to create mailboxes
without local part or secondary mail.
without local part.
"""
mail_domain = factories.MailDomainEnabledFactory()
access = factories.MailDomainAccessFactory(
@@ -219,18 +219,54 @@ def test_api_mailboxes__create_administrator_missing_fields():
assert not models.Mailbox.objects.exists()
assert response.json() == {"local_part": ["This field is required."]}
@responses.activate
@pytest.mark.parametrize(
"role",
[enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN],
)
def test_api_mailboxes__create_without_secondary_email(role, caplog):
"""
Creating a new mailbox should not require a secondary email.
We should be able to create a mailbox but not send any email notification.
"""
mail_domain = factories.MailDomainEnabledFactory()
access = factories.MailDomainAccessFactory(role=role, domain=mail_domain)
client = APIClient()
client.force_login(access.user)
mailbox_values = serializers.MailboxSerializer(
factories.MailboxFactory.build()
).data
del mailbox_values["secondary_email"]
responses.add(
responses.GET,
re.compile(r".*/token/"),
body=TOKEN_OK,
status=status.HTTP_200_OK,
content_type="application/json",
)
responses.add(
responses.POST,
re.compile(rf".*/domains/{mail_domain.name}/mailboxes/"),
body=response_mailbox_created(
f"{mailbox_values['local_part']}@{mail_domain.name}"
),
status=status.HTTP_201_CREATED,
content_type="application/json",
)
response = client.post(
f"/api/v1.0/mail-domains/{mail_domain.slug}/mailboxes/",
mailbox_values,
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert not models.Mailbox.objects.exists()
assert response.json() == {"secondary_email": ["This field is required."]}
assert response.status_code == status.HTTP_201_CREATED
mailbox = models.Mailbox.objects.get()
assert (
caplog.records[0].message
== f"Email notification for {mailbox} creation not sent "
"because no secondary email found"
)
@pytest.mark.parametrize(

View File

@@ -81,18 +81,6 @@ def test_models_mailboxes__domain_cannot_be_null():
# SECONDARY_EMAIL FIELD
def test_models_mailboxes__secondary_email_cannot_be_empty():
"""The "secondary_email" field should not be empty."""
with pytest.raises(exceptions.ValidationError, match="This field cannot be blank"):
factories.MailboxFactory(secondary_email="")
def test_models_mailboxes__secondary_email_cannot_be_null():
"""The "secondary_email" field should not be null."""
with pytest.raises(exceptions.ValidationError, match="This field cannot be null"):
factories.MailboxFactory(secondary_email=None)
@pytest.mark.parametrize(
"domain_status",
[

View File

@@ -334,9 +334,6 @@ class DimailAPIClient:
last_name=dimail_mailbox["surName"],
local_part=address.username,
domain=domain,
secondary_email=dimail_mailbox["email"],
# secondary email is mandatory. Unfortunately, dimail doesn't
# store any. We temporarily give current email as secondary email.
status=enums.MailboxStatusChoices.ENABLED,
password=make_password(None), # unusable password
)
@@ -408,11 +405,18 @@ class DimailAPIClient:
mailbox.status = enums.MailDomainStatusChoices.ENABLED
mailbox.save()
if mailbox.secondary_email:
# send confirmation email
self.notify_mailbox_creation(
recipient=mailbox.secondary_email,
mailbox_data=response.json(),
)
else:
logger.warning(
"Email notification for %s creation not sent "
"because no secondary email found",
mailbox,
)
def check_domain(self, domain):
"""Send a request to dimail to check domain health."""