Re: [Freeipa-devel] [PATCH] 552 handle setattr/addattr better
On 09/29/2010 11:03 PM, Rob Crittenden wrote: When doing an addattr check to see if we are creating a multi-value attribute and see if that is allowed by the Param and/or the attribute in the schema (SINGLE-VALUE). Pavel, check my fix in the exception callback. It was passing attrs_list but that isn't set until later. I decided to send an empty list instead. Also catch RDN update exceptions and return an error about primary keys (which this essentially means). ticket 230 rob NACK. The patch isn't all bad, but the single-value check is in the wrong place. As a result, it only applies when someone tries to add a new value to attributes already present in the original entry. It won't fire when someone is trying to add more than one value if there was none before and it also won't fire when creating new entries. I reworked your patch a bit a merged it with my patch number 32, because they overlap in functionality. See freeipa-devel thread: [PATCH] Check if attribute is single-value before trying to add values to it. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Check if attribute is single-value before trying to add values to it.
On 10/14/2010 12:01 AM, Rob Crittenden wrote:
Pavel Zuna wrote:
This patch adds a check in ldap2 for single-value attributes. DS doesn't
seem to care much about attributes being defined as SINGLE-VALUE except
for things like uidNumber and gidNumber (I suspect this is handled by
the DNA plugin).
Ticket #246
Pavel
This is similar to ticket 220 which I have a pending patch for (patch
552). I think both patches are valid but we should test them together to
be sure. Can you do that?
rob
I had to NACK your patch number 552, because the check was in the wrong place.
Both patches overlap in functionality, so I decided to merge them into a new
version of my original patch.
I split the single-value check into two parts:
First part is in baseldap classes (LDAPCreate, LDAPUpdate) and it checks if
we're not trying to add more values to a Param defined attribute, that is not
flagged as multivalue.
Second part is in the ldap2 backend. It checks if we're not trying to add more
values to an attribute, that is defined as SINGLE-VALUE in the schema.
Unfortunately, it seems that python-ldap isn't capable of reporting the
SINGLE-VALUE flag reliably and DS doesn't enforce it at all. In other words,
this check is a bit weak, but still better than nothing.
I hope you don't mind I merged both patches, but it seemed simpler and we can
knock out 2 tickets in one commit. :)
Ticket #230
Ticket #246
Pavel
From adff41671b7f04f718085711401e7328390151ae Mon Sep 17 00:00:00 2001
From: Pavel Zuna pz...@redhat.com
Date: Thu, 14 Oct 2010 13:05:43 -0400
Subject: [PATCH 1/2] Disallow RDN change and single-value bypass using setattr/addattr.
Merge of my original patch number 32 and Rob's patch number 552.
Ticket #230
Ticket #246
---
ipalib/errors.py | 33 -
ipalib/frontend.py |2 +-
ipalib/plugins/baseldap.py | 14 +-
ipaserver/plugins/ldap2.py | 44 +++-
4 files changed, 77 insertions(+), 16 deletions(-)
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 42d43ce..db13a43 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1162,7 +1162,7 @@ class DatabaseError(ExecutionError):
errno = 4203
-format = _('%(desc)s:%(info)s')
+format = _('%(desc)s: %(info)s')
class LimitsExceeded(ExecutionError):
@@ -1195,6 +1195,37 @@ class ObjectclassViolation(ExecutionError):
errno = 4205
format = _('%(info)s')
+class NotAllowedOnRDN(ExecutionError):
+
+**4206** Raised when an RDN value is modified.
+
+For example:
+
+ raise NotAllowedOnRDN()
+Traceback (most recent call last):
+ ...
+NotAllowedOnRDN: modifying primary key is not allowed
+
+
+errno = 4206
+format = _('modifying primary key is not allowed')
+
+
+class OnlyOneValueAllowed(ExecutionError):
+
+**4207** Raised when trying to set more than one value to single-value attributes
+
+For example:
+
+ raise OnlyOneValueAllowed(attr='ipasearchtimelimit')
+Traceback (most recent call last):
+ ...
+OnlyOneValueAllowed: ipasearchtimelimit: attribute is single-value
+
+
+errno = 4207
+format = _('%(attr)s: attribute is single-value')
+
class CertificateError(ExecutionError):
diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index c9c070d..96649d9 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -504,7 +504,7 @@ class Command(HasParam):
a dictionary. The incoming attribute may be a string or
a list.
-Any attribute found that is also a param is silently dropped.
+Any attribute found that is also a param is validated.
append controls whether this returns a list of values or a single
value.
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 2335a7a..caa616a 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -157,6 +157,14 @@ _attr_options = (
),
)
+# addattr can cause parameters to have more than one value even if not defined
+# as multivalue, make sure this isn't the case
+def _check_single_value_attrs(params, entry_attrs):
+for (a, v) in entry_attrs.iteritems():
+if isinstance(v, (list, tuple)) and len(v) 1:
+if a in params and not params[a].multivalue:
+raise errors.OnlyOneValueAllowed(attr=a)
+
class CallbackInterface(Method):
@@ -277,6 +285,8 @@ class LDAPCreate(CallbackInterface, crud.Create):
self, ldap, dn, entry_attrs, attrs_list, *keys, **options
)
+_check_single_value_attrs(self.params, entry_attrs)
+
try:
ldap.add_entry(dn, entry_attrs, normalize=self.obj.normalize_dn)
except errors.ExecutionError, e:
@@ -464,7 +474,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
except errors.ExecutionError, e:
try:
(dn,
[Freeipa-devel] [PATCH] Add fail-safe defaults to time and size limits in ldap2 searches.
There was no default value set even though we were using config.get and it was
throwing exceptions if someone deleted one of the related config values.
Pavel
From 5dfda61f3995f4d5ae5813b7f70f2d2658a687f0 Mon Sep 17 00:00:00 2001
From: Pavel Zuna pz...@redhat.com
Date: Thu, 14 Oct 2010 10:54:24 -0400
Subject: [PATCH 2/2] Add fail-safe defaults to time and size limits in ldap2 searches.
---
ipaserver/plugins/ldap2.py |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 096d3a3..1d18bbb 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -515,9 +515,9 @@ class ldap2(CrudBackend, Encoder):
if time_limit is None or size_limit is None:
(cdn, config) = self.get_ipa_config()
if time_limit is None:
-time_limit = config.get('ipasearchtimelimit')[0]
+time_limit = config.get('ipasearchtimelimit', [-1])[0]
if size_limit is None:
-size_limit = config.get('ipasearchrecordslimit')[0]
+size_limit = config.get('ipasearchrecordslimit', [0])[0]
if not isinstance(size_limit, int):
size_limit = int(size_limit)
if not isinstance(time_limit, float):
--
1.7.1.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add fail-safe defaults to time and size limits in ldap2 searches.
On 10/14/2010 09:25 AM, Pavel Zuna wrote: There was no default value set even though we were using config.get and it was throwing exceptions if someone deleted one of the related config values. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Check if attribute is single-value before trying to add values to it.
Pavel Zuna wrote: On 10/14/2010 12:01 AM, Rob Crittenden wrote: Pavel Zuna wrote: This patch adds a check in ldap2 for single-value attributes. DS doesn't seem to care much about attributes being defined as SINGLE-VALUE except for things like uidNumber and gidNumber (I suspect this is handled by the DNA plugin). Ticket #246 Pavel This is similar to ticket 220 which I have a pending patch for (patch 552). I think both patches are valid but we should test them together to be sure. Can you do that? rob I had to NACK your patch number 552, because the check was in the wrong place. Both patches overlap in functionality, so I decided to merge them into a new version of my original patch. I split the single-value check into two parts: First part is in baseldap classes (LDAPCreate, LDAPUpdate) and it checks if we're not trying to add more values to a Param defined attribute, that is not flagged as multivalue. Second part is in the ldap2 backend. It checks if we're not trying to add more values to an attribute, that is defined as SINGLE-VALUE in the schema. Unfortunately, it seems that python-ldap isn't capable of reporting the SINGLE-VALUE flag reliably and DS doesn't enforce it at all. In other words, this check is a bit weak, but still better than nothing. Can you give me an example of an attribute definition that python-ldap doesn't parse correctly? Can you give me an example of an ldapmodify command that adds multiple values to a single valued attribute in 389? I hope you don't mind I merged both patches, but it seemed simpler and we can knock out 2 tickets in one commit. :) Ticket #230 Ticket #246 Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add fail-safe defaults to time and size limits in ldap2 searches.
I have noticed a change in behavior with this ... BEFORE: --sizelimit=0 returned 0 entries now , it is returning all the entries ... obviously 0 now assumes default ... what is the default ?? Thanks Jenny Adam Young wrote: On 10/14/2010 09:25 AM, Pavel Zuna wrote: There was no default value set even though we were using config.get and it was throwing exceptions if someone deleted one of the related config values. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Jenny Galipeau jgali...@redhat.com Principal Software QA Engineer Red Hat, Inc. Security Engineering Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] #316 Avoid installing files in /usr
On Thu, 14 Oct 2010 13:28:14 -0400 Rob Crittenden rcrit...@redhat.com wrote: Simo Sorce wrote: The default setup-ds.pl configuration installs ds scripts in /usr With this patch the customized scripts are kep in /var/lib/dirsrv/scripts-instance-name instead of /usr/lib/dirsrv/slapd-instance-name Simo. ack pushed to master Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] #318 Use openldap's ldappasswd
On Thu, 14 Oct 2010 13:30:33 -0400 Rob Crittenden rcrit...@redhat.com wrote: Simo Sorce wrote: The following patch makes the ldappasswd operation use the openldap's ldappasswd command, as well as avoiding to put passwords in the command line (visible through a ps) and instead using secure temporary files that are deleted immediately after the operation. Simo. ack thanks, pushed to master Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add flag to group-find to only search on private groups.
Pavel Zuna wrote: On 10/01/2010 02:47 PM, Pavel Zuna wrote: Ticket #251 Pavel New version of patch attached. This time it should work. :) I renamed the flag from --privateonly to --private. Normal searches do not return private groups at all, while searches with this flag only return private groups. Pavel This works a lot better than the last patch. The code itself is fine, I'd just ask that you add a test case for searching for private groups. The test that is in this patch seems more geared for removing multiple users at once (which is a good thing) but doesn't actually work without this change: --- a/tests/test_xmlrpc/test_user_plugin.py +++ b/tests/test_xmlrpc/test_user_plugin.py @@ -358,7 +358,7 @@ class test_user(Declarative): loginshell=[u'/bin/sh'], objectclass=objectclasses.user, sn=[u'User2'], -uid=[user1], +uid=[user2], uidnumber=[fuzzy_digits], ipauniqueid=[fuzzy_uuid], dn=u'uid=tuser2,cn=users,cn=accounts,' + api.env.basedn, So NACK for now but its very close. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 579 catch socket errors in client
Catch socket errors in the client. I ran into this playing around with the ipa command-line on an unconfigured machine. ticket 382 rob freeipa-579-socket.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
