Manually merged PR95 to enable py2+3 unit testing of safeinput and clarify which tests should pass and which should fail. Adjust email-validator requirements for various python versions with the note that python-2.7 does NOT really support email-validator-1.3.0 despite pypi metadata claiming it. Added comments about current limitations and pending work for the validation to be more robust and RFC 6532 compliant.

git-svn-id: svn+ssh://svn.code.sf.net/p/migrid/code/trunk@6108 b75ad72c-e7d7-11dd-a971-7dbc132099af

Author: jonasbardino

mig/shared/safeinput.py

@@ -117,7 +117,7 @@
 # Unicode letter categories as defined on
 # http://www.unicode.org/reports/tr44/#GC_Values_Table
 # TODO: should we go all in and allow even these very exotic modifiers?
-#_ACCENT_CATS = frozenset(('Lu', 'Ll', 'Lt', 'Lm', 'Lo', ))
+# _ACCENT_CATS = frozenset(('Lu', 'Ll', 'Lt', 'Lm', 'Lo', ))
 _ACCENT_CATS = frozenset(('Lu', 'Ll', 'Lt', ))
 
 # Use utf8 byte string representation here ("something" and not u"something")
@@ -879,13 +879,29 @@ def valid_email_address(addr, allow_real_name=True):
     John Doe <john@doe.org>
     """
 
+    # TODO: rework this as formataddr does NOT have RFC6532 support on py3
+    #       and assuming parse+format matches orig also introduces false
+    #       positives regarding e.g. extra white space in display name.
+    #       https://github.com/python/cpython/issues/70143
     (real_name, plain_addr) = name_address_pair = parseaddr(addr)
     if real_name and not allow_real_name:
         raise InputException("Only plain addresses without name allowed")
     if not silent_email_validator(plain_addr):
+        # NOTE: email-validator validate_email function ONLY accepts non-ascii
+        #       chars when passed unicode on py2, so we end up here for valid
+        #       RFC 6532 addresses when passed as native byte strings.
         raise InputException("No actual email address included")
-    merged = formataddr(name_address_pair)
-    if merged != addr:
+    try:
+        merged = formataddr(name_address_pair)
+        if merged != addr:
+            raise ValueError("parsed vs raw email mismatch")
+    except (ValueError, UnicodeEncodeError) as exc:
+        # TODO: eliminate this UnicodeEncodeError handling here once reworked
+        # NOTE: formataddr does NOT support non-ascii chars on py3 at all and
+        #       explicitly encodes to ascii so we end up here for valid RFC
+        #       6532 addresses. See note and link above for details.
+        # print("unicode err in %s" %
+        #       [addr, real_name, plain_addr, name_address_pair])
         raise InputException("Invalid email format")
 
 
@@ -2281,24 +2297,47 @@ def __str__(self):
         return force_utf8(force_unicode(self.value))
 
 
-def main(_print=print):
-    print = _print # workaround print as reserved word on PY2
+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",
-                     "Invalid R@D", 'Test HTML Invalid <code/>'):
+                     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,19 +2348,40 @@ 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>',
+    # Simple valid addresses to accept
+    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>',
+                      '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)
+
+    # TODO: fix validation and toggle exit back on below
+    # Corner-case valid addresses that we currently don't accept
+    for test_addr in ('Test Æøå <æøå@abc.org>',
+                      'aBc@Def.org', '<valid@def.org>',
                       ' false-positive@ghi.org',
                       'False Positive  <abc@ghi.org>',
-                      ' Another False Positive  <abc@ghi.org>',
-                      'invalid <abc@ghi,org>', 'invalid <abc@',
-                      'invalid abc@def.org',
+                      ' Another False Positive  <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)
+
+    # Simple invalid addresses to fail on
+    for test_addr in ('', 'invalid', 'abc@dk', 'invalid <abc@ghi,org>',
+                      'invalid <abc@', 'invalid abc@def.org',
                       'Test <abc@ghi.org>, twice <def@def.org>',
                       'A !#¤%/) positive  <abc@ghi.org>',
                       'a@b.c<script>alert("XSS vulnerable");</script>',
@@ -2331,7 +2391,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 +2439,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 +2503,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 +2533,14 @@ 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)