saltstack/salt
1
0
Fork 0
mirror of https://github.com/saltstack/salt.git synced 2025-04-16 09:40:20 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

fixes saltstack/salt#64430 regression for user.present on handling groups with dupe GIDs

Browse source
This commit is contained in:
nicholasmhughes 2023-06-06 16:18:09 -04:00 • committed by Megan Wilhite
parent 0ffbb22b44
commit 2e4c57edaa
3 changed files with 110 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
changelog/64430.fixed.md Normal file
View file

@ -0,0 +1 @@
Fix regression for user.present on handling groups with dupe GIDs

75
salt/states/user.py
View file

@ -40,11 +40,29 @@ def _group_changes(cur, wanted, remove=False):
"""
Determine if the groups need to be changed
"""
old = set(cur)
new = set(wanted)
if (remove and old != new) or (not remove and not new.issubset(old)):
return True
return False
cur = set(cur)
wanted = set(wanted)
if cur == wanted or (not remove and wanted.issubset(cur)):
return False
all_grps = {name: __salt__["group.info"](name) for name in cur.union(wanted)}
if remove:
diff = wanted.symmetric_difference(cur)
else:
diff = wanted.difference(cur)
remain = list(diff)
for diff_grp in diff:
for grp, info in all_grps.items():
if grp == diff_grp:
continue
if all_grps[diff_grp]["gid"] == info["gid"]:
# dupe detected
remain.remove(diff_grp)
return bool(remain)
def _changes(
@ -100,6 +118,15 @@ def _changes(
change = {}
wanted_groups = sorted(set((groups or []) + (optional_groups or [])))
lusr_groups_gids = [
__salt__["file.group_to_gid"](gname) for gname in lusr["groups"]
]
dupe_groups = {}
for idx, _gid in enumerate(lusr_groups_gids):
if lusr_groups_gids.count(_gid) > 1:
if _gid not in dupe_groups:
dupe_groups[_gid] = []
dupe_groups[_gid].append(lusr["groups"][idx])
if not remove_groups:
wanted_groups = sorted(set(wanted_groups + lusr["groups"]))
if uid and lusr["uid"] != uid:
@ -109,24 +136,44 @@ def _changes(
default_grp = __salt__["file.gid_to_group"](gid if gid is not None else lusr["gid"])
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)
# Remove default group from wanted_groups, as this requirement is
# already met
if default_grp in lusr["groups"] or default_grp in wanted_groups:
if default_grp in salt.utils.data.flatten(dupe_groups.values()):
dupe_gid = __salt__["file.group_to_gid"](default_grp)
for gname in dupe_groups[dupe_gid]:
if gname in lusr["groups"]:
lusr["groups"].remove(gname)
if gname in wanted_groups:
wanted_groups.remove(gname)
else:
if default_grp in lusr["groups"]:
lusr["groups"].remove(default_grp)
if default_grp in wanted_groups:
wanted_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 old_default_grp in salt.utils.data.flatten(dupe_groups.values()):
dupe_gid = __salt__["file.group_to_gid"](old_default_grp)
for gname in dupe_groups[dupe_gid]:
lusr["groups"].remove(gname)
else:
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
# already met
if default_grp in wanted_groups:
wanted_groups.remove(default_grp)
if name in salt.utils.data.flatten(dupe_groups.values()):
dupe_gid = __salt__["file.group_to_gid"](name)
for gname in dupe_groups[dupe_gid]:
lusr["groups"].remove(gname)
else:
lusr["groups"].remove(name)
if _group_changes(lusr["groups"], wanted_groups, remove_groups):
change["groups"] = wanted_groups
if wanted_groups or remove_groups:
change["groups"] = wanted_groups
if home and lusr["home"] != home:
change["home"] = home
if createhome:

57
tests/pytests/unit/states/test_user.py
View file

@ -123,8 +123,8 @@ def test_present_invalid_gid_change():
)
dunder_salt = {
"user.info": mock_info,
"file.group_to_gid": MagicMock(side_effect=["foo"]),
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
"file.group_to_gid": MagicMock(return_value="foo"),
"file.gid_to_group": MagicMock(return_value=5000),
}
with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict(
user.__salt__, dunder_salt
@ -148,8 +148,8 @@ def test_present_invalid_uid_gid_change():
)
dunder_salt = {
"user.info": mock_info,
"file.group_to_gid": MagicMock(side_effect=["foo"]),
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
"file.group_to_gid": MagicMock(return_value="foo"),
"file.gid_to_group": MagicMock(return_value=5000),
}
with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict(
user.__salt__, dunder_salt
@ -179,7 +179,7 @@ def test_present_uid_gid_change():
# get the before/after for the changes dict, and one last time to
# 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=[5000, 5001])
mock_group_to_gid = MagicMock(side_effect=[5000, 5000, 5001, 5001])
mock_gid_to_group = MagicMock(
side_effect=["othergroup", "foo", "othergroup", "othergroup"]
)
@ -254,12 +254,11 @@ def test_changes():
"file.gid_to_group": MagicMock(side_effect=[5000, 5000]),
}
def mock_exists(*args):
return True
with patch.dict(user.__grains__, {"kernel": "Linux"}), patch.dict(
user.__salt__, dunder_salt
), patch.dict(user.__opts__, {"test": False}), patch("os.path.isdir", mock_exists):
), patch.dict(user.__opts__, {"test": False}), patch(
"os.path.isdir", MagicMock(return_value=True)
):
ret = user._changes("foo", maxdays=999999, inactdays=0, warndays=7)
assert ret == {
"maxdays": 999999,
@ -459,3 +458,43 @@ def test_present_password_unlock():
else:
unlock_password.assert_called_once()
unlock_account.assert_not_called()
@pytest.mark.parametrize(
"current,wanted,remove,return_value,expected",
[
(["grp1"], ["grp1"], False, MagicMock(return_value={"gid": 100}), False),
(
["grp1"],
["grp1", "grp2"],
False,
MagicMock(side_effect=[{"gid": 100}, {"gid": 200}]),
True,
),
(
["grp1"],
["grp1", "grp2"],
False,
MagicMock(side_effect=[{"gid": 100}, {"gid": 100}]),
False,
),
(
["grp1", "grp2"],
["grp1"],
True,
MagicMock(side_effect=[{"gid": 100}, {"gid": 200}]),
True,
),
(
["grp1", "grp2"],
["grp1"],
True,
MagicMock(side_effect=[{"gid": 100}, {"gid": 100}]),
False,
),
],
)
def test__group_changes(current, wanted, remove, return_value, expected):
with patch.dict(user.__salt__, {"group.info": return_value}):
ret = user._group_changes(current, wanted, remove)
assert ret == expected