On Fri, 2011-05-27 at 19:21 +0200, Martin Kosek wrote: > On Fri, 2011-05-27 at 11:10 -0400, Rob Crittenden wrote: > > Martin Kosek wrote: > > > On Mon, 2011-05-16 at 17:46 -0400, Rob Crittenden wrote: > > >> Add option to limit the attributes allowed in an entry. > > >> > > >> Kerberos ticket policy can update policy in a user entry. This allowed > > >> set/addattr to be used to modify attributes outside of the ticket policy > > >> perview, also bypassing all validation/normalization. Likewise the > > >> ticket policy was updatable by the user plugin bypassing all validation. > > >> > > >> Add two new LDAPObject values to control this behavior: > > >> > > >> limit_object_classes: only attributes in these are allowed > > >> disallow_object_classes: attributes in these are disallowed > > >> > > >> By default both of these lists are empty so are skipped. > > >> > > >> ticket 744 > > >> > > >> rob > > > > > > NACK. I have some concerns with this patch. In function > > > _check_limit_object_class: > > > > > > 1) You change input attribute 'attrs' by removing the items from it. If > > > user passes the same list of attrs to be checked and the function is run > > > twice, the 'attrs' parameter in second run is corrupt. > > > > > > You can try it by running e.g. `ipa krbtpolicy-mod --maxrenew=24044' and > > > checking the value of this parameter in the function. > > > > Good catch, updated patch attached. > > > > > > > > 2) The purpose of this statement is not clear to me: > > > + if len(attrs)> 0 and allow_only: > > > + raise errors.ObjectclassViolation(info='attribute > > > "%(attribute)s" not allowed' % dict(attribute=attrs[0])) > > > Maybe just the exception text is misleading. > > > > This function has 2 modes: "allow only the attributes in these > > objectclasses" or "specifically deny the attributes in these > > objectclasses". This enforces the first type. If when we've gone through > > all the attributes there are any left over they must not be allowed so > > raise an error. This is documented in the function header. > > Thanks for explanation, now I get it. It all looks OK, ACK. > > Martin >
Checked again as I had some second thoughts. But no problem found. Pushed to master, ipa-2-0. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel