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

@@ -364,7 +364,7 @@ class idrange_add(LDAPCreate):
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
 
-        is_set = lambda x: (x in entry_attrs) and (x is not None)
+        is_set = lambda x: (x in entry_attrs) and (entry_attrs[x] is not None)
 
         # This needs to stay in options since there is no
         # ipanttrusteddomainname attribute in LDAP
@@ -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,16 @@ 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, ['*'])
+        except errors.NotFound:
+            self.obj.handle_not_found(*keys)
+
+        is_set = lambda x: (x in entry_attrs) and (entry_attrs[x] is not None)
+        in_updated_attrs = lambda x:\
+            (x in entry_attrs and entry_attrs[x] is not None) or\
+            (x not in entry_attrs and x in old_attrs
+                and old_attrs[x] is not None)
 
         # This needs to stay in options since there is no
         # ipanttrusteddomainname attribute in LDAP
@@ -496,6 +507,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 +517,37 @@ class idrange_mod(LDAPUpdate):
                             'not be found. Please specify the SID directly '
                             'using dom-sid option.'))
 
-        try:
+        if in_updated_attrs('ipanttrusteddomainsid'):
-            (old_dn, old_attrs) = ldap.get_entry(dn,
+            if in_updated_attrs('ipasecondarybaserid'):
-                                                ['ipabaseid',
+                raise errors.ValidationError(name='ID Range setup',
-                                                'ipaidrangesize',
+                    error=_('Options dom-sid and secondary-rid-base cannot '
-                                                'ipabaserid',
+                            'be used together'))
-                                                'ipasecondarybaserid'])
+
-        except errors.NotFound:
+            if not in_updated_attrs('ipabaserid'):
-            self.obj.handle_not_found(*keys)
+                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
-            self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
+                # 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 overlap
-        if all((base in entry_attrs) or (base in old_attrs)
+        if all(in_updated_attrs(base)
                for base in ('ipabaserid', 'ipasecondarybaserid')):
 
             # make sure we are working with updated attributes
-            rid_range_attributes = ('ipabaserid', 'ipasecondarybaserid', 'ipaidrangesize')
+            rid_range_attributes = ('ipabaserid', 'ipasecondarybaserid',
+                                    'ipaidrangesize')
             updated_values = dict()
 
             for attr in rid_range_attributes:
@@ -539,14 +564,19 @@ class idrange_mod(LDAPUpdate):
                             error=_("Primary RID range and secondary RID range"
                                  " cannot overlap"))
 
+        # check whether ids are in modified range
         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')
+
         if new_base_id is not None:
             new_base_id = int(new_base_id)
+
         new_range_size = entry_attrs.get('ipaidrangesize')
+
         if new_range_size is not None:
             new_range_size = int(new_range_size)
+
         self.obj.check_ids_in_modified_range(old_base_id, old_range_size,
                                              new_base_id, new_range_size)
 

tests/test_xmlrpc/test_range_plugin.py

@@ -69,6 +69,11 @@ testrange7_size = 50
 testrange7_base_rid = 600
 testrange7_secondary_base_rid=649
 
+testrange8 = u'testrange8'
+testrange8_base_id = 700
+testrange8_size = 50
+testrange8_base_rid = 700
+
 user1=u'tuser1'
 user1_uid = 900000
 group1=u'group1'
@@ -76,7 +81,7 @@ group1_gid = 900100
 
 class test_range(Declarative):
     cleanup_commands = [
-        ('idrange_del', [testrange1,testrange2,testrange3,testrange4,testrange5,testrange6,testrange7], {'continue': True}),
+        ('idrange_del', [testrange1,testrange2,testrange3,testrange4,testrange5,testrange6,testrange7, testrange8], {'continue': True}),
         ('user_del', [user1], {}),
         ('group_del', [group1], {}),
     ]
@@ -365,4 +370,43 @@ class test_range(Declarative):
                 summary=u'Deleted ID range "%s"' % testrange2,
             ),
         ),
+
+        dict(
+            desc='Create ID range %r' % (testrange8),
+            command=('idrange_add', [testrange8],
+                      dict(ipabaseid=testrange8_base_id,
+                          ipaidrangesize=testrange8_size)),
+            expected=dict(
+                result=dict(
+                    dn=DN(('cn',testrange8),('cn','ranges'),('cn','etc'),
+                          api.env.basedn),
+                    cn=[testrange8],
+                    objectclass=[u'ipaIDrange', u'ipadomainidrange'],
+                    ipabaseid=[unicode(testrange8_base_id)],
+                    ipaidrangesize=[unicode(testrange8_size)],
+                    iparangetype=[u'local domain range'],
+                ),
+                value=testrange8,
+                summary=u'Added ID range "%s"' % (testrange8),
+            ),
+        ),
+
+        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'),
+        ),
+
+        dict(
+            desc='Delete ID range %r' % testrange8,
+            command=('idrange_del', [testrange8], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=testrange8,
+                summary=u'Deleted ID range "%s"' % testrange8,
+            ),
+        ),
+
     ]