There is some validation that we only need to apply when an entry is being created, namely the key itself. This is to allow us to manage an otherwise illegal entry that finds its way into the system (i.e. migration).

Consider this. We migrate a group with a space in it. This isn't allowed in IPA but we also provide no way to delete it because the cn regex kicks out the group-del command.


The trick is adding appropriate context so we can know during validation how we got here. A command object has a bases field which contains the base classes associated with it, which appears to contain only the leaf baseclass. So using this we can tell how we got to validation and can skip based on that baseclass name.

I went back and forth a bit on where the right place to put this was, I'm open to more fine tuning. I initially skipped just the pattern validation then expanded it to apply to all validation in the Parameter.

rob
>From 4f66d031103ffd3eb2477ffbb28e07432374254f Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 3 Feb 2012 16:49:45 -0500
Subject: [PATCH] Only apply some validation rules to the add method.

There may be cases, for whatever reason, that an otherwise illegal
entry gets created that doesn't match the criteria for a valid
user/host/group name. If this happens (i.e. migration) there is no way
to remove this using the IPA tools because we always applied the name
pattern.

This adds a little bit of context to parameters so they know the
baseclass being executed. Using this we can limit when validation
rules get executed to just LDAPCreate.

https://fedorahosted.org/freeipa/ticket/2115

Related: https://fedorahosted.org/freeipa/ticket/2089
---
 ipalib/frontend.py                     |    2 +-
 ipalib/parameters.py                   |   26 ++++++++++++++----
 ipalib/plugins/group.py                |    1 +
 ipalib/plugins/host.py                 |    1 +
 ipalib/plugins/user.py                 |    1 +
 tests/test_xmlrpc/test_group_plugin.py |   35 +++++++++++++++++++++++++
 tests/test_xmlrpc/test_host_plugin.py  |   44 ++++++++++++++++++++++++++++++++
 tests/test_xmlrpc/test_user_plugin.py  |   28 ++++++++++++++++++++
 8 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 028e17e7945c5b193ccbfd84a2f2214e56d2a5fa..b04e57afa568eebd71ef95b15728c164341c3bb8 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -651,7 +651,7 @@ class Command(HasParam):
         """
         for param in self.params():
             value = kw.get(param.name, None)
-            param.validate(value, self.env.context)
+            param.validate(value, self.env.context, self.bases)
 
     def verify_client_version(self, client_version):
         """
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index d918a573778f533b37318622b2fd0ce2d265adf4..e711ba0e335c7add7136ace794817ee2a29a6819 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -348,6 +348,10 @@ class Param(ReadOnly):
         parameter is not `required`
       - sortorder: used to sort a list of parameters for Command. See
         `Command.finalize()` for further information
+      - onlyvalidateclasses: list of object classes to apply rules to. This
+        can be used to apply validation only when doing an add, for example.
+        An empty set means validate all (the default). This is an all or
+        nothing choice, either all rules apply to a class or none.
     """
 
     # This is a dummy type so that most of the functionality of Param can be
@@ -386,6 +390,7 @@ class Param(ReadOnly):
         ('csv_separator', str, ','),
         ('csv_skipspace', bool, True),
         ('option_group', unicode, None),
+        ('onlyvalidateclasses', frozenset, frozenset()),
 
         # The 'default' kwarg gets appended in Param.__init__():
         # ('default', self.type, None),
@@ -817,7 +822,7 @@ class Param(ReadOnly):
             error=ugettext(self.type_error),
         )
 
-    def validate(self, value, context=None):
+    def validate(self, value, context=None, classbases=[]):
         """
         Check validity of ``value``.
 
@@ -839,11 +844,11 @@ class Param(ReadOnly):
             if len(value) < 1:
                 raise ValueError('value: empty tuple must be converted to None')
             for (i, v) in enumerate(value):
-                self._validate_scalar(v, i)
+                self._validate_scalar(v, i, classbases)
         else:
-            self._validate_scalar(value)
+            self._validate_scalar(value, classbases=classbases)
 
-    def _validate_scalar(self, value, index=None):
+    def _validate_scalar(self, value, index=None, classbases=[]):
         if type(value) is not self.type:
             raise ValidationError(name=self.name,
                 error='need a %r; got %r (a %r)' % (
@@ -854,6 +859,9 @@ class Param(ReadOnly):
             raise TypeError(
                 TYPE_ERROR % ('index', int, index, type(index))
             )
+        if self.onlyvalidateclasses and \
+          not [x for x in classbases if x in self.onlyvalidateclasses]:
+            return
         for rule in self.all_rules:
             error = rule(ugettext, value)
             if error is not None:
@@ -1195,12 +1203,15 @@ class Int(Number):
                 maxvalue=self.maxvalue,
             )
 
-    def _validate_scalar(self, value, index=None):
+    def _validate_scalar(self, value, index=None, classbases=[]):
         """
         This duplicates _validate_scalar in the Param class with
         the exception that it allows both int and long types. The
         min/max rules handle size enforcement.
         """
+        if self.onlyvalidateclasses and \
+          not [x for x in classbases if x in self.onlyvalidateclasses]:
+            return
         if type(value)  not in (int, long):
             raise ValidationError(name=self.name,
                 error='need a %r; got %r (a %r)' % (
@@ -1622,7 +1633,10 @@ class Any(Param):
     def _convert_scalar(self, value, index=None):
         return value
 
-    def _validate_scalar(self, value, index=None):
+    def _validate_scalar(self, value, index=None, classbases=[]):
+        if self.onlyvalidateclasses and \
+          not [x for x in classbases if x in self.onlyvalidateclasses]:
+            return
         for rule in self.all_rules:
             error = rule(ugettext, value)
             if error is not None:
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index 49bc8232835157d5fffec8e3c4c573399536d818..037d2c307226b45df2bd471304e0449cd6469401 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -109,6 +109,7 @@ class group(LDAPObject):
             label=_('Group name'),
             primary_key=True,
             normalizer=lambda value: value.lower(),
+            onlyvalidateclasses=('ipalib.plugins.baseldap.LDAPCreate'),
         ),
         Str('description',
             cli_name='desc',
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 0cae656b76fe4c2baabeaeae05aa5de1a452b543..3ff4240c40936b4a9dee224f07facafcd2587986 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -258,6 +258,7 @@ class host(LDAPObject):
             label=_('Host name'),
             primary_key=True,
             normalizer=lambda value: value.lower(),
+            onlyvalidateclasses=('ipalib.plugins.baseldap.LDAPCreate'),
         ),
         Str('description?',
             cli_name='desc',
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 70a111dd34c66ff22f906834a1348c65dbfd1bd5..4a2838673a6c2fe7240a649e2df7fad242bfc1bf 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -183,6 +183,7 @@ class user(LDAPObject):
             primary_key=True,
             default_from=lambda givenname, sn: givenname[0] + sn,
             normalizer=lambda value: value.lower(),
+            onlyvalidateclasses=('ipalib.plugins.baseldap.LDAPCreate'),
         ),
         Str('givenname',
             cli_name='first',
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 86c0d90daebd5917ea51d124cb9946aa3607d2db..0cd07ebcc9a8bfc861ff8738ab88bba590bbd213 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -603,6 +603,41 @@ class test_group(Declarative):
         ),
 
 
+        # The assumption on these next 4 tests is that if we don't get a
+        # validation error then the request was processed normally.
+        dict(
+            desc='Test that validation is disabled on mods',
+            command=('group_mod', [invalidgroup1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on deletes',
+            command=('group_del', [invalidgroup1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on show',
+            command=('group_show', [invalidgroup1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on find',
+            command=('group_find', [invalidgroup1], {}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 groups matched',
+                result=[],
+            ),
+        ),
+
+
         ##### managed entry tests
         dict(
             desc='Create %r' % user1,
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 372ee256eaf411398ac95c9b9e65199489b59270..639339e5e06cf1138de829310abf343b86cb28ad 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -48,6 +48,8 @@ fqdn4 = u'testhost2.lab.%s' % api.env.domain
 dn4 = DN(('fqdn',fqdn4),('cn','computers'),('cn','accounts'),
          api.env.basedn)
 
+invalidfqdn1 = u'foo_bar.lab.%s' % api.env.domain
+
 # We can use the same cert we generated for the service tests
 fd = open('tests/test_xmlrpc/service.crt', 'r')
 servercert = fd.readlines()
@@ -661,4 +663,46 @@ class test_host(Declarative):
             ),
         ),
 
+
+        dict(
+            desc='Test that validation is enabled on adds',
+            command=('host_add', [invalidfqdn1], {}),
+            expected=errors.ValidationError(name='fqdn', error='may only include letters, numbers, and -'),
+        ),
+
+
+        # The assumption on these next 4 tests is that if we don't get a
+        # validation error then the request was processed normally.
+        dict(
+            desc='Test that validation is disabled on mods',
+            command=('host_mod', [invalidfqdn1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on deletes',
+            command=('host_del', [invalidfqdn1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on show',
+            command=('host_show', [invalidfqdn1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on find',
+            command=('host_find', [invalidfqdn1], {}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 hosts matched',
+                result=[],
+            ),
+        ),
+
     ]
diff --git a/tests/test_xmlrpc/test_user_plugin.py b/tests/test_xmlrpc/test_user_plugin.py
index 0370ec74d294f15b742aadacbdbf6fa296ebb718..307a71870fe57320c7d5d2845ba64d418df9364f 100644
--- a/tests/test_xmlrpc/test_user_plugin.py
+++ b/tests/test_xmlrpc/test_user_plugin.py
@@ -634,6 +634,34 @@ class test_user(Declarative):
             expected=errors.ValidationError(name='uid', error='can be at most 33 characters'),
         ),
 
+        # The assumption on these next 4 tests is that if we don't get a
+        # validation error then the request was processed normally.
+        dict(
+            desc='Test that validation is disabled on deletes',
+            command=('user_del', [invaliduser1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on show',
+            command=('user_show', [invaliduser1], {}),
+            expected=errors.NotFound(reason='no such entry'),
+        ),
+
+
+        dict(
+            desc='Test that validation is disabled on find',
+            command=('user_find', [invaliduser1], {}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 users matched',
+                result=[],
+            ),
+        ),
+
+
         dict(
             desc='Create %r' % group1,
             command=(
-- 
1.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to