From 35ab9bf4e464c0559968adc3c285423486a3724d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 15 Jun 2024 16:18:01 -0300 Subject: [PATCH] refactor: adjust mail ingestion api (#7523) --- ietf/api/tests.py | 61 ++++++++++++++++++++++++++++++++++++----- ietf/api/views.py | 70 ++++++++++++++++++++++------------------------- 2 files changed, 87 insertions(+), 44 deletions(-) diff --git a/ietf/api/tests.py b/ietf/api/tests.py index 66bc7a3be7..fd8eb52cd6 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -10,6 +10,7 @@ from importlib import import_module from pathlib import Path +from random import randrange from django.apps import apps from django.conf import settings @@ -1072,8 +1073,20 @@ def test_ingest_email( self.assertEqual(r.status_code, 400) self.assertFalse(any(m.called for m in mocks)) - # test that valid requests call handlers appropriately + # bad destination message_b64 = base64.b64encode(b"This is a message").decode() + r = self.client.post( + url, + {"dest": "not-a-destination", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_dest"}) + self.assertFalse(any(m.called for m in mocks)) + + # test that valid requests call handlers appropriately r = self.client.post( url, {"dest": "iana-review", "message": message_b64}, @@ -1081,6 +1094,8 @@ def test_ingest_email( headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "ok"}) 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}))) @@ -1093,20 +1108,44 @@ def test_ingest_email( headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "ok"}) 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() + # bad nomcom-feedback dest + for bad_nomcom_dest in [ + "nomcom-feedback", # no suffix + "nomcom-feedback-", # no year + "nomcom-feedback-squid", # not a year, + "nomcom-feedback-2024-2025", # also not a year + ]: + r = self.client.post( + url, + {"dest": bad_nomcom_dest, "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_dest"}) + self.assertFalse(any(m.called for m in mocks)) + + # good nomcom-feedback dest + random_year = randrange(100000) r = self.client.post( url, - {"dest": "nomcom-feedback", "message": message_b64, "year": 2024}, # arbitrary year + {"dest": f"nomcom-feedback-{random_year}", "message": message_b64}, content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "ok"}) self.assertTrue(mock_nomcom_ingest.called) - self.assertEqual(mock_nomcom_ingest.call_args, mock.call(b"This is a message", 2024)) + self.assertEqual(mock_nomcom_ingest.call_args, mock.call(b"This is a message", random_year)) self.assertFalse(any(m.called for m in (mocks - {mock_nomcom_ingest}))) mock_nomcom_ingest.reset_mock() @@ -1118,7 +1157,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) 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}))) @@ -1138,7 +1179,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) 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}))) @@ -1167,7 +1210,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) 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}))) @@ -1192,7 +1237,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) 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}))) diff --git a/ietf/api/views.py b/ietf/api/views.py index 3c3ea0d5a9..6aaed4b6a9 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -8,6 +8,7 @@ import pytz import re +from contextlib import suppress from django.conf import settings from django.contrib.auth import authenticate from django.contrib.auth.decorators import login_required @@ -533,32 +534,13 @@ def role_holder_addresses(request): "type": "object", "properties": { "dest": { - "enum": [ - "iana-review", - "ipr-response", - "nomcom-feedback", - ] + "type": "string", }, "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"], - }, } ) @@ -630,49 +612,63 @@ def as_emailmessage(self) -> Optional[EmailMessage]: @requires_api_token @csrf_exempt def ingest_email(request): + """Ingest incoming email + + Returns a 4xx or 5xx status code if the HTTP request was invalid or something went + wrong while processing it. If the request was valid, returns a 200. This may or may + not indicate that the message was accepted. + """ - def _err(code, text): + def _http_err(code, text): return HttpResponse(text, status=code, content_type="text/plain") + def _api_response(result): + return JsonResponse(data={"result": result}) + if request.method != "POST": - return _err(405, "Method not allowed") + return _http_err(405, "Method not allowed") if request.content_type != "application/json": - return _err(415, "Content-Type must be application/json") + return _http_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}") + return _http_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}") + return _http_err(400, f"JSON schema error at {err.json_path}: {err.message}") except Exception: - return _err(400, "Invalid request format") + return _http_err(400, "Invalid request format") try: message = base64.b64decode(payload["message"], validate=True) except binascii.Error: - return _err(400, "Invalid message: bad base64 encoding") + return _http_err(400, "Invalid message: bad base64 encoding") dest = payload["dest"] + valid_dest = False try: if dest == "iana-review": + valid_dest = True iana_ingest_review_email(message) elif dest == "ipr-response": + valid_dest = True 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 + elif dest.startswith("nomcom-feedback-"): + maybe_year = dest[len("nomcom-feedback-"):] + if maybe_year.isdecimal(): + valid_dest = True + nomcom_ingest_feedback_email(message, int(maybe_year)) except EmailIngestionError as err: error_email = err.as_emailmessage() if error_email is not None: - send_smtp(error_email) - return _err(400, err.msg) + with suppress(Exception): # send_smtp logs its own exceptions, ignore them here + send_smtp(error_email) + return _api_response("bad_msg") + + if not valid_dest: + return _api_response("bad_dest") - return HttpResponse(status=200) + return _api_response("ok")