From 08c981fe31324f8513bf54e02bcb71439911e0d4 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Fri, 30 Aug 2024 14:09:13 -0600 Subject: [PATCH] Fix issues with user.update endpoint User may have nologin shell causing the attempt to run cp in a subprocess to fail due to failure to su to the user in question (due to error in pam_open_session()). After review of history for why we were subprocessing out to perform this task, I failed to find a compelling reason why we could not simply use shutil.copytree without switching the user context. This also fixes a bug whereby home directory was not being saved when the user tried to switch their home directory to a subdirectory of their current one. --- .../middlewared/plugins/account.py | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/middlewared/middlewared/plugins/account.py b/src/middlewared/middlewared/plugins/account.py index bc5c96738a748..f69c66bf733bd 100644 --- a/src/middlewared/middlewared/plugins/account.py +++ b/src/middlewared/middlewared/plugins/account.py @@ -753,17 +753,14 @@ def do_update(self, app, audit_callback, pk, data): # Copy the home directory if it changed home_copy = False home_old = None - if ( - has_home and - 'home' in data and - data['home'] != user['home'] and - not data['home'].startswith(f'{user["home"]}/') - ): + if has_home and 'home' in data: + username = data.get('username', user['username']) if had_home: home_copy = True home_old = user['home'] - if data.get('home_create', False): - data['home'] = os.path.join(data['home'], data.get('username') or user['username']) + + if data.get('home_create', False) and not data['home'].endswith(username): + data['home'] = os.path.join(data['home'], username) # After this point user dict has values from data user.update(data) @@ -1283,7 +1280,7 @@ async def setup_local_administrator(self, app, username, password, options): @private @job(lock=lambda args: f'copy_home_to_{args[1]}') async def do_home_copy(self, job, home_old, home_new, username, new_mode, uid): - if home_old in DEFAULT_HOME_PATHS: + if home_old in DEFAULT_HOME_PATHS or home_old == home_new: return if new_mode is not None: @@ -1303,11 +1300,7 @@ async def do_home_copy(self, job, home_old, home_new, username, new_mode, uid): }) await perm_job.wait() - - command = f"/bin/cp -a {shlex.quote(home_old) + '/' + '.'} {shlex.quote(home_new + '/')}" - do_copy = await run(["/usr/bin/su", "-", username, "-c", command], check=False) - if do_copy.returncode != 0: - raise CallError(f"Failed to copy homedir [{home_old}] to [{home_new}]: {do_copy.stderr.decode()}") + shutil.copytree(home_old, home_new, dirs_exist_ok=True) @private async def common_validation(self, verrors, data, schema, group_ids, old=None):