fixes #62502 user.present incapable of removing optional groups

changelog/62502.fixed

@@ -0,0 +1 @@
+Fix user.present to allow removing groups using optional_groups parameter and enforcing idempotent group membership.

salt/states/user.py

@@ -98,7 +98,7 @@ def _changes(
         return False
 
     change = {}
-    if groups is None:
+    if not remove_groups and groups is None:
         groups = lusr["groups"]
     wanted_groups = sorted(set((groups or []) + (optional_groups or [])))
     if uid and lusr["uid"] != uid:

tests/pytests/functional/states/test_user.py

@@ -363,3 +363,51 @@ def test_user_present_existing(states, username):
     assert ret.changes["profile"] == win_profile
     assert "description" in ret.changes
     assert ret.changes["description"] == win_description
+
+
+def test_user_present_change_groups(modules, states, username, group_1, group_2):
+    ret = states.user.present(
+        name=username,
+        groups=[group_1.name, group_2.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["groups"] == [group_1.name, group_2.name]
+
+    # run again and remove group_2
+    ret = states.user.present(
+        name=username,
+        groups=[group_1.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["groups"] == [group_1.name]
+
+
+def test_user_present_change_optional_groups(
+    modules, states, username, group_1, group_2
+):
+    ret = states.user.present(
+        name=username,
+        optional_groups=[group_1.name, group_2.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["optional_groups"] == [group_1.name, group_2.name]
+
+    # run again and remove group_2
+    ret = states.user.present(
+        name=username,
+        optional_groups=[group_1.name],
+    )
+    assert ret.result is True
+
+    user_info = modules.user.info(username)
+    assert user_info
+    assert user_info["optional_groups"] == [group_1.name]

tests/pytests/unit/states/test_user.py

@@ -178,8 +178,10 @@ 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=["foo", "othergroup"])
-    mock_gid_to_group = MagicMock(side_effect=[5000, 5000, 5001, 5001])
+    mock_group_to_gid = MagicMock(side_effect=[5000, 5001])
+    mock_gid_to_group = MagicMock(
+        side_effect=["othergroup", "foo", "othergroup", "othergroup"]
+    )
     dunder_salt = {
         "user.info": mock_info,
         "user.chuid": Mock(),
@@ -197,7 +199,7 @@ def test_present_uid_gid_change():
         )
         assert ret == {
             "comment": "Updated user foo",
-            "changes": {"gid": 5001, "uid": 5001, "groups": ["othergroup"]},
+            "changes": {"gid": 5001, "uid": 5001, "groups": []},
             "name": "foo",
             "result": True,
         }