diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c260c..cefea6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/backend/mailbox_manager/admin.py b/src/backend/mailbox_manager/admin.py index c3bb5e0..1c764da 100644 --- a/src/backend/mailbox_manager/admin.py +++ b/src/backend/mailbox_manager/admin.py @@ -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): @@ -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) @@ -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") diff --git a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py index 212f179..7e8bff7 100644 --- a/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py +++ b/src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py @@ -11,6 +11,7 @@ from django.test.utils import override_settings 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 @@ -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, diff --git a/src/backend/mailbox_manager/tests/test_utils_dimail_client.py b/src/backend/mailbox_manager/tests/test_utils_dimail_client.py new file mode 100644 index 0000000..d5e79e2 --- /dev/null +++ b/src/backend/mailbox_manager/tests/test_utils_dimail_client.py @@ -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": "johndoe@wrongdomain.com", + "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"]] diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index 9cbfaa5..f2856c4 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -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 @@ -13,6 +16,8 @@ import requests from rest_framework import status from urllib3.util import Retry +from mailbox_manager import models + logger = getLogger(__name__) adapter = requests.adapters.HTTPAdapter( @@ -123,7 +128,7 @@ class DimailAPIClient: 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}" ) @@ -163,3 +168,66 @@ class DimailAPIClient: 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 + + 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