On 05/10/2012 12:05 PM, Ondrej Hamada wrote:
On 05/09/2012 04:49 PM, Petr Viktorin wrote:
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.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Suggestion: you can use ipalib.utils.validate_hostname function with
check_fqdn param set to False. Sorry for not mentioning it before.
Otherwise ACK
Attached patch uses your suggestion. Thanks.
--
Petr³
From 3324c86b05f372d41766da6d3ca2ef0076d6ccea 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 | 17 ++++++--
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, 105 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 85a81723175f38f10c711530971f173a54f1150a..895ec682ac2ee1d6b57e48711e22c75cb5f05105 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -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_hostname(hostname, check_fqdn=False, 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..d51287bcd0174818126131c1bebcb558720a0cc7 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
