On 05/29/2013 12:25 PM, Martin Kosek wrote:
On 05/28/2013 03:48 PM, Alexander Bokovoy wrote:
On Tue, 28 May 2013, Tomas Babej wrote:
On 05/28/2013 02:35 PM, Alexander Bokovoy wrote:
On Mon, 27 May 2013, Tomas Babej wrote:
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.
So I tried to run this test on a machine where there is already trust
established and I think there should be done some changes.
I perused the log. Seems that the failures you're experiencing are not
relevant to the patch itself,
since the newly added tests passed.
This is problem with test_range_plugin.py tests that has been there for quite
a while, the parameters
of the ranges such as size, and base ID/RID/secondary RID are hardcoded in
the test case.
Yep.
Probably it would be wise to add pre-start procedure to pull existing
ranges and define constants for the ranges so that they don't overlap
with existing ones. Perhaps selecting something from a top of the range
space...
Attached is the log
I agree. This has not been relevant until now, since we did not do much
testing on IPA instances
with trusts set up, and even then there's random factor in having the overlap
with the already created
trust range.
I'd propose fixing this in a separate effort as a part of continouous
integration improvements. I see it
as a separate issue of its own.
What do you think?
Please file a separate ticket then.
ACK for this one.
May-be-NACK.
Would it make sense to replace the error with DependentEntry error? We use in
cases like this elsewhere and I think it makes more sense in this case too.
Martin
Sure, I changed the error class in idrange.py and tests accordingly.
I ran the unit tests again to verify the changes.
Here is the updated patch.
Tomas
From c0bcbc1b91c2a9d964d458054210477459f30a7b Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
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 | 19 ++++-
tests/test_xmlrpc/test_range_plugin.py | 144 ++++++++++++++++++++++++++++++---
2 files changed, 152 insertions(+), 11 deletions(-)
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 54f6fbb3e19b9aa01dfde2a8d0c5da4498632386..d548794428fbbc7981112d6c441c980fd9e06157 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -434,14 +434,31 @@ 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.DependentEntry(
+ label='Active Trust',
+ key=keys[0],
+ dependent=result['result'][0]['cn'][0])
+
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..ce70433112b3216304356b520026d79be66543cf 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -27,61 +27,166 @@ 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 = u'testtrust'
+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:
+ pass
+
+ 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 +514,23 @@ 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.DependentEntry(
+ label='Active Trust',
+ key=testrange10,
+ dependent=testtrust),
+ ),
+
]
--
1.8.1.4
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
