fuller email validition (request [SF#216291])

Justus Pendleton · Justus Pendleton · commit ea7f777b9666 · 2007-09-12T21:11:14.000Z
Checks email syntax more rigorously.
Perform address checks against "address" and any "alternate_addresses".
Changed all of the unit tests to have addresses that pass this new check.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -49,6 +49,7 @@ Fixed:
 - respect umask on filestorage backends (Ulrik Mikaelsson) (sf bug 1744328)
 - cope with spam robots posting multiple instances of the same form
 - include the author of property-only changes in generated messages
+- fuller email validation in templates (sf feature 1216291)
 
 
 2007-02-15 1.3.3
diff --git a/templates/classic/detectors/userauditor.py b/templates/classic/detectors/userauditor.py
@@ -18,44 +18,73 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 #
-#$Id: userauditor.py,v 1.8 2007-09-11 21:28:29 jpend Exp $
+#$Id: userauditor.py,v 1.9 2007-09-12 21:11:13 jpend Exp $
+
+import re
+
+# regular expression thanks to: http://www.regular-expressions.info/email.html
+# this is the "99.99% solution for syntax only".
+email_regexp = (r"[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*", r"(localhost|(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9]))")
+email_rfc = re.compile('^' + email_regexp[0] + '@' + email_regexp[1] + '$', re.IGNORECASE)
+email_local = re.compile('^' + email_regexp[0] + '$', re.IGNORECASE)
+
+def valid_address(address):
+    ''' If we see an @-symbol in the address then check against the full
+        RFC syntax. Otherwise it is a local-only address so only check
+        the local part of the RFC syntax.
+    '''
+    if '@' in address:
+        return email_rfc.match(address)
+    else:
+        return email_local.match(address)
+
+def get_addresses(user):
+    ''' iterate over all known addresses in a newvalues dict
+        this takes of the address/alterate_addresses handling
+    '''
+    if user.has_key('address'):
+        yield user['address']
+    if user.get('alternate_addresses', None):
+        for address in user['alternate_addresses'].split('\n'):
+            yield address
 
 def audit_user_fields(db, cl, nodeid, newvalues):
     ''' Make sure user properties are valid.
 
-        - email address has no spaces in it
+        - email address is syntactically valid
         - email address is unique
         - roles specified exist
         - timezone is valid
     '''
-    if newvalues.has_key('address'):
-        address = newvalues['address']
-        if address:
-            if ' ' in address:
-                raise ValueError, 'Email address must not contain spaces'
-            user = db.user.stringFind(address=address)
-            if len(user):
-                raise ValueError, 'Email address already in use'
+
+    for address in get_addresses(newvalues):
+        if not valid_address(address):
+            raise ValueError, 'Email address syntax is invalid'
+
+        check_main = db.user.stringFind(address=address)
+        # make sure none of the alts are owned by anyone other than us (x!=nodeid)
+        check_alts = [x for x in db.user.filter(None, {'alternate_addresses' : address}) if x != nodeid]
+        if check_main or check_alts:
+            raise ValueError, 'Email address %s already in use' % address
 
     for rolename in [r.lower().strip() for r in newvalues.get('roles', '').split(',')]:
             if rolename and not db.security.role.has_key(rolename):
                 raise ValueError, 'Role "%s" does not exist'%rolename
 
-    if newvalues.has_key('timezone'):
-        tz = newvalues['timezone']
-        if tz:
-            # if they set a new timezone validate the timezone by attempting to
-            # use it before we store it to the db.
-            import roundup.date
-            import datetime
-            try:
-                TZ = roundup.date.get_timezone(tz)
-                dt = datetime.datetime.now()
-                local = TZ.localize(dt).utctimetuple()
-            except IOError:
-                raise ValueError, 'Timezone "%s" does not exist' % tz
-            except ValueError:
-                raise ValueError, 'Timezone "%s" exceeds valid range [-23...23]' % tz
+    tz = newvalues.get('timezone', None)
+    if tz:
+        # if they set a new timezone validate the timezone by attempting to
+        # use it before we store it to the db.
+        import roundup.date
+        import datetime
+        try:
+            TZ = roundup.date.get_timezone(tz)
+            dt = datetime.datetime.now()
+            local = TZ.localize(dt).utctimetuple()
+        except IOError:
+            raise ValueError, 'Timezone "%s" does not exist' % tz
+        except ValueError:
+            raise ValueError, 'Timezone "%s" exceeds valid range [-23...23]' % tz
 
 def init(db):
     # fire before changes are made
diff --git a/templates/minimal/detectors/userauditor.py b/templates/minimal/detectors/userauditor.py
@@ -18,43 +18,73 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 #
-#$Id: userauditor.py,v 1.7 2007-09-11 21:28:29 jpend Exp $
+#$Id: userauditor.py,v 1.8 2007-09-12 21:11:14 jpend Exp $
+
+import re
+
+# regular expression thanks to: http://www.regular-expressions.info/email.html
+# this is the "99.99% solution for syntax only".
+email_regexp = (r"[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*", r"(localhost|(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9]))")
+email_rfc = re.compile('^' + email_regexp[0] + '@' + email_regexp[1] + '$', re.IGNORECASE)
+email_local = re.compile('^' + email_regexp[0] + '$', re.IGNORECASE)
+
+def valid_address(address):
+    ''' If we see an @-symbol in the address then check against the full
+        RFC syntax. Otherwise it is a local-only address so only check
+        the local part of the RFC syntax.
+    '''
+    if '@' in address:
+        return email_rfc.match(address)
+    else:
+        return email_local.match(address)
+
+def get_addresses(user):
+    ''' iterate over all known addresses in a newvalues dict
+        this takes of the address/alterate_addresses handling
+    '''
+    if user.has_key('address'):
+        yield user['address']
+    if user.get('alternate_addresses', None):
+        for address in user['alternate_addresses'].split('\n'):
+            yield address
 
 def audit_user_fields(db, cl, nodeid, newvalues):
     ''' Make sure user properties are valid.
 
-        - email address has no spaces in it
+        - email address is syntactically valid
         - email address is unique
         - roles specified exist
         - timezone is valid
     '''
-    if newvalues.has_key('address'):
-        address = newvalues['address']
-        if ' ' in address:
-            raise ValueError, 'Email address must not contain spaces'
-        user = db.user.stringFind(address=address)
-        if len(user):
-            raise ValueError, 'Email address already in use'
+
+    for address in get_addresses(newvalues):
+        if not valid_address(address):
+            raise ValueError, 'Email address syntax is invalid'
+
+        check_main = db.user.stringFind(address=address)
+        # make sure none of the alts are owned by anyone other than us (x!=nodeid)
+        check_alts = [x for x in db.user.filter(None, {'alternate_addresses' : address}) if x != nodeid]
+        if check_main or check_alts:
+            raise ValueError, 'Email address %s already in use' % address
 
     for rolename in [r.lower().strip() for r in newvalues.get('roles', '').split(',')]:
             if rolename and not db.security.role.has_key(rolename):
                 raise ValueError, 'Role "%s" does not exist'%rolename
 
-    if newvalues.has_key('timezone'):
-        tz = newvalues['timezone']
-        if tz:
-            # if they set a new timezone validate the timezone by attempting to
-            # use it before we store it to the db.
-            import roundup.date
-            import datetime
-            try:
-                TZ = roundup.date.get_timezone(tz)
-                dt = datetime.datetime.now()
-                local = TZ.localize(dt).utctimetuple()
-            except IOError:
-                raise ValueError, 'Timezone "%s" does not exist' % tz
-            except ValueError:
-                raise ValueError, 'Timezone "%s" exceeds valid range [-23...23]' % tz
+    tz = newvalues.get('timezone', None)
+    if tz:
+        # if they set a new timezone validate the timezone by attempting to
+        # use it before we store it to the db.
+        import roundup.date
+        import datetime
+        try:
+            TZ = roundup.date.get_timezone(tz)
+            dt = datetime.datetime.now()
+            local = TZ.localize(dt).utctimetuple()
+        except IOError:
+            raise ValueError, 'Timezone "%s" does not exist' % tz
+        except ValueError:
+            raise ValueError, 'Timezone "%s" exceeds valid range [-23...23]' % tz
 
 def init(db):
     # fire before changes are made
diff --git a/test/test_cgi.py b/test/test_cgi.py
@@ -8,7 +8,7 @@
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
 #
-# $Id: test_cgi.py,v 1.30 2007-07-05 19:21:57 schlatterbeck Exp $
+# $Id: test_cgi.py,v 1.31 2007-09-12 21:11:14 jpend Exp $
 
 import unittest, os, shutil, errno, sys, difflib, cgi, re
 
@@ -70,7 +70,7 @@ def setUp(self):
         self.db = self.instance.open('admin')
         self.db.user.create(username='Chef', address='chef@bork.bork.bork',
             realname='Bork, Chef', roles='User')
-        self.db.user.create(username='mary', address='mary@test',
+        self.db.user.create(username='mary', address='mary@test.test',
             roles='User', realname='Contrary, Mary')
 
         test = self.instance.backend.Class(self.db, "test",
diff --git a/test/test_mailgw.py b/test/test_mailgw.py
diff --git a/test/test_userauditor.py b/test/test_userauditor.py
