Attachment 313645 Details for Bug 450613 – Re-based patch to use same regex as shadow-utils and change validation to be more obvious

[patch] Re-based patch to use same regex as shadow-utils and change validation to be more obvious

freeipa-71-goodname.patch (text/plain), 20.62 KB, created by Rob Crittenden on 2008-08-06 21:13:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Crittenden

Created: 2008-08-06 21:13:25 UTC

Size: 20.62 KB

patch

obsolete

>From c485f610090521b69e9ea987b90aab75300dc96d Mon Sep 17 00:00:00 2001
>From: Rob Crittenden <rcrit@ipa.greyoak.com>
>Date: Wed, 6 Aug 2008 13:00:36 -0400
>Subject: [PATCH] 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
>---
> ipa-admintools/ipa-addgroup                     |   11 ++-
> ipa-admintools/ipa-addservice                   |    1 -
> ipa-admintools/ipa-adduser                      |   15 +++--
> ipa-admintools/ipa-moduser                      |    6 +-
> ipa-python/ipaadminutil.py                      |    9 +++
> ipa-python/ipautil.py                           |   15 ++++-
> ipa-python/ipavalidate.py                       |   76 +++++++++++++++-------
> ipa-python/radius_util.py                       |    2 +-
> ipa-python/test/test_ipavalidate.py             |   80 +++++++++++++----------
> ipa-server/ipa-gui/ipagui/forms/group.py        |    4 +-
> ipa-server/ipa-gui/ipagui/forms/user.py         |    3 +-
> ipa-server/ipa-gui/ipagui/helpers/validators.py |   28 ++++++++
> 12 files changed, 171 insertions(+), 79 deletions(-)
>
>diff --git a/ipa-admintools/ipa-addgroup b/ipa-admintools/ipa-addgroup
>index cc81383..d99d588 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 ddf75a8..29d0bb9 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 17c75ed..9db4516 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 4b3bd5c..4ea4063 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 d94ced4..2733764 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 3526cc7..f3018ed 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 4dc7fe1..63e0a76 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 fb3e581..3d2e83e 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 de4693c..8b79fbf 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 4835e91..dc3ac06 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 22a4965..62fc0df 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 6e78781..2727259 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 
>+$'),
>+    }
>-- 
>1.5.4.1
>

Actions: View | Diff

Attachments on bug 450613: 313561 | 313645