diff --git a/plinth/modules/storage/forms.py b/plinth/modules/storage/forms.py index a8f77103b..5fff0ad26 100644 --- a/plinth/modules/storage/forms.py +++ b/plinth/modules/storage/forms.py @@ -68,7 +68,7 @@ class DirectoryValidator: privileged.validate_directory(value, self.check_creatable, self.check_writable, - _run_as_user=self.username) + for_user=self.username) except FileNotFoundError: raise ValidationError(_('Directory does not exist.'), 'invalid') except NotADirectoryError: diff --git a/plinth/modules/storage/privileged.py b/plinth/modules/storage/privileged.py index 9138a7373..e5422eced 100644 --- a/plinth/modules/storage/privileged.py +++ b/plinth/modules/storage/privileged.py @@ -6,7 +6,7 @@ import re import stat import subprocess -from plinth import utils +from plinth import action_utils, utils from plinth.actions import privileged @@ -330,10 +330,8 @@ def usage_info() -> str: @privileged def validate_directory(directory: str, check_creatable: bool, - check_writable: bool): + check_writable: bool, for_user: str): """Validate a directory.""" - if os.geteuid() == 0: - raise RuntimeError('You must not be root to run this command') def part_exists(path): """Return part of the path that exists.""" @@ -352,9 +350,15 @@ def validate_directory(directory: str, check_creatable: bool, if not os.path.isdir(directory): raise NotADirectoryError - if not os.access(directory, os.R_OK): + try: + action_utils.run_as_user(['test', '-r', directory], username=for_user, + check=True) + except subprocess.CalledProcessError: raise PermissionError('read') if check_writable or check_creatable: - if not os.access(directory, os.W_OK): + try: + action_utils.run_as_user(['test', '-w', directory], + username=for_user, check=True) + except subprocess.CalledProcessError: raise PermissionError('write') diff --git a/plinth/modules/storage/tests/test_storage.py b/plinth/modules/storage/tests/test_storage.py index fcce70101..2b6f01e8a 100644 --- a/plinth/modules/storage/tests/test_storage.py +++ b/plinth/modules/storage/tests/test_storage.py @@ -31,7 +31,7 @@ def _is_container(): return process.returncode == 0 -pytestmark = pytest.mark.usefixtures('mock_privileged') +pytestmark = pytest.mark.usefixtures('mock_privileged', 'mock_run_as_user') privileged_modules_to_mock = ['plinth.modules.storage.privileged'] skip_if_container = pytest.mark.skipif(_is_container(), reason='running inside a container') @@ -337,9 +337,11 @@ def _assert_validate_directory(path, error, check_writable=False, match = None if not error.args else error.args[0] with pytest.raises(error.__class__, match=match): privileged.validate_directory(path, check_creatable, - check_writable) + check_writable, + for_user='test-non-existent') else: - privileged.validate_directory(path, check_creatable, check_writable) + privileged.validate_directory(path, check_creatable, check_writable, + for_user='test-non-existent') @pytest.mark.usefixtures('needs_not_root')