On 05/26/2013 08:56 PM, Alexander Bokovoy wrote:
On Fri, 24 May 2013, Tomas Babej wrote:
On 05/20/2013 04:29 PM, Alexander Bokovoy wrote:
On Mon, 20 May 2013, Tomas Babej wrote:
On 05/16/2013 11:16 AM, Ana Krivokapic wrote:
On 05/15/2013 03:41 PM, Tomas Babej wrote:
Hi,
When removing an ID range using idrange-del command, validation
in pre_callback ensures that the range does not belong to any
active trust. In such case, ValidationError is raised.
https://fedorahosted.org/freeipa/ticket/3615
Tomas
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
I suggest adding some unit tests to cover this change in
functionality.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
I incorporated the unit tests. It had to use direct access to LDAP
using ldapmodify since we need to create a mock AD trusted range
first.
Tomas
We got rid of openldap utilities now. While using python.ldap module,
I also made the tests much more robust and added a new test case.
In general patches look fine, there is one small nitpick.
I'll run tests on Monday and then will provide final ACK.
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -22,66 +22,171 @@ Test the `ipalib/plugins/idrange.py` module, and
XML-RPC in general.
"""
from ipalib import api, errors, _
+from ipapython.ipautil import run
This import is unused, can be removed.
Fixed, thanks for catching that.
Updated patch attached.
Tomas
From 3ef84ba0c664a43b7ae19ce22ac9d4e61e23a929 Mon Sep 17 00:00:00 2001
From: Tomas Babej <[email protected]>
Date: Wed, 15 May 2013 15:37:15 +0200
Subject: [PATCH] Do not allow removal of ID range of an active trust
When removing an ID range using idrange-del command, validation
in pre_callback ensures that the range does not belong to any
active trust. In such case, ValidationError is raised.
Unit tests to cover the functionality has been added.
https://fedorahosted.org/freeipa/ticket/3615
---
ipalib/plugins/idrange.py | 17 +++-
tests/test_xmlrpc/test_range_plugin.py | 142 ++++++++++++++++++++++++++++++---
2 files changed, 148 insertions(+), 11 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 54f6fbb3e19b9aa01dfde2a8d0c5da4498632386..a0309f82cc14117212c355547dac25b8c4e0f1e3 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -434,14 +434,29 @@ class idrange_del(LDAPDelete):
def pre_callback(self, ldap, dn, *keys, **options):
try:
- (old_dn, old_attrs) = ldap.get_entry(dn, ['ipabaseid', 'ipaidrangesize'])
+ (old_dn, old_attrs) = ldap.get_entry(dn, ['ipabaseid',
+ 'ipaidrangesize',
+ 'ipanttrusteddomainsid'])
except errors.NotFound:
self.obj.handle_not_found(*keys)
+ # Check whether we leave any object with id in deleted range
old_base_id = int(old_attrs.get('ipabaseid', [0])[0])
old_range_size = int(old_attrs.get('ipaidrangesize', [0])[0])
self.obj.check_ids_in_modified_range(
old_base_id, old_range_size, 0, 0)
+
+ # Check whether the range does not belong to the active trust
+ range_sid = old_attrs.get('ipanttrusteddomainsid')
+
+ if range_sid is not None:
+ range_sid = range_sid[0]
+ result = api.Command['trust_find'](ipanttrusteddomainsid=range_sid)
+
+ if result['count'] > 0:
+ raise errors.ValidationError(name='ID Range constraint',
+ error=_("ID range of an active trust cannot be deleted."))
+
return dn
class idrange_find(LDAPSearch):
diff --git a/tests/test_xmlrpc/test_range_plugin.py b/tests/test_xmlrpc/test_range_plugin.py
index be8eac593a04c52aaaff61f980cfd5fd0899fabd..7018f63e96a7659b822787874ec786b18e629d50 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -27,61 +27,165 @@ from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
from tests.test_xmlrpc import objectclasses
from ipapython.dn import *
+import ldap, ldap.sasl, ldap.modlist
+
testrange1 = u'testrange1'
testrange1_base_id = 900000
testrange1_size = 99999
testrange1_base_rid = 10000
-testrange1_secondary_base_rid=200000
+testrange1_secondary_base_rid = 200000
testrange2 = u'testrange2'
testrange2_base_id = 100
testrange2_size = 50
testrange2_base_rid = 100
-testrange2_secondary_base_rid=1000
+testrange2_secondary_base_rid = 1000
testrange3 = u'testrange3'
testrange3_base_id = 200
testrange3_size = 50
testrange3_base_rid = 70
-testrange3_secondary_base_rid=1100
+testrange3_secondary_base_rid = 1100
testrange4 = u'testrange4'
testrange4_base_id = 300
testrange4_size = 50
testrange4_base_rid = 200
-testrange4_secondary_base_rid=1030
+testrange4_secondary_base_rid = 1030
testrange5 = u'testrange5'
testrange5_base_id = 400
testrange5_size = 50
testrange5_base_rid = 1020
-testrange5_secondary_base_rid=1200
+testrange5_secondary_base_rid = 1200
testrange6 = u'testrange6'
testrange6_base_id = 130
testrange6_size = 50
testrange6_base_rid = 500
-testrange6_secondary_base_rid=1300
+testrange6_secondary_base_rid = 1300
testrange7 = u'testrange7'
testrange7_base_id = 600
testrange7_size = 50
testrange7_base_rid = 600
-testrange7_secondary_base_rid=649
+testrange7_secondary_base_rid = 649
testrange8 = u'testrange8'
testrange8_base_id = 700
testrange8_size = 50
testrange8_base_rid = 700
-user1=u'tuser1'
+testrange9 = u'testrange9'
+testrange9_base_id = 800
+testrange9_size = 50
+testrange9_base_rid = 800
+
+testrange10 = u'testrange10'
+testrange10_base_id = 900
+testrange10_size = 50
+testrange10_base_rid = 900
+
+testrange9_dn = "cn={name},cn=ranges,cn=etc,{basedn}".format(name=testrange9,
+ basedn=api.env.basedn)
+
+testrange9_add = dict(
+ objectClass=["ipaIDrange", "ipatrustedaddomainrange"],
+ ipaBaseID="{base_id}".format(base_id=testrange9_base_id),
+ ipaBaseRID="{base_rid}".format(base_rid=testrange9_base_rid),
+ ipaIDRangeSize="{size}".format(size=testrange9_size),
+ ipaNTTrustedDomainSID="S-1-5-21-259319770-2312917334-591429603",
+ )
+
+testrange10_dn = "cn={name},cn=ranges,cn=etc,{basedn}".format(name=testrange10,
+ basedn=api.env.basedn)
+
+testrange10_add = dict(
+ objectClass=["ipaIDrange", "ipatrustedaddomainrange"],
+ ipaBaseID="{base_id}".format(base_id=testrange10_base_id),
+ ipaBaseRID="{base_rid}".format(base_rid=testrange10_base_rid),
+ ipaIDRangeSize="{size}".format(size=testrange10_size),
+ ipaNTTrustedDomainSID="S-1-5-21-2997650941-1802118864-3094776726",
+ )
+
+testtrust_dn = "cn=testtrust,cn=trusts,{basedn}".format(basedn=api.env.basedn)
+
+testtrust_add = dict(
+ objectClass=["ipaNTTrustedDomain", "ipaIDobject", "top"],
+ ipaNTFlatName='TESTTRUST',
+ ipaNTTrustedDomainSID='S-1-5-21-2997650941-1802118864-3094776726',
+ ipaNTSIDBlacklistIncoming='S-1-0',
+ ipaNTTrustPartner='testtrust.mock',
+ ipaNTTrustType='2',
+ ipaNTTrustDirection='3',
+ ipaNTTrustAttributes='8',
+ )
+
+user1 = u'tuser1'
user1_uid = 900000
-group1=u'group1'
+group1 = u'group1'
group1_gid = 900100
+
class test_range(Declarative):
+
+ def __init__(self):
+ super(test_range, self).__init__()
+ self.connection = None
+
+ @classmethod
+ def connect_ldap(self):
+ self.connection = ldap.initialize('ldap://{host}'
+ .format(host=api.env.host))
+
+ auth = ldap.sasl.gssapi("")
+ self.connection.sasl_interactive_bind_s('', auth)
+
+ @classmethod
+ def add_entry(self, dn, mods):
+ ldif = ldap.modlist.addModlist(mods)
+ self.connection.add_s(dn, ldif)
+
+ @classmethod
+ def setUpClass(self):
+ super(test_range, self).setUpClass()
+
+ self.tearDownClass()
+
+ try:
+ self.connect_ldap()
+
+ self.add_entry(testrange9_dn, testrange9_add)
+ self.add_entry(testrange10_dn, testrange10_add)
+ self.add_entry(testtrust_dn, testtrust_add)
+
+ except ldap.ALREADY_EXISTS:
+ trusted_ad_range_ok = True
+
+ finally:
+ if self.connection is not None:
+ self.connection.unbind_s()
+
+ @classmethod
+ def tearDownClass(self):
+
+ try:
+ self.connect_ldap()
+ self.connection.delete_s(testrange9_dn)
+ self.connection.delete_s(testrange10_dn)
+ self.connection.delete_s(testtrust_dn)
+
+ except ldap.NO_SUCH_OBJECT:
+ pass
+
+ finally:
+ if self.connection is not None:
+ self.connection.unbind_s()
+
cleanup_commands = [
- ('idrange_del', [testrange1,testrange2,testrange3,testrange4,testrange5,testrange6,testrange7, testrange8], {'continue': True}),
+ ('idrange_del', [testrange1, testrange2, testrange3, testrange4,
+ testrange5, testrange6, testrange7, testrange8],
+ {'continue': True}),
('user_del', [user1], {}),
('group_del', [group1], {}),
]
@@ -409,4 +513,22 @@ class test_range(Declarative):
),
),
+ dict(
+ desc='Delete non-active AD trusted range %r' % testrange9,
+ command=('idrange_del', [testrange9], {}),
+ expected=dict(
+ result=dict(failed=u''),
+ value=testrange9,
+ summary=u'Deleted ID range "%s"' % testrange9,
+ ),
+ ),
+
+ dict(
+ desc='Try to delete active AD trusted range %r' % testrange10,
+ command=('idrange_del', [testrange10], {}),
+ expected=errors.ValidationError(
+ name='ID Range constraint',
+ error='ID range of an active trust cannot be deleted.'),
+ ),
+
]
--
1.8.1.4
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
