Refactor email and add tests for email and locales support

gettor.conf.json

 {
   "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,

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):
         """

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(

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,

share/locale/available_locales.json

 {
-  "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"
+  }
 }

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

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")

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()