From 0616b07d2dd0c9d135238d6bfe2d010768418062 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 23 Apr 2024 11:07:50 -0300 Subject: [PATCH] feat: email ingestion API (#7342) * feat: IANA review email ingestor API * refactor: Replace iana email api with generic one * chore: Add type hint * feat: Ingest ipr responses * feat: Ingest nomcom feedback * refactor: message -> msg * fix: Typo * feat: Send email on nomcom ingestion failure * feat: Send email on IPR mail ingestion error * feat: Check content type, handle more errs * fix: drop additionalProperties: false Unfortunately this does not mix well with the conditional "year" property. * test: Test ingest_email view * Revert "test: Test ingest_email view" This reverts commit e498022829f834a0d3cebcb0dafb0d5f5a5d162e. * test: Test ingest_email view * fix: pass new test * test: Test ingest_review_email * fix: Pass new test * test: Test ipr ingest_response_email * fix: pass new test * test: test nomcom ingest_feedback_email * chore: fix typo found in code reviw * fix: De-lint --- ietf/api/tests.py | 193 ++++++++++++++++++++++++++++++++++++++++++- ietf/api/urls.py | 4 +- ietf/api/views.py | 163 +++++++++++++++++++++++++++++++++++- ietf/ipr/tests.py | 40 ++++++++- ietf/ipr/utils.py | 30 +++++++ ietf/nomcom/tests.py | 47 ++++++++++- ietf/nomcom/utils.py | 32 +++++++ ietf/sync/iana.py | 19 +++++ ietf/sync/tests.py | 57 +++++++++++++ ietf/sync/views.py | 1 - 10 files changed, 577 insertions(+), 9 deletions(-) diff --git a/ietf/api/tests.py b/ietf/api/tests.py index 25b6ac5b7..c4e627c52 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -1,6 +1,6 @@ # Copyright The IETF Trust 2015-2020, All Rights Reserved # -*- coding: utf-8 -*- - +import base64 import datetime import json import html @@ -36,11 +36,12 @@ from ietf.person.factories import PersonFactory, random_faker, EmailFactory from ietf.person.models import Email, User from ietf.person.models import PersonalApiKey from ietf.stats.models import MeetingRegistration -from ietf.utils.mail import outbox, get_payload_text +from ietf.utils.mail import empty_outbox, outbox, get_payload_text from ietf.utils.models import DumpInfo from ietf.utils.test_utils import TestCase, login_testing_unauthorized, reload_db_objects from .ietf_utils import is_valid_token, requires_api_token +from .views import EmailIngestionError OMITTED_APPS = ( 'ietf.secr.meetings', @@ -1013,6 +1014,194 @@ class CustomApiTests(TestCase): sorted(e.address for e in emails), ) + @override_settings(APP_API_TOKENS={"ietf.api.views.ingest_email": "valid-token"}) + @mock.patch("ietf.api.views.iana_ingest_review_email") + @mock.patch("ietf.api.views.ipr_ingest_response_email") + @mock.patch("ietf.api.views.nomcom_ingest_feedback_email") + def test_ingest_email( + self, mock_nomcom_ingest, mock_ipr_ingest, mock_iana_ingest + ): + mocks = {mock_nomcom_ingest, mock_ipr_ingest, mock_iana_ingest} + empty_outbox() + url = urlreverse("ietf.api.views.ingest_email") + + # test various bad calls + r = self.client.get(url) + self.assertEqual(r.status_code, 403) + self.assertFalse(any(m.called for m in mocks)) + + r = self.client.post(url) + self.assertEqual(r.status_code, 403) + self.assertFalse(any(m.called for m in mocks)) + + r = self.client.get(url, headers={"X-Api-Key": "valid-token"}) + self.assertEqual(r.status_code, 405) + self.assertFalse(any(m.called for m in mocks)) + + r = self.client.post(url, headers={"X-Api-Key": "valid-token"}) + self.assertEqual(r.status_code, 415) + self.assertFalse(any(m.called for m in mocks)) + + r = self.client.post( + url, content_type="application/json", headers={"X-Api-Key": "valid-token"} + ) + self.assertEqual(r.status_code, 400) + self.assertFalse(any(m.called for m in mocks)) + + r = self.client.post( + url, + "this is not JSON!", + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 400) + self.assertFalse(any(m.called for m in mocks)) + + r = self.client.post( + url, + {"json": "yes", "valid_schema": False}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 400) + self.assertFalse(any(m.called for m in mocks)) + + # test that valid requests call handlers appropriately + message_b64 = base64.b64encode(b"This is a message").decode() + r = self.client.post( + url, + {"dest": "iana-review", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertTrue(mock_iana_ingest.called) + self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) + self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) + mock_iana_ingest.reset_mock() + + r = self.client.post( + url, + {"dest": "ipr-response", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertTrue(mock_ipr_ingest.called) + self.assertEqual(mock_ipr_ingest.call_args, mock.call(b"This is a message")) + self.assertFalse(any(m.called for m in (mocks - {mock_ipr_ingest}))) + mock_ipr_ingest.reset_mock() + + r = self.client.post( + url, + {"dest": "nomcom-feedback", "message": message_b64, "year": 2024}, # arbitrary year + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertTrue(mock_nomcom_ingest.called) + self.assertEqual(mock_nomcom_ingest.call_args, mock.call(b"This is a message", 2024)) + self.assertFalse(any(m.called for m in (mocks - {mock_nomcom_ingest}))) + mock_nomcom_ingest.reset_mock() + + # test that exceptions lead to email being sent - assumes that iana-review handling is representative + mock_iana_ingest.side_effect = EmailIngestionError("Error: don't send email") + r = self.client.post( + url, + {"dest": "iana-review", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 400) + self.assertTrue(mock_iana_ingest.called) + self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) + self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) + self.assertEqual(len(outbox), 0) # implicitly tests that _none_ of the earlier tests sent email + mock_iana_ingest.reset_mock() + + # test default recipients and attached original message + mock_iana_ingest.side_effect = EmailIngestionError( + "Error: do send email", + email_body="This is my email\n", + email_original_message=b"This is the original message" + ) + with override_settings(ADMINS=[("Some Admin", "admin@example.com")]): + r = self.client.post( + url, + {"dest": "iana-review", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 400) + self.assertTrue(mock_iana_ingest.called) + self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) + self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) + self.assertEqual(len(outbox), 1) + self.assertIn("admin@example.com", outbox[0]["To"]) + self.assertEqual("Error: do send email", outbox[0]["Subject"]) + self.assertEqual("This is my email\n", get_payload_text(outbox[0].get_body())) + attachments = list(a for a in outbox[0].iter_attachments()) + self.assertEqual(len(attachments), 1) + self.assertEqual(attachments[0].get_filename(), "original-message") + self.assertEqual(attachments[0].get_content_type(), "application/octet-stream") + self.assertEqual(attachments[0].get_content(), b"This is the original message") + mock_iana_ingest.reset_mock() + empty_outbox() + + # test overridden recipients and no attached original message + mock_iana_ingest.side_effect = EmailIngestionError( + "Error: do send email", + email_body="This is my email\n", + email_recipients=("thatguy@example.com") + ) + with override_settings(ADMINS=[("Some Admin", "admin@example.com")]): + r = self.client.post( + url, + {"dest": "iana-review", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 400) + self.assertTrue(mock_iana_ingest.called) + self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) + self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) + self.assertEqual(len(outbox), 1) + self.assertNotIn("admin@example.com", outbox[0]["To"]) + self.assertIn("thatguy@example.com", outbox[0]["To"]) + self.assertEqual("Error: do send email", outbox[0]["Subject"]) + self.assertEqual("This is my email\n", get_payload_text(outbox[0])) + mock_iana_ingest.reset_mock() + empty_outbox() + + # test attached traceback + mock_iana_ingest.side_effect = EmailIngestionError( + "Error: do send email", + email_body="This is my email\n", + email_attach_traceback=True, + ) + with override_settings(ADMINS=[("Some Admin", "admin@example.com")]): + r = self.client.post( + url, + {"dest": "iana-review", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 400) + self.assertTrue(mock_iana_ingest.called) + self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) + self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) + self.assertEqual(len(outbox), 1) + self.assertIn("admin@example.com", outbox[0]["To"]) + self.assertEqual("Error: do send email", outbox[0]["Subject"]) + self.assertEqual("This is my email\n", get_payload_text(outbox[0].get_body())) + attachments = list(a for a in outbox[0].iter_attachments()) + self.assertEqual(len(attachments), 1) + self.assertEqual(attachments[0].get_filename(), "traceback.txt") + self.assertEqual(attachments[0].get_content_type(), "text/plain") + self.assertIn("ietf.api.views.EmailIngestionError: Error: do send email", attachments[0].get_content()) + mock_iana_ingest.reset_mock() + empty_outbox() + class DirectAuthApiTests(TestCase): diff --git a/ietf/api/urls.py b/ietf/api/urls.py index 4fab83172..fb2184a3f 100644 --- a/ietf/api/urls.py +++ b/ietf/api/urls.py @@ -24,7 +24,9 @@ urlpatterns = [ # --- Custom API endpoints, sorted alphabetically --- # Email alias information for drafts url(r'^doc/draft-aliases/$', api_views.draft_aliases), - # GPRD: export of personal information for the logged-in person + # email ingestor + url(r'email/$', api_views.ingest_email), + # GDPR: export of personal information for the logged-in person url(r'^export/personal-information/$', api_views.PersonalInformationExportView.as_view()), # Email alias information for groups url(r'^group/group-aliases/$', api_views.group_aliases), diff --git a/ietf/api/views.py b/ietf/api/views.py index e992db119..6f97efbdb 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -1,10 +1,13 @@ # Copyright The IETF Trust 2017-2020, All Rights Reserved # -*- coding: utf-8 -*- +import base64 +import binascii import json +import jsonschema +import pytz import re -import pytz from django.conf import settings from django.contrib.auth import authenticate from django.contrib.auth.decorators import login_required @@ -18,11 +21,15 @@ from django.utils.decorators import method_decorator from django.views.decorators.csrf import csrf_exempt from django.views.decorators.gzip import gzip_page from django.views.generic.detail import DetailView +from email.message import EmailMessage from jwcrypto.jwk import JWK from tastypie.exceptions import BadRequest from tastypie.serializers import Serializer from tastypie.utils import is_valid_jsonp_callback_value from tastypie.utils.mime import determine_format, build_content_type +from textwrap import dedent +from traceback import format_exception, extract_tb +from typing import Iterable, Optional import ietf from ietf.api import _api_list @@ -32,12 +39,16 @@ from ietf.doc.utils import DraftAliasGenerator, fuzzy_find_documents from ietf.group.utils import GroupAliasGenerator, role_holder_emails from ietf.ietfauth.utils import role_required from ietf.ietfauth.views import send_account_creation_email +from ietf.ipr.utils import ingest_response_email as ipr_ingest_response_email from ietf.meeting.models import Meeting from ietf.nomcom.models import Volunteer, NomCom +from ietf.nomcom.utils import ingest_feedback_email as nomcom_ingest_feedback_email from ietf.person.models import Person, Email from ietf.stats.models import MeetingRegistration +from ietf.sync.iana import ingest_review_email as iana_ingest_review_email from ietf.utils import log from ietf.utils.decorators import require_api_key +from ietf.utils.mail import send_smtp from ietf.utils.models import DumpInfo @@ -515,3 +526,153 @@ def role_holder_addresses(request): } ) return HttpResponse(status=405) + + +_response_email_json_validator = jsonschema.Draft202012Validator( + schema={ + "type": "object", + "properties": { + "dest": { + "enum": [ + "iana-review", + "ipr-response", + "nomcom-feedback", + ] + }, + "message": { + "type": "string", # base64-encoded mail message + }, + }, + "required": ["dest", "message"], + "if": { + # If dest == "nomcom-feedback"... + "properties": { + "dest": {"const": "nomcom-feedback"}, + } + }, + "then": { + # ... then also require year, an integer, be present + "properties": { + "year": { + "type": "integer", + }, + }, + "required": ["year"], + }, + } +) + + +class EmailIngestionError(Exception): + """Exception indicating ingestion failed""" + def __init__( + self, + msg="Message rejected", + *, + email_body: Optional[str] = None, + email_recipients: Optional[Iterable[str]] = None, + email_attach_traceback=False, + email_original_message: Optional[bytes]=None, + ): + self.msg = msg + self.email_body = email_body + self.email_subject = msg + self.email_recipients = email_recipients + self.email_attach_traceback = email_attach_traceback + self.email_original_message = email_original_message + self.email_from = settings.SERVER_EMAIL + + @staticmethod + def _summarize_error(error): + frame = extract_tb(error.__traceback__)[-1] + return dedent(f"""\ + Error details: + Exception type: {type(error).__module__}.{type(error).__name__} + File: {frame.filename} + Line: {frame.lineno}""") + + def as_emailmessage(self) -> Optional[EmailMessage]: + """Generate an EmailMessage to report an error""" + if self.email_body is None: + return None + error = self if self.__cause__ is None else self.__cause__ + format_values = dict( + error=error, + error_summary=self._summarize_error(error), + ) + msg = EmailMessage() + if self.email_recipients is None: + msg["To"] = tuple(adm[1] for adm in settings.ADMINS) + else: + msg["To"] = self.email_recipients + msg["From"] = self.email_from + msg["Subject"] = self.msg + msg.set_content( + self.email_body.format(**format_values) + ) + if self.email_attach_traceback: + msg.add_attachment( + "".join(format_exception(None, error, error.__traceback__)), + filename="traceback.txt", + ) + if self.email_original_message is not None: + # Attach incoming message if it was provided. Send as a generic media + # type because we don't know for sure that it was actually a valid + # message. + msg.add_attachment( + self.email_original_message, + 'application', 'octet-stream', # media type + filename='original-message', + ) + return msg + + +@requires_api_token +@csrf_exempt +def ingest_email(request): + + def _err(code, text): + return HttpResponse(text, status=code, content_type="text/plain") + + if request.method != "POST": + return _err(405, "Method not allowed") + + if request.content_type != "application/json": + return _err(415, "Content-Type must be application/json") + + # Validate + try: + payload = json.loads(request.body) + _response_email_json_validator.validate(payload) + except json.decoder.JSONDecodeError as err: + return _err(400, f"JSON parse error at line {err.lineno} col {err.colno}: {err.msg}") + except jsonschema.exceptions.ValidationError as err: + return _err(400, f"JSON schema error at {err.json_path}: {err.message}") + except Exception: + return _err(400, "Invalid request format") + + try: + message = base64.b64decode(payload["message"], validate=True) + except binascii.Error: + return _err(400, "Invalid message: bad base64 encoding") + + dest = payload["dest"] + try: + if dest == "iana-review": + iana_ingest_review_email(message) + elif dest == "ipr-response": + ipr_ingest_response_email(message) + elif dest == "nomcom-feedback": + year = payload["year"] + nomcom_ingest_feedback_email(message, year) + else: + # Should never get here - json schema validation should enforce the enum + log.unreachable(date="2024-04-04") + return _err(400, "Invalid dest") # return something reasonable if we got here unexpectedly + except EmailIngestionError as err: + error_email = err.as_emailmessage() + if error_email is not None: + send_smtp(error_email) + return _err(400, err.msg) + + return HttpResponse(status=200) diff --git a/ietf/ipr/tests.py b/ietf/ipr/tests.py index 73b5d0dc5..e6964445d 100644 --- a/ietf/ipr/tests.py +++ b/ietf/ipr/tests.py @@ -3,18 +3,20 @@ import datetime - +import mock from pyquery import PyQuery from urllib.parse import quote, urlparse from zoneinfo import ZoneInfo from django.conf import settings +from django.test.utils import override_settings from django.urls import reverse as urlreverse from django.utils import timezone import debug # pyflakes:ignore +from ietf.api.views import EmailIngestionError from ietf.doc.factories import ( DocumentFactory, WgDraftFactory, @@ -34,8 +36,9 @@ from ietf.ipr.mail import (process_response_email, get_reply_to, get_update_subm from ietf.ipr.models import (IprDisclosureBase,GenericIprDisclosure,HolderIprDisclosure, ThirdPartyIprDisclosure) from ietf.ipr.templatetags.ipr_filters import no_revisions_message -from ietf.ipr.utils import get_genitive, get_ipr_summary +from ietf.ipr.utils import get_genitive, get_ipr_summary, ingest_response_email from ietf.mailtrigger.utils import gather_address_lists +from ietf.message.factories import MessageFactory from ietf.message.models import Message from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.test_utils import TestCase, login_testing_unauthorized @@ -769,6 +772,39 @@ Subject: test result = process_response_email(message_bytes) self.assertIsNone(result) + @override_settings(ADMINS=(("Some Admin", "admin@example.com"),)) + @mock.patch("ietf.ipr.utils.process_response_email") + def test_ingest_response_email(self, mock_process_response_email): + message = b"What a nice message" + mock_process_response_email.side_effect = ValueError("ouch!") + with self.assertRaises(EmailIngestionError) as context: + ingest_response_email(message) + self.assertIsNone(context.exception.email_recipients) # default recipients + self.assertIsNotNone(context.exception.email_body) # body set + self.assertIsNotNone(context.exception.email_original_message) # original message attached + self.assertEqual(context.exception.email_attach_traceback, True) + self.assertTrue(mock_process_response_email.called) + self.assertEqual(mock_process_response_email.call_args, mock.call(message)) + mock_process_response_email.reset_mock() + + mock_process_response_email.side_effect = None + mock_process_response_email.return_value = None # rejected message + with self.assertRaises(EmailIngestionError) as context: + ingest_response_email(message) + self.assertIsNone(context.exception.email_recipients) # default recipients + self.assertIsNotNone(context.exception.email_body) # body set + self.assertIsNotNone(context.exception.email_original_message) # original message attached + self.assertEqual(context.exception.email_attach_traceback, True) + self.assertTrue(mock_process_response_email.called) + self.assertEqual(mock_process_response_email.call_args, mock.call(message)) + mock_process_response_email.reset_mock() + + # successful operation + mock_process_response_email.return_value = MessageFactory() + ingest_response_email(message) + self.assertTrue(mock_process_response_email.called) + self.assertEqual(mock_process_response_email.call_args, mock.call(message)) + def test_ajax_search(self): url = urlreverse('ietf.ipr.views.ajax_search') response=self.client.get(url+'?q=disclosure') diff --git a/ietf/ipr/utils.py b/ietf/ipr/utils.py index c4f17c482..06af1535f 100644 --- a/ietf/ipr/utils.py +++ b/ietf/ipr/utils.py @@ -1,6 +1,9 @@ # Copyright The IETF Trust 2014-2020, All Rights Reserved # -*- coding: utf-8 -*- +from textwrap import dedent + +from ietf.ipr.mail import process_response_email from ietf.ipr.models import IprDocRel import debug # pyflakes:ignore @@ -86,3 +89,30 @@ def generate_draft_recursive_txt(): f.write(data) +def ingest_response_email(message: bytes): + from ietf.api.views import EmailIngestionError # avoid circular import + try: + result = process_response_email(message) + except Exception as err: + raise EmailIngestionError( + "Datatracker IPR email ingestion error", + email_body=dedent("""\ + An error occurred while ingesting IPR email into the Datatracker. The original message is attached. + + {error_summary} + """), + email_original_message=message, + email_attach_traceback=True, + ) from err + + if result is None: + raise EmailIngestionError( + "Datatracker IPR email ingestion rejected", + email_body=dedent("""\ + A message was rejected while ingesting IPR email into the Datatracker. The original message is attached. + + {error_summary} + """), + email_original_message=message, + email_attach_traceback=True, + ) diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index cdbb860c4..9a615c91d 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -24,6 +24,7 @@ from django.utils.encoding import force_str import debug # pyflakes:ignore +from ietf.api.views import EmailIngestionError from ietf.dbtemplate.factories import DBTemplateFactory from ietf.dbtemplate.models import DBTemplate from ietf.doc.factories import DocEventFactory, WgDocumentAuthorFactory, \ @@ -37,14 +38,15 @@ from ietf.nomcom.test_data import nomcom_test_data, generate_cert, check_comment MEMBER_USER, SECRETARIAT_USER, EMAIL_DOMAIN, NOMCOM_YEAR from ietf.nomcom.models import NomineePosition, Position, Nominee, \ NomineePositionStateName, Feedback, FeedbackTypeName, \ - Nomination, FeedbackLastSeen, TopicFeedbackLastSeen, ReminderDates + Nomination, FeedbackLastSeen, TopicFeedbackLastSeen, ReminderDates, \ + NomCom from ietf.nomcom.management.commands.send_reminders import Command, is_time_to_send from ietf.nomcom.factories import NomComFactory, FeedbackFactory, TopicFactory, \ nomcom_kwargs_for_year, provide_private_key_to_test_client, \ key from ietf.nomcom.utils import get_nomcom_by_year, make_nomineeposition, \ get_hash_nominee_position, is_eligible, list_eligible, \ - get_eligibility_date, suggest_affiliation, \ + get_eligibility_date, suggest_affiliation, ingest_feedback_email, \ decorate_volunteers_with_qualifications from ietf.person.factories import PersonFactory, EmailFactory from ietf.person.models import Email, Person @@ -1114,6 +1116,47 @@ class FeedbackTest(TestCase): self.assertNotEqual(feedback.comments, comment_text) self.assertEqual(check_comments(feedback.comments, comment_text, self.privatekey_file), True) + @mock.patch("ietf.nomcom.utils.create_feedback_email") + def test_ingest_feedback_email(self, mock_create_feedback_email): + message = b"This is nomcom feedback" + no_nomcom_year = date_today().year + 10 # a guess at a year with no nomcoms + while NomCom.objects.filter(group__acronym__icontains=no_nomcom_year).exists(): + no_nomcom_year += 1 + inactive_nomcom = NomComFactory(group__state_id="conclude", group__acronym=f"nomcom{no_nomcom_year + 1}") + + # cases where the nomcom does not exist, so admins are notified + for bad_year in (no_nomcom_year, inactive_nomcom.year()): + with self.assertRaises(EmailIngestionError) as context: + ingest_feedback_email(message, bad_year) + self.assertIn("does not exist", context.exception.msg) + self.assertIsNotNone(context.exception.email_body) # error message to be sent + self.assertIsNone(context.exception.email_recipients) # default recipients (i.e., admin) + self.assertIsNone(context.exception.email_original_message) # no original message + self.assertFalse(context.exception.email_attach_traceback) # no traceback + self.assertFalse(mock_create_feedback_email.called) + + # nomcom exists but an error occurs, so feedback goes to the nomcom chair + active_nomcom = NomComFactory(group__acronym=f"nomcom{no_nomcom_year + 2}") + mock_create_feedback_email.side_effect = ValueError("ouch!") + with self.assertRaises(EmailIngestionError) as context: + ingest_feedback_email(message, active_nomcom.year()) + self.assertIn(f"Error ingesting nomcom {active_nomcom.year()}", context.exception.msg) + self.assertIsNotNone(context.exception.email_body) # error message to be sent + self.assertEqual(context.exception.email_recipients, active_nomcom.chair_emails()) + self.assertEqual(context.exception.email_original_message, message) + self.assertFalse(context.exception.email_attach_traceback) # no traceback + self.assertTrue(mock_create_feedback_email.called) + self.assertEqual(mock_create_feedback_email.call_args, mock.call(active_nomcom, message)) + mock_create_feedback_email.reset_mock() + + # and, finally, success + mock_create_feedback_email.side_effect = None + mock_create_feedback_email.return_value = FeedbackFactory(author="someone@example.com") + ingest_feedback_email(message, active_nomcom.year()) + self.assertTrue(mock_create_feedback_email.called) + self.assertEqual(mock_create_feedback_email.call_args, mock.call(active_nomcom, message)) + + class ReminderTest(TestCase): def setUp(self): diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 23e89026d..53e775deb 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -16,6 +16,7 @@ from email.errors import HeaderParseError from email.header import decode_header from email.iterators import typed_subpart_iterator from email.utils import parseaddr +from textwrap import dedent from django.db.models import Q, Count from django.conf import settings @@ -715,3 +716,34 @@ def extract_volunteers(year): decorate_volunteers_with_qualifications(volunteers,nomcom=nomcom) volunteers = sorted(volunteers,key=lambda v:(not v.eligible,v.person.last_name())) return nomcom, volunteers + + +def ingest_feedback_email(message: bytes, year: int): + from ietf.api.views import EmailIngestionError # avoid circular import + from .models import NomCom + try: + nomcom = NomCom.objects.get(group__acronym__icontains=str(year), + group__state__slug='active') + except NomCom.DoesNotExist: + raise EmailIngestionError( + f"Error ingesting nomcom email: nomcom {year} does not exist or is not active", + email_body=dedent(f"""\ + An email for nomcom {year} was posted to ingest_feedback_email, but no + active nomcom exists for that year. + """), + ) + + try: + feedback = create_feedback_email(nomcom, message) + except Exception as err: + raise EmailIngestionError( + f"Error ingesting nomcom {year} feedback email", + email_recipients=nomcom.chair_emails(), + email_body=dedent(f"""\ + An error occurred while ingesting feedback email for nomcom {year}. + + {{error_summary}} + """), + email_original_message=message, + ) from err + log("Received nomcom email from %s" % feedback.author) diff --git a/ietf/sync/iana.py b/ietf/sync/iana.py index 9ce54a687..f46fe407d 100644 --- a/ietf/sync/iana.py +++ b/ietf/sync/iana.py @@ -304,3 +304,22 @@ def add_review_comment(doc_name, review_time, by, comment): e.by = by e.save() + + +def ingest_review_email(message: bytes): + from ietf.api.views import EmailIngestionError # avoid circular import + try: + doc_name, review_time, by, comment = parse_review_email(message) + except Exception as err: + raise EmailIngestionError("Unable to parse message as IANA review email") from err + log(f"Read IANA review email for {doc_name} at {review_time} by {by}") + if by.name == "(System)": + log("WARNING: person responsible for email does not have a IANA role") # (sic) + try: + add_review_comment(doc_name, review_time, by, comment) + except Document.DoesNotExist: + log(f"ERROR: unknown document {doc_name}") + raise EmailIngestionError(f"Unknown document {doc_name}") + except Exception as err: + raise EmailIngestionError("Error ingesting IANA review email") from err + diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py index d59b31a95..db5619095 100644 --- a/ietf/sync/tests.py +++ b/ietf/sync/tests.py @@ -19,10 +19,12 @@ from django.test.utils import override_settings import debug # pyflakes:ignore +from ietf.api.views import EmailIngestionError from ietf.doc.factories import WgDraftFactory, RfcFactory, DocumentAuthorFactory, DocEventFactory from ietf.doc.models import Document, DocEvent, DeletedEvent, DocTagName, RelatedDocument, State, StateDocEvent from ietf.doc.utils import add_state_change_event from ietf.group.factories import GroupFactory +from ietf.person.factories import PersonFactory from ietf.person.models import Person from ietf.sync import iana, rfceditor, tasks from ietf.utils.mail import outbox, empty_outbox @@ -214,6 +216,61 @@ ICANN iana.add_review_comment(doc_name, review_time, by, comment) self.assertEqual(DocEvent.objects.filter(doc=draft, type="iana_review").count(), events_before+1) + @mock.patch("ietf.sync.iana.add_review_comment") + @mock.patch("ietf.sync.iana.parse_review_email") + def test_ingest_review_email(self, mock_parse_review_email, mock_add_review_comment): + mock_parse_review_email.side_effect = ValueError("ouch!") + message = b"message" + + # Error parsing mail + with self.assertRaises(EmailIngestionError) as context: + iana.ingest_review_email(message) + self.assertIsNone(context.exception.as_emailmessage()) # no email + self.assertEqual("Unable to parse message as IANA review email", str(context.exception)) + self.assertTrue(mock_parse_review_email.called) + self.assertEqual(mock_parse_review_email.call_args, mock.call(message)) + self.assertFalse(mock_add_review_comment.called) + mock_parse_review_email.reset_mock() + + args = ( + "doc-name", + datetime.datetime.now(tz=datetime.timezone.utc), + PersonFactory(), + "yadda yadda yadda", + ) + mock_parse_review_email.side_effect = None + mock_parse_review_email.return_value = args + mock_add_review_comment.side_effect = Document.DoesNotExist + with self.assertRaises(EmailIngestionError) as context: + iana.ingest_review_email(message) + self.assertIsNone(context.exception.as_emailmessage()) # no email + self.assertEqual(str(context.exception), "Unknown document doc-name") + self.assertTrue(mock_parse_review_email.called) + self.assertEqual(mock_parse_review_email.call_args, mock.call(message)) + self.assertTrue(mock_add_review_comment.called) + self.assertEqual(mock_add_review_comment.call_args, mock.call(*args)) + mock_parse_review_email.reset_mock() + mock_add_review_comment.reset_mock() + + mock_add_review_comment.side_effect = ValueError("ouch!") + with self.assertRaises(EmailIngestionError) as context: + iana.ingest_review_email(message) + self.assertIsNone(context.exception.as_emailmessage()) # no email + self.assertEqual("Error ingesting IANA review email", str(context.exception)) + self.assertTrue(mock_parse_review_email.called) + self.assertEqual(mock_parse_review_email.call_args, mock.call(message)) + self.assertTrue(mock_add_review_comment.called) + self.assertEqual(mock_add_review_comment.call_args, mock.call(*args)) + mock_parse_review_email.reset_mock() + mock_add_review_comment.reset_mock() + + mock_add_review_comment.side_effect = None + iana.ingest_review_email(message) + self.assertTrue(mock_parse_review_email.called) + self.assertEqual(mock_parse_review_email.call_args, mock.call(message)) + self.assertTrue(mock_add_review_comment.called) + self.assertEqual(mock_add_review_comment.call_args, mock.call(*args)) + def test_notify_page(self): # check that we can get the notify page url = urlreverse("ietf.sync.views.notify", kwargs=dict(org="iana", notification="changes")) diff --git a/ietf/sync/views.py b/ietf/sync/views.py index 30b2a928e..788e982f7 100644 --- a/ietf/sync/views.py +++ b/ietf/sync/views.py @@ -1,7 +1,6 @@ # Copyright The IETF Trust 2012-2020, All Rights Reserved # -*- coding: utf-8 -*- - import datetime import subprocess import os