Petr Viktorin wrote:
On 03/15/2012 10:04 PM, Rob Crittenden wrote:
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index
9562ff98729ead6ac9e56d504f6ee0a7c0ca377a..f3c89a0fc5e3f00ed7f132dbff2510d89bc7370d
100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -887,12 +877,29 @@ last, after all sets and adds."""),
# normalize all values
changedattrs = setattrs | addattrs | delattrs
for attr in changedattrs:
- # remove duplicite and invalid values
- entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if
val]))
- if not entry_attrs[attr]:
- entry_attrs[attr] = None
- elif isinstance(entry_attrs[attr], (tuple, list)) and
len(entry_attrs[attr]) == 1:
- entry_attrs[attr] = entry_attrs[attr][0]
+ if attr in self.obj.params:
+ # convert single-value params to scalars
+ # Need to use the LDAPObject's params, not self's, because the
+ # CRUD classes filter their disallowed parameters out.
+ # Yet {set,add,del}attr are powerful enough to change these
+ # (e.g. Config's ipacertificatesubjectbase)
+ if not self.obj.params[attr].multivalue:
+ if len(entry_attrs[attr]) == 1:
+ entry_attrs[attr] = entry_attrs[attr][0]
+ elif not entry_attrs[attr]:
+ entry_attrs[attr] = None
+ else:
+ raise errors.OnlyOneValueAllowed(attr=attr)
+ # validate and convert params
+ entry_attrs[attr] = self.obj.params[attr](entry_attrs[attr])
+ else:
+ # unknown attribute: remove duplicite and invalid values
+ entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if
val]))
+ if not entry_attrs[attr]:
+ entry_attrs[attr] = None
+ elif isinstance(entry_attrs[attr], (tuple, list)) and
len(entry_attrs[attr]) == 1:
+ entry_attrs[attr] = entry_attrs[attr][0]
+


You've included an unrelated patch (my 0016).


That's what I get for mixing my review and dev branch. Correct patch attached.

rob
>From 2aaa1ecd1da8d42b2d82b08f27bf03f271976d88 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Thu, 15 Mar 2012 17:01:20 -0400
Subject: [PATCH] Don't allow hosts and services of IPA masters to be
 disabled.

https://fedorahosted.org/freeipa/ticket/2487
---
 ipalib/plugins/baseldap.py               |   41 ++++++++++++++----------
 ipalib/plugins/host.py                   |    2 +
 ipalib/plugins/service.py                |   22 +++++++++---
 tests/test_xmlrpc/test_attr.py           |   50 ++++++++++++++++++++++++++++++
 tests/test_xmlrpc/test_host_plugin.py    |   10 +++++-
 tests/test_xmlrpc/test_service_plugin.py |   14 ++++++++
 6 files changed, 115 insertions(+), 24 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 9562ff98729ead6ac9e56d504f6ee0a7c0ca377a..f3c89a0fc5e3f00ed7f132dbff2510d89bc7370d 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -396,7 +396,7 @@ def host_is_master(ldap, fqdn):
     master_dn = str(DN('cn=%s' % fqdn, 'cn=masters,cn=ipa,cn=etc', api.env.basedn))
     try:
         (dn, entry_attrs) = ldap.get_entry(master_dn, ['objectclass'])
-        raise errors.ValidationError(name='hostname', error=_('An IPA master host cannot be deleted'))
+        raise errors.ValidationError(name='hostname', error=_('An IPA master host cannot be deleted or disabled'))
     except errors.NotFound:
         # Good, not a master
         return
@@ -759,8 +759,6 @@ last, after all sets and adds."""),
         Convert a string in the form of name/value pairs into a dictionary.
         The incoming attribute may be a string or a list.

-        Any attribute found that is also a param is validated.
-
         :param attrs: A list of name/value pairs

         :param append: controls whether this returns a list of values or a single
@@ -776,14 +774,6 @@ last, after all sets and adds."""),
             if len(value) == 0:
                 # None means "delete this attribute"
                 value = None
-            if attr in self.params:
-                try:
-                   value = self.params[attr](value)
-                except errors.ValidationError, err:
-                    (name, error) = str(err.strerror).split(':')
-                    raise errors.ValidationError(name=attr, error=error)
-                if self.api.env.in_server:
-                    value = self.params[attr].encode(value)
             if append and attr in newdict:
                 if type(value) in (tuple,):
                     newdict[attr] += list(value)
@@ -887,12 +877,29 @@ last, after all sets and adds."""),
         # normalize all values
         changedattrs = setattrs | addattrs | delattrs
         for attr in changedattrs:
-            # remove duplicite and invalid values
-            entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if val]))
-            if not entry_attrs[attr]:
-                entry_attrs[attr] = None
-            elif isinstance(entry_attrs[attr], (tuple, list)) and len(entry_attrs[attr]) == 1:
-                entry_attrs[attr] = entry_attrs[attr][0]
+            if attr in self.obj.params:
+                # convert single-value params to scalars
+                # Need to use the LDAPObject's params, not self's, because the
+                # CRUD classes filter their disallowed parameters out.
+                # Yet {set,add,del}attr are powerful enough to change these
+                # (e.g. Config's ipacertificatesubjectbase)
+                if not self.obj.params[attr].multivalue:
+                    if len(entry_attrs[attr]) == 1:
+                        entry_attrs[attr] = entry_attrs[attr][0]
+                    elif not entry_attrs[attr]:
+                        entry_attrs[attr] = None
+                    else:
+                        raise errors.OnlyOneValueAllowed(attr=attr)
+                # validate and convert params
+                entry_attrs[attr] = self.obj.params[attr](entry_attrs[attr])
+            else:
+                # unknown attribute: remove duplicite and invalid values
+                entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if val]))
+                if not entry_attrs[attr]:
+                    entry_attrs[attr] = None
+                elif isinstance(entry_attrs[attr], (tuple, list)) and len(entry_attrs[attr]) == 1:
+                    entry_attrs[attr] = entry_attrs[attr][0]
+

 class LDAPCreate(BaseLDAPCommand, crud.Create):
     """
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 9db98e713e52ae8671a4813e59885c6f4a03c2ba..662cff3114be5aae399ff4c5fb43ca3d7e46c703 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -850,6 +850,8 @@ class host_disable(LDAPQuery):
         else:
             fqdn = keys[-1]

+        host_is_master(ldap, fqdn)
+
         # See if we actually do anthing here, and if not raise an exception
         done_work = False

diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index e75d71f03183688e3f01aa3a0fdfa76ec4838505..7c563b306eae2beeab524644ac9b639b9c9c985c 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -200,6 +200,18 @@ def set_certificate_attrs(entry_attrs):
     entry_attrs['md5_fingerprint'] = unicode(nss.data_to_hex(nss.md5_digest(cert.der_data), 64)[0])
     entry_attrs['sha1_fingerprint'] = unicode(nss.data_to_hex(nss.sha1_digest(cert.der_data), 64)[0])

+def check_required_principal(ldap, hostname, service):
+    """
+    Raise an error if the host of this prinicipal is an IPA master and one
+    of the principals required for proper execution.
+    """
+    try:
+        host_is_master(ldap, hostname)
+    except errors.ValidationError, e:
+        service_types = ['HTTP', 'ldap', 'DNS' 'dogtagldap']
+        if service in service_types:
+            raise errors.ValidationError(name='principal', error=_('This principal is required by the IPA master'))
+
 class service(LDAPObject):
     """
     Service object.
@@ -296,12 +308,7 @@ class service_del(LDAPDelete):
         # deleted. This is a limited few though. If the user has their own
         # custom services allow them to manage them.
         (service, hostname, realm) = split_principal(keys[-1])
-        try:
-            host_is_master(ldap, hostname)
-        except errors.ValidationError, e:
-            service_types = ['HTTP', 'ldap', 'DNS' 'dogtagldap']
-            if service in service_types:
-                raise errors.ValidationError(name='principal', error=_('This principal is required by the IPA master'))
+        check_required_principal(ldap, hostname, service)
         if self.api.env.enable_ra:
             (dn, entry_attrs) = ldap.get_entry(dn, ['usercertificate'])
             cert = entry_attrs.get('usercertificate')
@@ -465,6 +472,9 @@ class service_disable(LDAPQuery):
         dn = self.obj.get_dn(*keys, **options)
         (dn, entry_attrs) = ldap.get_entry(dn, ['usercertificate'])

+        (service, hostname, realm) = split_principal(keys[-1])
+        check_required_principal(ldap, hostname, service)
+
         # See if we do any work at all here and if not raise an exception
         done_work = False

diff --git a/tests/test_xmlrpc/test_attr.py b/tests/test_xmlrpc/test_attr.py
index ef239709dba58cca42ad0127e8223d172885e6cd..83111925264567913544d4a6462cdbc493504747 100644
--- a/tests/test_xmlrpc/test_attr.py
+++ b/tests/test_xmlrpc/test_attr.py
@@ -402,4 +402,54 @@ class test_attr(Declarative):
             ),
         ),

+        dict(
+            desc='Lock %r using setattr' % user1,
+            command=(
+                'user_mod', [user1], dict(setattr=u'nsaccountlock=True')
+            ),
+            expected=dict(
+                result=dict(
+                    givenname=[u'Finkle'],
+                    homedirectory=[u'/home/tuser1'],
+                    loginshell=[u'/bin/sh'],
+                    sn=[u'User1'],
+                    uid=[user1],
+                    uidnumber=[fuzzy_digits],
+                    gidnumber=[fuzzy_digits],
+                    mail=[u't...@example.com', u'te...@example.com'],
+                    memberof_group=[u'ipausers'],
+                    telephonenumber=[u'202-888-9833'],
+                    nsaccountlock=True,
+                    has_keytab=False,
+                    has_password=False,
+                ),
+                summary=u'Modified user "tuser1"',
+                value=user1,
+            ),
+        ),
+
+        dict(
+            desc='Try adding a new group search fields config entry',
+            command=(
+                'config_mod', [], dict(addattr=u'ipagroupsearchfields=newattr')
+            ),
+            expected=errors.OnlyOneValueAllowed(attr='ipagroupsearchfields'),
+        ),
+
+        dict(
+            desc='Try adding a new cert subject base config entry',
+            command=(
+                'config_mod', [], dict(addattr=u'ipacertificatesubjectbase=0=DOMAIN.COM')
+            ),
+            expected=errors.OnlyOneValueAllowed(attr='ipacertificatesubjectbase'),
+        ),
+
+        dict(
+            desc='Try deleting a required config entry',
+            command=(
+                'config_mod', [], dict(delattr=u'ipasearchrecordslimit=100')
+            ),
+            expected=errors.RequirementError(name='ipasearchrecordslimit'),
+        ),
+
     ]
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 7068d9a3f02ec9b7dfe00e0551888ac084738da7..8f5bd7cf347dd2f712ff441c90dbf61315be8c6b 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -673,9 +673,17 @@ class test_host(Declarative):
         dict(
             desc='Delete the current host (master?) %s should be caught' % api.env.host,
             command=('host_del', [api.env.host], {}),
-            expected=errors.ValidationError(name='fqdn', error='An IPA master host cannot be deleted'),
+            expected=errors.ValidationError(name='fqdn', error='An IPA master host cannot be deleted or disabled'),
         ),

+
+        dict(
+            desc='Disable the current host (master?) %s should be caught' % api.env.host,
+            command=('host_disable', [api.env.host], {}),
+            expected=errors.ValidationError(name='fqdn', error='An IPA master host cannot be deleted or disabled'),
+        ),
+
+
         dict(
             desc='Test that validation is enabled on adds',
             command=('host_add', [invalidfqdn1], {}),
diff --git a/tests/test_xmlrpc/test_service_plugin.py b/tests/test_xmlrpc/test_service_plugin.py
index 7eccd206647838567ea50a7cc0521ba7494a42a2..501cf024edc17e86ff62e79a2626b2eba3b9818c 100644
--- a/tests/test_xmlrpc/test_service_plugin.py
+++ b/tests/test_xmlrpc/test_service_plugin.py
@@ -487,4 +487,18 @@ class test_service(Declarative):
         ),


+        dict(
+            desc='Disable the current host (master?) %s HTTP service, should be caught' % api.env.host,
+            command=('service_disable', ['HTTP/%s' % api.env.host], {}),
+            expected=errors.ValidationError(name='principal', error='This principal is required by the IPA master'),
+        ),
+
+
+        dict(
+            desc='Disable the current host (master?) %s ldap service, should be caught' % api.env.host,
+            command=('service_disable', ['ldap/%s' % api.env.host], {}),
+            expected=errors.ValidationError(name='principal', error='This principal is required by the IPA master'),
+        ),
+
+
     ]
--
1.7.6

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

Reply via email to