On 12/12/2012 04:32 PM, Martin Kosek wrote:
On 10/26/2012 03:43 PM, Tomas Babej wrote:
Hi,
creating an id range with overlapping primary and secondary
rid range using idrange-add or idrange-mod command now
raises ValidationError. Unit tests have been added to
test_range_plugin.py.
https://fedorahosted.org/freeipa/ticket/3171
Tomas
1) Add command can cause crash:
# ipa idrange-add range9 --base-id=1000 --rid-base=1000 --secondary-rid-base=
--range-size 1000
ipa: ERROR: an internal error has occurred
2) I don't like this construct very much:
updated_values = dict(zip(rid_range_attributes,[None]*3))
This would look better, IMO:
updated_values = dict((attr, None) for attr in rid_range_attributes)
Why do you need this dict pre-created anyway? You overwrite all keys here:
+ for attr in rid_range_attributes:
+ if attr in entry_attrs:
+ updated_values[attr] = entry_attrs[attr]
+ else:
+ updated_values[attr] = int(old_attrs[attr][0])
3) [nitpick] We don't end ValidationError with '.':
+ raise errors.ValidationError(name='ID Range setup',
+ error=_("Primary rid range and secondary rid range"\
+ " cannot overlap."))
There is also a duplication of the same error message...
4) The -mod operation will also crash:
# ipa idrange-add range9 --base-id=1000 --rid-base=1000
--secondary-rid-base=2000 --range-size 1000
# ipa idrange-mod range9 --secondary-rid-base=
ipa: ERROR: an internal error has occurred
Martin
New patch version as well as diff between
patch versions (for your convenience) attached.
Tomas
>From fa94a7ec849a01e895424aa9b35997c37f4d3a0b Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Fri, 26 Oct 2012 07:43:05 -0400
Subject: [PATCH] Forbid overlapping rid ranges for the same id range
Creating an id range with overlapping primary and secondary
rid range using idrange-add or idrange-mod command now
raises ValidationError. Unit tests have been added to
test_range_plugin.py.
https://fedorahosted.org/freeipa/ticket/3171
---
ipalib/plugins/idrange.py | 70 +++++++++++++++++++++++++++++++---
tests/test_xmlrpc/test_range_plugin.py | 42 ++++++++++++++++----
2 files changed, 98 insertions(+), 14 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 75470811750f745c9b89268074830439bfc06bce..e7cf0fd22576f0186e4cf5f2f5fa596ffbf6a34a 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -277,6 +277,24 @@ class idrange(LDAPObject):
raise errors.ValidationError(name='domain SID',
error=_('SID is not recognized as a valid SID for a trusted domain'))
+ # checks that primary and secondary rid ranges do not overlap
+ def are_rid_ranges_overlapping(self, rid_base, secondary_rid_base, size):
+
+ # if any of these is None, the check does not apply
+ if(any([attr is None for attr in [rid_base,secondary_rid_base, size]])):
+ return False
+
+ # sort the bases
+ if rid_base > secondary_rid_base:
+ rid_base, secondary_rid_base = secondary_rid_base, rid_base
+
+ # rid_base is now <= secondary_rid_base,
+ # so the following check is sufficient
+ if rid_base + size <= secondary_rid_base:
+ return False
+ else:
+ return True
+
class idrange_add(LDAPCreate):
__doc__ = _("""
Add new ID range.
@@ -315,13 +333,15 @@ class idrange_add(LDAPCreate):
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
assert isinstance(dn, DN)
- if 'ipanttrusteddomainsid' in options:
- if 'ipasecondarybaserid' in options:
+ is_set = lambda x : (x in entry_attrs) and not (x is None)
+
+ if is_set('ipanttrusteddomainsid'):
+ if is_set('ipasecondarybaserid'):
raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and secondary_rid_base cannot ' \
'be used together'))
- if 'ipabaserid' not in options:
+ if not is_set('ipabaserid'):
raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and rid_base must ' \
'be used together'))
@@ -330,12 +350,22 @@ class idrange_add(LDAPCreate):
self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
# Finally, add trusted AD domain range object class
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
+
else:
- if (('ipasecondarybaserid' in options) != ('ipabaserid' in options)):
+ 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'))
+ if (is_set('ipabaserid') and is_set('ipasecondarybaserid')):
+ if self.obj.are_rid_ranges_overlapping(
+ entry_attrs['ipabaserid'],
+ entry_attrs['ipasecondarybaserid'],
+ entry_attrs['ipaidrangesize']):
+ raise errors.ValidationError(name='ID Range setup',
+ error=_("Primary rid range and secondary rid range"\
+ " cannot overlap"))
+
entry_attrs['objectclass'].append('ipadomainidrange')
return dn
@@ -403,15 +433,43 @@ class idrange_mod(LDAPUpdate):
assert isinstance(dn, DN)
attrs_list.append('objectclass')
+ is_set = lambda x : (x in entry_attrs) and not (x is None)
+
try:
- (old_dn, old_attrs) = ldap.get_entry(dn, ['ipabaseid', 'ipaidrangesize'])
+ (old_dn, old_attrs) = ldap.get_entry(dn,
+ ['ipabaseid',
+ 'ipaidrangesize',
+ 'ipabaserid',
+ 'ipasecondarybaserid'])
except errors.NotFound:
self.obj.handle_not_found(*keys)
- if 'ipanttrusteddomainsid' in options:
+ if is_set('ipanttrusteddomainsid'):
# Validate SID as the one of trusted domains
self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
+ # ensure that primary and secondary rid ranges do not overlap
+ if all([(base in entry_attrs) or (base in old_attrs)
+ for base in ['ipabaserid','ipasecondarybaserid']]):
+
+ # make sure we are working with updated attributes
+ rid_range_attributes = ['ipabaserid','ipasecondarybaserid','ipaidrangesize']
+ updated_values = dict()
+
+ for attr in rid_range_attributes:
+ if is_set(attr):
+ updated_values[attr] = entry_attrs[attr]
+ else:
+ updated_values[attr] = int(old_attrs[attr][0])
+
+ if self.obj.are_rid_ranges_overlapping(
+ updated_values['ipabaserid'],
+ updated_values['ipasecondarybaserid'],
+ updated_values['ipaidrangesize']):
+ raise errors.ValidationError(name='ID Range setup',
+ error=_("Primary rid range and secondary rid range"\
+ " cannot overlap"))
+
old_base_id = int(old_attrs.get('ipabaseid', [0])[0])
old_range_size = int(old_attrs.get('ipaidrangesize', [0])[0])
new_base_id = entry_attrs.get('ipabaseid')
diff --git a/tests/test_xmlrpc/test_range_plugin.py b/tests/test_xmlrpc/test_range_plugin.py
index 56c6db528b6c9cc4e1ac8cf22232fdfa46953389..310044f6a595fd0363e60b3a046920c3abcc0799 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -30,6 +30,8 @@ from ipapython.dn import *
testrange1 = u'testrange1'
testrange1_base_id = 900000
testrange1_size = 99999
+testrange1_base_rid = 10000
+testrange1_secondary_base_rid=200000
testrange2 = u'testrange2'
testrange2_base_id = 100
@@ -61,6 +63,11 @@ testrange6_size = 50
testrange6_base_rid = 500
testrange6_secondary_base_rid=1300
+testrange7 = u'testrange7'
+testrange7_base_id = 600
+testrange7_size = 50
+testrange7_base_rid = 600
+testrange7_secondary_base_rid=649
user1=u'tuser1'
user1_uid = 900000
@@ -69,7 +76,7 @@ group1_gid = 900100
class test_range(Declarative):
cleanup_commands = [
- ('idrange_del', [testrange1,testrange2,testrange3,testrange4,testrange5,testrange6], {}),
+ ('idrange_del', [testrange1,testrange2,testrange3,testrange4,testrange5,testrange6,testrange7], {'continue': True}),
('user_del', [user1], {}),
('group_del', [group1], {}),
]
@@ -79,7 +86,7 @@ class test_range(Declarative):
desc='Create ID range %r' % (testrange1),
command=('idrange_add', [testrange1],
dict(ipabaseid=testrange1_base_id, ipaidrangesize=testrange1_size,
- ipabaserid=10000, ipasecondarybaserid=20000)),
+ ipabaserid=testrange1_base_rid, ipasecondarybaserid=testrange1_secondary_base_rid)),
expected=dict(
result=dict(
dn=DN(('cn',testrange1),('cn','ranges'),('cn','etc'),
@@ -87,8 +94,8 @@ class test_range(Declarative):
cn=[testrange1],
objectclass=[u'ipaIDrange', u'ipadomainidrange'],
ipabaseid=[unicode(testrange1_base_id)],
- ipabaserid=[u'10000'],
- ipasecondarybaserid=[u'20000'],
+ ipabaserid=[unicode(testrange1_base_rid)],
+ ipasecondarybaserid=[unicode(testrange1_secondary_base_rid)],
ipaidrangesize=[unicode(testrange1_size)],
iparangetype=[u'local domain range'],
),
@@ -106,8 +113,8 @@ class test_range(Declarative):
api.env.basedn),
cn=[testrange1],
ipabaseid=[unicode(testrange1_base_id)],
- ipabaserid=[u'10000'],
- ipasecondarybaserid=[u'20000'],
+ ipabaserid=[unicode(testrange1_base_rid)],
+ ipasecondarybaserid=[unicode(testrange1_secondary_base_rid)],
ipaidrangesize=[unicode(testrange1_size)],
iparangetype=[u'local domain range'],
),
@@ -210,8 +217,8 @@ class test_range(Declarative):
result=dict(
cn=[testrange1],
ipabaseid=[unicode(testrange1_base_id)],
- ipabaserid=[u'10000'],
- ipasecondarybaserid=[u'20000'],
+ ipabaserid=[unicode(testrange1_base_rid)],
+ ipasecondarybaserid=[unicode(testrange1_secondary_base_rid)],
ipaidrangesize=[u'90000'],
iparangetype=[u'local domain range'],
),
@@ -287,6 +294,14 @@ class test_range(Declarative):
),
dict(
+ desc='Try to modify ID range %r so that its rid ranges are overlapping themselves' % (testrange2),
+ command=('idrange_mod', [testrange2],
+ dict(ipabaserid=(testrange2_base_rid*10))),
+ expected=errors.ValidationError(
+ name='ID Range setup', error='Primary rid range and secondary rid range cannot overlap'),
+ ),
+
+ dict(
desc='Try to create ID range %r with overlapping rid range' % (testrange3),
command=('idrange_add', [testrange3],
dict(ipabaseid=testrange3_base_id,
@@ -331,6 +346,17 @@ class test_range(Declarative):
),
dict(
+ desc='Try to create ID range %r with rid ranges overlapping themselves' % (testrange7),
+ command=('idrange_add', [testrange7],
+ dict(ipabaseid=testrange7_base_id,
+ ipaidrangesize=testrange7_size,
+ ipabaserid=testrange7_base_rid,
+ ipasecondarybaserid=testrange7_secondary_base_rid)),
+ expected=errors.ValidationError(
+ name='ID Range setup', error='Primary rid range and secondary rid range cannot overlap'),
+ ),
+
+ dict(
desc='Delete ID range %r' % testrange2,
command=('idrange_del', [testrange2], {}),
expected=dict(
--
1.8.0.1
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 05aac70..e7cf0fd 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -280,6 +280,10 @@ class idrange(LDAPObject):
# checks that primary and secondary rid ranges do not overlap
def are_rid_ranges_overlapping(self, rid_base, secondary_rid_base, size):
+ # if any of these is None, the check does not apply
+ if(any([attr is None for attr in [rid_base,secondary_rid_base, size]])):
+ return False
+
# sort the bases
if rid_base > secondary_rid_base:
rid_base, secondary_rid_base = secondary_rid_base, rid_base
@@ -329,13 +333,15 @@ class idrange_add(LDAPCreate):
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
assert isinstance(dn, DN)
- if 'ipanttrusteddomainsid' in options:
- if 'ipasecondarybaserid' in options:
+ is_set = lambda x : (x in entry_attrs) and not (x is None)
+
+ if is_set('ipanttrusteddomainsid'):
+ if is_set('ipasecondarybaserid'):
raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and secondary_rid_base cannot ' \
'be used together'))
- if 'ipabaserid' not in options:
+ if not is_set('ipabaserid'):
raise errors.ValidationError(name='ID Range setup',
error=_('Options dom_sid and rid_base must ' \
'be used together'))
@@ -344,20 +350,21 @@ class idrange_add(LDAPCreate):
self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
# Finally, add trusted AD domain range object class
entry_attrs['objectclass'].append('ipatrustedaddomainrange')
+
else:
- if (('ipasecondarybaserid' in options) != ('ipabaserid' in options)):
+ 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'))
- if (('ipabaserid' in options) and ('ipasecondarybaserid' in options)):
+ if (is_set('ipabaserid') and is_set('ipasecondarybaserid')):
if self.obj.are_rid_ranges_overlapping(
- options['ipabaserid'],
- options['ipasecondarybaserid'],
- options['ipaidrangesize']):
+ entry_attrs['ipabaserid'],
+ entry_attrs['ipasecondarybaserid'],
+ entry_attrs['ipaidrangesize']):
raise errors.ValidationError(name='ID Range setup',
error=_("Primary rid range and secondary rid range"\
- " cannot overlap."))
+ " cannot overlap"))
entry_attrs['objectclass'].append('ipadomainidrange')
@@ -426,12 +433,18 @@ class idrange_mod(LDAPUpdate):
assert isinstance(dn, DN)
attrs_list.append('objectclass')
+ is_set = lambda x : (x in entry_attrs) and not (x is None)
+
try:
- (old_dn, old_attrs) = ldap.get_entry(dn, ['ipabaseid', 'ipaidrangesize','ipabaserid','ipasecondarybaserid'])
+ (old_dn, old_attrs) = ldap.get_entry(dn,
+ ['ipabaseid',
+ 'ipaidrangesize',
+ 'ipabaserid',
+ 'ipasecondarybaserid'])
except errors.NotFound:
self.obj.handle_not_found(*keys)
- if 'ipanttrusteddomainsid' in options:
+ if is_set('ipanttrusteddomainsid'):
# Validate SID as the one of trusted domains
self.obj.validate_trusted_domain_sid(options['ipanttrusteddomainsid'])
@@ -441,10 +454,10 @@ class idrange_mod(LDAPUpdate):
# make sure we are working with updated attributes
rid_range_attributes = ['ipabaserid','ipasecondarybaserid','ipaidrangesize']
- updated_values = dict(zip(rid_range_attributes,[None]*3))
+ updated_values = dict()
for attr in rid_range_attributes:
- if attr in entry_attrs:
+ if is_set(attr):
updated_values[attr] = entry_attrs[attr]
else:
updated_values[attr] = int(old_attrs[attr][0])
@@ -455,7 +468,7 @@ class idrange_mod(LDAPUpdate):
updated_values['ipaidrangesize']):
raise errors.ValidationError(name='ID Range setup',
error=_("Primary rid range and secondary rid range"\
- " cannot overlap."))
+ " cannot overlap"))
old_base_id = int(old_attrs.get('ipabaseid', [0])[0])
old_range_size = int(old_attrs.get('ipaidrangesize', [0])[0])
diff --git a/tests/test_xmlrpc/test_range_plugin.py b/tests/test_xmlrpc/test_range_plugin.py
index 56898d1..310044f 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -298,7 +298,7 @@ class test_range(Declarative):
command=('idrange_mod', [testrange2],
dict(ipabaserid=(testrange2_base_rid*10))),
expected=errors.ValidationError(
- name='ID Range setup', error='Primary rid range and secondary rid range cannot overlap.'),
+ name='ID Range setup', error='Primary rid range and secondary rid range cannot overlap'),
),
dict(
@@ -353,7 +353,7 @@ class test_range(Declarative):
ipabaserid=testrange7_base_rid,
ipasecondarybaserid=testrange7_secondary_base_rid)),
expected=errors.ValidationError(
- name='ID Range setup', error='Primary rid range and secondary rid range cannot overlap.'),
+ name='ID Range setup', error='Primary rid range and secondary rid range cannot overlap'),
),
dict(
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
