From 95ba6566b1e905bd4fd9b6ee3db4cbf83446c298 Mon Sep 17 00:00:00 2001
From: Nick Rhodes <nrhodes91@gmail.com>
Date: Sun, 2 Oct 2022 11:20:56 +0100
Subject: [PATCH] If mode changes set perms["cmode"] in file.check_perms

This mirrors the user/group checks which set `perms["cuser"]` etc when
there are changes expected. These values are used to determine if we
need to return changes in `ret["changes"]`. Before this commit
`file.chec_perms` was returning `mode` changes for new files which
didn't match the original behaviour.
---
 changelog/62818.fixed                         |  2 ++
 salt/modules/file.py                          | 20 ++++++++++---------
 salt/states/file.py                           |  2 +-
 .../unit/modules/file/test_file_selinux.py    |  2 +-
 4 files changed, 15 insertions(+), 11 deletions(-)
 create mode 100644 changelog/62818.fixed

diff --git a/changelog/62818.fixed b/changelog/62818.fixed
new file mode 100644
index 00000000000..944d2227c7c
--- /dev/null
+++ b/changelog/62818.fixed
@@ -0,0 +1,2 @@
+Include UID and GID checks in modules.file.check_perms as well as comparing
+ownership by username and group name.
diff --git a/salt/modules/file.py b/salt/modules/file.py
index 2b6a7928f9c..49ec728ce56 100644
--- a/salt/modules/file.py
+++ b/salt/modules/file.py
@@ -5169,13 +5169,12 @@ def check_perms(
 
     # Mode changes if needed
     if mode is not None:
-        # File is a symlink, ignore the mode setting
-        # if follow_symlinks is False
-        if not (is_link and not follow_symlinks):
-            if __opts__["test"] is True:
-                ret["changes"]["mode"] = mode
-            else:
+        if not __opts__["test"] is True:
+            # File is a symlink, ignore the mode setting
+            # if follow_symlinks is False
+            if not (is_link and not follow_symlinks):
                 if not mode == cur["mode"]:
+                    perms["cmode"] = mode
                     set_mode(name, mode)
 
     # verify user/group/mode changes
@@ -5217,9 +5216,12 @@ def check_perms(
         # if follow_symlinks is False
         if not (is_link and not follow_symlinks):
             if not mode == post["mode"]:
-                ret["result"] = False
-                ret["comment"].append("Failed to change mode to {}".format(mode))
-            else:
+                if __opts__["test"] is True:
+                    ret["changes"]["mode"] = mode
+                else:
+                    ret["result"] = False
+                    ret["comment"].append("Failed to change mode to {}".format(mode))
+            elif "cmode" in perms:
                 ret["changes"]["mode"] = mode
 
     # Modify attributes of file if needed
diff --git a/salt/states/file.py b/salt/states/file.py
index 1062cea35c5..a3cd20348f2 100644
--- a/salt/states/file.py
+++ b/salt/states/file.py
@@ -755,7 +755,7 @@ def _check_directory(
                     if (
                         group is not None
                         and not group == stats.get("group")
-                        and not user == stats.get("gid")
+                        and not group == stats.get("gid")
                     ):
                         fchange["group"] = group
                     smode = salt.utils.files.normalize_mode(stats.get("mode"))
diff --git a/tests/pytests/unit/modules/file/test_file_selinux.py b/tests/pytests/unit/modules/file/test_file_selinux.py
index 843353f8ac2..fea2e7df1c9 100644
--- a/tests/pytests/unit/modules/file/test_file_selinux.py
+++ b/tests/pytests/unit/modules/file/test_file_selinux.py
@@ -112,7 +112,7 @@ def test_file_check_perms(tfile3):
             "name": tfile3,
             "result": True,
         },
-        {"luser": "root", "lmode": "0644", "lgroup": "root"},
+        {"cmode": "0664", "luser": "root", "lmode": "0644", "lgroup": "root"},
     )
 
     # Disable lsattr calls