Skip to content

Commit d9dc4f5

Browse files
rjsparksrjsparks
committed
Merged in [19493] from jennifer@painless-security.com:
 Create management command base class that sends emails on exceptions. Fixes ietf-tools#3356 and ietf-tools#3357. - Legacy-Id: 19527 Note: SVN reference [19493] has been migrated to Git commit 968b775
2 parents 279c1f3 + 968b775 commit d9dc4f5

File tree

8 files changed

+434
-22
lines changed

8 files changed

+434
-22
lines changed

‎ietf/ipr/management/commands/process_email.py‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,49 @@
44

55
import io
66
import sys
7+
from textwrap import dedent
78

8-
from django.core.management.base import BaseCommand, CommandError
9+
from django.core.management import CommandError
910

11+
from ietf.utils.management.base import EmailOnFailureCommand
1012
from ietf.ipr.mail import process_response_email
1113

1214
import debug # pyflakes:ignore
1315

14-
class Command(BaseCommand):
16+
class Command(EmailOnFailureCommand):
1517
help = ("Process incoming email responses to ipr mail")
18+
msg_bytes = None
1619

1720
def add_arguments(self, parser):
21+
super().add_arguments(parser)
1822
parser.add_argument('--email-file', dest='email', help='File containing email (default: stdin)')
1923

2024
def handle(self, *args, **options):
2125
email = options.get('email', None)
22-
msg = None
23-
2426
if not email:
2527
msg = sys.stdin.read()
28+
self.msg_bytes = msg.encode()
2629
else:
27-
msg = io.open(email, "r").read()
30+
self.msg_bytes = io.open(email, "rb").read()
31+
msg = self.msg_bytes.decode()
2832

2933
try:
3034
process_response_email(msg)
3135
except ValueError as e:
3236
raise CommandError(e)
37+
38+
failure_subject = 'Error during ipr email processing'
39+
failure_message = dedent("""\
40+
An error occurred in the ipr process_email management command.
41+
42+
{error_summary}
43+
""")
44+
def make_failure_message(self, error, **extra):
45+
msg = super().make_failure_message(error, **extra)
46+
if self.msg_bytes is not None:
47+
msg.add_attachment(
48+
self.msg_bytes,
49+
'application', 'octet-stream', # mime type
50+
filename='original-message',
51+
)
52+
return msg

‎ietf/ipr/management/tests.py‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Copyright The IETF Trust 2021, All Rights Reserved
2+
# -*- coding: utf-8 -*-
3+
"""Tests of ipr management commands"""
4+
import mock
5+
6+
from django.core.management import call_command
7+
from django.test.utils import override_settings
8+
9+
from ietf.utils.test_utils import TestCase, name_of_file_containing
10+
11+
12+
@override_settings(ADMINS=(('Some Admin', 'admin@example.com'),))
13+
class ProcessEmailTests(TestCase):
14+
@mock.patch('ietf.ipr.management.commands.process_email.process_response_email')
15+
def test_process_email(self, process_mock):
16+
"""The process_email command should process the correct email"""
17+
with name_of_file_containing('contents') as filename:
18+
call_command('process_email', email_file=filename)
19+
self.assertEqual(process_mock.call_count, 1, 'process_response_email should be called once')
20+
self.assertEqual(
21+
process_mock.call_args.args,
22+
('contents',),
23+
'process_response_email should receive the correct contents'
24+
)
25+
26+
@mock.patch('ietf.utils.management.base.send_smtp')
27+
@mock.patch('ietf.ipr.management.commands.process_email.process_response_email')
28+
def test_send_error_to_admin(self, process_mock, send_smtp_mock):
29+
"""The process_email command should email the admins on error"""
30+
# arrange an mock error during processing
31+
process_mock.side_effect = RuntimeError('mock error')
32+
33+
with name_of_file_containing('contents') as filename:
34+
call_command('process_email', email_file=filename)
35+
36+
self.assertTrue(send_smtp_mock.called)
37+
(msg,) = send_smtp_mock.call_args.args
38+
self.assertEqual(msg['to'], 'admin@example.com', 'Admins should be emailed on error')
39+
self.assertIn('error', msg['subject'].lower(), 'Error email subject should indicate error')
40+
self.assertTrue(msg.is_multipart(), 'Error email should have attachments')
41+
parts = msg.get_payload()
42+
self.assertEqual(len(parts), 3, 'Error email should contain message, traceback, and original message')
43+
content = parts[0].get_payload()
44+
traceback = parts[1].get_payload()
45+
original = parts[2].get_payload(decode=True).decode() # convert octet-stream to string
46+
self.assertIn('RuntimeError', content, 'Error type should be included in error email')
47+
self.assertIn('mock.py', content, 'File where error occurred should be included in error email')
48+
self.assertIn('traceback', traceback.lower(), 'Traceback should be attached to error email')
49+
self.assertEqual(original, 'contents', 'Original message should be attached to error email')

‎ietf/nomcom/management/commands/feedback_email.py‎

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,47 +4,82 @@
44

55
import io
66
import sys
7+
from textwrap import dedent
78

8-
from django.core.management.base import BaseCommand, CommandError
9+
from django.core.management import CommandError
910

1011
from ietf.utils.log import log
12+
from ietf.utils.management.base import EmailOnFailureCommand
1113
from ietf.nomcom.models import NomCom
1214
from ietf.nomcom.utils import create_feedback_email
1315
from ietf.nomcom.fields import EncryptedException
1416

15-
import debug # pyflakes:ignore
17+
import debug # pyflakes:ignore
1618

17-
class Command(BaseCommand):
19+
20+
class Command(EmailOnFailureCommand):
1821
help = ("Receive nomcom email, encrypt and save it.")
22+
nomcom = None
23+
msg = None # incoming message
1924

2025
def add_arguments(self, parser):
21-
parser.add_argument('--nomcom-year', dest='year', help='NomCom year')
22-
parser.add_argument('--email-file', dest='email', help='File containing email (default: stdin)')
26+
super().add_arguments(parser)
27+
parser.add_argument('--nomcom-year', dest='year', help='NomCom year')
28+
parser.add_argument('--email-file', dest='email', help='File containing email (default: stdin)')
2329

2430
def handle(self, *args, **options):
2531
email = options.get('email', None)
2632
year = options.get('year', None)
27-
msg = None
28-
nomcom = None
2933
help_message = 'Usage: feeback_email --nomcom-year <nomcom-year> --email-file <email-file>'
3034

3135
if not year:
3236
log("Error: missing nomcom-year")
33-
raise CommandError("Missing nomcom-year\n\n"+help_message)
34-
35-
if not email:
36-
msg = io.open(sys.stdin.fileno(), 'rb').read()
37-
else:
38-
msg = io.open(email, "rb").read()
37+
raise CommandError("Missing nomcom-year\n\n" + help_message)
3938

4039
try:
41-
nomcom = NomCom.objects.get(group__acronym__icontains=year,
42-
group__state__slug='active')
40+
self.nomcom = NomCom.objects.get(group__acronym__icontains=year,
41+
group__state__slug='active')
4342
except NomCom.DoesNotExist:
4443
raise CommandError("NomCom %s does not exist or it isn't active" % year)
4544

45+
if not email:
46+
self.msg = io.open(sys.stdin.fileno(), 'rb').read()
47+
else:
48+
self.msg = io.open(email, "rb").read()
49+
4650
try:
47-
feedback = create_feedback_email(nomcom, msg)
51+
feedback = create_feedback_email(self.nomcom, self.msg)
4852
log("Received nomcom email from %s" % feedback.author)
4953
except (EncryptedException, ValueError) as e:
5054
raise CommandError(e)
55+
56+
# Configuration for the email to be sent on failure
57+
failure_email_includes_traceback = False # error messages might contain pieces of the feedback email
58+
failure_subject = '{nomcom}: error during feedback email processing'
59+
failure_message = dedent("""\
60+
An error occurred in the nomcom feedback_email management command while
61+
processing feedback for {nomcom}.
62+
63+
{error_summary}
64+
""")
65+
@property
66+
def failure_recipients(self):
67+
return self.nomcom.chair_emails() if self.nomcom else super().failure_recipients
68+
69+
def make_failure_message(self, error, **extra):
70+
failure_message = super().make_failure_message(
71+
error,
72+
nomcom=self.nomcom or 'nomcom',
73+
**extra
74+
)
75+
if self.nomcom and self.msg:
76+
# Attach incoming message if we have it and are sending to the nomcom chair.
77+
# Do not attach it if we are sending to the admins. Send as a generic
78+
# mime type because we don't know for sure that it was actually a valid
79+
# message.
80+
failure_message.add_attachment(
81+
self.msg,
82+
'application', 'octet-stream', # mime type
83+
filename='original-message',
84+
)
85+
return failure_message

‎ietf/nomcom/management/tests.py‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# Copyright The IETF Trust 2021, All Rights Reserved
2+
# -*- coding: utf-8 -*-
3+
"""Tests of nomcom management commands"""
4+
import mock
5+
6+
from collections import namedtuple
7+
8+
from django.core.management import call_command
9+
from django.test.utils import override_settings
10+
11+
from ietf.nomcom.factories import NomComFactory
12+
from ietf.utils.test_utils import TestCase, name_of_file_containing
13+
14+
15+
@override_settings(ADMINS=(('Some Admin', 'admin@example.com'),))
16+
class FeedbackEmailTests(TestCase):
17+
def setUp(self):
18+
self.year = 2021
19+
self.nomcom = NomComFactory(group__acronym=f'nomcom{self.year}')
20+
21+
@mock.patch('ietf.utils.management.base.send_smtp')
22+
def test_send_error_to_admins(self, send_smtp_mock):
23+
"""If a nomcom chair cannot be identified, mail goes to admins
24+
25+
This email should not contain either the full traceback or the original message.
26+
"""
27+
# Call with the wrong nomcom year so the admin will be contacted
28+
with name_of_file_containing('feedback message') as filename:
29+
call_command('feedback_email', nomcom_year=self.year + 1, email_file=filename)
30+
31+
self.assertTrue(send_smtp_mock.called)
32+
(msg,) = send_smtp_mock.call_args.args # get the message to be sent
33+
self.assertEqual(msg['to'], 'admin@example.com', 'Email recipient should be the admins')
34+
self.assertIn('error', msg['subject'], 'Email subject should indicate error')
35+
self.assertFalse(msg.is_multipart(), 'Nomcom feedback error sent to admin should not have attachments')
36+
content = msg.get_payload()
37+
self.assertIn('CommandError', content, 'Admin email should contain error type')
38+
self.assertIn('feedback_email.py', content, 'Admin email should contain file where error occurred')
39+
self.assertNotIn('traceback', content.lower(), 'Admin email should not contain traceback')
40+
self.assertNotIn(f'NomCom {self.year} does not exist', content,
41+
'Admin email should not contain error message')
42+
# not going to check the line - that's too likely to change
43+
44+
@mock.patch('ietf.utils.management.base.send_smtp')
45+
@mock.patch('ietf.nomcom.management.commands.feedback_email.create_feedback_email')
46+
def test_send_error_to_chair(self, create_feedback_mock, send_smtp_mock):
47+
# mock an exception in create_feedback_email()
48+
create_feedback_mock.side_effect = RuntimeError('mock error')
49+
50+
with name_of_file_containing('feedback message') as filename:
51+
call_command('feedback_email', nomcom_year=self.year, email_file=filename)
52+
53+
self.assertTrue(send_smtp_mock.called)
54+
(msg,) = send_smtp_mock.call_args.args # get the message to be sent
55+
self.assertCountEqual(
56+
[addr.strip() for addr in msg['to'].split(',')],
57+
self.nomcom.chair_emails(),
58+
'Email recipient should be the nomcom chair(s)',
59+
)
60+
self.assertIn('error', msg['subject'], 'Email subject should indicate error')
61+
self.assertTrue(msg.is_multipart(), 'Chair feedback error should have attachments')
62+
parts = msg.get_payload()
63+
content = parts[0].get_payload()
64+
# decode=True decodes the base64 encoding, .decode() converts the octet-stream bytes to a string
65+
attachment = parts[1].get_payload(decode=True).decode()
66+
self.assertIn('RuntimeError', content, 'Nomcom email should contain error type')
67+
self.assertIn('mock.py', content, 'Nomcom email should contain file where error occurred')
68+
self.assertIn('feedback message', attachment, 'Nomcom email should include original message')
69+
70+
@mock.patch('ietf.nomcom.management.commands.feedback_email.create_feedback_email')
71+
def test_feedback_email(self, create_feedback_mock):
72+
"""The feedback_email command should create feedback"""
73+
# mock up the return value
74+
create_feedback_mock.return_value = namedtuple('mock_feedback', 'author')('author@example.com')
75+
76+
with name_of_file_containing('feedback message') as filename:
77+
call_command('feedback_email', nomcom_year=self.year, email_file=filename)
78+
79+
self.assertEqual(create_feedback_mock.call_count, 1, 'create_feedback_email() should be called once')
80+
self.assertEqual(
81+
create_feedback_mock.call_args.args,
82+
(self.nomcom, b'feedback message'),
83+
'feedback_email should process the correct email for the correct nomcom'
84+
)

‎ietf/nomcom/models.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,15 @@ def encrypt(self, cleartext):
102102
else:
103103
raise EncryptedException(error)
104104

105+
def chair_emails(self):
106+
if not hasattr(self, '_cached_chair_emails'):
107+
if self.group:
108+
self._cached_chair_emails = list(
109+
self.group.role_set.filter(name_id='chair').values_list('email__address', flat=True)
110+
)
111+
else:
112+
self._cached_chair_emails = []
113+
return self._cached_chair_emails
105114

106115
def delete_nomcom(sender, **kwargs):
107116
nomcom = kwargs.get('instance', None)

0 commit comments

Comments
 (0)