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=1200000 --range-size=200000 --rid-base=1000 
--secondary-rid-base=1000000 local_range
----------------------------
Added ID range "local_range"
----------------------------
   Range name: local_range
   First Posix ID of the range: 1200000
   Number of IDs in the range: 200000
   First RID of the corresponding RID range: 1000
   First RID of the secondary RID range: 1000000
   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 old_attrs['objectclass']:
+                entry_attrs['objectclass'].append('ipatrustedaddomainrange')
+
         else:
             # secondary base rid must be set if and only if base rid is set
             if in_updated_attrs('ipasecondarybaserid') !=\
-- 
1.7.11.7

>From 07ccc2320233e01b1cb74d873602f11dd765bd90 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              | 60 +++++++++++++++++++++++++---------
 tests/test_xmlrpc/test_range_plugin.py | 46 +++++++++++++++++++++++++-
 2 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 445473394223f3f7c69d1e32be71d7c6944e6252..8ca5f79f8ef6e767f6bf37dcfd044d8ff655975b 100644
--- a/ipalib/plugins/idrange.py
+++ b/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,14 @@ 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: any((x in attrs and attrs[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 overlap
-        if all((base in entry_attrs) or (base in old_attrs)
-                for base in ('ipabaserid', 'ipasecondarybaserid')):
+        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 +562,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)
 
diff --git a/tests/test_xmlrpc/test_range_plugin.py b/tests/test_xmlrpc/test_range_plugin.py
index 2e00609a7829a836f924ac04ff34d040bcb2a544..be8eac593a04c52aaaff61f980cfd5fd0899fabd 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/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,
+            ),
+        ),
+
     ]
-- 
1.7.11.7

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to