From 4d1998da85d6486ac324815831a904bf7c2296be Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 31 Aug 2022 11:34:46 +0100 Subject: [PATCH 1/7] Add tests for mailserver getters --- .../libraries/test_routing_controller.py | 105 ++++++++++++++++++ pii-secret-exclude.txt | 1 + 2 files changed, 106 insertions(+) create mode 100644 mail/tests/libraries/test_routing_controller.py diff --git a/mail/tests/libraries/test_routing_controller.py b/mail/tests/libraries/test_routing_controller.py new file mode 100644 index 00000000..487e90b6 --- /dev/null +++ b/mail/tests/libraries/test_routing_controller.py @@ -0,0 +1,105 @@ +import unittest +from unittest.mock import patch + +from django.test import override_settings + +from mail.libraries.routing_controller import ( + get_hmrc_to_dit_mailserver, + get_mock_hmrc_mailserver, + get_spire_standin_mailserver, + get_spire_to_dit_mailserver, +) + + +class RoutingControllerTest(unittest.TestCase): + @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( + INCOMING_EMAIL_USER="incoming.email.user@example.com", + INCOMING_EMAIL_PASSWORD="testpassword", + INCOMING_EMAIL_HOSTNAME="host.example.com", + INCOMING_EMAIL_POP3_PORT="123", + ) + def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthentication): + spire_to_dit_mailserver = get_spire_to_dit_mailserver() + + mock_BasicAuthentication.asset_called_with( + user="incoming.email.user@example.com", + password="testpassword", + ) + mock_MailServer.assert_called_with( + mock_BasicAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(spire_to_dit_mailserver, mock_MailServer()) + + @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( + HMRC_TO_DIT_EMAIL_USER="hmrc.to.dit.email.user@example.com", + HMRC_TO_DIT_EMAIL_PASSWORD="testpassword", + HMRC_TO_DIT_EMAIL_HOSTNAME="host.example.com", + HMRC_TO_DIT_EMAIL_POP3_PORT="123", + ) + def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthentication): + hmrc_to_dit_mailserver = get_hmrc_to_dit_mailserver() + + mock_BasicAuthentication.asset_called_with( + user="hmrc.to.dit.email.user@example.com", + password="testpassword", + ) + mock_MailServer.assert_called_with( + mock_BasicAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(hmrc_to_dit_mailserver, mock_MailServer()) + + @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( + MOCK_HMRC_EMAIL_USER="mock.hmrc.email.user@example.com", + MOCK_HMRC_EMAIL_PASSWORD="testpassword", + 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.asset_called_with( + user="hmrc.to.dit.email.user@example.com", + password="testpassword", + ) + mock_MailServer.assert_called_with( + mock_BasicAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(mock_hmrc_mailserver, mock_MailServer()) + + @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch("mail.libraries.routing_controller.MailServer") + @override_settings( + SPIRE_STANDIN_EMAIL_USER="spire.standin.email.user@example.com", + SPIRE_STANDIN_EMAIL_PASSWORD="testpassword", + SPIRE_STANDIN_EMAIL_HOSTNAME="host.example.com", + SPIRE_STANDIN_EMAIL_POP3_PORT="123", + ) + def test_get_spire_standin_mailserver(self, mock_MailServer, mock_BasicAuthentication): + spire_standin_mailserver = get_spire_standin_mailserver() + + mock_BasicAuthentication.asset_called_with( + user="hmrc.to.dit.email.user@example.com", + password="testpassword", + ) + mock_MailServer.assert_called_with( + mock_BasicAuthentication(), + hostname="host.example.com", + pop3_port="123", + ) + + self.assertEqual(spire_standin_mailserver, mock_MailServer()) 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 From 6024468d04e9134a0d6cf3f74075cd1ae4dd862b Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 31 Aug 2022 11:35:57 +0100 Subject: [PATCH 2/7] Remove unused spire standin server getter --- mail/libraries/routing_controller.py | 13 ---------- .../libraries/test_routing_controller.py | 24 ------------------- 2 files changed, 37 deletions(-) diff --git a/mail/libraries/routing_controller.py b/mail/libraries/routing_controller.py index 2f1189ce..05724174 100644 --- a/mail/libraries/routing_controller.py +++ b/mail/libraries/routing_controller.py @@ -78,19 +78,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 index 487e90b6..45a42551 100644 --- a/mail/tests/libraries/test_routing_controller.py +++ b/mail/tests/libraries/test_routing_controller.py @@ -6,7 +6,6 @@ from mail.libraries.routing_controller import ( get_hmrc_to_dit_mailserver, get_mock_hmrc_mailserver, - get_spire_standin_mailserver, get_spire_to_dit_mailserver, ) @@ -80,26 +79,3 @@ def test_get_mock_hmrc_mailserver(self, mock_MailServer, mock_BasicAuthenticatio ) self.assertEqual(mock_hmrc_mailserver, mock_MailServer()) - - @patch("mail.libraries.routing_controller.BasicAuthentication") - @patch("mail.libraries.routing_controller.MailServer") - @override_settings( - SPIRE_STANDIN_EMAIL_USER="spire.standin.email.user@example.com", - SPIRE_STANDIN_EMAIL_PASSWORD="testpassword", - SPIRE_STANDIN_EMAIL_HOSTNAME="host.example.com", - SPIRE_STANDIN_EMAIL_POP3_PORT="123", - ) - def test_get_spire_standin_mailserver(self, mock_MailServer, mock_BasicAuthentication): - spire_standin_mailserver = get_spire_standin_mailserver() - - mock_BasicAuthentication.asset_called_with( - user="hmrc.to.dit.email.user@example.com", - password="testpassword", - ) - mock_MailServer.assert_called_with( - mock_BasicAuthentication(), - hostname="host.example.com", - pop3_port="123", - ) - - self.assertEqual(spire_standin_mailserver, mock_MailServer()) From 0defe231a3443c7928848bc96417ef15624ef92b Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 31 Aug 2022 11:39:12 +0100 Subject: [PATCH 3/7] Use non-flagging password string in tests --- mail/tests/libraries/test_routing_controller.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mail/tests/libraries/test_routing_controller.py b/mail/tests/libraries/test_routing_controller.py index 45a42551..a34cdf9b 100644 --- a/mail/tests/libraries/test_routing_controller.py +++ b/mail/tests/libraries/test_routing_controller.py @@ -15,7 +15,7 @@ class RoutingControllerTest(unittest.TestCase): @patch("mail.libraries.routing_controller.MailServer") @override_settings( INCOMING_EMAIL_USER="incoming.email.user@example.com", - INCOMING_EMAIL_PASSWORD="testpassword", + INCOMING_EMAIL_PASSWORD="shhh", INCOMING_EMAIL_HOSTNAME="host.example.com", INCOMING_EMAIL_POP3_PORT="123", ) @@ -24,7 +24,7 @@ def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthentica mock_BasicAuthentication.asset_called_with( user="incoming.email.user@example.com", - password="testpassword", + password="shhh", ) mock_MailServer.assert_called_with( mock_BasicAuthentication(), @@ -38,7 +38,7 @@ def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthentica @patch("mail.libraries.routing_controller.MailServer") @override_settings( HMRC_TO_DIT_EMAIL_USER="hmrc.to.dit.email.user@example.com", - HMRC_TO_DIT_EMAIL_PASSWORD="testpassword", + HMRC_TO_DIT_EMAIL_PASSWORD="shhh", HMRC_TO_DIT_EMAIL_HOSTNAME="host.example.com", HMRC_TO_DIT_EMAIL_POP3_PORT="123", ) @@ -47,7 +47,7 @@ def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthenticat mock_BasicAuthentication.asset_called_with( user="hmrc.to.dit.email.user@example.com", - password="testpassword", + password="shhh", ) mock_MailServer.assert_called_with( mock_BasicAuthentication(), @@ -61,7 +61,7 @@ def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthenticat @patch("mail.libraries.routing_controller.MailServer") @override_settings( MOCK_HMRC_EMAIL_USER="mock.hmrc.email.user@example.com", - MOCK_HMRC_EMAIL_PASSWORD="testpassword", + MOCK_HMRC_EMAIL_PASSWORD="shhh", MOCK_HMRC_EMAIL_HOSTNAME="host.example.com", MOCK_HMRC_EMAIL_POP3_PORT="123", ) @@ -70,7 +70,7 @@ def test_get_mock_hmrc_mailserver(self, mock_MailServer, mock_BasicAuthenticatio mock_BasicAuthentication.asset_called_with( user="hmrc.to.dit.email.user@example.com", - password="testpassword", + password="shhh", ) mock_MailServer.assert_called_with( mock_BasicAuthentication(), From 8b0ed2bf857855da4d13c7dc594af4e851feef81 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 31 Aug 2022 12:01:21 +0100 Subject: [PATCH 4/7] Mock mailserver getters for relevant tests --- mail/tests/test_resend_email.py | 30 +++++++++++-- mail/tests/test_select_email_for_sending.py | 50 ++++++++++++++++++--- 2 files changed, 72 insertions(+), 8 deletions(-) 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, From 1da838d9dea1f90bc832c3b4b43c97db244d246a Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 31 Aug 2022 12:11:15 +0100 Subject: [PATCH 5/7] Use MS modern authentication for mail server connections --- conf/settings.py | 4 ++ local.env | 4 ++ mail/libraries/routing_controller.py | 14 +++-- .../libraries/test_routing_controller.py | 52 +++++++++++++------ 4 files changed, 52 insertions(+), 22 deletions(-) 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 05724174..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( diff --git a/mail/tests/libraries/test_routing_controller.py b/mail/tests/libraries/test_routing_controller.py index a34cdf9b..9b66db56 100644 --- a/mail/tests/libraries/test_routing_controller.py +++ b/mail/tests/libraries/test_routing_controller.py @@ -3,6 +3,7 @@ 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, @@ -11,66 +12,83 @@ class RoutingControllerTest(unittest.TestCase): - @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch( + "mail.libraries.routing_controller.ModernAuthentication", + spec=ModernAuthentication, + ) @patch("mail.libraries.routing_controller.MailServer") @override_settings( INCOMING_EMAIL_USER="incoming.email.user@example.com", - INCOMING_EMAIL_PASSWORD="shhh", 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", # nosec + AZURE_AUTH_TENANT_ID="azure-auth-tenant-id", ) - def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthentication): + def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_ModernAuthentication): spire_to_dit_mailserver = get_spire_to_dit_mailserver() - mock_BasicAuthentication.asset_called_with( + mock_ModernAuthentication.assert_called_with( user="incoming.email.user@example.com", - password="shhh", + client_id="azure-auth-client-id", + client_secret="azure-auth-client-secret", # nosec + tenant_id="azure-auth-tenant-id", ) mock_MailServer.assert_called_with( - mock_BasicAuthentication(), + mock_ModernAuthentication(), hostname="host.example.com", pop3_port="123", ) self.assertEqual(spire_to_dit_mailserver, mock_MailServer()) - @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch( + "mail.libraries.routing_controller.ModernAuthentication", + spec=ModernAuthentication, + ) @patch("mail.libraries.routing_controller.MailServer") @override_settings( HMRC_TO_DIT_EMAIL_USER="hmrc.to.dit.email.user@example.com", - HMRC_TO_DIT_EMAIL_PASSWORD="shhh", 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", # nosec + AZURE_AUTH_TENANT_ID="azure-auth-tenant-id", ) - def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_BasicAuthentication): + def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_ModernAuthentication): hmrc_to_dit_mailserver = get_hmrc_to_dit_mailserver() - mock_BasicAuthentication.asset_called_with( + mock_ModernAuthentication.assert_called_with( user="hmrc.to.dit.email.user@example.com", - password="shhh", + client_id="azure-auth-client-id", + client_secret="azure-auth-client-secret", # nosec + tenant_id="azure-auth-tenant-id", ) mock_MailServer.assert_called_with( - mock_BasicAuthentication(), + mock_ModernAuthentication(), hostname="host.example.com", pop3_port="123", ) self.assertEqual(hmrc_to_dit_mailserver, mock_MailServer()) - @patch("mail.libraries.routing_controller.BasicAuthentication") + @patch( + "mail.libraries.routing_controller.BasicAuthentication", + spec=BasicAuthentication, + ) @patch("mail.libraries.routing_controller.MailServer") @override_settings( MOCK_HMRC_EMAIL_USER="mock.hmrc.email.user@example.com", - MOCK_HMRC_EMAIL_PASSWORD="shhh", + MOCK_HMRC_EMAIL_PASSWORD="shhh", # nosec 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.asset_called_with( - user="hmrc.to.dit.email.user@example.com", - password="shhh", + mock_BasicAuthentication.assert_called_with( + user="mock.hmrc.email.user@example.com", + password="shhh", # nosec ) mock_MailServer.assert_called_with( mock_BasicAuthentication(), From e595131382481d7ac9677a5d6958de6f7cfc7c30 Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Wed, 31 Aug 2022 13:36:02 +0100 Subject: [PATCH 6/7] Correctly skip bandit violations on test password --- .../libraries/test_routing_controller.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/mail/tests/libraries/test_routing_controller.py b/mail/tests/libraries/test_routing_controller.py index 9b66db56..fee776e6 100644 --- a/mail/tests/libraries/test_routing_controller.py +++ b/mail/tests/libraries/test_routing_controller.py @@ -17,21 +17,21 @@ class RoutingControllerTest(unittest.TestCase): spec=ModernAuthentication, ) @patch("mail.libraries.routing_controller.MailServer") - @override_settings( + @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", # nosec + 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( + mock_ModernAuthentication.assert_called_with( # nosec user="incoming.email.user@example.com", client_id="azure-auth-client-id", - client_secret="azure-auth-client-secret", # nosec + client_secret="azure-auth-client-secret", tenant_id="azure-auth-tenant-id", ) mock_MailServer.assert_called_with( @@ -47,21 +47,21 @@ def test_get_spire_to_dit_mailserver(self, mock_MailServer, mock_ModernAuthentic spec=ModernAuthentication, ) @patch("mail.libraries.routing_controller.MailServer") - @override_settings( + @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", # nosec + 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( + 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", # nosec + client_secret="azure-auth-client-secret", tenant_id="azure-auth-tenant-id", ) mock_MailServer.assert_called_with( @@ -77,18 +77,18 @@ def test_get_hmrc_to_dit_mailserver(self, mock_MailServer, mock_ModernAuthentica spec=BasicAuthentication, ) @patch("mail.libraries.routing_controller.MailServer") - @override_settings( + @override_settings( # nosec MOCK_HMRC_EMAIL_USER="mock.hmrc.email.user@example.com", - MOCK_HMRC_EMAIL_PASSWORD="shhh", # nosec + 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( + mock_BasicAuthentication.assert_called_with( # nosec user="mock.hmrc.email.user@example.com", - password="shhh", # nosec + password="shhh", ) mock_MailServer.assert_called_with( mock_BasicAuthentication(), From b695c5ddd2df893940b61ae02c94df4aeeb7e9aa Mon Sep 17 00:00:00 2001 From: Kevin Carrogan Date: Thu, 1 Sep 2022 13:36:24 +0100 Subject: [PATCH 7/7] Add vault link to readme for .env credentials --- README.md | 1 + 1 file changed, 1 insertion(+) 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`