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
copy them without switching the user context.

This commit adds a new `copytree` filesystem utility that provides the
following features that are relevant to copying home directories.

1. ability to control behavior regarding whether the copy operation
will walk into child datasets. Generally this is not desired because
we have no design or provisions for creating child datasets within a
user home directory. This situation is most likely to arise when users
mistakenly set an incorrect path for a home directory, e.g.
/mnt/tank/share rather than /mnt/tank/share/wilbur

2. awareness of various ACL types on TrueNAS. We need to rigidly preserve
the permissions from the source when writing to a new destination in
case the administrator made a configuration mistake like in (1) in order
to avoid unintentional data disclosure.

3. awareness of the special .zfs directory. Some users enable snapdir
visibility. This means that conventional copy methods can end up
attempting to copy the entirety of the .zfs directory to a new path
resulting in excessive time spent copying and space usage.

4. ability to use block cloning. This speeds up the actual data copy and
reduces the amount of space used on the filesystem when the sysadmin
changes a user home directory path.
  • Loading branch information
anodos325 committed Sep 9, 2024
1 parent 73e7082 commit cd2448c
Show file tree
Hide file tree
Showing 10 changed files with 1,355 additions and 41 deletions.
34 changes: 15 additions & 19 deletions src/middlewared/middlewared/plugins/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from pathlib import Path
from contextlib import suppress

from dataclasses import asdict
from middlewared.api import api_method
from middlewared.api.current import *
from middlewared.schema import accepts, Bool, Dict, Int, List, Password, Patch, returns, SID, Str
Expand All @@ -21,6 +22,7 @@
from middlewared.utils import run, filter_list
from middlewared.utils.crypto import generate_nt_hash, sha512_crypt
from middlewared.utils.directoryservices.constants import DSType, DSStatus
from middlewared.utils.filesystem.copy import copytree, CopyTreeConfig
from middlewared.utils.nss import pwd, grp
from middlewared.utils.nss.nss_common import NssModule
from middlewared.utils.privilege import credential_has_full_admin, privileges_group_mapping
Expand Down Expand Up @@ -753,18 +755,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 had_home:
home_copy = True
home_old = user['home']
if has_home and 'home' in data:
if data.get('home_create', False):
data['home'] = os.path.join(data['home'], data.get('username') or user['username'])

if had_home and user['home'] != data['home']:
home_copy = True
home_old = user['home']

# After this point user dict has values from data
user.update(data)

Expand Down Expand Up @@ -1282,32 +1280,30 @@ 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:
def do_home_copy(self, job, home_old, home_new, username, new_mode, uid):
if home_old in DEFAULT_HOME_PATH:
return

# We need to set permission and strip ACL first before copying files
if new_mode is not None:
perm_job = await self.middleware.call('filesystem.setperm', {
perm_job = self.middleware.call_sync('filesystem.setperm', {
'uid': uid,
'path': home_new,
'mode': new_mode,
'options': {'stripacl': True},
})
else:
current_mode = stat.S_IMODE((await self.middleware.call('filesystem.stat', home_old))['mode'])
perm_job = await self.middleware.call('filesystem.setperm', {
current_mode = stat.S_IMODE(self.middleware.call_sync('filesystem.stat', home_old)['mode'])
perm_job = self.middleware.call_sync('filesystem.setperm', {
'uid': uid,
'path': home_new,
'mode': f'{current_mode:03o}',
'options': {'stripacl': True},
})

await perm_job.wait()
perm_job.wait_sync()

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()}")
return asdict(copytree(home_old, home_new, CopyTreeConfig(exist_ok=True, job=job)))

@private
async def common_validation(self, verrors, data, schema, group_ids, old=None):
Expand Down
10 changes: 5 additions & 5 deletions src/middlewared/middlewared/plugins/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from middlewared.utils.filesystem.acl import acl_is_present
from middlewared.utils.filesystem.constants import FileType
from middlewared.utils.filesystem.directory import DirectoryIterator, DirectoryRequestMask
from middlewared.utils.filesystem.utils import timespec_convert
from middlewared.utils.filesystem.utils import timespec_convert_float
from middlewared.utils.mount import getmntinfo
from middlewared.utils.nss import pwd, grp
from middlewared.utils.path import FSLocation, path_location, is_child_realpath
Expand Down Expand Up @@ -454,10 +454,10 @@ def stat(self, _path):
'mode': st['st'].stx_mode,
'uid': st['st'].stx_uid,
'gid': st['st'].stx_gid,
'atime': timespec_convert(st['st'].stx_atime),
'mtime': timespec_convert(st['st'].stx_mtime),
'ctime': timespec_convert(st['st'].stx_ctime),
'btime': timespec_convert(st['st'].stx_btime),
'atime': timespec_convert_float(st['st'].stx_atime),
'mtime': timespec_convert_float(st['st'].stx_mtime),
'ctime': timespec_convert_float(st['st'].stx_ctime),
'btime': timespec_convert_float(st['st'].stx_btime),
'mount_id': st['st'].stx_mnt_id,
'dev': os.makedev(st['st'].stx_dev_major, st['st'].stx_dev_minor),
'inode': st['st'].stx_ino,
Expand Down
Loading

0 comments on commit cd2448c

Please sign in to comment.