Add optional error message to pattern validator

The pattern validator by default displays the pattern that is being
matched against. This isn't helpful, particularly for very hairy patterns.
This adds a new parameter, pattern_errmsg, that is displayed on errors
if set.

ticket #11

5 files changed, 51 insertions, 3 deletions

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index f97c42218..c86db758e 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -1082,6 +1082,7 @@ class Data(Param):
         ('maxlength', int, None),
         ('length', int, None),
         ('pattern', (basestring,), None),
+        ('pattern_errmsg', (basestring,), None),
     )
 
     def __init__(self, name, *rules, **kw):
@@ -1123,9 +1124,12 @@ class Data(Param):
         """
         assert type(value) is self.type
         if self.re.match(value) is None:
-            return _('must match pattern "%(pattern)s"') % dict(
-                pattern=self.pattern,
-            )
+            if self.re_errmsg:
+                return self.re_errmsg % dict(pattern=self.pattern,)
+            else:
+                return _('must match pattern "%(pattern)s"') % dict(
+                    pattern=self.pattern,
+                )
 
 
 class Bytes(Data):
@@ -1148,6 +1152,7 @@ class Bytes(Data):
             self.re = None
         else:
             self.re = re.compile(kw['pattern'])
+        self.re_errmsg = kw.get('pattern_errmsg', None)
         super(Bytes, self).__init__(name, *rules, **kw)
 
     def _rule_minlength(self, _, value):
@@ -1201,6 +1206,7 @@ class Str(Data):
             self.re = None
         else:
             self.re = re.compile(kw['pattern'], re.UNICODE)
+        self.re_errmsg = kw.get('pattern_errmsg', None)
         super(Str, self).__init__(name, *rules, **kw)
 
     def _convert_scalar(self, value, index=None):
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index 2558c38ab..4fd630d42 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -89,6 +89,9 @@ class group(LDAPObject):
 
     takes_params = (
         Str('cn',
+            pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?$',
+            pattern_errmsg='may only include letters, numbers, _, -, . and $',
+            maxlength=33,
             cli_name='name',
             label=_('Group name'),
             primary_key=True,
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index de5ff2d27..f698aa703 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -72,6 +72,9 @@ class user(LDAPObject):
 
     takes_params = (
         Str('uid',
+            pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,30}[a-zA-Z0-9_.$-]?$',
+            pattern_errmsg='may only include letters, numbers, _, -, . and $',
+            maxlength=33,
             cli_name='login',
             label=_('User login'),
             primary_key=True,
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 620f3eacd..a8940a0d0 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -28,6 +28,9 @@ from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
 group1 = u'testgroup1'
 group2 = u'testgroup2'
 
+invalidgroup1=u'+tgroup1'
+invalidgroup2=u'tgroup1234567890123456789012345678901234567890'
+
 
 class test_group(Declarative):
     cleanup_commands = [
@@ -511,4 +514,18 @@ class test_group(Declarative):
             expected=errors.NotFound(reason='no such entry'),
         ),
 
+        dict(
+            desc='Test an invalid group name %r' % invalidgroup1,
+            command=('group_add', [invalidgroup1], dict(description=u'Test')),
+            expected=errors.ValidationError(name='cn', error='may only include letters, numbers, _, -, . and $'),
+        ),
+
+
+        dict(
+            desc='Test a group name that is too long %r' % invalidgroup2,
+            command=('group_add', [invalidgroup2], dict(description=u'Test')),
+            expected=errors.ValidationError(name='cn', error='can be at most 33 characters'),
+        ),
+
+
     ]
diff --git a/tests/test_xmlrpc/test_user_plugin.py b/tests/test_xmlrpc/test_user_plugin.py
index 467eacf4e..18c5b45f2 100644
--- a/tests/test_xmlrpc/test_user_plugin.py
+++ b/tests/test_xmlrpc/test_user_plugin.py
@@ -31,6 +31,9 @@ from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
 user_memberof = (u'cn=ipausers,cn=groups,cn=accounts,%s' % api.env.basedn,)
 user1=u'tuser1'
 
+invaliduser1=u'+tuser1'
+invaliduser2=u'tuser1234567890123456789012345678901234567890'
+
 
 class test_user(Declarative):
 
@@ -78,6 +81,7 @@ class test_user(Declarative):
                     objectclass=objectclasses.user,
                     sn=[u'User1'],
                     uid=[user1],
+                    uidnumber=[fuzzy_digits],
                     ipauniqueid=[fuzzy_uuid],
                     dn=u'uid=tuser1,cn=users,cn=accounts,' + api.env.basedn,
                 ),
@@ -183,6 +187,8 @@ class test_user(Declarative):
                         sn=[u'Administrator'],
                         uid=[u'admin'],
                         memberof_group=[u'admins'],
+                        memberof_rolegroup=[u'replicaadmin'],
+                        memberof_taskgroup=[u'managereplica', u'deletereplica'],
                     ),
                     dict(
                         dn=u'uid=tuser1,cn=users,cn=accounts,' + api.env.basedn,
@@ -299,4 +305,17 @@ class test_user(Declarative):
         ),
 
 
+        dict(
+            desc='Test an invalid login name %r' % invaliduser1,
+            command=('user_add', [invaliduser1], dict(givenname=u'Test', sn=u'User1')),
+            expected=errors.ValidationError(name='uid', error='may only include letters, numbers, _, -, . and $'),
+        ),
+
+
+        dict(
+            desc='Test a login name that is too long %r' % invaliduser2,
+            command=('user_add', [invaliduser2], dict(givenname=u'Test', sn=u'User1')),
+            expected=errors.ValidationError(name='uid', error='can be at most 33 characters'),
+        ),
+
     ]