storage: Don't use privileged action feature to run as different user

- Instead implement running specific commands inside the privileged action as a
specific user.

Tests:

- In transmission, setting the download directory is valid if

  - A parent level directory is writable by transmission daemon and child does
  not exist.

  - A leaf level directory is writable by transmission daemon when leaf exists.

  - A leaf level exists and is not a directory.

- In MiniDLNA, setting the directory works only if it exists and is readable.
Work when write permission is not available.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Joseph Nuthalapati <njoseph@riseup.net>

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:

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')

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')