Rework input validation to more closely match what we require in the UI

5 files changed, 102 insertions, 77 deletions

diff --git a/ipa-admintools/ipa-addgroup b/ipa-admintools/ipa-addgroup
index 15d42a91e..8169dea12 100644
--- a/ipa-admintools/ipa-addgroup
+++ b/ipa-admintools/ipa-addgroup
@@ -62,28 +62,28 @@ def main():
     if (len(args) != 2):
         while (cont != True):
             cn = raw_input("Group name: ")
-            if (ipavalidate.plain(cn, notEmpty=True)):
-                print "Field is required and must be letters or '."
+            if (ipavalidate.String(cn, notEmpty=True)):
+                print "Please enter a value"
             else:
                 cont = True
     else:
         cn = args[1]
-        if (ipavalidate.plain(cn, notEmpty=True)):
-            print "Group name is required and must be letters or '."
+        if (ipavalidate.ipastsring(cn, notEmpty=True)):
+            print "Please enter a value"
             return 1
 
     cont = False
     if not options.desc:
         while (cont != True):
             desc = raw_input("Description: ")
-            if (ipavalidate.plain(desc, notEmpty=True)):
-                print "Field is required and must be letters or '."
+            if (ipavalidate.String(desc, notEmpty=True)):
+                print "Please enter a value"
             else:
                 cont = True
     else:
         desc = options.desc
-        if (ipavalidate.plain(desc, notEmpty=True)):
-            print "First name is required and must be letters or '."
+        if (ipavalidate.String(desc, notEmpty=True)):
+            print "Please enter a value"
             return 1
 
     if options.gid:
diff --git a/ipa-admintools/ipa-adduser b/ipa-admintools/ipa-adduser
index 96b43e0d5..298834258 100644
--- a/ipa-admintools/ipa-adduser
+++ b/ipa-admintools/ipa-adduser
@@ -92,42 +92,42 @@ def main():
     if not options.gn:
         while (cont != True):
             givenname = raw_input("First name: ")
-            if (ipavalidate.plain(givenname, notEmpty=True)):
-                print "Field is required and must be letters or '"
+            if (ipavalidate.String(givenname, notEmpty=True)):
+                print "Please enter a value"
             else:
                 cont = True
     else:
         givenname = options.gn
-        if (ipavalidate.plain(givenname, notEmpty=True)):
-            print "First name is required and must be letters or '"
+        if (ipavalidate.String(givenname, notEmpty=True)):
+            print "Please enter a value"
             return 1
 
     cont = False
     if not options.sn:
         while (cont != True):
             lastname = raw_input("Last name: ")
-            if (ipavalidate.plain(lastname, notEmpty=True)):
-                print "Field is required and must be letters or '"
+            if (ipavalidate.String(lastname, notEmpty=True)):
+                print "Please enter a value"
             else:
                 cont = True
     else:
         lastname = options.sn
-        if (ipavalidate.plain(lastname, notEmpty=True)):
-            print "Last name is required and must be letters or '"
+        if (ipavalidate.String(lastname, notEmpty=True)):
+            print "Please enter a value"
             return 1
 
     cont = False
     if (len(args) != 2):
         while (cont != True):
             username = raw_input("Login name: ")
-            if (ipavalidate.plain(username, notEmpty=True)):
-                print "Field is required and must be letters or '"
+            if (ipavalidate.Plain(username, notEmpty=True, allowSpaces=False)):
+                print "Please enter a value"
             else:
                 cont = True
     else:
         username = args[1]
-        if (ipavalidate.plain(username, notEmpty=True)):
-            print "Username is required and must be letters or '"
+        if (ipavalidate.Plain(username, notEmpty=True, allowSpaces=False)):
+            print "Username is required and may only include letters and numbers"
             return 1
 
     if not options.password:
@@ -147,7 +147,7 @@ def main():
 
     if options.mail:
         mail = options.mail
-        if (ipavalidate.email(mail)):
+        if (ipavalidate.Email(mail)):
             print "The email provided seem not a valid email."
             return 1
 
@@ -158,8 +158,8 @@ def main():
         if not options.gecos:
             while (cont != True):
                 gecos = raw_input("gecos []: ")
-                if (ipavalidate.plain(gecos, notEmpty=False)):
-                    print "Must be letters, numbers, spaces or '"
+                if (ipavalidate.String(gecos, notEmpty=False)):
+                    print "Please enter a value"
                 else:
                     cont = True
         cont = False
@@ -168,8 +168,8 @@ def main():
                 directory = raw_input("home directory [/home/"+username+"]: ")
                 if directory == "":
                      directory = "/home/"+username
-                if (ipavalidate.path(directory, notEmpty=False)):
-                    print "Must be letters, numbers, spaces or '"
+                if (ipavalidate.Path(directory, notEmpty=False)):
+                    print "Please enter a value"
                 else:
                     cont = True
         cont = False
diff --git a/ipa-admintools/ipa-usermod b/ipa-admintools/ipa-usermod
index 9d3e7794c..5d33700f9 100644
--- a/ipa-admintools/ipa-usermod
+++ b/ipa-admintools/ipa-usermod
@@ -124,8 +124,8 @@ def main():
         if not options.gn:
             while (cont != True):
                 givenname = raw_input("First name: [%s] " % user.getValue('givenname'))
-                if (ipavalidate.plain(givenname, notEmpty=False)):
-                    print "Must be letters or '"
+                if (ipavalidate.String(givenname, notEmpty=False)):
+                    print "Please enter a value"
                 else:
                     cont = True
                 if len(givenname) < 1:
@@ -133,16 +133,16 @@ def main():
                     cont = True
         else:
             givenname = options.gn
-            if (ipavalidate.plain(givenname, notEmpty=True)):
-                print "First name must be letters or '"
+            if (ipavalidate.String(givenname, notEmpty=True)):
+                print "Please enter a value"
                 return 1
 
         cont = False
         if not options.sn:
             while (cont != True):
                 lastname = raw_input(" Last name: [%s] " % user.getValue('sn'))
-                if (ipavalidate.plain(lastname, notEmpty=False)):
-                    print "Must be letters or '"
+                if (ipavalidate.String(lastname, notEmpty=False)):
+                    print "Please enter a value"
                 else:
                     cont = True
                 if len(lastname) < 1:
@@ -150,21 +150,21 @@ def main():
                     cont = True
         else:
             lastname = options.sn
-            if (ipavalidate.plain(lastname, notEmpty=True)):
-                print "Last name must be letters or '"
+            if (ipavalidate.String(lastname, notEmpty=True)):
+                print "Please enter a value"
                 return 1
 
         cont = False
         if not options.mail:
             while (cont != True):
                 mail = raw_input("E-mail addr: [%s]" % user.getValue('mail'))
-                if (ipavalidate.email(mail, notEmpty=False)):
-                    print "Must include a user and domain name"
+                if (ipavalidate.Email(mail, notEmpty=False)):
+                    print "E-mail must include a user and domain name"
                 else:
                     cont = True
         else:
             mail = options.mail
-            if (ipavalidate.email(mail)):
+            if (ipavalidate.Email(mail)):
                 print "E-mail must include a user and domain name"
                 return 1
 
@@ -174,8 +174,8 @@ def main():
         if not options.gecos:
             while (cont != True):
                 gecos = raw_input("gecos: [%s] " % user.getValue('gecos'))
-                if (ipavalidate.plain(gecos, notEmpty=False)):
-                    print "Must be letters, numbers, spaces or '"
+                if (ipavalidate.String(gecos, notEmpty=False)):
+                    print "Please enter a value"
                 else:
                     cont = True
 
@@ -183,8 +183,8 @@ def main():
         if not options.directory:
             while (cont != True):
                 directory = raw_input("home directory: [%s] " % user.getValue('homeDirectory'))
-                if (ipavalidate.path(gecos, notEmpty=False)):
-                    print "Must be letters, numbers, spaces or '"
+                if (ipavalidate.Path(gecos, notEmpty=False)):
+                    print "Valid path is required"
                 else:
                     cont = True
         cont = False
diff --git a/ipa-python/ipavalidate.py b/ipa-python/ipavalidate.py
index 918c34a62..1750b8397 100644
--- a/ipa-python/ipavalidate.py
+++ b/ipa-python/ipavalidate.py
@@ -19,7 +19,7 @@
 
 import re
 
-def email(mail, notEmpty=True):
+def Email(mail, notEmpty=True):
     """Do some basic validation of an e-mail address.
        Return 0 if ok
        Return 1 if not
@@ -49,7 +49,7 @@ def email(mail, notEmpty=True):
 
     return 0
 
-def plain(text, notEmpty=False):
+def Plain(text, notEmpty=False, allowSpaces=True):
     """Do some basic validation of a plain text field
        Return 0 if ok
        Return 1 if not
@@ -57,23 +57,33 @@ def plain(text, notEmpty=False):
        If notEmpty is True the this will return an error if the field
        is "" or None.
     """
-    textRE = re.compile(r"^[a-zA-Z_\-0-9\'\ ]*$")
-
-    if not text and notEmpty is True:
-        return 1
-
-    if text is None:
+    if (text is None) or (not text.strip()):
         if notEmpty is True:
             return 1
         else:
             return 0
 
+    if allowSpaces:
+        textRE = re.compile(r"^[a-zA-Z_\-0-9\'\ ]*$")
+    else:
+        textRE = re.compile(r"^[a-zA-Z_\-0-9\']*$")
     if not textRE.search(text):
         return 1
 
     return 0
 
-def path(text, notEmpty=False):
+def String(text, notEmpty=False):
+    """A string type. This is much looser in what it allows than plain"""
+
+    if text is None or not text.strip():
+        if notEmpty is True:
+            return 1
+        else:
+            return 0
+
+    return 0
+
+def Path(text, notEmpty=False):
     """Do some basic validation of a path
        Return 0 if ok
        Return 1 if not
diff --git a/ipa-python/test/test_ipavalidate.py b/ipa-python/test/test_ipavalidate.py
index 507f7e022..677866d04 100644
--- a/ipa-python/test/test_ipavalidate.py
+++ b/ipa-python/test/test_ipavalidate.py
@@ -31,39 +31,54 @@ class TestValidate(unittest.TestCase):
     def tearDown(self):
         pass
 
-    def test_validemail(self):
-        self.assertEqual(0, ipavalidate.email("test@freeipa.org"))
-        self.assertEqual(0, ipavalidate.email("", notEmpty=False))
+    def test_validEmail(self):
+        self.assertEqual(0, ipavalidate.Email("test@freeipa.org"))
+        self.assertEqual(0, ipavalidate.Email("", notEmpty=False))
 
-    def test_invalidemail(self):
-        self.assertEqual(1, ipavalidate.email("test"))
-        self.assertEqual(1, ipavalidate.email("test@freeipa"))
-        self.assertEqual(1, ipavalidate.email("test@.com"))
-        self.assertEqual(1, ipavalidate.email(""))
-        self.assertEqual(1, ipavalidate.email(None))
+    def test_invalidEmail(self):
+        self.assertEqual(1, ipavalidate.Email("test"))
+        self.assertEqual(1, ipavalidate.Email("test@freeipa"))
+        self.assertEqual(1, ipavalidate.Email("test@.com"))
+        self.assertEqual(1, ipavalidate.Email(""))
+        self.assertEqual(1, ipavalidate.Email(None))
 
-    def test_validplain(self):
-        self.assertEqual(0, ipavalidate.plain("Joe User"))
-        self.assertEqual(0, ipavalidate.plain("Joe O'Malley"))
-        self.assertEqual(0, ipavalidate.plain("", notEmpty=False))
-        self.assertEqual(0, ipavalidate.plain(None, notEmpty=False))
+    def test_validPlain(self):
+        self.assertEqual(0, ipavalidate.Plain("Joe User"))
+        self.assertEqual(0, ipavalidate.Plain("Joe O'Malley"))
+        self.assertEqual(0, ipavalidate.Plain("", notEmpty=False))
+        self.assertEqual(0, ipavalidate.Plain(None, notEmpty=False))
+        self.assertEqual(0, ipavalidate.Plain("JoeUser", allowSpaces=False))
+        self.assertEqual(0, ipavalidate.Plain("JoeUser", allowSpaces=True))
 
-    def test_invalidplain(self):
-        self.assertEqual(1, ipavalidate.plain("Joe (User)"))
-        self.assertEqual(1, ipavalidate.plain("", notEmpty=True))
-        self.assertEqual(1, ipavalidate.plain(None, notEmpty=True))
+    def test_invalidPlain(self):
+        self.assertEqual(1, ipavalidate.Plain("Joe (User)"))
+        self.assertEqual(1, ipavalidate.Plain("Joe C. User"))
+        self.assertEqual(1, ipavalidate.Plain("", notEmpty=True))
+        self.assertEqual(1, ipavalidate.Plain(None, notEmpty=True))
+        self.assertEqual(1, ipavalidate.Plain("Joe User", allowSpaces=False))
 
-    def test_validpath(self):
-        self.assertEqual(0, ipavalidate.path("/"))
-        self.assertEqual(0, ipavalidate.path("/home/user"))
-        self.assertEqual(0, ipavalidate.path("../home/user"))
-        self.assertEqual(0, ipavalidate.path("", notEmpty=False))
-        self.assertEqual(0, ipavalidate.path(None, notEmpty=False))
+    def test_validString(self):
+        self.assertEqual(0, ipavalidate.String("Joe User"))
+        self.assertEqual(0, ipavalidate.String("Joe O'Malley"))
+        self.assertEqual(1, ipavalidate.Plain("Joe C. User"))
+        self.assertEqual(0, ipavalidate.String("", notEmpty=False))
+        self.assertEqual(0, ipavalidate.String(None, notEmpty=False))
 
-    def test_invalidpath(self):
-        self.assertEqual(1, ipavalidate.path("(foo)"))
-        self.assertEqual(1, ipavalidate.path("", notEmpty=True))
-        self.assertEqual(1, ipavalidate.path(None, notEmpty=True))
+    def test_invalidString(self):
+        self.assertEqual(1, ipavalidate.String("", notEmpty=True))
+        self.assertEqual(1, ipavalidate.String(None, notEmpty=True))
+
+    def test_validPath(self):
+        self.assertEqual(0, ipavalidate.Path("/"))
+        self.assertEqual(0, ipavalidate.Path("/home/user"))
+        self.assertEqual(0, ipavalidate.Path("../home/user"))
+        self.assertEqual(0, ipavalidate.Path("", notEmpty=False))
+        self.assertEqual(0, ipavalidate.Path(None, notEmpty=False))
+
+    def test_invalidPath(self):
+        self.assertEqual(1, ipavalidate.Path("(foo)"))
+        self.assertEqual(1, ipavalidate.Path("", notEmpty=True))
+        self.assertEqual(1, ipavalidate.Path(None, notEmpty=True))
 
 if __name__ == '__main__':
     unittest.main()