From e803d6d2fcd56705d81563b97b44eced035de28a Mon Sep 17 00:00:00 2001
From: themylogin <themylogin@gmail.com>
Date: Fri, 26 Jul 2024 17:11:15 +0200
Subject: [PATCH] Ensure correct user homedir ownership

---
 .../middlewared/plugins/account.py            |  9 +++++++-
 .../middlewared/plugins/filesystem_/acl.py    |  2 +-
 .../middlewared/plugins/keychain.py           |  2 ++
 tests/api2/test_account_home.py               | 21 +++++++++++++++++++
 4 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 tests/api2/test_account_home.py

diff --git a/src/middlewared/middlewared/plugins/account.py b/src/middlewared/middlewared/plugins/account.py
index d71c10666e302..c3e31da50a39a 100644
--- a/src/middlewared/middlewared/plugins/account.py
+++ b/src/middlewared/middlewared/plugins/account.py
@@ -842,7 +842,14 @@ def do_update(self, app, audit_callback, pk, data):
     def recreate_homedir_if_not_exists(self, user, group, mode):
         # sigh, nothing is stopping someone from removing the homedir
         # from the CLI so recreate the original directory in this case
-        if not os.path.isdir(user['home']):
+        if os.path.isdir(user['home']):
+            if user['home'].startswith('/mnt/'):
+                self.middleware.call_sync('filesystem.setperm', {
+                    'path': user['home'],
+                    'uid': user['uid'],
+                    'gid': group['bsdgrp_gid'],
+                }).wait_sync(raise_error=True)
+        else:
             if os.path.exists(user['home']):
                 raise CallError(f'{user["home"]!r} already exists and is not a directory')
 
diff --git a/src/middlewared/middlewared/plugins/filesystem_/acl.py b/src/middlewared/middlewared/plugins/filesystem_/acl.py
index b0ffeaf6d3cd1..5946099425e4b 100644
--- a/src/middlewared/middlewared/plugins/filesystem_/acl.py
+++ b/src/middlewared/middlewared/plugins/filesystem_/acl.py
@@ -246,7 +246,7 @@ def setperm(self, job, data):
         uid = -1 if data['uid'] is None else data.get('uid', -1)
         gid = -1 if data['gid'] is None else data.get('gid', -1)
 
-        loc = self._common_perm_path_validate("filesystem.setperm", data, verrors)
+        self._common_perm_path_validate("filesystem.setperm", data, verrors)
 
         current_acl = self.middleware.call_sync('filesystem.getacl', data['path'])
         acl_is_trivial = current_acl['trivial']
diff --git a/src/middlewared/middlewared/plugins/keychain.py b/src/middlewared/middlewared/plugins/keychain.py
index 2d62d0a547bec..5a42a700901f4 100644
--- a/src/middlewared/middlewared/plugins/keychain.py
+++ b/src/middlewared/middlewared/plugins/keychain.py
@@ -704,6 +704,8 @@ def ssh_pair(self, data):
 
         # Write public key in user authorized_keys for SSH
         with open(f"{dotsshdir}/authorized_keys", "a+") as f:
+            os.fchmod(f.fileno(), 0o600)
+            os.fchown(f.fileno(), user["uid"], user["group"]["bsdgrp_gid"])
             f.seek(0)
             if data["public_key"] not in f.read():
                 f.write("\n" + data["public_key"] + "\n")
diff --git a/tests/api2/test_account_home.py b/tests/api2/test_account_home.py
new file mode 100644
index 0000000000000..33cada6bbb0a2
--- /dev/null
+++ b/tests/api2/test_account_home.py
@@ -0,0 +1,21 @@
+from middlewared.test.integration.assets.account import user
+from middlewared.test.integration.assets.pool import dataset
+from middlewared.test.integration.utils import call
+
+
+def test_chown_on_update():
+    with dataset("unpriv_homedir") as homedir:
+        with user({
+            "username": "unpriv",
+            "full_name": "unpriv",
+            "group_create": True,
+            "password": "pass",
+        }) as u:
+            path = f"/mnt/{homedir}"
+
+            call("user.update", u["id"], {"home": path})
+
+            assert {
+                "user": "unpriv",
+                "group": "unpriv",
+            }.items() < call("filesystem.stat", path).items()