Martin Kosek wrote:
On Thu, 2012-02-23 at 11:33 -0500, Rob Crittenden wrote:
Martin Kosek wrote:
On Wed, 2012-02-22 at 17:47 -0500, Rob Crittenden wrote:
Don't allow a host that is a master or its IPA services to be deleted.

I'm taking a pretty limited view of services, preventing deletion of
just the IPA services I could think of. I don't want to prevent someone
from deleting an nfs service they set up, for example.

I'm raising a ValidationError here. I don't know what value it would add
to have a custom exception but I can add one if desired.

rob

Generally it looks OK. At first I was concerned if we don't blow up
during ipa-replica-manage del, but it worked fine.

I have just 2 minor issues:
1) There is wrong attribute name in new service-del ValidationError,
which is confusing:

# ipa service-del
ldap/vm-068.idm.lab.bos.redhat....@idm.lab.bos.redhat.com
ipa: ERROR: invalid 'hostname': This service cannot be removed from an
IPA master

Yeah, I waffled on that myself. I used hostname since that is what was
blowing up. I can change it.

Yes please. This may confuse users as we always try to have attribute
name in ValidationError. We may want to reword the error text in that
case too.


2) I would move function host_is_master rather to ipalib/util.py as its
not really related with base classes in baseldap.py

I added in there because it requires LDAP to execute. You can't call
this without an ldpa handle, etc. I think it should remain there to
avoid confusion.


Done. Added some unit tests too.

rob
>From 4b922839f533b834c98b16274723851c33a94de0 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Wed, 22 Feb 2012 17:42:38 -0500
Subject: [PATCH] Don't allow IPA master hosts or important services be
 deleted.

Deleting these would cause the IPA master to blow up.

For services I'm taking a conservative approach and only limiting the
deletion of known services we care about.

https://fedorahosted.org/freeipa/ticket/2425
---
 ipalib/plugins/baseldap.py               |   14 ++++++++++++++
 ipalib/plugins/host.py                   |    1 +
 ipalib/plugins/service.py                |   10 ++++++++++
 tests/test_xmlrpc/test_host_plugin.py    |    9 +++++++++
 tests/test_xmlrpc/test_service_plugin.py |   17 +++++++++++++++++
 5 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index d619f14ee06d4157b456e2f21ead314f16c44efb..725704ee0e2d784a1f5ad8691d90888366f8efec 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -387,6 +387,20 @@ def remove_external_post_callback(memberattr, membertype, externalattr, ldap, co
 
     return (completed + completed_external, dn)
 
+def host_is_master(ldap, fqdn):
+    """
+    Check to see if this host is a master.
+
+    Raises an exception if a master, otherwise returns nothing.
+    """
+    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'))
+    except errors.NotFound:
+        # Good, not a master
+        return
+
 
 class LDAPObject(Object):
     """
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 682b81420577d7f4e153867c7435803f841ab86a..012817e6b23176491792bfe6743653152ddc6175 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -560,6 +560,7 @@ class host_del(LDAPDelete):
             fqdn = hostentry['fqdn'][0]
         else:
             fqdn = keys[-1]
+        host_is_master(ldap, fqdn)
         # Remove all service records for this host
         truncated = True
         while truncated:
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index dad3ded434d241ae55e1352889c577ba1a08d8c4..71e4ac4657193612860ed88729aa7b848838a908 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -288,6 +288,16 @@ class service_del(LDAPDelete):
     msg_summary = _('Deleted service "%(value)s"')
     member_attributes = ['managedby']
     def pre_callback(self, ldap, dn, *keys, **options):
+        # In the case of services we don't want IPA master services to be
+        # 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'))
         if self.api.env.enable_ra:
             (dn, entry_attrs) = ldap.get_entry(dn, ['usercertificate'])
             cert = entry_attrs.get('usercertificate')
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 372ee256eaf411398ac95c9b9e65199489b59270..0323ac39a812e4e69344e8ebf3cec50a10838efb 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -661,4 +661,13 @@ class test_host(Declarative):
             ),
         ),
 
+
+        # This test will only succeed when running against lite-server.py
+        # on same box as IPA install.
+        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'),
+        ),
+
     ]
diff --git a/tests/test_xmlrpc/test_service_plugin.py b/tests/test_xmlrpc/test_service_plugin.py
index d36dac984e4c1e3e4352c8653a787a2f0fd3d030..e97fb7c878ab6b9ec0ef04e5e78b40416c219f92 100644
--- a/tests/test_xmlrpc/test_service_plugin.py
+++ b/tests/test_xmlrpc/test_service_plugin.py
@@ -467,4 +467,21 @@ class test_service(Declarative):
             expected=errors.HostService()
         ),
 
+
+        # These tests will only succeed when running against lite-server.py
+        # on same box as IPA install.
+        dict(
+            desc='Delete the current host (master?) %s HTTP service, should be caught' % api.env.host,
+            command=('service_del', ['HTTP/%s' % api.env.host], {}),
+            expected=errors.ValidationError(name='principal', error='This principal is required by the IPA master'),
+        ),
+
+
+        dict(
+            desc='Delete the current host (master?) %s ldap service, should be caught' % api.env.host,
+            command=('service_del', ['ldap/%s' % api.env.host], {}),
+            expected=errors.ValidationError(name='principal', error='This principal is required by the IPA master'),
+        ),
+
+
     ]
-- 
1.7.6.5

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

Reply via email to