On 01/05/2012 05:40 PM, Alexander Bokovoy wrote:
On Thu, 05 Jan 2012, Ondrej Hamada wrote:
https://fedorahosted.org/freeipa/ticket/2085

New version of SSSD begins ignoring sourcehost value of HBAC rules by
default. In order to match this behaviour the sourcehost option in
hbactest is optional now, but the value of sourcehost is ignored in all
rules. Every rule's sourcehost value is set to 'ALL' what turns sourchost
value comparation off. For every such a rule warning is displayed to
inform the user about changes.

Also the unit tests for hbactest plugin were updated. Every test was
doubled. The second ones test the plugin without sourcehost option. They
are supposed to have the same result.
Looks OK for me. One possibility to enhance the tests is to verify
that sourcehost-based version also checks warning being set to
something other than None.

ACK
Done. I've also made one modification - warnings appear only when srchost option is used and unittests checks whether they've appeared. I've also updated the text in docstring to be mentioning srchost option as optional only.

--
Regards,

Ondrej Hamada
FreeIPA team
jabber: oh...@jabbim.cz
IRC: ohamada

From 65afccdfc3f4a663f541d26d39963e98e8139093 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada <oham...@redhat.com>
Date: Sat, 7 Jan 2012 20:17:25 +0100
Subject: [PATCH] HBAC test optional sourcehost option

New version of SSSD begins ignoring sourcehost value of HBAC rules by
default. In order to match this behaviour the sourcehost option in
hbactest is optional now, but the value of sourcehost is ignored in all
rules. Every rule's sourcehost value is set to 'ALL' what turns sourchost
value comparation off. If srchost option is used, warning is displayed to
inform the user about changes. Text of plugin help was also updated.

Also the unit tests for hbactest plugin were updated. Every test was
doubled. The second ones test the plugin without sourcehost option. They
are supposed to have the same result.

https://fedorahosted.org/freeipa/ticket/2085
---
 API.txt                                   |    5 +-
 VERSION                                   |    2 +-
 ipalib/plugins/hbactest.py                |   66 ++++++++++++++++-----------
 tests/test_xmlrpc/test_hbactest_plugin.py |   71 +++++++++++++++++++++++++++++
 4 files changed, 115 insertions(+), 29 deletions(-)

diff --git a/API.txt b/API.txt
index aba3d8aa0250113d137878c97903922ff14ee664..493d5a3ff235e2a6f2599896114365af54086d16 100644
--- a/API.txt
+++ b/API.txt
@@ -1455,9 +1455,9 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('value', <type 'unicode'>, None)
 command: hbactest
-args: 0,8,5
+args: 0,8,6
 option: Str('user', cli_name='user', primary_key=True)
-option: Str('sourcehost', cli_name='srchost')
+option: Str('sourcehost?', cli_name='srchost')
 option: Str('targethost', cli_name='host')
 option: Str('service', cli_name='service')
 option: Str('rules*', cli_name='rules', csv=True)
@@ -1465,6 +1465,7 @@ option: Flag('nodetail?', autofill=True, cli_name='nodetail', default=False)
 option: Flag('enabled?', autofill=True, cli_name='enabled', default=False)
 option: Flag('disabled?', autofill=True, cli_name='disabled', default=False)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
+output: Output('warning', (<type 'list'>, <type 'tuple'>, <type 'NoneType'>), None)
 output: Output('matched', (<type 'list'>, <type 'tuple'>, <type 'NoneType'>), None)
 output: Output('notmatched', (<type 'list'>, <type 'tuple'>, <type 'NoneType'>), None)
 output: Output('error', (<type 'list'>, <type 'tuple'>, <type 'NoneType'>), None)
diff --git a/VERSION b/VERSION
index 081643745c47e78e7739f2b1092be762acd14e5f..b6ef09fc5119d04cc54e031762fa3e21987d2867 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=18
+IPA_API_VERSION_MINOR=19
diff --git a/ipalib/plugins/hbactest.py b/ipalib/plugins/hbactest.py
index fbc3dbb2e15076665bf712ce94821d46bf52f6df..f1b608d21a69bbd57577455157d8e78e11a54733 100644
--- a/ipalib/plugins/hbactest.py
+++ b/ipalib/plugins/hbactest.py
@@ -28,20 +28,21 @@ __doc__ = _("""
 Simulate use of Host-based access controls
 
 HBAC rules control who can access what services on what hosts and from where.
-You can use HBAC to control which users or groups on a source host can
-access a service, or group of services, on a target host.
+You can use HBAC to control which users or groups can access a service,
+or group of services, on a target host.
 
 Since applying HBAC rules implies use of a production environment,
 this plugin aims to provide simulation of HBAC rules evaluation without
 having access to the production environment.
 
- Test user coming from source host to a service on a named host against
+ Test user coming to a service on a named host against
  existing enabled rules.
 
- ipa hbactest --user= --srchost= --host= --service=
+ ipa hbactest --user= --host= --service=
               [--rules=rules-list] [--nodetail] [--enabled] [--disabled]
+              [--srchost= ]
 
- --user, --srchost, --host, and --service are mandatory, others are optional.
+ --user, --host, and --service are mandatory, others are optional.
 
  If --rules is specified simulate enabling of the specified rules and test
  the login of the user using only these rules.
@@ -57,10 +58,12 @@ having access to the production environment.
 
  If no --rules specified, simulation is run against all IPA enabled rules.
 
+ If --srchost is specified, it will be ignored. It is left because of compatibility reasons only.
+
 EXAMPLES:
 
     1. Use all enabled HBAC rules in IPA database to simulate:
-    $ ipa  hbactest --user=a1a --srchost=foo --host=bar --service=sshd
+    $ ipa  hbactest --user=a1a --host=bar --service=sshd
     --------------------
     Access granted: True
     --------------------
@@ -70,13 +73,13 @@ EXAMPLES:
       matched: allow_all
 
     2. Disable detailed summary of how rules were applied:
-    $ ipa hbactest --user=a1a --srchost=foo --host=bar --service=sshd --nodetail
+    $ ipa hbactest --user=a1a --host=bar --service=sshd --nodetail
     --------------------
     Access granted: True
     --------------------
 
     3. Test explicitly specified HBAC rules:
-    $ ipa hbactest --user=a1a --srchost=foo --host=bar --service=sshd \
+    $ ipa hbactest --user=a1a --host=bar --service=sshd \
           --rules=my-second-rule,myrule
     ---------------------
     Access granted: False
@@ -85,7 +88,7 @@ EXAMPLES:
       notmatched: myrule
 
     4. Use all enabled HBAC rules in IPA database + explicitly specified rules:
-    $ ipa hbactest --user=a1a --srchost=foo --host=bar --service=sshd \
+    $ ipa hbactest --user=a1a --host=bar --service=sshd \
           --rules=my-second-rule,myrule --enabled
     --------------------
     Access granted: True
@@ -96,14 +99,14 @@ EXAMPLES:
       matched: allow_all
 
     5. Test all disabled HBAC rules in IPA database:
-    $ ipa hbactest --user=a1a --srchost=foo --host=bar --service=sshd --disabled
+    $ ipa hbactest --user=a1a --host=bar --service=sshd --disabled
     ---------------------
     Access granted: False
     ---------------------
       notmatched: new-rule
 
     6. Test all disabled HBAC rules in IPA database + explicitly specified rules:
-    $ ipa hbactest --user=a1a --srchost=foo --host=bar --service=sshd \
+    $ ipa hbactest --user=a1a --host=bar --service=sshd \
           --rules=my-second-rule,myrule --disabled
     ---------------------
     Access granted: False
@@ -113,7 +116,7 @@ EXAMPLES:
       notmatched: myrule
 
     7. Test all (enabled and disabled) HBAC rules in IPA database:
-    $ ipa hbactest --user=a1a --srchost=foo --host=bar --service=sshd \
+    $ ipa hbactest --user=a1a --host=bar --service=sshd \
           --enabled --disabled
     --------------------
     Access granted: True
@@ -139,8 +142,9 @@ def convert_to_ipa_rule(rule):
         )
     for element in structure:
         category = '%scategory' % (element[0])
-        if category in rule and rule[category][0] == u'all':
+        if (category in rule and rule[category][0] == u'all') or (element[0] == 'sourcehost'):
             # rule applies to all elements
+            # sourcehost is always set to 'all'
             element[4].category = set([pyhbac.HBAC_CATEGORY_ALL])
         else:
             # rule is about specific entities
@@ -162,6 +166,7 @@ class hbactest(Command):
 
     has_output = (
         output.summary,
+        output.Output('warning', (list, tuple, NoneType),   _('Warning')),
         output.Output('matched', (list, tuple, NoneType),   _('Matched rules')),
         output.Output('notmatched', (list, tuple, NoneType), _('Not matched rules')),
         output.Output('error', (list, tuple, NoneType), _('Non-existent or invalid rules')),
@@ -174,7 +179,7 @@ class hbactest(Command):
             label=_('User name'),
             primary_key=True,
         ),
-        Str('sourcehost',
+        Str('sourcehost?',
             cli_name='srchost',
             label=_('Source host'),
         ),
@@ -265,7 +270,7 @@ class hbactest(Command):
             # Error, unresolved rules are left in --rules
             return {'summary' : unicode(_(u'Unresolved rules in --rules')),
                     'error': testrules, 'matched': None, 'notmatched': None,
-                    'value' : False}
+                    'warning' : None, 'value' : False}
 
         # Rules are converted to pyhbac format, build request and then test it
         request = pyhbac.HbacRequest()
@@ -290,16 +295,20 @@ class hbactest(Command):
             except:
                 pass
 
-        if options['sourcehost'] != u'all':
-            try:
-                request.srchost.name = self.canonicalize(options['sourcehost'])
-                srchost_result = self.api.Command.host_show(request.srchost.name)['result']
-                groups = srchost_result['memberof_hostgroup']
-                if 'memberofindirect_hostgroup' in srchost_result:
-                    groups += search_result['memberofindirect_hostgroup']
-                request.srchost.groups = sorted(set(groups))
-            except:
-                 pass
+        if options.get('sourcehost'):
+            warning_flag = True
+            if options['sourcehost'] != u'all':
+                try:
+                    request.srchost.name = self.canonicalize(options['sourcehost'])
+                    srchost_result = self.api.Command.host_show(request.srchost.name)['result']
+                    groups = srchost_result['memberof_hostgroup']
+                    if 'memberofindirect_hostgroup' in srchost_result:
+                        groups += search_result['memberofindirect_hostgroup']
+                    request.srchost.groups = sorted(set(groups))
+                except:
+                     pass
+        else:
+            warning_flag = False
 
         if options['targethost'] != u'all':
             try:
@@ -315,8 +324,9 @@ class hbactest(Command):
         matched_rules = []
         notmatched_rules = []
         error_rules = []
+        warning_rules = []
 
-        result = {'matched':None, 'notmatched':None, 'error':None}
+        result = {'warning':None, 'matched':None, 'notmatched':None, 'error':None}
         if not options['nodetail']:
             # Validate runs rules one-by-one and reports failed ones
             for ipa_rule in rules:
@@ -326,6 +336,8 @@ class hbactest(Command):
                         matched_rules.append(ipa_rule.name)
                     if res == pyhbac.HBAC_EVAL_DENY:
                         notmatched_rules.append(ipa_rule.name)
+                    if warning_flag:
+                        warning_rules.append(u'Sourcehost value of rule "%s" is ignored' % (ipa_rule.name))
                 except pyhbac.HbacError as (code, rule_name):
                     if code == pyhbac.HBAC_EVAL_ERROR:
                         error_rules.append(rule_name)
@@ -348,6 +360,8 @@ class hbactest(Command):
             result['notmatched'] = notmatched_rules
         if len(error_rules) > 0:
             result['error'] = error_rules
+        if len(warning_rules) > 0:
+            result['warning'] = warning_rules
 
         result['value'] = access_granted
         return result
diff --git a/tests/test_xmlrpc/test_hbactest_plugin.py b/tests/test_xmlrpc/test_hbactest_plugin.py
index 7e4607c858dec9c08021e30c326b042b1831c4c7..7899d540615f80a3293b29a7ebf13ac14705001a 100644
--- a/tests/test_xmlrpc/test_hbactest_plugin.py
+++ b/tests/test_xmlrpc/test_hbactest_plugin.py
@@ -48,6 +48,13 @@ class test_hbactest(XMLRPC_test):
     test_sourcehostgroup = u'hbacrule_test_src_hostgroup'
     test_service = u'ssh'
 
+    # Auxiliary funcion for checking existence of warning for specified rule
+    def check_rule_presence(self,rule_name,warnings):
+        for warning in warnings:
+            if rule_name in warning:
+                return True
+        return False
+
     def test_0_hbactest_addrules(self):
         """
         Prepare data by adding test HBAC rules using `xmlrpc.hbacrule_add'.
@@ -114,6 +121,19 @@ class test_hbactest(XMLRPC_test):
         assert type(ret['error']) == NoneType
         for i in [0,1,2,3]:
             assert self.rule_names[i] in ret['matched']
+            assert self.rule_names[i] in ret['warning'][i]
+
+        # same test without sourcehost value
+        ret = api.Command['hbactest'](
+            user=self.test_user,
+            targethost=self.test_host,
+            service=self.test_service,
+            rules=self.rule_names
+        )
+        assert ret['value'] == True
+        assert type(ret['error']) == NoneType
+        for i in [0,1,2,3]:
+            assert self.rule_names[i] in ret['matched']
 
     def test_b_hbactest_check_rules_nodetail(self):
         """
@@ -131,6 +151,20 @@ class test_hbactest(XMLRPC_test):
         assert ret['error'] == None
         assert ret['matched'] == None
         assert ret['notmatched'] == None
+        assert ret['warning'] == None
+
+        # same test without sourcehost value
+        ret = api.Command['hbactest'](
+            user=self.test_user,
+            targethost=self.test_host,
+            service=self.test_service,
+            rules=self.rule_names,
+            nodetail=True
+        )
+        assert ret['value'] == True
+        assert ret['error'] == None
+        assert ret['matched'] == None
+        assert ret['notmatched'] == None
 
     def test_c_hbactest_check_rules_enabled_detail(self):
         """
@@ -148,6 +182,17 @@ class test_hbactest(XMLRPC_test):
         # Thus, check that our two enabled rules are in matched, nothing more
         for i in [0,2]:
             assert self.rule_names[i] in ret['matched']
+            assert self.check_rule_presence(self.rule_names[i], ret['warning'])
+
+        # same test without sourcehost value
+        ret = api.Command['hbactest'](
+            user=self.test_user,
+            targethost=self.test_host,
+            service=self.test_service,
+            enabled=True
+        )
+        for i in [0,2]:
+            assert self.rule_names[i] in ret['matched']
 
     def test_d_hbactest_check_rules_disabled_detail(self):
         """
@@ -165,6 +210,17 @@ class test_hbactest(XMLRPC_test):
         # Thus, check that our two disabled rules are in matched, nothing more
         for i in [1,3]:
             assert self.rule_names[i] in ret['matched']
+            assert self.check_rule_presence(self.rule_names[i], ret['warning'])
+
+        # same test without sourcehost value
+        ret = api.Command['hbactest'](
+            user=self.test_user,
+            targethost=self.test_host,
+            service=self.test_service,
+            disabled=True
+        )
+        for i in [1,3]:
+            assert self.rule_names[i] in ret['matched']
 
     def test_e_hbactest_check_non_existing_rule_detail(self):
         """
@@ -185,6 +241,21 @@ class test_hbactest(XMLRPC_test):
         for rule in self.rule_names:
             assert u'%s_1x1' % (rule) in ret['error']
 
+        # same test without sourcehost value
+        ret = api.Command['hbactest'](
+            user=self.test_user,
+            targethost=self.test_host,
+            service=self.test_service,
+            rules=[u'%s_1x1' % (rule) for rule in self.rule_names],
+            nodetail=True
+        )
+
+        assert ret['value'] == False
+        assert ret['matched'] == None
+        assert ret['notmatched'] == None
+        for rule in self.rule_names:
+            assert u'%s_1x1' % (rule) in ret['error']
+
     def test_f_hbactest_clear_testing_data(self):
         """
         Clear data for HBAC test plugin testing.
-- 
1.7.6.5

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

Reply via email to