Fix user.present detecting primary group being moved to "groups" arg

This commit is contained in:
Erik Johnson 2020-02-04 16:43:10 -06:00 committed by Daniel Wozniak
parent 84421e884b
commit fd8ed64b17
3 changed files with 55 additions and 7 deletions

View file

@ -112,12 +112,21 @@ def _changes(
if gid is not None and lusr["gid"] not in (gid, __salt__["file.group_to_gid"](gid)):
change["gid"] = gid
default_grp = __salt__["file.gid_to_group"](gid if gid is not None else lusr["gid"])
# remove the default group from the list for comparison purposes
old_default_grp = __salt__["file.gid_to_group"](lusr["gid"])
# Remove the default group from the list for comparison purposes.
if default_grp in lusr["groups"]:
lusr["groups"].remove(default_grp)
# If the group is being changed, make sure that the old primary group is
# also removed from the list. Otherwise, if a user's gid is being changed
# and their old primary group is reassigned as an additional group, Salt
# will not properly detect the need for the change.
if old_default_grp != default_grp and old_default_grp in lusr["groups"]:
lusr["groups"].remove(old_default_grp)
# If there's a group by the same name as the user, remove it from the list
# for comparison purposes.
if name in lusr["groups"] and name not in wanted_groups:
lusr["groups"].remove(name)
# remove default group from wanted_groups, as this requirement is
# Remove default group from wanted_groups, as this requirement is
# already met
if default_grp in wanted_groups:
wanted_groups.remove(default_grp)

View file

@ -56,6 +56,7 @@ class UserTest(ModuleCase, SaltReturnAssertsMixin):
"""
user_name = "salt-test"
alt_group = "salt-test-altgroup"
user_home = (
"/var/lib/{0}".format(user_name)
if not salt.utils.platform.is_windows()
@ -308,6 +309,44 @@ class UserTest(ModuleCase, SaltReturnAssertsMixin):
user_info = self.run_function("user.info", [self.user_name])
self.assertTrue(os.path.exists(user_info["home"]))
def test_user_present_change_gid_but_keep_group(self):
"""
This tests the case in which the default group is changed at the same
time as it is also moved into the "groups" list.
"""
# Add the groups
ret = self.run_state("group.present", name=self.user_name)
self.assertSaltTrueReturn(ret)
ret = self.run_state("group.present", name=self.alt_group)
self.assertSaltTrueReturn(ret)
# Add the user
ret = self.run_state("user.present", name=self.user_name, gid=self.alt_group,)
self.assertSaltTrueReturn(ret)
# Now change the gid and move alt_group to the groups list
ret = self.run_state(
"user.present",
name=self.user_name,
gid=self.user_name,
groups=[self.alt_group],
allow_gid_change=True,
)
self.assertSaltTrueReturn(ret)
# Be extra sure that we did what we intended
gid = self.run_function("file.group_to_gid", [self.user_name])
uinfo = self.run_function("user.info", [self.user_name])
assert uinfo["gid"] == gid, uinfo["gid"]
assert uinfo["groups"] == [self.user_name, self.alt_group], uinfo["groups"]
# Do the cleanup here so we don't have to put all of this in the
# tearDown to be executed after each test.
self.assertSaltTrueReturn(self.run_state("user.absent", name=self.user_name))
self.assertSaltTrueReturn(self.run_state("group.absent", name=self.user_name))
self.assertSaltTrueReturn(self.run_state("group.absent", name=self.user_name))
def tearDown(self):
if salt.utils.platform.is_darwin():
check_user = self.run_function("user.list_users")

View file

@ -113,7 +113,7 @@ class UserTestCase(TestCase, LoaderModuleMockMixin):
dunder_salt = {
"user.info": mock_info,
"file.group_to_gid": MagicMock(side_effect=["foo"]),
"file.gid_to_group": MagicMock(side_effect=[5000]),
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
}
# side_effect used because these mocks should only be called once
with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict(
@ -140,7 +140,7 @@ class UserTestCase(TestCase, LoaderModuleMockMixin):
dunder_salt = {
"user.info": mock_info,
"file.group_to_gid": MagicMock(side_effect=["foo"]),
"file.gid_to_group": MagicMock(side_effect=[5000]),
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
}
# side_effect used because these mocks should only be called once
with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict(
@ -167,7 +167,7 @@ class UserTestCase(TestCase, LoaderModuleMockMixin):
dunder_salt = {
"user.info": mock_info,
"file.group_to_gid": MagicMock(side_effect=["foo"]),
"file.gid_to_group": MagicMock(side_effect=[5000]),
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
}
# side_effect used because these mocks should only be called once
with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict(
@ -200,7 +200,7 @@ class UserTestCase(TestCase, LoaderModuleMockMixin):
# confirm that no changes still need to be made.
mock_info = MagicMock(side_effect=[before, before, after, after])
mock_group_to_gid = MagicMock(side_effect=["foo", "othergroup"])
mock_gid_to_group = MagicMock(side_effect=[5000, 5001])
mock_gid_to_group = MagicMock(side_effect=[5000, 5000, 5001, 5001])
dunder_salt = {
"user.info": mock_info,
"user.chuid": Mock(),
@ -271,7 +271,7 @@ class UserTestCase(TestCase, LoaderModuleMockMixin):
"shadow.info": shadow_info,
"shadow.default_hash": shadow_hash,
"file.group_to_gid": MagicMock(side_effect=["foo"]),
"file.gid_to_group": MagicMock(side_effect=[5000]),
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
}
def mock_exists(*args):