On 06/06/2013 04:04 PM, Tomas Babej wrote:
> On 05/31/2013 07:35 PM, Ana Krivokapic wrote:
>> On 05/28/2013 04:49 PM, Ana Krivokapic wrote:
>>> Hello,
>>>
>>> This patch addresses https://fedorahosted.org/freeipa/ticket/3634
>>>
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel@redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> This updated patch applies on top of tbabej's patches 0053-0055.
>>
>> As suggested by Tomás(
>> (https://www.redhat.com/archives/freeipa-devel/2013-May/msg00352.html), I
>> refactored support of "mock" LDAP objects to tests/util, and modified
>> test_range_plugin and test_cli to use it.
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> I looked thoroughly at the issue here..
>
> The ticket is a little bit confusing about that, but you need to require
> primary/secondary rid base for the range after ipa-adtrust-install has been 
> run.
>
> Currently, the way your patch works, the bases are required only if at least
> one trust exists.
>
> [root@vm-002 labtool]# ipa-adtrust-install
>
> The log file for this installation can be found in 
> /var/log/ipaserver-install.log
> [snip]
> Setup complete
> [snip]
>
> [root@vm-002 labtool]# ipa idrange-add local
> First Posix ID of the range: 10
> Number of IDs in the range: 20
> ----------------------
> Added ID range "local"
> ----------------------
>   Range name: local
>   First Posix ID of the range: 10
>   Number of IDs in the range: 20
>   Range type: local domain range
>
> After adding the trust, everything works ok:
>
> [root@vm-002 labtool]# ipa trust-find
> ---------------
> 1 trust matched
> ---------------
>   Realm name: test
>   Domain NetBIOS name: TEST
>   Domain Security Identifier: S-1-5-21-259319770-2312917334-591429603
>   Trust type: Active Directory domain
>
> [root@vm-002 labtool]# ipa idrange-add local
> First Posix ID of the range: 10
> Number of IDs in the range: 10
> First RID of the corresponding RID range: 10
> First RID of the secondary RID range: 20
> ----------------------
> Added ID range "local"
> ----------------------
>   Range name: local
>   First Posix ID of the range: 10
>   Number of IDs in the range: 10
>   First RID of the corresponding RID range: 10
>   First RID of the secondary RID range: 20
>   Range type: local domain range
>
> We should require for primary/secondary rid base after ipa-adtrust-install has
> been run even if no trust is established.
>
> Tomas

This patch introduces a new command which can be used to determine if
ipa-adtrust-install has been run on the system.

Tests have been amended accordingly.

This patch applies on top of tbabej's patches 70 & 71.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From 3f1801dbb29f80d8f4c523363b010278e062fbf8 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Mon, 10 Jun 2013 18:57:08 -0400
Subject: [PATCH] Require rid-base and secondary-rid-base in idrange-add after
 ipa-adtrust-install

Add a new API command 'adtrust_install_run', which can be used to determine
whether ipa-adtrust-install has been run on the system. This new command is not
visible in IPA CLI.

Use this command in idrange_add to conditionally require rid-base and
secondary-rid-base options.

Add tests to cover the new functionality

https://fedorahosted.org/freeipa/ticket/3634
---
 API.txt                                |  4 ++
 VERSION                                |  2 +-
 ipalib/plugins/idrange.py              | 35 +++++++++++++-
 ipalib/plugins/trust.py                | 36 ++++++++++++--
 tests/test_cmdline/test_cli.py         | 82 ++++++++++++++++++++++++++++++++
 tests/test_xmlrpc/test_range_plugin.py | 85 ++++++++++------------------------
 tests/util.py                          | 37 ++++++++++++---
 7 files changed, 208 insertions(+), 73 deletions(-)

diff --git a/API.txt b/API.txt
index 1313460de66d8e12fc7a068cda0cf30658bcdd1b..f1498a45b1229eedb5319ae91f9e66bfd5c96574 100644
--- a/API.txt
+++ b/API.txt
@@ -101,6 +101,10 @@ command: aci_show
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('value', <type 'unicode'>, None)
+command: adtrust_install_run
+args: 0,1,1
+option: Str('version?', exclude='webui')
+output: Output('result', None, None)
 command: automember_add
 args: 1,7,3
 arg: Str('cn', cli_name='automember_rule')
diff --git a/VERSION b/VERSION
index a95ccb91457c4caf9767843951b8290b15b377d6..c713b18b7ce42db7fb290912ec6028342015f142 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=59
+IPA_API_VERSION_MINOR=60
diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 54b835e244fb60ee212a9c00223d4294ff8f4363..16caf067e93d75225c9465ebe924fff9bb6dc12e 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -356,7 +356,7 @@ class idrange_add(LDAPCreate):
 
     may be given for a new ID range for the local domain while
 
-        --rid-bas
+        --rid-base
         --dom-sid
 
     must be given to add a new range for a trusted AD domain.
@@ -381,6 +381,9 @@ def interactive_prompt_callback(self, kw):
 
         Also ensure that secondary-rid-base is prompted for when rid-base is
         specified and vice versa, in case that dom-sid was not specified.
+
+        Also ensure that rid-base and secondary-rid-base is prompted for
+        if ipa-adtrust-install has been run on the system.
         """
 
         # dom-sid can be specified using dom-sid or dom-name options
@@ -410,6 +413,22 @@ def interactive_prompt_callback(self, kw):
                 value = self.prompt_param(self.params['ipabaserid'])
                 kw.update(dict(ipabaserid=value))
 
+        # Prompt for rid-base and secondary-rid-base if ipa-adtrust-install
+        # has been run on the system
+        adtrust_install_run = api.Command['adtrust_install_run']()['result']
+
+        if adtrust_install_run:
+            rid_base = kw.get('ipabaserid', None)
+            secondary_rid_base = kw.get('ipasecondarybaserid', None)
+
+            if rid_base is None:
+                value = self.prompt_param(self.params['ipabaserid'])
+                kw.update(dict(ipabaserid=value))
+
+            if secondary_rid_base is None:
+                value = self.prompt_param(self.params['ipasecondarybaserid'])
+                kw.update(dict(ipasecondarybaserid=value))
+
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
 
@@ -495,6 +514,20 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
                             error=_("Primary RID range and secondary RID range"
                                     " cannot overlap"))
 
+            # rid-base and secondary-rid-base must be set if
+            # ipa-adtrust-install has been run on the system
+            adtrust_install_run = api.Command['adtrust_install_run']()['result']
+
+            if adtrust_install_run and not (
+                    is_set('ipabaserid') and is_set('ipasecondarybaserid')):
+                raise errors.ValidationError(
+                    name='ID Range setup',
+                    error=_(
+                        'You must specify both rid-base and '
+                        'secondary-rid-base options, because '
+                        'ipa-adtrust-install has already been run.'
+                    )
+                )
         return dn
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 3cb0ed98005ae5bd11b39f8ae01c9470d1bfc9c4..72866447bef7b9a71fbbfb5c4316c91c037801c6 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -20,12 +20,9 @@
 
 from ipalib.plugins.baseldap import *
 from ipalib.plugins.dns import dns_container_exists
-from ipalib import api, Str, StrEnum, Password, DefaultFrom, _, ngettext, Object
-from ipalib.parameters import Enum
+from ipalib import api, Str, StrEnum, Password, _, ngettext
 from ipalib import Command
 from ipalib import errors
-from ipapython import ipautil
-from ipalib import util
 try:
     import pysss_murmur #pylint: disable=F0401
     _murmur_installed = True
@@ -764,3 +761,34 @@ def execute(self, *keys, **options):
         return dict(result=result)
 
 api.register(trust_resolve)
+
+
+class adtrust_install_run(Command):
+    NO_CLI = True
+
+    __doc__ = _('Determine whether ipa-adtrust-install has been run on this '
+                'system')
+
+    def execute(self, *keys, **options):
+        ldap = self.api.Backend.ldap2
+        cifs = DN(api.env.container_service, api.env.basedn)
+        agents = DN(
+            ('cn', 'adtrust agents'),
+            ('cn', 'sysaccounts'),
+            ('cn', 'etc'),
+            api.env.basedn
+        )
+
+        try:
+            ldap.get_entries(
+                cifs,
+                ldap.SCOPE_ONELEVEL,
+                "(&(krbprincipalname=cifs/*@%s)(memberof=%s))" %
+                (api.env.realm, agents)
+            )
+        except errors.NotFound:
+            return dict(result=False)
+
+        return dict(result=True)
+
+api.register(adtrust_install_run)
diff --git a/tests/test_cmdline/test_cli.py b/tests/test_cmdline/test_cli.py
index bd1281e1d682b055ede9685a10a9cec91a3c76fd..2765a70b983cd8242299b942e7635f37c1796bc5 100644
--- a/tests/test_cmdline/test_cli.py
+++ b/tests/test_cmdline/test_cli.py
@@ -325,3 +325,85 @@ def test_dnszone_add(self):
             force=False,
             version=API_VERSION
         )
+
+    def test_idrange_add(self):
+        """
+        Test idrange-add with interative prompt
+        """
+        def test_with_interactive_input():
+            with self.fake_stdin('5\n500000\n'):
+                self.check_command(
+                    'idrange_add range1 --base-id=1 --range-size=1',
+                    'idrange_add',
+                    cn=u'range1',
+                    ipabaseid=u'1',
+                    ipaidrangesize=u'1',
+                    ipabaserid=5,
+                    ipasecondarybaserid=500000,
+                    all=False,
+                    raw=False,
+                    version=API_VERSION
+                )
+
+        def test_with_command_line_options():
+            self.check_command(
+                'idrange_add range1 --base-id=1 --range-size=1 '
+                '--rid-base=5 --secondary-rid-base=500000',
+                'idrange_add',
+                cn=u'range1',
+                ipabaseid=u'1',
+                ipaidrangesize=u'1',
+                ipabaserid=u'5',
+                ipasecondarybaserid=u'500000',
+                all=False,
+                raw=False,
+                version=API_VERSION
+            )
+
+        def test_without_options():
+            self.check_command(
+                'idrange_add range1 --base-id=1 --range-size=1',
+                'idrange_add',
+                cn=u'range1',
+                ipabaseid=u'1',
+                ipaidrangesize=u'1',
+                all=False,
+                raw=False,
+                version=API_VERSION
+            )
+
+        host, realm = str(api.env.host), str(api.env.realm)
+        cifs_dn = 'krbprincipalname=cifs/%s@%s,cn=services,cn=accounts,%s' % \
+                  (host, realm, api.env.basedn)
+        adtrust_install_run = api.Command['adtrust_install_run']()['result']
+        mockldap = None
+
+        if not adtrust_install_run:
+            # ipa-adtrust-install not run - no need to pass rid-base
+            # and secondary-rid-base
+            test_without_options()
+
+            # Create a mock service object to test against
+            cifs_add = dict(
+                krbprincipalname='cifs/%s@%s' % (host, realm),
+                ipakrbprincipalalias='cifs/%s@%s' % (host, realm),
+                memberof='cn=adtrust agents,cn=sysaccounts,cn=etc,%s'
+                         % api.env.basedn,
+                managedby='fqdn=%s,cn=computers,cn=accounts,%s'
+                          % (host, api.env.basedn),
+                objectclass=['krbprincipal', 'krbprincipalaux',
+                             'krbticketpolicyaux', 'ipaobject', 'ipaservice',
+                             'pkiuser', 'ipakrbprincipal', 'top']
+            )
+
+            mockldap = util.MockLDAP()
+            mockldap.add_entry(cifs_dn, cifs_add)
+
+        # Pass rid-base and secondary-rid-base interactively
+        test_with_interactive_input()
+
+        # Pass rid-base and secondary-rid-base on the command-line
+        test_with_command_line_options()
+
+        if not adtrust_install_run:
+            mockldap.del_entry(cifs_dn)
diff --git a/tests/test_xmlrpc/test_range_plugin.py b/tests/test_xmlrpc/test_range_plugin.py
index f0f881ab914b8c8ff84737be607cd709cceee377..22badd02f6f558a881915e28a887556c047865d2 100644
--- a/tests/test_xmlrpc/test_range_plugin.py
+++ b/tests/test_xmlrpc/test_range_plugin.py
@@ -21,13 +21,12 @@
 Test the `ipalib/plugins/idrange.py` module, and XML-RPC in general.
 """
 
-from ipalib import api, errors, _
-from tests.util import assert_equal, Fuzzy
-from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
+from ipalib import api, errors
+from xmlrpc_test import Declarative, fuzzy_uuid
 from tests.test_xmlrpc import objectclasses
-from ipapython.dn import *
+from tests.util import MockLDAP
+from ipapython.dn import DN
 
-import ldap, ldap.sasl, ldap.modlist
 
 id_shift = 0
 rid_shift = 0
@@ -99,6 +98,7 @@
 testrange8_base_id = id_shift + 700
 testrange8_size = 50
 testrange8_base_rid = rid_shift + 700
+testrange8_secondary_base_rid = 800
 
 testrange9 = u'testrange9'
 testrange9_base_id = id_shift + 800
@@ -155,59 +155,26 @@
 
 
 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)
+    def setUpClass(cls):
+        super(test_range, cls).setUpClass()
+        cls.tearDownClass()
+        cls.mockldap = MockLDAP()
+        cls.mockldap.add_entry(testrange9_dn, testrange9_add)
+        cls.mockldap.add_entry(testrange10_dn, testrange10_add)
+        cls.mockldap.add_entry(testtrust_dn, testtrust_add)
+        cls.mockldap.unbind()
 
     @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()
+    def tearDownClass(cls):
+        cls.mockldap = MockLDAP()
+        cls.mockldap.del_entry(testrange9_dn)
+        cls.mockldap.del_entry(testrange10_dn)
+        cls.mockldap.del_entry(testtrust_dn)
+        cls.mockldap.unbind()
 
     cleanup_commands = [
         ('idrange_del', [testrange1, testrange2, testrange3, testrange4,
@@ -508,7 +475,9 @@ def tearDownClass(self):
             desc='Create ID range %r' % (testrange8),
             command=('idrange_add', [testrange8],
                       dict(ipabaseid=testrange8_base_id,
-                          ipaidrangesize=testrange8_size)),
+                          ipaidrangesize=testrange8_size,
+                          ipabaserid=testrange8_base_rid,
+                          ipasecondarybaserid=testrange8_secondary_base_rid)),
             expected=dict(
                 result=dict(
                     dn=DN(('cn',testrange8),('cn','ranges'),('cn','etc'),
@@ -518,6 +487,8 @@ def tearDownClass(self):
                     ipabaseid=[unicode(testrange8_base_id)],
                     ipaidrangesize=[unicode(testrange8_size)],
                     iparangetype=[u'local domain range'],
+                    ipabaserid=[unicode(testrange8_base_rid)],
+                    ipasecondarybaserid=[unicode(testrange8_secondary_base_rid)]
                 ),
                 value=testrange8,
                 summary=u'Added ID range "%s"' % (testrange8),
@@ -525,14 +496,6 @@ def tearDownClass(self):
         ),
 
         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(
diff --git a/tests/util.py b/tests/util.py
index 117d2c834563dcdc09799d4ce4baa1349390e111..30fafdc76c3a139d954c6d7a5ad4c05360fe6de6 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -24,6 +24,9 @@
 import inspect
 import os
 from os import path
+import ldap
+import ldap.sasl
+import ldap.modlist
 import tempfile
 import shutil
 import re
@@ -32,6 +35,7 @@
 from ipalib.request import context
 from ipapython.dn import DN
 
+
 class TempDir(object):
     def __init__(self):
         self.__path = tempfile.mkdtemp(prefix='ipa.tests.')
@@ -451,12 +455,6 @@ def tearDown(self):
         context.__dict__.clear()
 
 
-
-
-
-
-
-
 def check_TypeError(value, type_, name, callback, *args, **kw):
     """
     Tests a standard TypeError raised with `errors.raise_TypeError`.
@@ -635,3 +633,30 @@ def __process(self, name_, args_, kw_):
 
     def _calledall(self):
         return self.__i == len(self.__calls)
+
+
+class MockLDAP(object):
+    def __init__(self):
+        self.connection = ldap.initialize(
+            'ldap://{host}'.format(host=ipalib.api.env.host)
+        )
+
+        auth = ldap.sasl.gssapi('')
+        self.connection.sasl_interactive_bind_s('', auth)
+
+    def add_entry(self, dn, mods):
+        try:
+            ldif = ldap.modlist.addModlist(mods)
+            self.connection.add_s(dn, ldif)
+        except ldap.ALREADY_EXISTS:
+            pass
+
+    def del_entry(self, dn):
+        try:
+            self.connection.delete_s(dn)
+        except ldap.NO_SUCH_OBJECT:
+            pass
+
+    def unbind(self):
+        if self.connection is not None:
+            self.connection.unbind_s()
-- 
1.8.1.4

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

Reply via email to