Re: [Freeipa-devel] [PATCH] 987 Don't allow IPA master hosts and services to be disabled

2012-03-19 Thread Martin Kosek
On Fri, 2012-03-16 at 08:29 -0400, Rob Crittenden wrote:
 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

I still think this is not the one. It somehow got squashed with Petr3's
*attr patch.

Martin

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


Re: [Freeipa-devel] [PATCH] 987 Don't allow IPA master hosts and services to be disabled

2012-03-19 Thread Rob Crittenden

Martin Kosek wrote:

On Fri, 2012-03-16 at 08:29 -0400, Rob Crittenden wrote:

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


I still think this is not the one. It somehow got squashed with Petr3's
*attr patch.

Martin



Ok, was a little more careful this time.

rob
From 12ffb0d7305a60faa31f08ffc1a884aa40e7ea4b Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Mon, 19 Mar 2012 10:16:49 -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   |2 +-
 ipalib/plugins/host.py   |2 ++
 ipalib/plugins/service.py|   22 --
 tests/test_xmlrpc/test_host_plugin.py|   10 +-
 tests/test_xmlrpc/test_service_plugin.py |   14 ++
 5 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 9562ff98729ead6ac9e56d504f6ee0a7c0ca377a..92540d8ac38a9fe033327d98c949469cb4745a97 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
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 

Re: [Freeipa-devel] [PATCH] 987 Don't allow IPA master hosts and services to be disabled

2012-03-19 Thread Martin Kosek
On Mon, 2012-03-19 at 10:17 -0400, Rob Crittenden wrote:
 Martin Kosek wrote:
  On Fri, 2012-03-16 at 08:29 -0400, Rob Crittenden wrote:
  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
 
  I still think this is not the one. It somehow got squashed with Petr3's
  *attr patch.
 
  Martin
 
 
 Ok, was a little more careful this time.
 
 rob

Yup, its much better now. ACK. Pushed to master, ipa-2-2.

Martin

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


Re: [Freeipa-devel] [PATCH] 987 Don't allow IPA master hosts and services to be disabled

2012-03-16 Thread Petr Viktorin

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).

--
PetrĀ³

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