On Fri, 2011-04-01 at 13:51 -0400, Rob Crittenden wrote: > Martin Kosek wrote: > > Target branches: master, ipa-2-0 > > --- > > > > Most of the pwpolicy_* commands do include cospriority in the result > > and potentially in the attribute rights (--all --rights). Especially > > when --raw output is requested. This patch fixes it for all > > pwpolicy commands. > > > > https://fedorahosted.org/freeipa/ticket/1103 > > > > nack. I see a couple of problems. > > You should test for rights before doing the cosentry_show(). If rights > is False then we won't add the data whatever it is so it is more > efficient to exit earlier.
We have to call cosentry_show every time (except for the case when we pull data for the global policy) because we read cospriority attribute. But the function was indeed not efficient (it called cosentry_show twice), I rewrote it. > > Same with pwpolicy_name == global_policy_name. I think you should drop > the try/except and make it: > > if not rights or pwpolicy_name == global_policy_name: > return > > ... > > It should never be the case that the cosentry is not found so I'd let it > fail if that does occur. Fixed. > > I think that keys[-1] can be None so be aware. Fixed. > > You hardcode rights == False in pwpolicy_find(), a good thing. I think > you should add make it explict rights=False and add a comment explaining > that you can't get accessrights with a find. Fixed. Fixed patch attached. Martin
>From 987c5aeab203495002722a6238b2c7726ad38f9b Mon Sep 17 00:00:00 2001 From: Martin Kosek <mko...@redhat.com> Date: Thu, 24 Mar 2011 16:30:10 +0100 Subject: [PATCH] Password policy commands do not include cospriority Most of the pwpolicy_* commands do include cospriority in the result and potentially in the attribute rights (--all --rights). Especially when --raw output is requested. This patch fixes it for all pwpolicy commands. https://fedorahosted.org/freeipa/ticket/1103 --- ipalib/plugins/pwpolicy.py | 56 ++++++++++++++++++------------------------- 1 files changed, 24 insertions(+), 32 deletions(-) diff --git a/ipalib/plugins/pwpolicy.py b/ipalib/plugins/pwpolicy.py index caf918c7af655510dd4311fa8e2c2a0b67a125e9..4e1961f59f2b3b79f52d03dfd039a7f4c2b9260e 100644 --- a/ipalib/plugins/pwpolicy.py +++ b/ipalib/plugins/pwpolicy.py @@ -156,7 +156,8 @@ class cosentry_find(LDAPSearch): api.register(cosentry_find) -global_policy_dn = 'cn=global_policy,cn=%s,cn=kerberos,%s' % (api.env.realm, api.env.basedn) +global_policy_name = 'global_policy' +global_policy_dn = 'cn=%s,cn=%s,cn=kerberos,%s' % (global_policy_name, api.env.realm, api.env.basedn) class pwpolicy(LDAPObject): """ @@ -304,6 +305,18 @@ class pwpolicy(LDAPObject): error=_('Maximum password life must be greater than minimum.'), ) + def add_cospriority(self, entry, pwpolicy_name, rights=True): + if pwpolicy_name and pwpolicy_name != global_policy_name: + cos_entry = self.api.Command.cosentry_show( + pwpolicy_name, + rights=rights, all=rights + )['result'] + if cos_entry.get('cospriority') is not None: + entry['cospriority'] = cos_entry['cospriority'] + if rights: + entry['attributelevelrights']['cospriority'] = \ + cos_entry['attributelevelrights']['cospriority'] + api.register(pwpolicy) @@ -327,9 +340,8 @@ class pwpolicy_add(LDAPCreate): def post_callback(self, ldap, dn, entry_attrs, *keys, **options): self.log.info('%r' % entry_attrs) - if not options.get('raw', False): - if options.get('cospriority') is not None: - entry_attrs['cospriority'] = [unicode(options['cospriority'])] + # attribute rights are not allowed for pwpolicy_add + self.obj.add_cospriority(entry_attrs, keys[-1], rights=False) self.obj.convert_time_for_output(entry_attrs, **options) return dn @@ -381,9 +393,8 @@ class pwpolicy_mod(LDAPUpdate): return dn def post_callback(self, ldap, dn, entry_attrs, *keys, **options): - if not options.get('raw', False): - if options.get('cospriority') is not None: - entry_attrs['cospriority'] = [unicode(options['cospriority'])] + rights = options.get('all', False) and options.get('rights', False) + self.obj.add_cospriority(entry_attrs, keys[-1], rights) self.obj.convert_time_for_output(entry_attrs, **options) return dn @@ -418,20 +429,8 @@ class pwpolicy_show(LDAPRetrieve): return dn def post_callback(self, ldap, dn, entry_attrs, *keys, **options): - if not options.get('raw', False): - if keys[-1] is not None and keys[-1] != 'global_policy': - try: - cos_entry = self.api.Command.cosentry_show( - keys[-1] - )['result'] - if cos_entry.get('cospriority') is not None: - entry_attrs['cospriority'] = cos_entry['cospriority'] - except errors.NotFound: - pass - if options.get('rights', False) and options.get('all', False) and \ - (keys[-1] is not None and keys[-1] != 'global_policy'): - cos_entry = self.api.Command.cosentry_show(keys[-1], rights=True, all=True)['result'] - entry_attrs['attributelevelrights']['cospriority'] = cos_entry['attributelevelrights']['cospriority'] + rights = options.get('all', False) and options.get('rights', False) + self.obj.add_cospriority(entry_attrs, keys[-1], rights) self.obj.convert_time_for_output(entry_attrs, **options) return dn @@ -443,17 +442,10 @@ class pwpolicy_find(LDAPSearch): Search for group password policies. """ def post_callback(self, ldap, entries, truncated, *args, **options): - if not options.get('raw', False): - for e in entries: - try: - cos_entry = self.api.Command.cosentry_show( - e[1]['cn'][0] - )['result'] - if cos_entry.get('cospriority') is not None: - e[1]['cospriority'] = cos_entry['cospriority'] - except errors.NotFound: - pass - self.obj.convert_time_for_output(e[1], **options) + for e in entries: + # attribute rights are not allowed for pwpolicy_find + self.obj.add_cospriority(e[1], e[1]['cn'][0], rights=False) + self.obj.convert_time_for_output(e[1], **options) api.register(pwpolicy_find) -- 1.7.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
