diff --git a/README.md b/README.md index cc5984ed..4bf94f96 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,7 @@ To run in docker do the following ```properties EMAIL_SMTP_PORT=587 ``` +- You may need to acquire additional credentials from [Vault](https://vault.ci.uktrade.digital/) - To build and run a local Postfix [mail server](https://github.com/uktrade/mailserver) - To initilize database `PIPENV_DOTENV_LOCATION=.env pipenv run ./manage.py migrate` diff --git a/conf/settings.py b/conf/settings.py index bba59b77..b2f0379c 100644 --- a/conf/settings.py +++ b/conf/settings.py @@ -237,3 +237,7 @@ INSTALLED_APPS.append("elasticapm.contrib.django") DEFAULT_ENCODING = "iso-8859-1" + +AZURE_AUTH_CLIENT_ID = env.str("AZURE_AUTH_CLIENT_ID") +AZURE_AUTH_CLIENT_SECRET = env.str("AZURE_AUTH_CLIENT_SECRET") +AZURE_AUTH_TENANT_ID = env.str("AZURE_AUTH_TENANT_ID") diff --git a/local.env b/local.env index bac41124..83b13f55 100644 --- a/local.env +++ b/local.env @@ -50,3 +50,7 @@ SPIRE_STANDIN_EMAIL_PASSWORD=password SPIRE_STANDIN_EMAIL_POP3_PORT=995 LITE_API_URL=https://lite.example.com + +AZURE_AUTH_CLIENT_ID= +AZURE_AUTH_CLIENT_SECRET= +AZURE_AUTH_TENANT_ID= diff --git a/mail/libraries/routing_controller.py b/mail/libraries/routing_controller.py index 2f1189ce..f1f681eb 100644 --- a/mail/libraries/routing_controller.py +++ b/mail/libraries/routing_controller.py @@ -7,7 +7,7 @@ from rest_framework.exceptions import ValidationError from conf.settings import SPIRE_ADDRESS -from mail.auth import BasicAuthentication +from mail.auth import BasicAuthentication, ModernAuthentication from mail.enums import ExtractTypeEnum, MailReadStatuses, ReceptionStatusEnum, SourceEnum from mail.libraries.builders import build_email_message from mail.libraries.data_processors import ( @@ -35,9 +35,11 @@ def get_spire_to_dit_mailserver() -> MailServer: These are licenceData and usageReply emails. They are processed by the service and sent to HMRC. """ - auth = BasicAuthentication( + auth = ModernAuthentication( user=settings.INCOMING_EMAIL_USER, - password=settings.INCOMING_EMAIL_PASSWORD, + client_id=settings.AZURE_AUTH_CLIENT_ID, + client_secret=settings.AZURE_AUTH_CLIENT_SECRET, + tenant_id=settings.AZURE_AUTH_TENANT_ID, ) return MailServer( @@ -53,9 +55,11 @@ def get_hmrc_to_dit_mailserver() -> MailServer: These are licenceReply and usageData emails """ - auth = BasicAuthentication( + auth = ModernAuthentication( user=settings.HMRC_TO_DIT_EMAIL_USER, - password=settings.HMRC_TO_DIT_EMAIL_PASSWORD, + client_id=settings.AZURE_AUTH_CLIENT_ID, + client_secret=settings.AZURE_AUTH_CLIENT_SECRET, + tenant_id=settings.AZURE_AUTH_TENANT_ID, ) return MailServer( @@ -78,19 +82,6 @@ def get_mock_hmrc_mailserver() -> MailServer: ) -def get_spire_standin_mailserver() -> MailServer: - auth = BasicAuthentication( - user=settings.SPIRE_STANDIN_EMAIL_USER, - password=settings.SPIRE_STANDIN_EMAIL_PASSWORD, - ) - - return MailServer( - auth, - hostname=settings.SPIRE_STANDIN_EMAIL_HOSTNAME, - pop3_port=settings.SPIRE_STANDIN_EMAIL_POP3_PORT, - ) - - def check_and_route_emails(): logger.info("Checking for emails") hmrc_to_dit_server = get_hmrc_to_dit_mailserver() diff --git a/mail/tests/libraries/test_routing_controller.py b/mail/tests/libraries/test_routing_controller.py new file mode 100644 index 00000000..fee776e6 --- /dev/null +++ b/mail/tests/libraries/test_routing_controller.py @@ -0,0 +1,99 @@ +import unittest +from unittest.mock import patch + +from django.test import override_settings + +from mail.auth import BasicAuthentication, ModernAuthentication +from mail.libraries.routing_controller import ( + get_hmrc_to_dit_mailserver, + get_mock_hmrc_mailserver, + get_spire_to_dit_mailserver, +) + + +class RoutingControllerTest(unittest.TestCase): + @patch( + "mail.libraries.routing_controller.ModernAuthentication", + spec=ModernAuthentication, + ) + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( # nosec + INCOMING_EMAIL_USER="incoming.email.user@example.com", + INCOMING_EMAIL_HOSTNAME="host.example.com", + INCOMING_EMAIL_POP3_PORT="123", + AZURE_AUTH_CLIENT_ID="azure-auth-client-id", + AZURE_AUTH_CLIENT_SECRET="azure-auth-client-secret", + AZURE_AUTH_TENANT_ID="azure-auth-tenant-id", + ) + def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_ModernAuthentication): + spire_to_dit_mailserver = get_spire_to_dit_mailserver() + + mock_ModernAuthentication.assert_called_with( # nosec + user="incoming.email.user@example.com", + client_id="azure-auth-client-id", + client_secret="azure-auth-client-secret", + tenant_id="azure-auth-tenant-id", + ) + mock_MailServer.assert_called_with( + mock_ModernAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(spire_to_dit_mailserver, mock_MailServer()) + + @patch( + "mail.libraries.routing_controller.ModernAuthentication", + spec=ModernAuthentication, + ) + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( # nosec + HMRC_TO_DIT_EMAIL_USER="hmrc.to.dit.email.user@example.com", + HMRC_TO_DIT_EMAIL_HOSTNAME="host.example.com", + HMRC_TO_DIT_EMAIL_POP3_PORT="123", + AZURE_AUTH_CLIENT_ID="azure-auth-client-id", + AZURE_AUTH_CLIENT_SECRET="azure-auth-client-secret", + AZURE_AUTH_TENANT_ID="azure-auth-tenant-id", + ) + def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_ModernAuthentication): + hmrc_to_dit_mailserver = get_hmrc_to_dit_mailserver() + + mock_ModernAuthentication.assert_called_with( # nosec + user="hmrc.to.dit.email.user@example.com", + client_id="azure-auth-client-id", + client_secret="azure-auth-client-secret", + tenant_id="azure-auth-tenant-id", + ) + mock_MailServer.assert_called_with( + mock_ModernAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(hmrc_to_dit_mailserver, mock_MailServer()) + + @patch( + "mail.libraries.routing_controller.BasicAuthentication", + spec=BasicAuthentication, + ) + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( # nosec + MOCK_HMRC_EMAIL_USER="mock.hmrc.email.user@example.com", + MOCK_HMRC_EMAIL_PASSWORD="shhh", + MOCK_HMRC_EMAIL_HOSTNAME="host.example.com", + MOCK_HMRC_EMAIL_POP3_PORT="123", + ) + def test_get_mock_hmrc_mailserver(self, mock_MailServer, mock_BasicAuthentication): + mock_hmrc_mailserver = get_mock_hmrc_mailserver() + + mock_BasicAuthentication.assert_called_with( # nosec + user="mock.hmrc.email.user@example.com", + password="shhh", + ) + mock_MailServer.assert_called_with( + mock_BasicAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(mock_hmrc_mailserver, mock_MailServer()) diff --git a/mail/tests/test_resend_email.py b/mail/tests/test_resend_email.py index dbb88b32..1ee22a00 100644 --- a/mail/tests/test_resend_email.py +++ b/mail/tests/test_resend_email.py @@ -12,9 +12,17 @@ class LITEHMRCResendEmailTests(LiteHMRCTestClient): + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_resend_licence_data_mail_to_hmrc(self, email_dtos, send_mail): + def test_resend_licence_data_mail_to_hmrc( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): """ Tests resending of licence data mail to HMRC Initially we setup an email and send it to HMRC and this sets the mail is in the @@ -56,9 +64,17 @@ def test_resend_licence_data_mail_to_hmrc(self, email_dtos, send_mail): self.assertEqual(mail.extract_type, ExtractTypeEnum.LICENCE_DATA) self.assertEqual(send_mail.call_count, 2) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_resend_licence_reply_mail_to_spire(self, email_dtos, send_mail): + def test_resend_licence_reply_mail_to_spire( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): source_run_number = 49530 hmrc_run_number = 49543 filename = self.licence_reply_file_name @@ -115,9 +131,17 @@ def test_resend_licence_reply_mail_to_spire(self, email_dtos, send_mail): self.assertEqual(mail.extract_type, ExtractTypeEnum.LICENCE_REPLY) send_mail.assert_called_once() + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_resend_usage_data_mail_to_spire(self, email_dtos, send_mail): + def test_resend_usage_data_mail_to_spire( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): source_run_number = 49530 hmrc_run_number = 49543 filename = self.licence_usage_file_name diff --git a/mail/tests/test_select_email_for_sending.py b/mail/tests/test_select_email_for_sending.py index 65cafd73..3c09711e 100644 --- a/mail/tests/test_select_email_for_sending.py +++ b/mail/tests/test_select_email_for_sending.py @@ -102,9 +102,17 @@ def test_email_selected_if_no_spire_data(self): self.assertEqual(mail, mail_1) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_case1_sending_of_pending_licencedata_mails(self, email_dtos, send_mail): + def test_case1_sending_of_pending_licencedata_mails( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): """ Ensure pending mails are sent and status updated as expected. @@ -154,9 +162,17 @@ def test_case1_sending_of_pending_licencedata_mails(self, email_dtos, send_mail) send_mail.assert_called_once() self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_PENDING) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_case2_sending_of_pending_usagedata_mails(self, email_dtos, send_mail): + def test_case2_sending_of_pending_usagedata_mails( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): """ Case2: When only usageData mails are pending. Multiple mails are possible if none are received over the weekend and both arrived on monday. @@ -211,9 +227,17 @@ def test_case2_sending_of_pending_usagedata_mails(self, email_dtos, send_mail): self.assertEqual(send_mail.call_count, int(i + 1)) self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_SENT) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1(self, email_dtos, send_mail): + def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): """ Case3.1: When both licenceData and usageData mails are pending. This is possible if we haven't received any usageData files over the weekend and on monday they have @@ -293,9 +317,17 @@ def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1(self, email_ elif mail.extract_type == ExtractTypeEnum.USAGE_DATA: self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_SENT) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2(self, email_dtos, send_mail): + def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): """ Another variation of case3 is, @@ -361,9 +393,17 @@ def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2(self, email_ send_mail.assert_called_once() self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_PENDING) + @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver") + @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver") @mock.patch("mail.libraries.routing_controller.send") @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") - def test_case4_sending_of_pending_licencedata_when_waiting_for_reply(self, email_dtos, send_mail): + def test_case4_sending_of_pending_licencedata_when_waiting_for_reply( + self, + email_dtos, + send_mail, + mock_get_hmrc_to_dit_mailserver, + mock_get_spire_to_dit_mailserver, + ): """ Another variation of case3 is, diff --git a/pii-secret-exclude.txt b/pii-secret-exclude.txt index 7becf6f2..9f38e493 100644 --- a/pii-secret-exclude.txt +++ b/pii-secret-exclude.txt @@ -18,3 +18,4 @@ mail/tests/files/licence_payload_file mail/libraries/helpers.py mail/auth.py mail/tests/test_auth.py +mail/tests/libraries/test_routing_controller.py