URL: https://github.com/freeipa/freeipa/pull/6031
Author: rcritten
Title: #6031: Improve sudooption docs, make the option multi-value
Action: opened
PR body:
"""
I don't know why this wasn't always multi-value but if one wanted
to set multiple options they needed to call add-option multiple
times. The LDAP attribute is already multi-value.
This shouldn't cause API issues as it understood the attribute as
multi-value just didn't expose it. Client output on the CLI will
look a bit different:
Added option "('one', 'two')" to Sudo Rule "test"
or
Added option "(u'one', u'Two')" to Sudo Rule "test"
instead of with this change:
Added option "one,two" to Sudo Rule "test"
https://pagure.io/freeipa/issue/2278
Signed-off-by: Rob Crittenden <rcrit...@redhat.com>
"""
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/6031/head:pr6031
git checkout pr6031
From 1f860a3d82fc66dd165fbd5fd52a279a4ef013dc Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Mon, 27 Sep 2021 16:54:05 -0400
Subject: [PATCH] Improve sudooption docs, make the option multi-value
I don't know why this wasn't always multi-value but if one wanted
to set multiple options they needed to call add-option multiple
times. The LDAP attribute is already multi-value.
This shouldn't cause API issues as it understood the attribute as
multi-value just didn't expose it. Client output on the CLI will
look a bit different:
Added option "('one', 'two')" to Sudo Rule "test"
or
Added option "(u'one', u'Two')" to Sudo Rule "test"
instead of with this change:
Added option "one,two" to Sudo Rule "test"
https://pagure.io/freeipa/issue/2278
Signed-off-by: Rob Crittenden <rcrit...@redhat.com>
---
ipaclient/plugins/sudorule.py | 3 ++-
ipaserver/plugins/sudorule.py | 26 ++++++++++++++------
ipatests/test_xmlrpc/test_sudorule_plugin.py | 25 +++++++++++++++++++
3 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/ipaclient/plugins/sudorule.py b/ipaclient/plugins/sudorule.py
index a87628044cb..9e377fc2e57 100644
--- a/ipaclient/plugins/sudorule.py
+++ b/ipaclient/plugins/sudorule.py
@@ -41,7 +41,8 @@ class sudorule_add_option(MethodOverride):
def output_for_cli(self, textui, result, cn, **options):
textui.print_dashed(
_('Added option "%(option)s" to Sudo Rule "%(rule)s"')
- % dict(option=options['ipasudoopt'], rule=cn))
+ % dict(option=','.join(options['ipasudoopt']), rule=cn)
+ )
super(sudorule_add_option, self).output_for_cli(textui, result, cn,
**options)
diff --git a/ipaserver/plugins/sudorule.py b/ipaserver/plugins/sudorule.py
index 68806571552..b5fc9bcab0c 100644
--- a/ipaserver/plugins/sudorule.py
+++ b/ipaserver/plugins/sudorule.py
@@ -57,6 +57,11 @@
RunAsGroup: The group(s) whose gid rights Sudo will be invoked with.
Options: The various Sudoers Options that can modify Sudo's behavior.
""") + _("""
+Each option needs to be added separately and no validation is done whether
+the option is known by sudo or is in a valid format. Environment variables
+also need to be set individually. For example env_keep="FOO BAR" in sudoers
+needs be represented as --sudooption env_keep=FOO --sudooption env_keep+=BAR.
+""") + _("""
An order can be added to a sudorule to control the order in which they
are evaluated (if the client supports it). This order is an integer and
must be unique.
@@ -89,6 +94,10 @@
""") + _("""
Set a default Sudo option:
ipa sudorule-add-option defaults --sudooption '!authenticate'
+""") + _("""
+ Set multiple default Sudo options:
+ ipa sudorule-add-option defaults --sudooption '!authenticate' \
+ --sudooption mail_badpass
""") + _("""
Set SELinux type and role transitions on a rule:
ipa sudorule-add-option sysadmin_sudo --sudooption type=unconfined_t
@@ -353,7 +362,7 @@ class sudorule(LDAPObject):
label=_('RunAs External Group'),
doc=_('External Group the commands can run as (sudorule-find only)'),
),
- Str('ipasudoopt?',
+ Str('ipasudoopt*',
label=_('Sudo Option'),
flags=['no_create', 'no_update', 'no_search'],
),
@@ -989,7 +998,7 @@ class sudorule_add_option(LDAPQuery):
has_output = output.standard_entry
takes_options = (
- Str('ipasudoopt',
+ Str('ipasudoopt+',
cli_name='sudooption',
label=_('Sudo Option'),
),
@@ -1000,16 +1009,17 @@ def execute(self, cn, **options):
dn = self.obj.get_dn(cn)
- if not options['ipasudoopt'].strip():
+ if len(options.get('ipasudoopt',[])) == 0:
raise errors.EmptyModlist()
entry_attrs = ldap.get_entry(dn, ['ipasudoopt'])
try:
- if options['ipasudoopt'] not in entry_attrs['ipasudoopt']:
- entry_attrs.setdefault('ipasudoopt', []).append(
- options['ipasudoopt'])
- else:
- raise errors.DuplicateEntry
+ entry_attrs.setdefault('ipasudoopt', [])
+ for option in options['ipasudoopt']:
+ if option not in entry_attrs['ipasudoopt']:
+ entry_attrs['ipasudoopt'].append(option)
+ else:
+ raise errors.DuplicateEntry
except KeyError:
entry_attrs.setdefault('ipasudoopt', []).append(
options['ipasudoopt'])
diff --git a/ipatests/test_xmlrpc/test_sudorule_plugin.py b/ipatests/test_xmlrpc/test_sudorule_plugin.py
index 5cdb27f085a..c3ff76796cc 100644
--- a/ipatests/test_xmlrpc/test_sudorule_plugin.py
+++ b/ipatests/test_xmlrpc/test_sudorule_plugin.py
@@ -25,6 +25,7 @@
import six
from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, assert_attr_equal
+from ipatests.test_xmlrpc.xmlrpc_test import assert_deepequal
from ipalib import api
from ipalib import errors
@@ -61,6 +62,7 @@ class test_sudorule(XMLRPC_test):
test_runasgroup = u'manager'
test_category = u'all'
test_option = u'authenticate'
+ test_option2 = u'fqdn'
test_invalid_user = u'+invalid#user'
test_invalid_host = u'+invalid&host.nonexist.com'
@@ -426,6 +428,29 @@ def test_b_sudorule_remove_option(self):
)
assert ret['completed'] == 2
+ def test_c_sudorule_add_multiple_options(self):
+ """
+ Test adding two options to Sudo rule using
+ `xmlrpc.sudorule_add_option`.
+ """
+ ret = api.Command['sudorule_add_option'](
+ self.rule_name, ipasudoopt=(self.test_option, self.test_option2)
+ )
+ entry = ret['result']
+ assert_deepequal(entry.get('ipasudoopt'),
+ (self.test_option, self.test_option2))
+
+ def test_d_sudorule_remove_one_option(self):
+ """
+ Test removing an option from Sudo rule using
+ `xmlrpc.sudorule_remove_option'.
+ """
+ ret = api.Command['sudorule_remove_option'](
+ self.rule_name, ipasudoopt=self.test_option
+ )
+ entry = ret['result']
+ assert_deepequal(entry.get('ipasudoopt'), (self.test_option2,))
+
def test_a_sudorule_add_host(self):
"""
Test adding host and hostgroup to Sudo rule using
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
