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

2 files changed, 103 insertions, 18 deletions

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 754708117..84e1057ac 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -277,6 +277,25 @@ 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,27 +334,39 @@ 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 (x is not 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 ' \
+                    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 ' \
+                    error=_('Options dom_sid and rid_base must '
                             'be used together'))
 
             # Validate SID as the one of trusted domains
             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 ' \
+                    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
@@ -377,7 +408,7 @@ class idrange_find(LDAPSearch):
         return (filters, base_dn, ldap.SCOPE_ONELEVEL)
 
     def post_callback(self, ldap, entries, truncated, *args, **options):
-        for dn,entry in entries:
+        for dn, entry in entries:
             self.obj.handle_iparangetype(entry, options)
         return truncated
 
@@ -403,15 +434,43 @@ class idrange_mod(LDAPUpdate):
         assert isinstance(dn, DN)
         attrs_list.append('objectclass')
 
+        is_set = lambda x: (x in entry_attrs) and (x is not 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 56c6db528..2e00609a7 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(