Skip to content

Commit

Permalink
Fix issues with user.update endpoint
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
anodos325 committed Aug 31, 2024
1 parent 1218dea commit 08c981f
Showing 1 changed file with 7 additions and 14 deletions.
21 changes: 7 additions & 14 deletions src/middlewared/middlewared/plugins/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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):
Expand Down

0 comments on commit 08c981f

Please sign in to comment.