Minimal changes to enforce safeinput assertions & repair email validation.

The wiring of the pre-existing safeinput tests into the automated suite
was executing them but did not raise assertion errors upon test strings
not being handled correctly thus did not enforce correctness.

Break the test values into pass validation/fail value lists. Loop these
lists and use a similar approach to testcore wherein a non-zero exit is
triggered for if a test string passes is allowed/rejected when it should
not be and wire these exits to trip AssertionError when under test.

In doing so it was discovered that email validation was not behaving the
same across versions and this turns out to be a combination of dependency
versioning problems in combination with overly broad assertion handling
in the suite masking issues. Address this here.

Author: albu-diku

mig/shared/safeinput.py

@@ -884,8 +884,14 @@ def valid_email_address(addr, allow_real_name=True):
         raise InputException("Only plain addresses without name allowed")
     if not silent_email_validator(plain_addr):
         raise InputException("No actual email address included")
-    merged = formataddr(name_address_pair)
-    if merged != addr:
+    raise_error = False
+    try:
+        merged = formataddr(name_address_pair)
+        if merged != addr:
+            raise InputException("")
+    except (InputException, UnicodeEncodeError) as exc:
+        raise_error = True
+    if raise_error:
         raise InputException("Invalid email format")
 
 
@@ -2281,24 +2287,49 @@ def __str__(self):
         return force_utf8(force_unicode(self.value))
 
 
-def main(_print=print):
+def main(_exit=sys.exit, _print=print):
     print = _print # workaround print as reserved word on PY2
 
     for test_org in ('UCPH', 'Some University, Some Dept.', 'Green Shoes Ltd.',
-                     u'Unicode Org', "Invalid R+D", "Invalid R/D",
+                     u'Unicode Org'):
+        try:
+            print('Testing valid_organization: %r' % test_org)
+            print('Filtered organization: %s' % filter_organization(test_org))
+            valid_organization(test_org)
+            print('Accepted raw organization!')
+        except InputException as exc:
+            print('Rejected raw organization %r: %s' % (test_org, exc))
+            _exit(1)
+
+    for test_org in (
+                     "Invalid R+D", "Invalid R/D",
                      "Invalid R@D", 'Test HTML Invalid <code/>'):
         try:
             print('Testing valid_organization: %r' % test_org)
             print('Filtered organization: %s' % filter_organization(test_org))
             valid_organization(test_org)
             print('Accepted raw organization!')
-        except Exception as exc:
+            _exit(1)
+        except InputException as exc:
             print('Rejected raw organization %r: %s' % (test_org, exc))
 
     for test_path in ('test.txt', 'Test Æøå', 'Test Überh4x0r',
                       'Test valid Jean-Luc Géraud', 'Test valid Źacãŕ',
                       'Test valid special%&()!$¶â€', 'Test look-alike-å å',
-                      'Test north Atlantic Barður Ðýþ', 'Test exotic لرحيم',
+                      'Test north Atlantic Barður Ðýþ'):
+        try:
+            print('Testing valid_path: %s' % test_path)
+            print('Filtered path: %s' % filter_path(test_path))
+            # print 'DEBUG %s only in %s' % ([test_path],
+            #                               [VALID_PATH_CHARACTERS])
+            valid_path(test_path)
+            print('Accepted raw path!')
+        except InputException as exc:
+            print('Rejected raw path %r: %s' % (test_path, exc))
+            _exit(1)
+
+    for test_path in (
+                      'Test exotic لرحيم',
                       'Test Invalid ?', 'Test Invalid `',
                       'Test invalid <', 'Test Invalid >',
                       'Test Invalid *', 'Test Invalid "'):
@@ -2309,14 +2340,26 @@ def main(_print=print):
             #                               [VALID_PATH_CHARACTERS])
             valid_path(test_path)
             print('Accepted raw path!')
-        except Exception as exc:
+            _exit(1)
+        except InputException as exc:
             print('Rejected raw path %r: %s' % (test_path, exc))
 
-    for test_addr in ('', 'invalid', 'abc@dk', 'abc@def.org', 'abc@def.gh.org',
-                      'aBc@Def.org', '<invalid@def.org>',
-                      'Test <abc@def.org>', 'Test Æøå <æøå@abc.org>',
+    for test_addr in ('abc@def.org', 'abc@def.gh.org',
+                      'Test <abc@def.org>',
                       'Test <abc.def@ghi.org>', 'Test <abc+def@ghi.org>',
                       'A valid-name <abc@ghi.org>',
+                      ):
+        try:
+            print('Testing valid_email_address: %s' % test_addr)
+            valid_email_address(test_addr)
+            print('Accepted raw address! %s' % [parseaddr(test_addr)])
+        except InputException as exc:
+            print('Rejected raw address %r: %s' % (test_addr, exc))
+            _exit(1)
+
+    for test_addr in (
+                      '', 'invalid', 'abc@dk', 'Test Æøå <æøå@abc.org>',
+                      'aBc@Def.org', '<invalid@def.org>',
                       ' false-positive@ghi.org',
                       'False Positive  <abc@ghi.org>',
                       ' Another False Positive  <abc@ghi.org>',
@@ -2331,7 +2374,8 @@ def main(_print=print):
             print('Testing valid_email_address: %s' % test_addr)
             valid_email_address(test_addr)
             print('Accepted raw address! %s' % [parseaddr(test_addr)])
-        except Exception as exc:
+            _exit(1)
+        except InputException as exc:
             print('Rejected raw address %r: %s' % (test_addr, exc))
 
     # OpenID 2.0 version
@@ -2378,19 +2422,24 @@ def main(_print=print):
     print("Accepted:")
     for (key, val) in accepted.items():
         print("\t%s: %s" % (key, val))
-    print("Rejected:")
-    for (key, val) in rejected.items():
-        print("\t%s: %s" % (key, val))
+    if rejected:
+        print("Rejected:")
+        for (key, val) in rejected.items():
+            print("\t%s: %s" % (key, val))
+        _exit(1)
+
     user_arguments_dict['openid.sreg.fullname'] = [
         force_unicode('Jonas Æøå Bardino')]
     (accepted, rejected) = validated_input(
         user_arguments_dict, autocreate_defaults)
     print("Accepted:")
     for (key, val) in accepted.items():
         print("\t%s: %s" % (key, val))
-    print("Rejected:")
-    for (key, val) in rejected.items():
-        print("\t%s: %s" % (key, val))
+    if rejected:
+        print("Rejected:")
+        for (key, val) in rejected.items():
+            print("\t%s: %s" % (key, val))
+        _exit(1)
 
     # OpenID Connect version
     autocreate_defaults = {
@@ -2437,9 +2486,12 @@ def main(_print=print):
     print("Accepted:")
     for (key, val) in accepted.items():
         print("\t%s: %s" % (key, val))
-    print("Rejected:")
-    for (key, val) in rejected.items():
-        print("\t%s: %s" % (key, val))
+    if rejected:
+        print("Rejected:")
+        for (key, val) in rejected.items():
+            print("\t%s: %s" % (key, val))
+        _exit(1)
+
     user_arguments_dict = {'oidc.claim.aud': ['http://somedomain.org'],
                            'oidc.claim.country': ['DK'],
                            'oidc.claim.email': ['bardino@nbi.ku.dk,bardino@science.ku.dk'],
@@ -2464,9 +2516,13 @@ def main(_print=print):
     print("Accepted:")
     for (key, val) in accepted.items():
         print("\t%s: %s" % (key, val))
-    print("Rejected:")
-    for (key, val) in rejected.items():
-        print("\t%s: %s" % (key, val))
+    if rejected:
+        print("Rejected:")
+        for (key, val) in rejected.items():
+            print("\t%s: %s" % (key, val))
+        _exit(1)
+
+    _exit(0)
 
 if __name__ == '__main__':
     main()

requirements.txt

@@ -4,7 +4,11 @@ future
 pyotp;python_version >= "3"
 pyotp<2.4;python_version < "3"
 pyyaml
-email-validator
+dnspython;python_version >= "3"
+dnspython<2;python_version < "3"
+email-validator<2.1;python_version >= "3.7"
+email-validator<2.0;python_version >= "3" and python_version < "3.7"
+email-validator<1.3;python_version < "3"
 
 # NOTE: additional optional dependencies depending on site conf are listed
 #       in recommended.txt and can be installed in the same manner by pointing

tests/test_mig_shared_safeinput.py

@@ -36,7 +36,7 @@
 sys.path.append(os.path.realpath(os.path.join(os.path.dirname(__file__), ".")))
 
 from support import MigTestCase, testmain
-from mig.shared.safeinput import \
+from mig.shared.safeinput import main as safeinput_main, InputException, \
     filter_commonname, valid_commonname
 
 PY2 = sys.version_info[0] == 2
@@ -56,12 +56,21 @@ def is_string_of_unicode(value):
 
 class MigSharedSafeinput(MigTestCase):
 
-    def test_basic_import(self):
-        safeimport = importlib.import_module("mig.shared.safeinput")
-
     def test_existing_main(self):
-        safeimport = importlib.import_module("mig.shared.safeinput")
-        safeimport.main(_print=lambda _: None)
+        def raise_on_error_exit(exit_code):
+            if exit_code != 0:
+                if raise_on_error_exit.last_print is not None:
+                    identifying_message = raise_on_error_exit.last_print
+                else:
+                    identifying_message = 'unknown'
+                raise AssertionError(
+                    'failure in unittest/testcore: %s' % (identifying_message,))
+        raise_on_error_exit.last_print = None
+
+        def record_last_print(value):
+            raise_on_error_exit.last_print = value
+
+        safeinput_main(_exit=raise_on_error_exit, _print=record_last_print)
 
     COMMONNAME_PERMITTED = (
         'Firstname Lastname',
@@ -81,15 +90,15 @@ def test_commonname_valid(self):
             saw_raise = False
             try:
                 valid_commonname(test_cn)
-            except Exception:
+            except InputException:
                 saw_raise = True
             self.assertFalse(saw_raise)
 
         for test_cn in self.COMMONNAME_PROHIBITED:
             saw_raise = False
             try:
                 valid_commonname(test_cn)
-            except Exception:
+            except InputException:
                 saw_raise = True
             self.assertTrue(saw_raise)