Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to

### Added

- ✨(dimail) synchronize mailboxes from dimail to our db #453
- ✨(ci) add security scan #429
- ✨(teams) register contacts on admin views

Expand Down
43 changes: 35 additions & 8 deletions src/backend/mailbox_manager/admin.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,35 @@
"""Admin classes and registrations for People's mailbox manager app."""

from django.contrib import admin
from django.contrib import admin, messages
from django.utils.translation import gettext_lazy as _

from requests import exceptions

from mailbox_manager import models
from mailbox_manager.utils.dimail import DimailAPIClient


@admin.action(description=_("Synchronise from dimail"))
def sync_mailboxes_from_dimail(modeladmin, request, queryset): # pylint: disable=unused-argument
"""Admin action to synchronize existing mailboxes from dimail to our database."""
client = DimailAPIClient()

for domain in queryset:
try:
imported_mailboxes = client.synchronize_mailboxes_from_dimail(domain)
except exceptions.HTTPError as err:
messages.error(
request,
_(f"Synchronisation failed for {domain.name} with message: [{err}]"),
)
else:
messages.success(
request,
_(
f"Synchronisation succeed for {domain.name}. "
f"Imported mailboxes: {', '.join(imported_mailboxes)}"
),
)


class UserMailDomainAccessInline(admin.TabularInline):
Expand All @@ -28,6 +54,14 @@ class MailDomainAdmin(admin.ModelAdmin):
search_fields = ("name",)
readonly_fields = ["created_at", "slug"]
inlines = (UserMailDomainAccessInline,)
actions = (sync_mailboxes_from_dimail,)


@admin.register(models.Mailbox)
class MailboxAdmin(admin.ModelAdmin):
"""Admin for mailbox model."""

list_display = ("__str__", "first_name", "last_name")


@admin.register(models.MailDomainAccess)
Expand All @@ -50,10 +84,3 @@ class MailDomainAccessInline(admin.TabularInline):
autocomplete_fields = ["user", "domain"]
model = models.MailDomainAccess
readonly_fields = ("created_at", "updated_at")


@admin.register(models.Mailbox)
class MailboxAdmin(admin.ModelAdmin):
"""Admin for mailbox model."""

list_display = ("__str__", "first_name", "last_name")
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from django.utils.translation import gettext_lazy as _

import pytest
import requests
import responses
from rest_framework import status
from rest_framework.test import APIClient
Expand Down Expand Up @@ -531,7 +532,7 @@ def test_api_mailboxes__handling_dimail_unexpected_error(mock_error):
content_type="application/json",
)

with pytest.raises(SystemError):
with pytest.raises(requests.exceptions.HTTPError):
response = client.post(
f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/",
mailbox_data,
Expand Down
156 changes: 156 additions & 0 deletions src/backend/mailbox_manager/tests/test_utils_dimail_client.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
"""
Unit tests for dimail client
"""

import re
from email.errors import HeaderParseError, NonASCIILocalPartDefect
from logging import Logger
from unittest import mock

import pytest
import responses
from rest_framework import status

from mailbox_manager import factories, models
from mailbox_manager.utils.dimail import DimailAPIClient

pytestmark = pytest.mark.django_db


def test_dimail_synchronization__already_sync():
"""
No mailbox should be created when everything is already synced.
"""
domain = factories.MailDomainEnabledFactory()
factories.MailboxFactory.create_batch(3, domain=domain)

pre_sync_mailboxes = models.Mailbox.objects.filter(domain=domain)
assert pre_sync_mailboxes.count() == 3

dimail_client = DimailAPIClient()
with responses.RequestsMock() as rsps:
# Ensure successful response using "responses":
rsps.add(
rsps.GET,
re.compile(r".*/token/"),
body='{"access_token": "dimail_people_token"}',
status=status.HTTP_200_OK,
content_type="application/json",
)
rsps.add(
rsps.GET,
re.compile(rf".*/domains/{domain.name}/mailboxes/"),
body=str(
[
{
"type": "mailbox",
"status": "broken",
"email": f"{mailbox.local_part}@{domain.name}",
"givenName": mailbox.first_name,
"surName": mailbox.last_name,
"displayName": f"{mailbox.first_name} {mailbox.last_name}",
}
for mailbox in pre_sync_mailboxes
]
),
status=status.HTTP_200_OK,
content_type="application/json",
)
imported_mailboxes = dimail_client.synchronize_mailboxes_from_dimail(domain)

post_sync_mailboxes = models.Mailbox.objects.filter(domain=domain)
assert post_sync_mailboxes.count() == 3
assert imported_mailboxes == []
assert set(models.Mailbox.objects.filter(domain=domain)) == set(pre_sync_mailboxes)


@mock.patch.object(Logger, "warning")
def test_dimail_synchronization__synchronize_mailboxes(mock_warning):
"""A mailbox existing solely on dimail should be synchronized
upon calling sync function on its domain"""
domain = factories.MailDomainEnabledFactory()
assert not models.Mailbox.objects.exists()

dimail_client = DimailAPIClient()
with responses.RequestsMock() as rsps:
# Ensure successful response using "responses":
rsps.add(
rsps.GET,
re.compile(r".*/token/"),
body='{"access_token": "dimail_people_token"}',
status=status.HTTP_200_OK,
content_type="application/json",
)

mailbox_valid = {
"type": "mailbox",
"status": "broken",
"email": f"oxadmin@{domain.name}",
"givenName": "Admin",
"surName": "Context",
"displayName": "Context Admin",
}
mailbox_with_wrong_domain = {
"type": "mailbox",
"status": "broken",
"email": "[email protected]",
"givenName": "John",
"surName": "Doe",
"displayName": "John Doe",
}
mailbox_with_invalid_domain = {
"type": "mailbox",
"status": "broken",
"email": f"naw@ake@{domain.name}",
"givenName": "Joe",
"surName": "Doe",
"displayName": "Joe Doe",
}
mailbox_with_invalid_local_part = {
"type": "mailbox",
"status": "broken",
"email": f"obalmaské@{domain.name}",
"givenName": "Jean",
"surName": "Vang",
"displayName": "Jean Vang",
}

rsps.add(
rsps.GET,
re.compile(rf".*/domains/{domain.name}/mailboxes/"),
body=str(
[
mailbox_valid,
mailbox_with_wrong_domain,
mailbox_with_invalid_domain,
mailbox_with_invalid_local_part,
]
),
status=status.HTTP_200_OK,
content_type="application/json",
)

imported_mailboxes = dimail_client.synchronize_mailboxes_from_dimail(domain)

# 3 imports failed: wrong domain, HeaderParseError, NonASCIILocalPartDefect
assert mock_warning.call_count == 3

# first we try to import email with a wrong domain
assert mock_warning.call_args_list[0][0] == (
"Import of email %s failed because of a wrong domain",
mailbox_with_wrong_domain["email"],
)

# then we try to import email with invalid domain
invalid_mailbox_log = mock_warning.call_args_list[1][0]
assert invalid_mailbox_log[1] == mailbox_with_invalid_domain["email"]
assert isinstance(invalid_mailbox_log[2], HeaderParseError)

# finally we try to import email with non ascii local part
non_ascii_mailbox_log = mock_warning.call_args_list[2][0]
assert non_ascii_mailbox_log[1] == mailbox_with_invalid_local_part["email"]
assert isinstance(non_ascii_mailbox_log[2], NonASCIILocalPartDefect)

mailbox = models.Mailbox.objects.get()
assert mailbox.local_part == "oxadmin"
assert imported_mailboxes == [mailbox_valid["email"]]
70 changes: 69 additions & 1 deletion src/backend/mailbox_manager/utils/dimail.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
"""A minimalist client to synchronize with mailbox provisioning API."""

import ast
import smtplib
from email.errors import HeaderParseError, NonASCIILocalPartDefect
from email.headerregistry import Address
from logging import getLogger

from django.conf import settings
Expand All @@ -13,6 +16,8 @@
from rest_framework import status
from urllib3.util import Retry

from mailbox_manager import models

logger = getLogger(__name__)

adapter = requests.adapters.HTTPAdapter(
Expand Down Expand Up @@ -123,7 +128,7 @@ def pass_dimail_unexpected_response(self, response):
logger.error(
"[DIMAIL] unexpected error : %s %s", response.status_code, error_content
)
raise SystemError(
raise requests.exceptions.HTTPError(
f"Unexpected response from dimail: {response.status_code} {error_content}"
)

Expand Down Expand Up @@ -163,3 +168,66 @@ def send_new_mailbox_notification(self, recipient, mailbox_data):
recipient,
exception,
)

def synchronize_mailboxes_from_dimail(self, domain):
"""Synchronize mailboxes from dimail - open xchange to our database.
This is useful in case of acquisition of a pre-existing mail domain.
Mailboxes created here are not new mailboxes and will not trigger mail notification."""

try:
response = session.get(
f"{self.API_URL}/domains/{domain.name}/mailboxes/",
headers=self.get_headers(),
verify=True,
timeout=10,
)
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_200_OK:
return self.pass_dimail_unexpected_response(response)

dimail_mailboxes = ast.literal_eval(
response.content.decode("utf-8")
) # format output str to proper list
Comment on lines +195 to +197
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is response.json() not proper here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think dimail's response is not json-ready !


people_mailboxes = models.Mailbox.objects.filter(domain=domain)
imported_mailboxes = []
for dimail_mailbox in dimail_mailboxes:
if not dimail_mailbox["email"] in [
str(people_mailbox) for people_mailbox in people_mailboxes
]:
try:
# sometimes dimail api returns email from another domain,
# so we decide to exclude this kind of email
address = Address(addr_spec=dimail_mailbox["email"])
if address.domain == domain.name:
# creates a mailbox on our end
mailbox = models.Mailbox.objects.create(
first_name=dimail_mailbox["givenName"],
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.
)
imported_mailboxes.append(str(mailbox))
else:
logger.warning(
"Import of email %s failed because of a wrong domain",
dimail_mailbox["email"],
)
except (HeaderParseError, NonASCIILocalPartDefect) as err:
logger.warning(
"Import of email %s failed with error %s",
dimail_mailbox["email"],
err,
)
return imported_mailboxes
Loading