Only apply validation rules when adding and updating.

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. So you can't, for example, delete a user with an illegal name.

Primary keys are cloned with query=True in PKQuery which causes no
rules to be applied on mod/show/find. This reverts a change from commit
3a5e26a0 which applies class rules when query=True (for enforcing no
white space).

Replace rdnattr with rdn_is_primary_key. This was meant to tell us when
an RDN change was necessary to do a rename. There could be a disconnect
where the rdnattr wasn't the primary key and in that case we don't
need to do an RDN change, so use a boolean instead so that it is
clear that RDN == primary key.

Add a test to ensure that nowhitespace is actually enforced.

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

Related: https://fedorahosted.org/freeipa/ticket/2089

Whitespace tickets:
https://fedorahosted.org/freeipa/ticket/1285
https://fedorahosted.org/freeipa/ticket/1286
https://fedorahosted.org/freeipa/ticket/1287

14 files changed, 145 insertions, 31 deletions

diff --git a/API.txt b/API.txt
index b5f5774b..dadd9c11 100644
--- a/API.txt
+++ b/API.txt
@@ -2036,12 +2036,12 @@ command: permission_add
 args: 1,12,3
 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, required=True)
 option: Str('permissions', attribute=True, cli_name='permissions', csv=True, multivalue=True, required=True)
-option: Str('attrs', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=True, required=False)
-option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=True, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
-option: Str('memberof', alwaysask=True, attribute=True, autofill=False, cli_name='memberof', multivalue=False, query=True, required=False)
-option: Str('filter', alwaysask=True, attribute=True, autofill=False, cli_name='filter', multivalue=False, query=True, required=False)
-option: Str('subtree', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=True, required=False)
-option: Str('targetgroup', alwaysask=True, attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, query=True, required=False)
+option: Str('attrs', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=False, required=False)
+option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
+option: Str('memberof', alwaysask=True, attribute=True, autofill=False, cli_name='memberof', multivalue=False, query=False, required=False)
+option: Str('filter', alwaysask=True, attribute=True, autofill=False, cli_name='filter', multivalue=False, query=False, required=False)
+option: Str('subtree', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=False, required=False)
+option: Str('targetgroup', alwaysask=True, attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, query=False, required=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -2092,12 +2092,12 @@ command: permission_mod
 args: 1,15,3
 arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
 option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, required=False)
-option: Str('attrs', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=True, required=False)
-option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=True, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
-option: Str('memberof', alwaysask=True, attribute=True, autofill=False, cli_name='memberof', multivalue=False, query=True, required=False)
-option: Str('filter', alwaysask=True, attribute=True, autofill=False, cli_name='filter', multivalue=False, query=True, required=False)
-option: Str('subtree', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=True, required=False)
-option: Str('targetgroup', alwaysask=True, attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, query=True, required=False)
+option: Str('attrs', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=False, required=False)
+option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
+option: Str('memberof', alwaysask=True, attribute=True, autofill=False, cli_name='memberof', multivalue=False, query=False, required=False)
+option: Str('filter', alwaysask=True, attribute=True, autofill=False, cli_name='filter', multivalue=False, query=False, required=False)
+option: Str('subtree', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=False, required=False)
+option: Str('targetgroup', alwaysask=True, attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, query=False, required=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Str('delattr*', cli_name='delattr', exclude='webui')
diff --git a/ipalib/crud.py b/ipalib/crud.py
index 833914cf..b9dfb025 100644
--- a/ipalib/crud.py
+++ b/ipalib/crud.py
@@ -144,7 +144,7 @@ class Create(Method):
                 continue
             if 'ask_create' in option.flags:
                 yield option.clone(
-                    attribute=attribute, query=True, required=False,
+                    attribute=attribute, query=False, required=False,
                     autofill=False, alwaysask=True
                 )
             else:
@@ -161,6 +161,8 @@ class PKQuery(Method):
 
     def get_args(self):
         if self.obj.primary_key:
+            # Don't enforce rules on the primary key so we can reference
+            # any stored entry, legal or not
             yield self.obj.primary_key.clone(attribute=True, query=True)
 
 
@@ -189,7 +191,7 @@ class Update(PKQuery):
                 continue
             if 'ask_update' in option.flags:
                 yield option.clone(
-                    attribute=attribute, query=True, required=False,
+                    attribute=attribute, query=False, required=False,
                     autofill=False, alwaysask=True
                 )
             elif 'req_update' in option.flags:
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index c533f9d0..b1525b4d 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -508,7 +508,8 @@ class Param(ReadOnly):
         self.class_rules = tuple(class_rules)
         self.rules = rules
         if self.query:
-            self.all_rules = self.class_rules
+            # by definition a query enforces no class or parameter rules
+            self.all_rules = ()
         else:
             self.all_rules = self.class_rules + self.rules
         for rule in self.all_rules:
diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index 8d743d75..31c143d8 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -645,7 +645,7 @@ class automountkey(LDAPObject):
     default_attributes = [
         'automountkey', 'automountinformation', 'description'
     ]
-    rdnattr = 'description'
+    rdn_is_primary_key = True
     rdn_separator = ' '
 
     takes_params = (
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 725704ee..2664160f 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -429,7 +429,7 @@ class LDAPObject(Object):
     rdn_attribute = ''
     uuid_attribute = ''
     attribute_members = {}
-    rdnattr = None
+    rdn_is_primary_key = False # Do we need RDN change to do a rename?
     password_attributes = []
     # Can bind as this entry (has userPassword or krbPrincipalKey)
     bindable = False
@@ -1178,7 +1178,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
     has_output_params = global_output_params
 
     def _get_rename_option(self):
-        rdnparam = getattr(self.obj.params, self.obj.rdnattr)
+        rdnparam = getattr(self.obj.params, self.obj.primary_key.name)
         return rdnparam.clone_rename('rename',
             cli_name='rename', required=False, label=_('Rename'),
             doc=_('Rename the %(ldap_obj_name)s object') % dict(
@@ -1189,7 +1189,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
     def get_options(self):
         for option in super(LDAPUpdate, self).get_options():
             yield option
-        if self.obj.rdnattr:
+        if self.obj.rdn_is_primary_key:
             yield self._get_rename_option()
 
     def execute(self, *keys, **options):
@@ -1229,18 +1229,19 @@ class LDAPUpdate(LDAPQuery, crud.Update):
 
         rdnupdate = False
         try:
-            if self.obj.rdnattr and 'rename' in options:
+            if self.obj.rdn_is_primary_key and 'rename' in options:
                 if not options['rename']:
                     raise errors.ValidationError(name='rename', error=u'can\'t be empty')
-                entry_attrs[self.obj.rdnattr] = options['rename']
+                entry_attrs[self.obj.primary_key.name] = options['rename']
 
-            if self.obj.rdnattr and self.obj.rdnattr in entry_attrs:
+            if self.obj.rdn_is_primary_key and self.obj.primary_key.name in entry_attrs:
                 # RDN change
-                ldap.update_entry_rdn(dn, unicode('%s=%s' % (self.obj.rdnattr,
-                    entry_attrs[self.obj.rdnattr])))
-                rdnkeys = keys[:-1] + (entry_attrs[self.obj.rdnattr], )
+                ldap.update_entry_rdn(dn,
+                    unicode('%s=%s' % (self.obj.primary_key.name,
+                    entry_attrs[self.obj.primary_key.name])))
+                rdnkeys = keys[:-1] + (entry_attrs[self.obj.primary_key.name], )
                 dn = self.obj.get_dn(*rdnkeys)
-                del entry_attrs[self.obj.rdnattr]
+                del entry_attrs[self.obj.primary_key.name]
                 options['rdnupdate'] = True
                 rdnupdate = True
 
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index b101d128..096cb9ea 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -95,7 +95,7 @@ class group(LDAPObject):
         'memberofindirect': ['group', 'netgroup', 'role', 'hbacrule',
         'sudorule'],
     }
-    rdnattr = 'cn'
+    rdn_is_primary_key = True
 
     label = _('User Groups')
     label_singular = _('User Group')
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index c9fd5649..ce2536d9 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -144,7 +144,7 @@ class permission(LDAPObject):
     attribute_members = {
         'member': ['privilege'],
     }
-    rdnattr='cn'
+    rdn_is_primary_key = True
 
     label = _('Permissions')
     label_singular = _('Permission')
diff --git a/ipalib/plugins/privilege.py b/ipalib/plugins/privilege.py
index 53f76512..53e1de22 100644
--- a/ipalib/plugins/privilege.py
+++ b/ipalib/plugins/privilege.py
@@ -60,7 +60,7 @@ class privilege(LDAPObject):
     reverse_members = {
         'member': ['permission'],
     }
-    rdnattr='cn'
+    rdn_is_primary_key = True
 
     label = _('Privileges')
     label_singular = _('Privilege')
diff --git a/ipalib/plugins/role.py b/ipalib/plugins/role.py
index ee6ebcdc..2837c418 100644
--- a/ipalib/plugins/role.py
+++ b/ipalib/plugins/role.py
@@ -76,7 +76,7 @@ class role(LDAPObject):
     reverse_members = {
         'member': ['privilege'],
     }
-    rdnattr='cn'
+    rdn_is_primary_key = True
 
     label = _('Roles')
     label_singular = _('Role')
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index d8da3a37..591132d3 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -168,7 +168,7 @@ class user(LDAPObject):
         'memberof': ['group', 'netgroup', 'role', 'hbacrule', 'sudorule'],
         'memberofindirect': ['group', 'netgroup', 'role', 'hbacrule', 'sudorule'],
     }
-    rdnattr = 'uid'
+    rdn_is_primary_key = True
     bindable = True
     password_attributes = [('userpassword', 'has_password'),
                            ('krbprincipalkey', 'has_keytab')]
diff --git a/tests/test_xmlrpc/test_group_plugin.py b/tests/test_xmlrpc/test_group_plugin.py
index 86c0d90d..3ef60f69 100644
--- a/tests/test_xmlrpc/test_group_plugin.py
+++ b/tests/test_xmlrpc/test_group_plugin.py
@@ -602,6 +602,28 @@ class test_group(Declarative):
             expected=errors.ValidationError(name='cn', 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=('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'),
+        ),
+
 
         ##### managed entry tests
         dict(
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 0323ac39..4f24b6e3 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -47,6 +47,7 @@ dn3 = DN(('fqdn',fqdn3),('cn','computers'),('cn','accounts'),
 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')
@@ -670,4 +671,45 @@ class test_host(Declarative):
             expected=errors.ValidationError(name='fqdn', error='An IPA master host cannot be deleted'),
         ),
 
+        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_role_plugin.py b/tests/test_xmlrpc/test_role_plugin.py
index 9c405cff..f871e268 100644
--- a/tests/test_xmlrpc/test_role_plugin.py
+++ b/tests/test_xmlrpc/test_role_plugin.py
@@ -33,6 +33,7 @@ role1 = u'test-role-1'
 role1_dn = DN(('cn',role1),api.env.container_rolegroup,
               api.env.basedn)
 renamedrole1 = u'test-role'
+invalidrole1 = u' whitespace '
 
 role2 = u'test-role-2'
 role2_dn = DN(('cn',role2),api.env.container_rolegroup,
@@ -101,6 +102,15 @@ class test_role(Declarative):
 
 
         dict(
+            desc='Create invalid %r' % invalidrole1,
+            command=('role_add', [invalidrole1],
+                dict(description=u'role desc 1')
+            ),
+            expected=errors.ValidationError(name='cn', error='Leading and trailing spaces are not allowed'),
+        ),
+
+
+        dict(
             desc='Create %r' % role1,
             command=('role_add', [role1],
                 dict(description=u'role desc 1')
diff --git a/tests/test_xmlrpc/test_user_plugin.py b/tests/test_xmlrpc/test_user_plugin.py
index d1cdb2f9..b21737f8 100644
--- a/tests/test_xmlrpc/test_user_plugin.py
+++ b/tests/test_xmlrpc/test_user_plugin.py
@@ -643,6 +643,42 @@ 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='Try to rename to invalid username %r' % user1,
+            command=('user_mod', [user1], dict(rename=invaliduser1)),
+            expected=errors.ValidationError(name='uid', error='may only include letters, numbers, _, -, . and $'),
+        ),
+
+
         dict(
             desc='Create %r' % group1,
             command=(