From 45fedde80677ea7fcbda773060d54f6129b622c8 Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Mon, 12 Sep 2022 11:36:49 -0400 Subject: [PATCH] fixes saltstack/salt#62657 groupadd.* functions hard code relative command name --- changelog/62657.fixed | 1 + salt/modules/groupadd.py | 52 ++-- tests/pytests/unit/modules/test_groupadd.py | 271 ++++++++++++++++++++ tests/unit/modules/test_groupadd.py | 259 ------------------- 4 files changed, 306 insertions(+), 277 deletions(-) create mode 100644 changelog/62657.fixed create mode 100644 tests/pytests/unit/modules/test_groupadd.py delete mode 100644 tests/unit/modules/test_groupadd.py diff --git a/changelog/62657.fixed b/changelog/62657.fixed new file mode 100644 index 00000000000..d6e83556c0e --- /dev/null +++ b/changelog/62657.fixed @@ -0,0 +1 @@ +Fix groupadd.* functions hard code relative command name diff --git a/salt/modules/groupadd.py b/salt/modules/groupadd.py index d2279f6f50a..6c5d587aadd 100644 --- a/salt/modules/groupadd.py +++ b/salt/modules/groupadd.py @@ -13,7 +13,9 @@ import logging import os import salt.utils.files +import salt.utils.path import salt.utils.stringutils +from salt.exceptions import CommandExecutionError try: import grp @@ -40,6 +42,16 @@ def __virtual__(): ) +def _which(cmd): + """ + Utility function wrapper to error out early if a command is not found + """ + _cmd = salt.utils.path.which(cmd) + if not _cmd: + raise CommandExecutionError("Command '{}' cannot be found".format(cmd)) + return _cmd + + def add(name, gid=None, system=False, root=None, non_unique=False): """ .. versionchanged:: 3006.0 @@ -69,7 +81,7 @@ def add(name, gid=None, system=False, root=None, non_unique=False): salt '*' group.add foo 3456 """ - cmd = ["groupadd"] + cmd = [_which("groupadd")] if gid: cmd.append("-g {}".format(gid)) if non_unique: @@ -103,7 +115,7 @@ def delete(name, root=None): salt '*' group.delete foo """ - cmd = ["groupdel"] + cmd = [_which("groupdel")] if root is not None: cmd.extend(("-R", root)) @@ -198,7 +210,7 @@ def _chattrib(name, key, value, param, root=None): if value == pre_info[key]: return True - cmd = ["groupmod"] + cmd = [_which("groupmod")] if root is not None: cmd.extend(("-R", root)) @@ -274,15 +286,15 @@ def adduser(name, username, root=None): if __grains__["kernel"] == "Linux": if on_redhat_5: - cmd = ["gpasswd", "-a", username, name] + cmd = [_which("gpasswd"), "-a", username, name] elif on_suse_11: - cmd = ["usermod", "-A", name, username] + cmd = [_which("usermod"), "-A", name, username] else: - cmd = ["gpasswd", "--add", username, name] + cmd = [_which("gpasswd"), "--add", username, name] if root is not None: cmd.extend(("--root", root)) else: - cmd = ["usermod", "-G", name, username] + cmd = [_which("usermod"), "-G", name, username] if root is not None: cmd.extend(("-R", root)) @@ -327,11 +339,11 @@ def deluser(name, username, root=None): if username in grp_info["members"]: if __grains__["kernel"] == "Linux": if on_redhat_5: - cmd = ["gpasswd", "-d", username, name] + cmd = [_which("gpasswd"), "-d", username, name] elif on_suse_11: - cmd = ["usermod", "-R", name, username] + cmd = [_which("usermod"), "-R", name, username] else: - cmd = ["gpasswd", "--del", username, name] + cmd = [_which("gpasswd"), "--del", username, name] if root is not None: cmd.extend(("--root", root)) retcode = __salt__["cmd.retcode"](cmd, python_shell=False) @@ -339,7 +351,7 @@ def deluser(name, username, root=None): out = __salt__["cmd.run_stdout"]( "id -Gn {}".format(username), python_shell=False ) - cmd = ["usermod", "-S"] + cmd = [_which("usermod"), "-S"] cmd.append(",".join([g for g in out.split() if g != str(name)])) cmd.append("{}".format(username)) retcode = __salt__["cmd.retcode"](cmd, python_shell=False) @@ -386,15 +398,16 @@ def members(name, members_list, root=None): if __grains__["kernel"] == "Linux": if on_redhat_5: - cmd = ["gpasswd", "-M", members_list, name] + cmd = [_which("gpasswd"), "-M", members_list, name] elif on_suse_11: for old_member in __salt__["group.info"](name).get("members"): __salt__["cmd.run"]( - "groupmod -R {} {}".format(old_member, name), python_shell=False + "{} -R {} {}".format(_which("groupmod"), old_member, name), + python_shell=False, ) - cmd = ["groupmod", "-A", members_list, name] + cmd = [_which("groupmod"), "-A", members_list, name] else: - cmd = ["gpasswd", "--members", members_list, name] + cmd = [_which("gpasswd"), "--members", members_list, name] if root is not None: cmd.extend(("--root", root)) retcode = __salt__["cmd.retcode"](cmd, python_shell=False) @@ -402,14 +415,17 @@ def members(name, members_list, root=None): retcode = 1 grp_info = __salt__["group.info"](name) if grp_info and name in grp_info["name"]: - __salt__["cmd.run"]("groupdel {}".format(name), python_shell=False) __salt__["cmd.run"]( - "groupadd -g {} {}".format(grp_info["gid"], name), python_shell=False + "{} {}".format(_which("groupdel"), name), python_shell=False + ) + __salt__["cmd.run"]( + "{} -g {} {}".format(_which("groupadd"), grp_info["gid"], name), + python_shell=False, ) for user in members_list.split(","): if user: retcode = __salt__["cmd.retcode"]( - ["usermod", "-G", name, user], python_shell=False + [_which("usermod"), "-G", name, user], python_shell=False ) if not retcode == 0: break diff --git a/tests/pytests/unit/modules/test_groupadd.py b/tests/pytests/unit/modules/test_groupadd.py new file mode 100644 index 00000000000..d9b5d5a6993 --- /dev/null +++ b/tests/pytests/unit/modules/test_groupadd.py @@ -0,0 +1,271 @@ +import pytest + +import salt.modules.groupadd as groupadd +from tests.support.mock import MagicMock, patch + +try: + import grp +except ImportError: + pass + + +pytestmark = [ + pytest.mark.skip_on_windows, +] + + +@pytest.fixture +def configure_loader_modules(): + return {groupadd: {}} + + +def test_add(): + """ + Tests if specified group was added + """ + mock = MagicMock(return_value={"retcode": 0}) + with patch.dict(groupadd.__salt__, {"cmd.run_all": mock}): + assert groupadd.add("test", 100) is True + + with patch.dict(groupadd.__grains__, {"kernel": "Linux"}): + with patch.dict(groupadd.__salt__, {"cmd.run_all": mock}): + assert groupadd.add("test", 100, True) is True + + +def test_info(): + """ + Tests the return of group information + """ + getgrnam = grp.struct_group(("foo", "*", 20, ["test"])) + with patch("grp.getgrnam", MagicMock(return_value=getgrnam)): + ret = {"passwd": "*", "gid": 20, "name": "foo", "members": ["test"]} + assert groupadd.info("foo") == ret + + +def test_format_info(): + """ + Tests the formatting of returned group information + """ + group = {"passwd": "*", "gid": 0, "name": "test", "members": ["root"]} + with patch("salt.modules.groupadd._format_info", MagicMock(return_value=group)): + data = grp.struct_group(("wheel", "*", 0, ["root"])) + ret = {"passwd": "*", "gid": 0, "name": "test", "members": ["root"]} + assert groupadd._format_info(data) == ret + + +def test_getent(): + """ + Tests the return of information on all groups + """ + getgrnam = grp.struct_group(("foo", "*", 20, ["test"])) + with patch("grp.getgrall", MagicMock(return_value=[getgrnam])): + ret = [{"passwd": "*", "gid": 20, "name": "foo", "members": ["test"]}] + assert groupadd.getent() == ret + + +def test_chgid_gid_same(): + """ + Tests if the group id is the same as argument + """ + mock = MagicMock(return_value={"gid": 10}) + with patch.object(groupadd, "info", mock): + assert groupadd.chgid("test", 10) is True + + +def test_chgid(): + """ + Tests the gid for a named group was changed + """ + mock = MagicMock(return_value=None) + with patch.dict(groupadd.__salt__, {"cmd.run": mock}): + mock = MagicMock(side_effect=[{"gid": 10}, {"gid": 500}]) + with patch.object(groupadd, "info", mock): + assert groupadd.chgid("test", 500) is True + + +def test_delete(): + """ + Tests if the specified group was deleted + """ + mock_ret = MagicMock(return_value={"retcode": 0}) + with patch.dict(groupadd.__salt__, {"cmd.run_all": mock_ret}): + assert groupadd.delete("test") is True + + +def test_adduser(): + """ + Tests if specified user gets added in the group. + """ + os_version_list = [ + { + "grains": { + "kernel": "Linux", + "os_family": "RedHat", + "osmajorrelease": "5", + }, + "cmd": ["/bin/gpasswd", "-a", "root", "test"], + }, + { + "grains": { + "kernel": "Linux", + "os_family": "Suse", + "osmajorrelease": "11", + }, + "cmd": ["/bin/usermod", "-A", "test", "root"], + }, + { + "grains": {"kernel": "Linux"}, + "cmd": ["/bin/gpasswd", "--add", "root", "test"], + }, + { + "grains": {"kernel": "OTHERKERNEL"}, + "cmd": ["/bin/usermod", "-G", "test", "root"], + }, + ] + with patch( + "salt.utils.path.which", + MagicMock( + side_effect=["/bin/gpasswd", "/bin/usermod", "/bin/gpasswd", "/bin/usermod"] + ), + ): + for os_version in os_version_list: + mock = MagicMock(return_value={"retcode": 0}) + with patch.dict(groupadd.__grains__, os_version["grains"]), patch.dict( + groupadd.__salt__, {"cmd.retcode": mock} + ): + assert groupadd.adduser("test", "root") is False + groupadd.__salt__["cmd.retcode"].assert_called_once_with( + os_version["cmd"], python_shell=False + ) + + +def test_deluser(): + """ + Tests if specified user gets deleted from the group. + """ + os_version_list = [ + { + "grains": { + "kernel": "Linux", + "os_family": "RedHat", + "osmajorrelease": "5", + }, + "cmd": ["/bin/gpasswd", "-d", "root", "test"], + }, + { + "grains": { + "kernel": "Linux", + "os_family": "Suse", + "osmajorrelease": "11", + }, + "cmd": ["/bin/usermod", "-R", "test", "root"], + }, + { + "grains": {"kernel": "Linux"}, + "cmd": ["/bin/gpasswd", "--del", "root", "test"], + }, + {"grains": {"kernel": "OpenBSD"}, "cmd": ["/bin/usermod", "-S", "foo", "root"]}, + ] + + with patch( + "salt.utils.path.which", + MagicMock( + side_effect=["/bin/gpasswd", "/bin/usermod", "/bin/gpasswd", "/bin/usermod"] + ), + ): + for os_version in os_version_list: + mock_retcode = MagicMock(return_value=0) + mock_stdout = MagicMock(return_value="test foo") + mock_info = MagicMock( + return_value={ + "passwd": "*", + "gid": 0, + "name": "test", + "members": ["root"], + } + ) + + with patch.dict(groupadd.__grains__, os_version["grains"]): + with patch.dict( + groupadd.__salt__, + { + "cmd.retcode": mock_retcode, + "group.info": mock_info, + "cmd.run_stdout": mock_stdout, + }, + ): + assert groupadd.deluser("test", "root") is True + groupadd.__salt__["cmd.retcode"].assert_called_once_with( + os_version["cmd"], python_shell=False + ) + + +def test_members(): + """ + Tests if members of the group, get replaced with a provided list. + """ + os_version_list = [ + { + "grains": { + "kernel": "Linux", + "os_family": "RedHat", + "osmajorrelease": "5", + }, + "cmd": ["/bin/gpasswd", "-M", "foo", "test"], + }, + { + "grains": { + "kernel": "Linux", + "os_family": "Suse", + "osmajorrelease": "11", + }, + "cmd": ["/bin/groupmod", "-A", "foo", "test"], + }, + { + "grains": {"kernel": "Linux"}, + "cmd": ["/bin/gpasswd", "--members", "foo", "test"], + }, + {"grains": {"kernel": "OpenBSD"}, "cmd": ["/bin/usermod", "-G", "test", "foo"]}, + ] + + with patch( + "salt.utils.path.which", + MagicMock( + side_effect=[ + "/bin/gpasswd", + "/bin/gpasswd", + "/bin/groupmod", + "/bin/gpasswd", + "/bin/groupdel", + "/bin/groupadd", + "/bin/usermod", + ] + ), + ): + for os_version in os_version_list: + mock_ret = MagicMock(return_value={"retcode": 0}) + mock_stdout = MagicMock(return_value={"cmd.run_stdout": 1}) + mock_info = MagicMock( + return_value={ + "passwd": "*", + "gid": 0, + "name": "test", + "members": ["root"], + } + ) + mock = MagicMock(return_value=True) + + with patch.dict(groupadd.__grains__, os_version["grains"]): + with patch.dict( + groupadd.__salt__, + { + "cmd.retcode": mock_ret, + "group.info": mock_info, + "cmd.run_stdout": mock_stdout, + "cmd.run": mock, + }, + ): + assert groupadd.members("test", "foo") is False + groupadd.__salt__["cmd.retcode"].assert_called_once_with( + os_version["cmd"], python_shell=False + ) diff --git a/tests/unit/modules/test_groupadd.py b/tests/unit/modules/test_groupadd.py deleted file mode 100644 index 1428ef521de..00000000000 --- a/tests/unit/modules/test_groupadd.py +++ /dev/null @@ -1,259 +0,0 @@ -""" - :codeauthor: Jayesh Kariya -""" - - -import salt.modules.groupadd as groupadd -import salt.utils.platform -from tests.support.mixins import LoaderModuleMockMixin -from tests.support.mock import MagicMock, patch -from tests.support.unit import TestCase, skipIf - -try: - import grp -except ImportError: - pass - - -@skipIf(salt.utils.platform.is_windows(), "Module not available on Windows") -class GroupAddTestCase(TestCase, LoaderModuleMockMixin): - """ - TestCase for salt.modules.groupadd - """ - - def setup_loader_modules(self): - return {groupadd: {}} - - # 'add' function tests: 1 - - def test_add(self): - """ - Tests if specified group was added - """ - mock = MagicMock(return_value={"retcode": 0}) - with patch.dict(groupadd.__salt__, {"cmd.run_all": mock}): - self.assertTrue(groupadd.add("test", 100)) - - with patch.dict(groupadd.__grains__, {"kernel": "Linux"}): - with patch.dict(groupadd.__salt__, {"cmd.run_all": mock}): - self.assertTrue(groupadd.add("test", 100, True)) - - # 'info' function tests: 1 - - def test_info(self): - """ - Tests the return of group information - """ - getgrnam = grp.struct_group(("foo", "*", 20, ["test"])) - with patch("grp.getgrnam", MagicMock(return_value=getgrnam)): - ret = {"passwd": "*", "gid": 20, "name": "foo", "members": ["test"]} - self.assertEqual(groupadd.info("foo"), ret) - - # '_format_info' function tests: 1 - - def test_format_info(self): - """ - Tests the formatting of returned group information - """ - group = {"passwd": "*", "gid": 0, "name": "test", "members": ["root"]} - with patch("salt.modules.groupadd._format_info", MagicMock(return_value=group)): - data = grp.struct_group(("wheel", "*", 0, ["root"])) - ret = {"passwd": "*", "gid": 0, "name": "test", "members": ["root"]} - self.assertDictEqual(groupadd._format_info(data), ret) - - # 'getent' function tests: 1 - - def test_getent(self): - """ - Tests the return of information on all groups - """ - getgrnam = grp.struct_group(("foo", "*", 20, ["test"])) - with patch("grp.getgrall", MagicMock(return_value=[getgrnam])): - ret = [{"passwd": "*", "gid": 20, "name": "foo", "members": ["test"]}] - self.assertEqual(groupadd.getent(), ret) - - # 'chgid' function tests: 2 - - def test_chgid_gid_same(self): - """ - Tests if the group id is the same as argument - """ - mock = MagicMock(return_value={"gid": 10}) - with patch.object(groupadd, "info", mock): - self.assertTrue(groupadd.chgid("test", 10)) - - def test_chgid(self): - """ - Tests the gid for a named group was changed - """ - mock = MagicMock(return_value=None) - with patch.dict(groupadd.__salt__, {"cmd.run": mock}): - mock = MagicMock(side_effect=[{"gid": 10}, {"gid": 500}]) - with patch.object(groupadd, "info", mock): - self.assertTrue(groupadd.chgid("test", 500)) - - # 'delete' function tests: 1 - - def test_delete(self): - """ - Tests if the specified group was deleted - """ - mock_ret = MagicMock(return_value={"retcode": 0}) - with patch.dict(groupadd.__salt__, {"cmd.run_all": mock_ret}): - self.assertTrue(groupadd.delete("test")) - - # 'adduser' function tests: 1 - - def test_adduser(self): - """ - Tests if specified user gets added in the group. - """ - os_version_list = [ - { - "grains": { - "kernel": "Linux", - "os_family": "RedHat", - "osmajorrelease": "5", - }, - "cmd": ["gpasswd", "-a", "root", "test"], - }, - { - "grains": { - "kernel": "Linux", - "os_family": "Suse", - "osmajorrelease": "11", - }, - "cmd": ["usermod", "-A", "test", "root"], - }, - { - "grains": {"kernel": "Linux"}, - "cmd": ["gpasswd", "--add", "root", "test"], - }, - { - "grains": {"kernel": "OTHERKERNEL"}, - "cmd": ["usermod", "-G", "test", "root"], - }, - ] - - for os_version in os_version_list: - mock = MagicMock(return_value={"retcode": 0}) - with patch.dict(groupadd.__grains__, os_version["grains"]): - with patch.dict(groupadd.__salt__, {"cmd.retcode": mock}): - self.assertFalse(groupadd.adduser("test", "root")) - groupadd.__salt__["cmd.retcode"].assert_called_once_with( - os_version["cmd"], python_shell=False - ) - - # 'deluser' function tests: 1 - - def test_deluser(self): - """ - Tests if specified user gets deleted from the group. - """ - os_version_list = [ - { - "grains": { - "kernel": "Linux", - "os_family": "RedHat", - "osmajorrelease": "5", - }, - "cmd": ["gpasswd", "-d", "root", "test"], - }, - { - "grains": { - "kernel": "Linux", - "os_family": "Suse", - "osmajorrelease": "11", - }, - "cmd": ["usermod", "-R", "test", "root"], - }, - { - "grains": {"kernel": "Linux"}, - "cmd": ["gpasswd", "--del", "root", "test"], - }, - {"grains": {"kernel": "OpenBSD"}, "cmd": ["usermod", "-S", "foo", "root"]}, - ] - - for os_version in os_version_list: - mock_retcode = MagicMock(return_value=0) - mock_stdout = MagicMock(return_value="test foo") - mock_info = MagicMock( - return_value={ - "passwd": "*", - "gid": 0, - "name": "test", - "members": ["root"], - } - ) - - with patch.dict(groupadd.__grains__, os_version["grains"]): - with patch.dict( - groupadd.__salt__, - { - "cmd.retcode": mock_retcode, - "group.info": mock_info, - "cmd.run_stdout": mock_stdout, - }, - ): - self.assertTrue(groupadd.deluser("test", "root")) - groupadd.__salt__["cmd.retcode"].assert_called_once_with( - os_version["cmd"], python_shell=False - ) - - # 'deluser' function tests: 1 - - def test_members(self): - """ - Tests if members of the group, get replaced with a provided list. - """ - os_version_list = [ - { - "grains": { - "kernel": "Linux", - "os_family": "RedHat", - "osmajorrelease": "5", - }, - "cmd": ["gpasswd", "-M", "foo", "test"], - }, - { - "grains": { - "kernel": "Linux", - "os_family": "Suse", - "osmajorrelease": "11", - }, - "cmd": ["groupmod", "-A", "foo", "test"], - }, - { - "grains": {"kernel": "Linux"}, - "cmd": ["gpasswd", "--members", "foo", "test"], - }, - {"grains": {"kernel": "OpenBSD"}, "cmd": ["usermod", "-G", "test", "foo"]}, - ] - - for os_version in os_version_list: - mock_ret = MagicMock(return_value={"retcode": 0}) - mock_stdout = MagicMock(return_value={"cmd.run_stdout": 1}) - mock_info = MagicMock( - return_value={ - "passwd": "*", - "gid": 0, - "name": "test", - "members": ["root"], - } - ) - mock = MagicMock(return_value=True) - - with patch.dict(groupadd.__grains__, os_version["grains"]): - with patch.dict( - groupadd.__salt__, - { - "cmd.retcode": mock_ret, - "group.info": mock_info, - "cmd.run_stdout": mock_stdout, - "cmd.run": mock, - }, - ): - self.assertFalse(groupadd.members("test", "foo")) - groupadd.__salt__["cmd.retcode"].assert_called_once_with( - os_version["cmd"], python_shell=False - )