Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 02/25/2013 02:16 PM, Tomas Babej wrote: On Fri 22 Feb 2013 04:34:55 PM CET, Martin Kosek wrote: On 02/22/2013 03:01 PM, Tomas Babej wrote: On 02/21/2013 02:22 PM, Martin Kosek wrote: On 02/20/2013 03:19 PM, Tomas Babej wrote: On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote: On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. Thanks! Sending the updated patch 0024. Tomas In patch 0024 your intention is OK, but the checking functions are not: is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) They return True even when the attribute is None because they check if *x* is None and not if *attrs[x]* is None. Example: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range This command should be NOOP, but is not: # ipa idrange-mod local_range --dom-sid= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin Thanks for the catch, all checking functions fixed. Sending the complete patchset, up to date. Tomas I think I am being a nitpicker now (sorry), but this condition also fails if the old_attrs has a setting, but I am removing it in a current -mod command: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range # ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin Yep. Ok, the command should now handle this use case as
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On Fri 22 Feb 2013 04:34:55 PM CET, Martin Kosek wrote: On 02/22/2013 03:01 PM, Tomas Babej wrote: On 02/21/2013 02:22 PM, Martin Kosek wrote: On 02/20/2013 03:19 PM, Tomas Babej wrote: On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote: On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. Thanks! Sending the updated patch 0024. Tomas In patch 0024 your intention is OK, but the checking functions are not: is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) They return True even when the attribute is None because they check if *x* is None and not if *attrs[x]* is None. Example: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range This command should be NOOP, but is not: # ipa idrange-mod local_range --dom-sid= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin Thanks for the catch, all checking functions fixed. Sending the complete patchset, up to date. Tomas I think I am being a nitpicker now (sorry), but this condition also fails if the old_attrs has a setting, but I am removing it in a current -mod command: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range # ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin Yep. Ok, the command should now handle this use case as well. Tomas From 61fcd3db8a14a17ed5854dfb4a9f11e2bb06784e Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 20 Feb
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 02/21/2013 02:22 PM, Martin Kosek wrote: On 02/20/2013 03:19 PM, Tomas Babej wrote: On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote: On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. Thanks! Sending the updated patch 0024. Tomas In patch 0024 your intention is OK, but the checking functions are not: is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) They return True even when the attribute is None because they check if *x* is None and not if *attrs[x]* is None. Example: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range This command should be NOOP, but is not: # ipa idrange-mod local_range --dom-sid= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin Thanks for the catch, all checking functions fixed. Sending the complete patchset, up to date. Tomas From 61fcd3db8a14a17ed5854dfb4a9f11e2bb06784e Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 20 Feb 2013 10:50:36 +0100 Subject: [PATCH] Add trusted domain range objectclass when using idrange-mod When modifing the idrange, one was able to add ipa NT trusted AD domain sid without objectclass ipatrustedaddomainrange being added. This patch fixes the issue. --- ipalib/plugins/idrange.py | 5 + 1 file changed, 5 insertions(+) diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py index 97b9d2489b51cf1fc62f49e0d3faf9674851d68a..087a1b128256063194e39bfa2369fd66af4b516f 100644 --- a/ipalib/plugins/idrange.py +++ b/ipalib/plugins/idrange.py @@ -531,6 +531,11 @@ class idrange_mod(LDAPUpdate): # perform this check only if the attribute was changed self.obj.validate_trusted_domain_sid( entry_attrs['ipanttrusteddomainsid']) + + # Add trusted AD domain range object class, if it wasn't there +if not 'ipatrustedaddomainrange' in
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 02/22/2013 03:01 PM, Tomas Babej wrote: On 02/21/2013 02:22 PM, Martin Kosek wrote: On 02/20/2013 03:19 PM, Tomas Babej wrote: On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote: On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. Thanks! Sending the updated patch 0024. Tomas In patch 0024 your intention is OK, but the checking functions are not: is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) They return True even when the attribute is None because they check if *x* is None and not if *attrs[x]* is None. Example: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range This command should be NOOP, but is not: # ipa idrange-mod local_range --dom-sid= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin Thanks for the catch, all checking functions fixed. Sending the complete patchset, up to date. Tomas I think I am being a nitpicker now (sorry), but this condition also fails if the old_attrs has a setting, but I am removing it in a current -mod command: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range # ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 02/20/2013 03:19 PM, Tomas Babej wrote: On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote: On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. Thanks! Sending the updated patch 0024. Tomas In patch 0024 your intention is OK, but the checking functions are not: is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) They return True even when the attribute is None because they check if *x* is None and not if *attrs[x]* is None. Example: # ipa idrange-add --base-id=120 --range-size=20 --rid-base=1000 --secondary-rid-base=100 local_range Added ID range local_range Range name: local_range First Posix ID of the range: 120 Number of IDs in the range: 20 First RID of the corresponding RID range: 1000 First RID of the secondary RID range: 100 Range type: local domain range This command should be NOOP, but is not: # ipa idrange-mod local_range --dom-sid= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Tomas From f21b135d546678544ccf05efd587b46bba88e07a Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Fri, 21 Dec 2012 05:34:37 -0500 Subject: [PATCH] Make options checks in idrange-add/mod consistent Both now enforce the following checks: - dom_sid and secondary_rid_base cannot be used together - rid_base must be used together if dom_rid is set - secondary_rid_base and rid_base must be used together if dom_rid is not set Unit test for third check has been added. http://fedorahosted.org/freeipa/ticket/3170 --- ipalib/plugins/idrange.py | 56 +- tests/test_xmlrpc/test_range_plugin.py | 46 +++- 2 files changed, 87 insertions(+), 15 deletions(-) diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py index 445473394223f3f7c69d1e32be71d7c6944e6252..97b9d2489b51cf1fc62f49e0d3faf9674851d68a 100644 --- a/ipalib/plugins/idrange.py +++ b/ipalib/plugins/idrange.py @@ -402,11 +402,13 @@ class idrange_add(LDAPCreate): entry_attrs['objectclass'].append('ipatrustedaddomainrange') else: + # secondary base rid must be set if and only if base rid is set if is_set('ipasecondarybaserid') != is_set('ipabaserid'): raise errors.ValidationError(name='ID Range setup', error=_('Options secondary-rid-base and rid-base must ' 'be used together')) +# and they must not overlap if is_set('ipabaserid') and is_set('ipasecondarybaserid'): if self.obj.are_rid_ranges_overlapping( entry_attrs['ipabaserid'], @@ -483,7 +485,14 @@ class idrange_mod(LDAPUpdate): assert isinstance(dn, DN) attrs_list.append('objectclass') +try: +(old_dn, old_attrs) = ldap.get_entry(dn, ['*']) +except errors.NotFound: +self.obj.handle_not_found(*keys) + is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) # This needs to stay in options since there is no # ipanttrusteddomainname attribute in LDAP @@ -496,6 +505,8 @@ class idrange_mod(LDAPUpdate): sid = self.obj.get_trusted_domain_sid_from_name( options['ipanttrusteddomainname']) +# we translate the name into sid so further validation can rely +# on ipanttrusteddomainsid attribute only if sid is not None: entry_attrs['ipanttrusteddomainsid'] = sid else: @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains -self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) + +if is_set('ipanttrusteddomainsid'): +# Validate SID as the one of trusted domains +# perform this check only if the attribute was changed +self.obj.validate_trusted_domain_sid( +entry_attrs['ipanttrusteddomainsid']) +else: +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) # ensure that primary and secondary rid ranges do not
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py
On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote: On Wed, 20 Feb 2013, Tomas Babej wrote: On 12/21/2012 12:15 PM, Tomas Babej wrote: Hi, Sending updated and rebased versions of patches 0024 and 0025. Tomas Sending rebased version, these got quite rotten. Thanks for updating them. @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate): 'not be found. Please specify the SID directly ' 'using dom-sid option.')) -try: -(old_dn, old_attrs) = ldap.get_entry(dn, -['ipabaseid', -'ipaidrangesize', -'ipabaserid', -'ipasecondarybaserid']) -except errors.NotFound: -self.obj.handle_not_found(*keys) +if in_updated_attrs('ipanttrusteddomainsid'): +if in_updated_attrs('ipasecondarybaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and secondary_rid_base cannot ' +'be used together')) Since we agreed to refer to options by their CLI name (--dom-sid and --secondary-rid-base) in the other patch, it makes sense to use it here too. -if is_set('ipanttrusteddomainsid'): -# Validate SID as the one of trusted domains - self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid']) +if not in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options dom_sid and rid_base must ' +'be used together')) Same here. +# secondary base rid must be set if and only if base rid is set +if in_updated_attrs('ipasecondarybaserid') !=\ +in_updated_attrs('ipabaserid'): +raise errors.ValidationError(name='ID Range setup', +error=_('Options secondary_rid_base and rid_base must ' +'be used together')) Same here. +dict( +desc='Try to modify ID range %r so it has only primary rid range set' % (testrange8), +command=('idrange_mod', [testrange8], + dict(ipabaserid=testrange8_base_rid)), +expected=errors.ValidationError( +name='ID Range setup', error='Options secondary_rid_base and rid_base must be used together'), +), And synchronize error message here too. Thanks! Sending the updated patch 0024. Tomas From d36c9aa5fb7dd044e212de4cd17adbc044d14777 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Fri, 21 Dec 2012 05:34:37 -0500 Subject: [PATCH] Make options checks in idrange-add/mod consistent Both now enforce the following checks: - dom_sid and secondary_rid_base cannot be used together - rid_base must be used together if dom_rid is set - secondary_rid_base and rid_base must be used together if dom_rid is not set Unit test for third check has been added. http://fedorahosted.org/freeipa/ticket/3170 --- ipalib/plugins/idrange.py | 56 +- tests/test_xmlrpc/test_range_plugin.py | 46 +++- 2 files changed, 87 insertions(+), 15 deletions(-) diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py index 445473394223f3f7c69d1e32be71d7c6944e6252..6a4c7a5b5de7971abf374572cd3bdadfaae93651 100644 --- a/ipalib/plugins/idrange.py +++ b/ipalib/plugins/idrange.py @@ -402,11 +402,13 @@ class idrange_add(LDAPCreate): entry_attrs['objectclass'].append('ipatrustedaddomainrange') else: + # secondary base rid must be set if and only if base rid is set if is_set('ipasecondarybaserid') != is_set('ipabaserid'): raise errors.ValidationError(name='ID Range setup', error=_('Options secondary-rid-base and rid-base must ' 'be used together')) +# and they must not overlap if is_set('ipabaserid') and is_set('ipasecondarybaserid'): if self.obj.are_rid_ranges_overlapping( entry_attrs['ipabaserid'], @@ -483,7 +485,14 @@ class idrange_mod(LDAPUpdate): assert isinstance(dn, DN) attrs_list.append('objectclass') +try: +(old_dn, old_attrs) = ldap.get_entry(dn, ['*']) +except errors.NotFound: +self.obj.handle_not_found(*keys) + is_set = lambda x: (x in entry_attrs) and (x is not None) +in_updated_attrs = lambda x: any((x in attrs and x is not None) + for attrs in (entry_attrs, old_attrs)) # This needs to stay in options since there is no # ipanttrusteddomainname attribute in LDAP @@ -496,6