Change user and group validators to match shadow-utils

This sets the regex to [a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?

Also change the validators to return True/False

450613, 457124

12 files changed, 171 insertions, 79 deletions

diff --git a/ipa-admintools/ipa-addgroup b/ipa-admintools/ipa-addgroup
index cc813836c..d99d588b8 100644
--- a/ipa-admintools/ipa-addgroup
+++ b/ipa-admintools/ipa-addgroup
@@ -27,6 +27,7 @@ try:
     import ipa.ipautil as ipautil
     import ipa.config
     import ipa.ipaerror
+    import ipa.ipaadminutil as ipaadminutil
 
     import xmlrpclib
     import kerberos
@@ -82,18 +83,20 @@ def main():
         usage()
 
     if (len(args) != 2):
-        cn = ipautil.user_input("Group name", allow_empty = False)
+        cn = ipautil.user_input_name("Group name")
     else:
         cn = args[1]
-        if (ipavalidate.String(cn, notEmpty=True)):
-            print "Please enter a value"
+        try:
+            ipaadminutil.check_name(cn)
+        except ValueError, e:
+            print "Group name " + str(e)
             return 1
 
     if not options.desc:
         desc = ipautil.user_input("Description", allow_empty = False)
     else:
         desc = options.desc
-        if (ipavalidate.String(desc, notEmpty=True)):
+        if (not ipavalidate.String(desc, notEmpty=True)):
             print "Please enter a value"
             return 1
 
diff --git a/ipa-admintools/ipa-addservice b/ipa-admintools/ipa-addservice
index ddf75a808..29d0bb966 100644
--- a/ipa-admintools/ipa-addservice
+++ b/ipa-admintools/ipa-addservice
@@ -23,7 +23,6 @@ try:
     import ipa
     import ipa.user
     import ipa.ipaclient as ipaclient
-    import ipa.ipavalidate as ipavalidate
     import ipa.ipautil as ipautil
     import ipa.config
 
diff --git a/ipa-admintools/ipa-adduser b/ipa-admintools/ipa-adduser
index 17c75edf8..9db45166d 100644
--- a/ipa-admintools/ipa-adduser
+++ b/ipa-admintools/ipa-adduser
@@ -26,6 +26,7 @@ try:
     import ipa.ipavalidate as ipavalidate
     import ipa.ipautil as ipautil
     import ipa.config
+    import ipa.ipaadminutil as ipaadminutil
 
     import xmlrpclib
     import kerberos
@@ -118,7 +119,7 @@ def main():
         givenname = ipautil.user_input("First name", allow_empty = False)
     else:
         givenname = options.gn
-        if (ipavalidate.String(givenname, notEmpty=True)):
+        if (not ipavalidate.String(givenname, notEmpty=True)):
             print "Please enter a value"
             return 1
 
@@ -126,16 +127,18 @@ def main():
         lastname = ipautil.user_input("Last name", allow_empty = False)
     else:
         lastname = options.sn
-        if (ipavalidate.String(lastname, notEmpty=True)):
+        if (not ipavalidate.String(lastname, notEmpty=True)):
             print "Please enter a value"
             return 1
 
     if (len(args) != 2):
-        username = ipautil.user_input_plain("Login name", allow_empty = False, allow_spaces = False)
+        username = ipautil.user_input_name("Login name")
     else:
         username = args[1]
-        if (ipavalidate.Plain(username, notEmpty=True, allowSpaces=False)):
-            print "Username is required and may only include letters and numbers"
+        try:
+            ipaadminutil.check_name(username)
+        except ValueError, e:
+            print "Login name " + str(e)
             return 1
 
     if options.password_prompt:
@@ -155,7 +158,7 @@ def main():
 
     if options.mail:
         mail = options.mail
-        if (ipavalidate.Email(mail)):
+        if (not ipavalidate.Email(mail)):
             print "The email provided seem not a valid email."
             return 1
 
diff --git a/ipa-admintools/ipa-moduser b/ipa-admintools/ipa-moduser
index 4b3bd5c9c..4ea40632e 100644
--- a/ipa-admintools/ipa-moduser
+++ b/ipa-admintools/ipa-moduser
@@ -143,7 +143,7 @@ def main():
             givenname = ipautil.user_input("First name", user.getValue('givenname'), allow_empty = False)
         else:
             givenname = options.gn
-            if (ipavalidate.String(givenname, notEmpty=True)):
+            if (not ipavalidate.String(givenname, notEmpty=True)):
                 print "Please enter a value"
                 return 1
 
@@ -151,7 +151,7 @@ def main():
             lastname = ipautil.user_input("Last name", user.getValue('sn'), allow_empty = False)
         else:
             lastname = options.sn
-            if (ipavalidate.String(lastname, notEmpty=True)):
+            if (not ipavalidate.String(lastname, notEmpty=True)):
                 print "Please enter a value"
                 return 1
 
@@ -159,7 +159,7 @@ def main():
             mail = ipautil.user_input_email("E-mail address", user.getValue('mail'), allow_empty = True)
         else:
             mail = options.mail
-            if (ipavalidate.Email(mail)):
+            if (not ipavalidate.Email(mail)):
                 print "E-mail must include a user and domain name"
                 return 1
 
diff --git a/ipa-python/ipaadminutil.py b/ipa-python/ipaadminutil.py
index d94ced4d9..27337642c 100644
--- a/ipa-python/ipaadminutil.py
+++ b/ipa-python/ipaadminutil.py
@@ -22,6 +22,7 @@ import tempfile
 import logging
 import subprocess
 import os
+import ipa.ipavalidate as ipavalidate
 
 def select_user(counter, users):
     """counter is the number of User objects in users
@@ -82,3 +83,11 @@ def select_group(counter, groups):
         print "Please enter a number between 1 and %s" % counter
 
     return groupindex
+
+def check_name(name):
+    """Helper to ensure that a user or group name is legal"""
+
+    if (not ipavalidate.GoodName(name, notEmpty=True)):
+        raise ValueError("may only include letters, numbers, _, -, . and $")
+
+    return
diff --git a/ipa-python/ipautil.py b/ipa-python/ipautil.py
index 3526cc7a8..f3018ed09 100644
--- a/ipa-python/ipautil.py
+++ b/ipa-python/ipautil.py
@@ -30,6 +30,7 @@ import stat
 import shutil
 
 from ipa import ipavalidate
+from ipa import ipaadminutil
 from types import *
 
 import re
@@ -529,13 +530,13 @@ def user_input_email(prompt, default = None, allow_empty = False):
         ret = user_input(prompt, default, allow_empty)
         if allow_empty and ret.lower() == "none":
             return ""
-        if not ipavalidate.Email(ret, not allow_empty):
+        if ipavalidate.Email(ret, not allow_empty):
             return ret.strip()
 
 def user_input_plain(prompt, default = None, allow_empty = True, allow_spaces = True):
     while True:
         ret = user_input(prompt, default, allow_empty)
-        if not ipavalidate.Plain(ret, not allow_empty, allow_spaces):
+        if ipavalidate.Plain(ret, not allow_empty, allow_spaces):
             return ret
 
 def user_input_path(prompt, default = None, allow_empty = True):
@@ -545,9 +546,17 @@ def user_input_path(prompt, default = None, allow_empty = True):
         ret = user_input(prompt, default, allow_empty)
         if allow_empty and ret.lower() == "none":
             return ""
-        if not ipavalidate.Path(ret, not allow_empty):
+        if ipavalidate.Path(ret, not allow_empty):
             return ret
 
+def user_input_name(prompt, default = None):
+    while True:
+        ret = user_input(prompt, default, False)
+        try:
+            ipaadminutil.check_name(ret)
+            return ret
+        except ValueError, e:
+            print prompt + " " + str(e)
 
 class AttributeValueCompleter:
     '''
diff --git a/ipa-python/ipavalidate.py b/ipa-python/ipavalidate.py
index 4dc7fe1a6..63e0a7614 100644
--- a/ipa-python/ipavalidate.py
+++ b/ipa-python/ipavalidate.py
@@ -21,8 +21,8 @@ import re
 
 def Email(mail, notEmpty=True):
     """Do some basic validation of an e-mail address.
-       Return 0 if ok
-       Return 1 if not
+       Return True if ok
+       Return False if not
 
        If notEmpty is True the this will return an error if the field
        is "" or None.
@@ -32,61 +32,61 @@ def Email(mail, notEmpty=True):
 
     if not mail or mail is None:
         if  notEmpty is True:
-            return 1
+            return False
         else:
-            return 0
+            return True
 
     mail = mail.strip()
     s = mail.split('@', 1)
     try:
         username, domain=s
     except ValueError:
-        return 1
+        return False
     if not usernameRE.search(username):
-        return 1
+        return False
     if not domainRE.search(domain):
-        return 1
+        return False
 
-    return 0
+    return True
 
 def Plain(text, notEmpty=False, allowSpaces=True):
     """Do some basic validation of a plain text field
-       Return 0 if ok
-       Return 1 if not
+       Return True if ok
+       Return False if not
 
        If notEmpty is True the this will return an error if the field
        is "" or None.
     """
     if (text is None) or (not text.strip()):
         if notEmpty is True:
-            return 1
+            return False
         else:
-            return 0
+            return True
 
     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 False
 
-    return 0
+    return True
 
 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
+            return False
         else:
-            return 0
+            return True
 
-    return 0
+    return True
 
 def Path(text, notEmpty=False):
     """Do some basic validation of a path
-       Return 0 if ok
-       Return 1 if not
+       Return True if ok
+       Return False if not
 
        If notEmpty is True the this will return an error if the field
        is "" or None.
@@ -94,16 +94,44 @@ def Path(text, notEmpty=False):
     textRE = re.compile(r"^[a-zA-Z_\-0-9\\ \.\/\\:]*$")
 
     if not text and notEmpty is True:
-        return 1
+        return False
 
     if text is None:
         if notEmpty is True:
-            return 1
+            return False
         else:
-            return 0
+            return True
 
     if not textRE.search(text):
-        return 1
+        return False
 
-    return 0
+    return True
 
+def GoodName(text, notEmpty=False):
+    """From shadow-utils:
+
+       User/group names must match gnu e-regex:
+       [a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?
+
+       as a non-POSIX, extension, allow "$" as the last char for
+       sake of Samba 3.x "add machine script"
+
+       Return True if ok
+       Return False if not
+    """
+    textRE = re.compile(r"^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?$")
+
+    if not text and notEmpty is True:
+        return False
+
+    if text is None:
+        if notEmpty is True:
+            return False
+        else:
+            return True
+
+    m = textRE.match(text)
+    if not m or text != m.group(0):
+        return False
+
+    return True
diff --git a/ipa-python/radius_util.py b/ipa-python/radius_util.py
index fb3e581e6..3d2e83e18 100644
--- a/ipa-python/radius_util.py
+++ b/ipa-python/radius_util.py
@@ -347,7 +347,7 @@ def validate_nastype(nastype, variable_name=None):
     return True
 
 def validate_desc(desc, variable_name=None):
-    if ipavalidate.Plain(desc) != 0:
+    if not ipavalidate.Plain(desc):
         print valid_desc_msg
         return False
     return True
diff --git a/ipa-python/test/test_ipavalidate.py b/ipa-python/test/test_ipavalidate.py
index de4693c36..8b79fbf07 100644
--- a/ipa-python/test/test_ipavalidate.py
+++ b/ipa-python/test/test_ipavalidate.py
@@ -32,54 +32,66 @@ class TestValidate(unittest.TestCase):
         pass
 
     def test_validEmail(self):
-        self.assertEqual(0, ipavalidate.Email("test@freeipa.org"))
-        self.assertEqual(0, ipavalidate.Email("", notEmpty=False))
+        self.assertEqual(True, ipavalidate.Email("test@freeipa.org"))
+        self.assertEqual(True, 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))
+        self.assertEqual(False, ipavalidate.Email("test"))
+        self.assertEqual(False, ipavalidate.Email("test@freeipa"))
+        self.assertEqual(False, ipavalidate.Email("test@.com"))
+        self.assertEqual(False, ipavalidate.Email(""))
+        self.assertEqual(False, 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))
-        self.assertEqual(0, ipavalidate.Plain("JoeUser", allowSpaces=False))
-        self.assertEqual(0, ipavalidate.Plain("JoeUser", allowSpaces=True))
+        self.assertEqual(True, ipavalidate.Plain("Joe User"))
+        self.assertEqual(True, ipavalidate.Plain("Joe O'Malley"))
+        self.assertEqual(True, ipavalidate.Plain("", notEmpty=False))
+        self.assertEqual(True, ipavalidate.Plain(None, notEmpty=False))
+        self.assertEqual(True, ipavalidate.Plain("JoeUser", allowSpaces=False))
+        self.assertEqual(True, ipavalidate.Plain("JoeUser", allowSpaces=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))
+        self.assertEqual(False, ipavalidate.Plain("Joe (User)"))
+        self.assertEqual(False, ipavalidate.Plain("Joe C. User"))
+        self.assertEqual(False, ipavalidate.Plain("", notEmpty=True))
+        self.assertEqual(False, ipavalidate.Plain(None, notEmpty=True))
+        self.assertEqual(False, ipavalidate.Plain("Joe User", allowSpaces=False))
+        self.assertEqual(False, ipavalidate.Plain("Joe C. User"))
 
     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))
+        self.assertEqual(True, ipavalidate.String("Joe User"))
+        self.assertEqual(True, ipavalidate.String("Joe O'Malley"))
+        self.assertEqual(True, ipavalidate.String("", notEmpty=False))
+        self.assertEqual(True, ipavalidate.String(None, notEmpty=False))
+        self.assertEqual(True, ipavalidate.String("Joe C. User"))
 
     def test_invalidString(self):
-        self.assertEqual(1, ipavalidate.String("", notEmpty=True))
-        self.assertEqual(1, ipavalidate.String(None, notEmpty=True))
+        self.assertEqual(False, ipavalidate.String("", notEmpty=True))
+        self.assertEqual(False, 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))
+        self.assertEqual(True, ipavalidate.Path("/"))
+        self.assertEqual(True, ipavalidate.Path("/home/user"))
+        self.assertEqual(True, ipavalidate.Path("../home/user"))
+        self.assertEqual(True, ipavalidate.Path("", notEmpty=False))
+        self.assertEqual(True, 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))
+        self.assertEqual(False, ipavalidate.Path("(foo)"))
+        self.assertEqual(False, ipavalidate.Path("", notEmpty=True))
+        self.assertEqual(False, ipavalidate.Path(None, notEmpty=True))
+
+    def test_validName(self):
+        self.assertEqual(True, ipavalidate.GoodName("foo"))
+        self.assertEqual(True, ipavalidate.GoodName("1foo"))
+        self.assertEqual(True, ipavalidate.GoodName("foo.bar"))
+        self.assertEqual(True, ipavalidate.GoodName("foo.bar$"))
+
+    def test_invalidName(self):
+        self.assertEqual(False, ipavalidate.GoodName("foo bar"))
+        self.assertEqual(False, ipavalidate.GoodName("foo%bar"))
+        self.assertEqual(False, ipavalidate.GoodName("*foo"))
+        self.assertEqual(False, ipavalidate.GoodName("$foo.bar$"))
 
 if __name__ == '__main__':
     unittest.main()
-
diff --git a/ipa-server/ipa-gui/ipagui/forms/group.py b/ipa-server/ipa-gui/ipagui/forms/group.py
index 4835e9168..dc3ac0608 100644
--- a/ipa-server/ipa-gui/ipagui/forms/group.py
+++ b/ipa-server/ipa-gui/ipagui/forms/group.py
@@ -36,7 +36,7 @@ class GroupFields(object):
     dn_to_info_json = widgets.HiddenField(name="dn_to_info_json")
 
 class GroupNewValidator(validators.Schema):
-    cn = validators.String(not_empty=True)
+    cn = GoodName(not_empty=True)
     description = validators.String(not_empty=False)
 
 
@@ -59,7 +59,7 @@ class GroupNewForm(widgets.Form):
 
 
 class GroupEditValidator(validators.Schema):
-    cn = validators.String(not_empty=False)
+    cn = GoodName(not_empty=False)
     gidnumber = validators.Int(not_empty=False)
     description = validators.String(not_empty=False)
 
diff --git a/ipa-server/ipa-gui/ipagui/forms/user.py b/ipa-server/ipa-gui/ipagui/forms/user.py
index 22a49653c..62fc0dfdc 100644
--- a/ipa-server/ipa-gui/ipagui/forms/user.py
+++ b/ipa-server/ipa-gui/ipagui/forms/user.py
@@ -86,7 +86,7 @@ class UserFields(object):
     custom_fields = []
 
 class UserNewValidator(validators.Schema):
-    uid = validators.PlainText(not_empty=True)
+    uid = GoodName(not_empty=True)
     krbprincipalkey = validators.String(not_empty=False)
     krbprincipalkey_confirm = validators.String(not_empty=False)
     givenname = validators.String(not_empty=True)
@@ -129,6 +129,7 @@ class UserNewForm(widgets.Form):
         super(UserNewForm,self).update_params(params)
 
 class UserEditValidator(validators.Schema):
+    uid = GoodName(not_empty=False)
     krbprincipalkey = validators.String(not_empty=False)
     krbprincipalkey_confirm = validators.String(not_empty=False)
     givenname = validators.String(not_empty=True)
diff --git a/ipa-server/ipa-gui/ipagui/helpers/validators.py b/ipa-server/ipa-gui/ipagui/helpers/validators.py
index 6e78781a4..27272595b 100644
--- a/ipa-server/ipa-gui/ipagui/helpers/validators.py
+++ b/ipa-server/ipa-gui/ipagui/helpers/validators.py
@@ -63,3 +63,31 @@ class UniqueList(FancyValidator):
         if orig > check:
             raise Invalid(self.message('notunique', state),
                           value, state)
+
+class GoodName(Regex):
+    """
+    Test that the field contains only letters, numbers, underscore,
+    dash, hyphen and $.
+
+    Examples::
+
+        >>> GoodName.to_python('_this9_')
+        '_this9_'
+        >>> GoodName.from_python('  this  ')
+        '  this  '
+        >>> GoodName(accept_python=False).from_python('  this  ')
+        Traceback (most recent call last):
+          ...
+        Invalid: Enter only letters, numbers, _ (underscore), - (dash) or $')
+        >>> GoodName(strip=True).to_python('  this  ')
+        'this'
+        >>> GoodName(strip=True).from_python('  this  ')
+        'this'
+    """
+
+    regex = r"^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?$"
+
+    messages = {
+        'invalid': _('Enter only letters, numbers, _ (underscore), - (dash) or 
+$'),
+    }