Skip to content

Commit

Permalink
🐛(maildomain) fix domain access creation
Browse files Browse the repository at this point in the history
During a new domain creation, a call to dimail is made
to create user/allow on dimail side before owner role creation
on our side.
So when user/allow creation on dimain side fails,
the owner role is not created on our side.
Therefore the domain is created but invisible on the user interface.
The user will probably try to create the same domain again
and see the error message 'this domain already exists'.
To avoid this we make sure to create owner role on our side
despite dimail failure and set domain to failed status to retry
later dimail access creation.
  • Loading branch information
sdemagny committed Dec 17, 2024
1 parent fa80edf commit e4936ef
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ and this project adheres to

## [Unreleased]

### Fixed

- 🐛(backend) fix manage roles on domain admin view

### Added

- ✨(organizations) add siret to name conversion #584
Expand Down
18 changes: 14 additions & 4 deletions src/backend/mailbox_manager/api/client/serializers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
"""Client serializers for People's mailbox manager app."""

import json
from logging import getLogger

from requests.exceptions import HTTPError
from rest_framework import exceptions, serializers

from core.api.client.serializers import UserSerializer
Expand All @@ -10,6 +12,8 @@
from mailbox_manager import enums, models
from mailbox_manager.utils.dimail import DimailAPIClient

logger = getLogger(__name__)


class MailboxSerializer(serializers.ModelSerializer):
"""Serialize mailbox."""
Expand Down Expand Up @@ -190,14 +194,20 @@ def create(self, validated_data):
"""
dimail = DimailAPIClient()

user = validated_data["user"]
domain = validated_data['domain']

if validated_data["role"] in [
enums.MailDomainRoleChoices.ADMIN,
enums.MailDomainRoleChoices.OWNER,
]:
dimail.create_user(validated_data["user"].sub)
dimail.create_allow(
validated_data["user"].sub, validated_data["domain"].name
)
try:
dimail.create_user(user.sub)
dimail.create_allow(user.sub, domain.name)
except HTTPError:
logger.exception("[DIMAIL] access creation failed %s")
domain.status = enums.MailDomainStatusChoices.FAILED
domain.save()

return super().create(validated_data)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,77 @@ def test_api_mail_domains__create_authenticated():
assert domain.accesses.filter(role="owner", user=user).exists()


def test_api_mail_domains__create_authenticated__dimail_failure():
"""
Despite a dimail failure for user and/or allow creation,
an authenticated user should be able to create a mail domain
and should automatically be added as owner of the newly created domain.
"""
user = core_factories.UserFactory()

client = APIClient()
client.force_login(user)

domain_name = "test.domain.fr"

with responses.RequestsMock() as rsps:
rsps.add(
rsps.POST,
re.compile(r".*/domains/"),
body=str(
{
"name": domain_name,
}
),
status=status.HTTP_201_CREATED,
content_type="application/json",
)
rsps.add(
rsps.POST,
re.compile(r".*/users/"),
body=str(
{
"name": "request-user-sub",
"is_admin": "false",
"uuid": "user-uuid-on-dimail",
"perms": [],
}
),
status=status.HTTP_201_CREATED,
content_type="application/json",
)
rsps.add(
rsps.POST,
re.compile(r".*/allows/"),
body=str({"user": "request-user-sub", "domain": str(domain_name)}),
status=status.HTTP_403_FORBIDDEN,
content_type="application/json",
)
response = client.post(
"/api/v1.0/mail-domains/",
{"name": domain_name, "context": "null", "features": ["webmail"]},
format="json",
)
assert response.status_code == status.HTTP_201_CREATED
domain = models.MailDomain.objects.get()

# response is as expected
assert response.json() == {
"id": str(domain.id),
"name": domain.name,
"slug": domain.slug,
"status": enums.MailDomainStatusChoices.FAILED,
"created_at": domain.created_at.isoformat().replace("+00:00", "Z"),
"updated_at": domain.updated_at.isoformat().replace("+00:00", "Z"),
"abilities": domain.get_abilities(user),
}

# a new domain with status "failed" is created and authenticated user is the owner
assert domain.status == enums.MailDomainStatusChoices.FAILED
assert domain.name == domain_name
assert domain.accesses.filter(role="owner", user=user).exists()


## SYNC TO DIMAIL
def test_api_mail_domains__create_dimail_domain(caplog):
"""
Expand Down
2 changes: 1 addition & 1 deletion src/backend/mailbox_manager/utils/dimail.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ def raise_exception_for_unexpected_response(self, response):
except json.decoder.JSONDecodeError:
error_content = response.content.decode(response.encoding)

logger.error(
logger.exception(
"[DIMAIL] unexpected error : %s %s", response.status_code, error_content
)
raise requests.exceptions.HTTPError(
Expand Down

0 comments on commit e4936ef

Please sign in to comment.