commit 58c45eabc58377595ad217522ab22c54c8a776e5 Author: hiro <hiro@torproject.org> Date: Thu May 23 16:39:32 2019 +0200 Refactor email and add tests for email and locales support --- gettor.conf.json | 1 - gettor/parse/email.py | 93 ++++++++++++++++++++++++------------- gettor/services/email/sendmail.py | 9 ++-- gettor/utils/settings.py | 1 - share/locale/available_locales.json | 15 ++++-- tests/conftests.py | 3 ++ tests/test_email_service.py | 33 +++++++++++++ tests/test_locales.py | 7 +-- 8 files changed, 118 insertions(+), 44 deletions(-) diff --git a/gettor.conf.json b/gettor.conf.json index 6a9b420..b3f331d 100644 --- a/gettor.conf.json +++ b/gettor.conf.json @@ -1,6 +1,5 @@ { "platforms": ["linux", "osx", "windows"], - "languages": ["en", "es", "pt"], "dbname": "/srv/gettor.torproject.org/home/gettor/gettor.db", "email_parser_logfile": "/srv/gettor.torproject.org/home/gettor/log/email_parser.log", "email_requests_limit": 5, diff --git a/gettor/parse/email.py b/gettor/parse/email.py index 7a88f70..99d90c6 100644 --- a/gettor/parse/email.py +++ b/gettor/parse/email.py @@ -28,7 +28,7 @@ from twisted.internet import defer from twisted.enterprise import adbapi from ..utils.db import SQLite3 - +from ..utils import strings class AddressError(Exception): """ @@ -57,26 +57,7 @@ class EmailParser(object): self.dkim = dkim self.to_addr = to_addr - - def parse(self, msg_str): - """ - Parse message content. Check if email address is well formed, if DKIM - signature is valid, and prevent service flooding. Finally, look for - commands to process the request. Current commands are: - - - links: request links for download. - - help: help request. - - :param msg_str (str): incomming message as string. - - :return dict with email address and command (`links` or `help`). - """ - - platforms = self.settings.get("platforms") - languages = self.settings.get("languages") - log.msg("Building email message from string.", system="email parser") - msg = message_from_string(msg_str) - + def normalize(self, msg): # Normalization will convert <Alice Wonderland> alice@wonderland.net # into alice@wonderland.net name, norm_addr = parseaddr(msg['From']) @@ -85,7 +66,10 @@ class EmailParser(object): "Normalizing and validating FROM email address.", system="email parser" ) + return name, norm_addr, to_name, norm_to_addr + + def validate(self, norm_addr, msg): # Validate_email will do a bunch of regexp to see if the email address # is well address. Additional options for validate_email are check_mx # and verify, which check if the SMTP host and email address exist. @@ -95,6 +79,8 @@ class EmailParser(object): "Email address normalized and validated.", system="email parser" ) + return True + else: log.err( "Error normalizing/validating email address.", @@ -102,17 +88,8 @@ class EmailParser(object): ) raise AddressError("Invalid email address {}".format(msg['From'])) - hid = hashlib.sha256(norm_addr.encode('utf-8')) - log.msg( - "Request from {}".format(hid.hexdigest()), system="email parser" - ) - - if self.to_addr: - if self.to_addr != norm_to_addr: - log.msg("Got request for a different instance of gettor") - log.msg("Intended recipient: {}".format(norm_to_addr)) - return {} + def dkim_verify(self, msg_str, norm_addr): # DKIM verification. Simply check that the server has verified the # message's signature if self.dkim: @@ -121,6 +98,7 @@ class EmailParser(object): # string, so DKIM will fail. Use the original string instead if dkim.verify(msg_str): log.msg("Valid DKIM signature.", system="email parser") + return True else: log.msg("Invalid DKIM signature.", system="email parser") username, domain = norm_addr.split("@") @@ -129,7 +107,12 @@ class EmailParser(object): hid.hexdigest(), domain ) ) + # Is this even useful like this? + else: + return True + + def build_request(self, msg_str, norm_addr, languages, platforms): # Search for commands keywords subject_re = re.compile(r"Subject: (.*)\r\n") subject = subject_re.search(msg_str) @@ -167,6 +150,54 @@ class EmailParser(object): return request + def parse(self, msg_str): + """ + Parse message content. Check if email address is well formed, if DKIM + signature is valid, and prevent service flooding. Finally, look for + commands to process the request. Current commands are: + + - links: request links for download. + - help: help request. + + :param msg_str (str): incomming message as string. + + :return dict with email address and command (`links` or `help`). + """ + + log.msg("Building email message from string.", system="email parser") + + platforms = self.settings.get("platforms") + languages = [*strings.get_locales().keys()] + msg = message_from_string(msg_str) + + name, norm_addr, to_name, norm_to_addr = self.normalize(msg) + + try: + self.validate(norm_addr, msg) + except AddressError as e: + log.message("Address error: {}".format(e.args)) + + hid = hashlib.sha256(norm_addr.encode('utf-8')) + log.msg( + "Request from {}".format(hid.hexdigest()), system="email parser" + ) + + if self.to_addr: + if self.to_addr != norm_to_addr: + log.msg("Got request for a different instance of gettor") + log.msg("Intended recipient: {}".format(norm_to_addr)) + return {} + + try: + self.dkim_verify(msg_str, norm_addr) + except ValueError as e: + log.msg("DKIM error: {}".format(e.args)) + + request = self.build_request(msg_str, norm_addr, languages, platforms) + + return request + + @defer.inlineCallbacks def parse_callback(self, request): """ diff --git a/gettor/services/email/sendmail.py b/gettor/services/email/sendmail.py index 52b50f5..0af0af8 100644 --- a/gettor/services/email/sendmail.py +++ b/gettor/services/email/sendmail.py @@ -124,7 +124,7 @@ class Sendmail(object): for request in help_requests: id = request[0] - date = request[4] + date = request[5] hid = hashlib.sha256(id.encode('utf-8')) log.info( @@ -164,11 +164,10 @@ class Sendmail(object): if not language: language = 'en' - locales = { 'en': 'en-US', - 'es': 'es-ES', - 'pt': 'pt-BR'} + locales = strings.get_locales() + strings.load_strings(language) - locale = locales[language] + locale = locales[language]['locale'] log.info("Getting links for {}.".format(platform)) links = yield self.conn.get_links( diff --git a/gettor/utils/settings.py b/gettor/utils/settings.py index 922cfe9..b301117 100644 --- a/gettor/utils/settings.py +++ b/gettor/utils/settings.py @@ -59,7 +59,6 @@ class Settings(object): else: self._settings = { "platforms": ["linux", "osx", "windows"], - "languages": ["en", "es", "pt"], "dbname": "/srv/gettor.torproject.org/home/gettor/gettor.db", "email_parser_logfile": "/srv/gettor.torproject.org/home/gettor/log/email_parser.log", "email_requests_limit": 5, diff --git a/share/locale/available_locales.json b/share/locale/available_locales.json index d91a253..8c3c037 100644 --- a/share/locale/available_locales.json +++ b/share/locale/available_locales.json @@ -1,5 +1,14 @@ { - "en": "English", - "es": "Español", - "pt": "Português Brasil" + "en": { + "language": "English", + "locale": "en-US" + }, + "es": { + "language": "Español", + "locale": "es-ES" + }, + "pt": { + "language": "Português Brasil", + "locale": "pt-BR" + } } diff --git a/tests/conftests.py b/tests/conftests.py index eaf1098..1f73f21 100644 --- a/tests/conftests.py +++ b/tests/conftests.py @@ -6,3 +6,6 @@ from gettor.utils import options from gettor.utils import strings from gettor.services.email import sendmail from gettor.parse.email import EmailParser, AddressError, DKIMError + +from email import message_from_string +from email.utils import parseaddr diff --git a/tests/test_email_service.py b/tests/test_email_service.py index 9abef57..9d50f5f 100644 --- a/tests/test_email_service.py +++ b/tests/test_email_service.py @@ -13,6 +13,7 @@ class EmailServiceTests(unittest.TestCase): def setUp(self): self.settings = conftests.options.parse_settings() self.sm_client = conftests.sendmail.Sendmail(self.settings) + self.locales = conftests.strings.get_locales() def tearDown(self): print("tearDown()") @@ -25,6 +26,38 @@ class EmailServiceTests(unittest.TestCase): request = ep.parse("From: \"silvia [hiro]\" <hiro@torproject.org>\n Subject: help\n Reply-To: hiro@torproject.org \nTo: gettor@torproject.org") self.assertEqual(request["command"], "help") + def test_normalize_msg(self): + ep = conftests.EmailParser(self.settings, "gettor@torproject.org") + msg_str = "From: \"silvia [hiro]\" <hiro@torproject.org>\n Subject: help\n Reply-To: hiro@torproject.org \nTo: gettor@torproject.org" + msg = conftests.message_from_string(msg_str) + request = ep.normalize(msg) + self.assertEqual(request, ('silvia [hiro]', 'hiro@torproject.org', '', 'gettor@torproject.org')) + + def test_validate_msg(self): + ep = conftests.EmailParser(self.settings, "gettor@torproject.org") + msg_str = "From: \"silvia [hiro]\" <hiro@torproject.org>\n Subject: help\n Reply-To: hiro@torproject.org \nTo: gettor@torproject.org" + msg = conftests.message_from_string(msg_str) + request = ep.validate("hiro@torproject.org", msg) + assert request + + def test_dkim_verify(self): + ep = conftests.EmailParser(self.settings, "gettor@torproject.org") + msg_str = "From: \"silvia [hiro]\" <hiro@torproject.org>\n Subject: help\n Reply-To: hiro@torproject.org \nTo: gettor@torproject.org" + msg = conftests.message_from_string(msg_str) + request = ep.dkim_verify(msg, "hiro@torproject.org") + assert request + + def test_build_request(self): + ep = conftests.EmailParser(self.settings, "gettor@torproject.org") + msg_str = "From: \"silvia [hiro]\" <hiro@torproject.org>\n Subject: \r\n Reply-To: hiro@torproject.org \nTo: gettor@torproject.org\r\n osx es" + msg = conftests.message_from_string(msg_str) + languages = [*self.locales.keys()] + platforms = self.settings.get('platforms') + request = ep.build_request(msg_str, "hiro@torproject.org", languages, platforms) + self.assertEqual(request["command"], "links") + self.assertEqual(request["platform"], "osx") + self.assertEqual(request["language"], "es") + def test_language_email_parser(self): ep = conftests.EmailParser(self.settings, "gettor@torproject.org") request = ep.parse("From: \"silvia [hiro]\" <hiro@torproject.org>\n Subject: \r\n Reply-To: hiro@torproject.org \nTo: gettor@torproject.org\n osx en") diff --git a/tests/test_locales.py b/tests/test_locales.py index 39cdac5..b6eb777 100644 --- a/tests/test_locales.py +++ b/tests/test_locales.py @@ -18,9 +18,6 @@ class EmailServiceTests(unittest.TestCase): def tearDown(self): print("tearDown()") - def test_get_available_locales(self): - self.assertEqual({"en": "English", "es": "Español", "pt": "Português Brasil"}, self.locales) - def test_load_en_strings(self): conftests.strings.load_strings("en") self.assertEqual(conftests.strings._("smtp_mirrors_subject"), "[GetTor] Mirrors") @@ -33,5 +30,9 @@ class EmailServiceTests(unittest.TestCase): conftests.strings.load_strings("es") self.assertEqual(conftests.strings._("smtp_help_subject"), "[GetTor] Ayuda") + def test_locale_supported(self): + self.assertEqual(self.locales['en']['language'], "English") + self.assertEqual(self.locales['es']['locale'], "es-ES") + if __name__ == "__main__": unittest.main()