On 05/04/2012 01:25 PM, Ondrej Hamada wrote:
On 04/30/2012 02:13 PM, Petr Viktorin wrote:


Change the externalhost attribute of hbacrule, netgroup
and sudorule into a full-fledged Parameter, and attach
a validator to it.

RFC 1123 specifies that only [-a-z0-9] are allowed, but apparently
Windows and some phones also use underscores in hostnames.
So the new validator allows the underscore.


https://fedorahosted.org/freeipa/ticket/2649



_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
1) Current validation of external hostnames does not require them to be
fully qualified, but you do. It's inconsistent.

2) one test case failed:
FAIL: Test adding an invalid external host to Sudo rule using
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
self.test(*self.arg)
File "/home/ohamada/2649/tests/test_xmlrpc/test_sudorule_plugin.py",
line 500, in test_a_sudorule_mod_externalhost_invalid_addattr
"character")
AssertionError


Thanks. Attaching updated patch.


--
PetrĀ³
From de7c93062120a32f828fdc6aa82c0794b42aff26 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 30 Apr 2012 07:29:08 -0400
Subject: [PATCH] Validate externalhost (when added by --addattr/--setattr)

Change the externalhost attribute of hbacrule, netgroup
and sudorule into a full-fledged Parameter, and attach
a validator to it.
The validator is relaxed to allow underscores, so that
some hosts with nonstandard names can be added.

Tests included.

https://fedorahosted.org/freeipa/ticket/2649
---
 ipalib/plugins/baseldap.py                |   19 +++++++--
 ipalib/plugins/hbacrule.py                |    1 +
 ipalib/plugins/netgroup.py                |    1 +
 ipalib/plugins/sudorule.py                |    1 +
 tests/test_xmlrpc/test_hbac_plugin.py     |    9 +++++
 tests/test_xmlrpc/test_netgroup_plugin.py |   62 +++++++++++++++++++++++++++++
 tests/test_xmlrpc/test_sudorule_plugin.py |   17 ++++++++
 7 files changed, 106 insertions(+), 4 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 85a81723175f38f10c711530971f173a54f1150a..353613b2b3dd9eed9ecf8e3ea42bca9d4e2e0cdf 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -33,7 +33,7 @@
 from ipalib.cli import to_cli, from_cli
 from ipalib import output
 from ipalib.text import _
-from ipalib.util import json_serialize, validate_hostname
+from ipalib.util import json_serialize, validate_hostname, validate_dns_label
 from ipalib.dn import *
 
 global_output_params = (
@@ -157,9 +157,6 @@
     Str('memberofindirect_hbacrule?',
         label='Indirect Member of HBAC rule',
     ),
-    Str('externalhost?',
-        label=_('External host'),
-    ),
     Str('sourcehost',
         label=_('Failed source hosts/hostgroups'),
     ),
@@ -313,6 +310,20 @@ def wait_for_value(ldap, dn, attr, value):
 
     return entry_attrs
 
+
+def validate_externalhost(ugettext, hostname):
+    try:
+        validate_dns_label(hostname, allow_underscore=True)
+    except ValueError, e:
+        return unicode(e)
+
+
+external_host_param = Str('externalhost*', validate_externalhost,
+        label=_('External host'),
+        flags=['no_create', 'no_update', 'no_search'],
+)
+
+
 def add_external_pre_callback(membertype, ldap, dn, keys, options):
     """
     Pre callback to validate external members.
diff --git a/ipalib/plugins/hbacrule.py b/ipalib/plugins/hbacrule.py
index eb5cb696eb835c717d734d31ee2c6333ac177030..33440ccde9ef63df7d087d17f0d5d224c75833fa 100644
--- a/ipalib/plugins/hbacrule.py
+++ b/ipalib/plugins/hbacrule.py
@@ -219,6 +219,7 @@ class hbacrule(LDAPObject):
             label=_('Service Groups'),
             flags=['no_create', 'no_update', 'no_search'],
         ),
+        external_host_param,
     )
 
 api.register(hbacrule)
diff --git a/ipalib/plugins/netgroup.py b/ipalib/plugins/netgroup.py
index d2a78098018fe23653fdfdd17ad73b9245905992..4236feeb7e557cfc3891329971ece419c14ba685 100644
--- a/ipalib/plugins/netgroup.py
+++ b/ipalib/plugins/netgroup.py
@@ -146,6 +146,7 @@ class netgroup(LDAPObject):
             doc=_('Host category the rule applies to'),
             values=(u'all', ),
         ),
+        external_host_param,
     )
 
 api.register(netgroup)
diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py
index 7432bc42bbb945cd0e8d25f3e320987af7fef26b..2c0358e879fc203106731ce966ed697e85c4e84f 100644
--- a/ipalib/plugins/sudorule.py
+++ b/ipalib/plugins/sudorule.py
@@ -217,6 +217,7 @@ class sudorule(LDAPObject):
             doc=_('Run with the gid of a specified POSIX group'),
             flags=['no_create', 'no_update', 'no_search'],
         ),
+        external_host_param,
     )
 
     order_not_unique_msg = _(
diff --git a/tests/test_xmlrpc/test_hbac_plugin.py b/tests/test_xmlrpc/test_hbac_plugin.py
index c7cb55bad4309f05fc0d9651f9e97d37ffe866ae..5ecb9014deae302404656e95bbd7b2ffd282f799 100644
--- a/tests/test_xmlrpc/test_hbac_plugin.py
+++ b/tests/test_xmlrpc/test_hbac_plugin.py
@@ -377,6 +377,15 @@ def test_c_hbacrule_add_same_external(self):
         entry = ret['result']
         assert_attr_equal(entry, 'externalhost', self.test_host_external)
 
+    @raises(errors.ValidationError)
+    def test_c_hbacrule_mod_invalid_external_setattr(self):
+        """
+        Test adding the same external host using `xmlrpc.hbacrule_add_host`.
+        """
+        ret = api.Command['hbacrule_mod'](
+            self.rule_name, setattr=self.test_invalid_sourcehost
+        )
+
     def test_c_hbacrule_remove_external_host(self):
         """
         Test removing external source host using `xmlrpc.hbacrule_remove_host`.
diff --git a/tests/test_xmlrpc/test_netgroup_plugin.py b/tests/test_xmlrpc/test_netgroup_plugin.py
index 03d5b9fa374cf17f6d898d574326e6eec36c8d40..8bca0a6eb603ca930f5f9a626b5b8d06d8cd41ba 100644
--- a/tests/test_xmlrpc/test_netgroup_plugin.py
+++ b/tests/test_xmlrpc/test_netgroup_plugin.py
@@ -46,6 +46,8 @@
 
 unknown_host = u'unknown'
 
+unknown_host2 = u'unknown2'
+
 hostgroup1 = u'hg1'
 hostgroup_dn1 = DN(('cn',hostgroup1),('cn','hostgroups'),('cn','accounts'),
                    api.env.basedn)
@@ -829,6 +831,66 @@ class test_netgroup(Declarative):
         ),
 
         dict(
+            desc='Add invalid host %r to netgroup %r using setattr' %
+                (invalidhost, netgroup1),
+            command=(
+                'netgroup_mod', [netgroup1],
+                dict(setattr='externalhost=%s' % invalidhost)
+            ),
+            expected=errors.ValidationError(name='externalhost',
+                error='only letters, numbers, _, and - are allowed. ' +
+                    'DNS label may not start or end with -'),
+        ),
+
+        dict(
+            desc='Add unknown host %r to netgroup %r using addattr' %
+                (unknown_host2, netgroup1),
+            command=(
+                'netgroup_mod', [netgroup1],
+                dict(addattr='externalhost=%s' % unknown_host2)
+            ),
+            expected=dict(
+                value=u'netgroup1',
+                summary=u'Modified netgroup "netgroup1"',
+                result={
+                        'memberhost_host': (host1,),
+                        'memberhost_hostgroup': (hostgroup1,),
+                        'memberuser_user': (user1,),
+                        'memberuser_group': (group1,),
+                        'member_netgroup': (netgroup2,),
+                        'cn': [netgroup1],
+                        'description': [u'Test netgroup 1'],
+                        'nisdomainname': [u'%s' % api.env.domain],
+                        'externalhost': [unknown_host, unknown_host2],
+                },
+            )
+        ),
+
+        dict(
+            desc='Remove unknown host %r from netgroup %r using delattr' %
+                (unknown_host2, netgroup1),
+            command=(
+                'netgroup_mod', [netgroup1],
+                dict(delattr='externalhost=%s' % unknown_host2)
+            ),
+            expected=dict(
+                value=u'netgroup1',
+                summary=u'Modified netgroup "netgroup1"',
+                result={
+                        'memberhost_host': (host1,),
+                        'memberhost_hostgroup': (hostgroup1,),
+                        'memberuser_user': (user1,),
+                        'memberuser_group': (group1,),
+                        'member_netgroup': (netgroup2,),
+                        'cn': [netgroup1],
+                        'description': [u'Test netgroup 1'],
+                        'nisdomainname': [u'%s' % api.env.domain],
+                        'externalhost': [unknown_host],
+                },
+            )
+        ),
+
+        dict(
             desc='Retrieve %r' % netgroup1,
             command=('netgroup_show', [netgroup1], {}),
             expected=dict(
diff --git a/tests/test_xmlrpc/test_sudorule_plugin.py b/tests/test_xmlrpc/test_sudorule_plugin.py
index 6aabd2b278fb1c4ef9c87587d70f2b8996595415..f0e6cd34f56698dd12a3601737f78106aa9048e1 100644
--- a/tests/test_xmlrpc/test_sudorule_plugin.py
+++ b/tests/test_xmlrpc/test_sudorule_plugin.py
@@ -484,6 +484,23 @@ def test_a_sudorule_add_externalhost_invalid(self):
         else:
             assert False
 
+    def test_a_sudorule_mod_externalhost_invalid_addattr(self):
+        """
+        Test adding an invalid external host to Sudo rule using
+        `xmlrpc.sudorule_mod --addattr`.
+        """
+        try:
+            api.Command['sudorule_mod'](
+                self.rule_name,
+                addattr='externalhost=%s' % self.test_invalid_host
+            )
+        except errors.ValidationError, e:
+            assert unicode(e) == ("invalid 'externalhost': only letters, " +
+                "numbers, _, and - are allowed. " +
+                "DNS label may not start or end with -")
+        else:
+            assert False
+
     def test_b_sudorule_remove_externalhost(self):
         """
         Test removing an external host from Sudo rule using
-- 
1.7.10.1

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

Reply via email to