Skip to content

Commit

Permalink
Merge pull request #161 from uktrade/LTD-2715-implement-modern-auth
Browse files Browse the repository at this point in the history
Ltd 2715 implement modern auth
  • Loading branch information
kevincarrogan authored Sep 1, 2022
2 parents fa687cd + b695c5d commit d28d3e2
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 26 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
4 changes: 4 additions & 0 deletions conf/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
4 changes: 4 additions & 0 deletions local.env
Original file line number Diff line number Diff line change
Expand Up @@ -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=<FROM_VAULT>
AZURE_AUTH_CLIENT_SECRET=<FROM_VAULT>
AZURE_AUTH_TENANT_ID=<FROM_VAULT>
27 changes: 9 additions & 18 deletions mail/libraries/routing_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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()
Expand Down
99 changes: 99 additions & 0 deletions mail/tests/libraries/test_routing_controller.py
Original file line number Diff line number Diff line change
@@ -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="[email protected]",
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="[email protected]",
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="[email protected]",
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="[email protected]",
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="[email protected]",
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="[email protected]",
password="shhh",
)
mock_MailServer.assert_called_with(
mock_BasicAuthentication(),
hostname="host.example.com",
pop3_port="123",
)

self.assertEqual(mock_hmrc_mailserver, mock_MailServer())
30 changes: 27 additions & 3 deletions mail/tests/test_resend_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
50 changes: 45 additions & 5 deletions mail/tests/test_select_email_for_sending.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions pii-secret-exclude.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit d28d3e2

Please sign in to comment.