backups: Use privileged decorator for sshfs actions

Tests:

- Mounting an SSH repository works
  - If an known error is thrown during mounting, a simplified error is shown.
- Unmounting an SSH repository works
  - If an known error is thrown during mounting, a simplified error is shown.
- Correct status of whether the repository is mounted is shown.
  - If an known error is thrown during mounting, a simplified error is shown.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/backups/privileged.py

@@ -1,15 +1,10 @@
-#!/usr/bin/python3
-# -*- mode: python -*-
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Configure backups and sshfs."""
-Actions for sshfs.
-"""
 
-import argparse
-import json
 import os
 import subprocess
-import sys
+
+from plinth.actions import privileged
 
 TIMEOUT = 30
 
@@ -18,40 +13,15 @@ class AlreadyMountedError(Exception):
     """Exception raised when mount point is already mounted."""
 
 
-def parse_arguments():
+@privileged
-    """Return parsed command line arguments as dictionary."""
+def mount(mountpoint: str, remote_path: str, ssh_keyfile: str = None,
-    parser = argparse.ArgumentParser()
+          password: str = None, user_known_hosts_file: str = '/dev/null'):
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    mount = subparsers.add_parser('mount', help='mount an ssh filesystem')
-    mount.add_argument('--mountpoint', help='Local mountpoint', required=True)
-    mount.add_argument('--path', help='Remote ssh path to mount',
-                       required=True)
-    mount.add_argument('--ssh-keyfile', help='Path of private ssh key',
-                       default=None, required=False)
-    mount.add_argument('--user-known-hosts-file',
-                       help='Path to a custom known_hosts file',
-                       default='/dev/null')
-    umount = subparsers.add_parser('umount', help='unmount an ssh filesystem')
-    umount.add_argument('--mountpoint', help='Mountpoint to unmount',
-                        required=True)
-    is_mounted = subparsers.add_parser(
-        'is-mounted', help='Check whether a mountpoint is mounted')
-    is_mounted.add_argument('--mountpoint', help='Mountpoint to check',
-                            required=True)
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
-def subcommand_mount(arguments):
     """Mount a remote ssh path via sshfs."""
     try:
-        validate_mountpoint(arguments.mountpoint)
+        _validate_mountpoint(mountpoint)
     except AlreadyMountedError:
         return
 
-    remote_path = arguments.path
     kwargs = {}
     # the shell would expand ~/ to the local home directory
     remote_path = remote_path.replace('~/', '').replace('~', '')
@@ -65,15 +35,14 @@ def subcommand_mount(arguments):
     # the course of ~11 days, and leaving the system in such state that the
     # only solution is a reboot.
     cmd = [
-        'sshfs', remote_path, arguments.mountpoint, '-o',
+        'sshfs', remote_path, mountpoint, '-o',
-        f'UserKnownHostsFile={arguments.user_known_hosts_file}', '-o',
+        f'UserKnownHostsFile={user_known_hosts_file}', '-o',
         'StrictHostKeyChecking=yes', '-o', 'reconnect', '-o',
         'ServerAliveInterval=15', '-o', 'ServerAliveCountMax=3'
     ]
-    if arguments.ssh_keyfile:
+    if ssh_keyfile:
-        cmd += ['-o', 'IdentityFile=' + arguments.ssh_keyfile]
+        cmd += ['-o', 'IdentityFile=' + ssh_keyfile]
     else:
-        password = read_password()
         if not password:
             raise ValueError('mount requires either a password or ssh_keyfile')
         cmd += ['-o', 'password_stdin']
@@ -82,13 +51,14 @@ def subcommand_mount(arguments):
     subprocess.run(cmd, check=True, timeout=TIMEOUT, **kwargs)
 
 
-def subcommand_umount(arguments):
+@privileged
+def subcommand_umount(mountpoint: str):
     """Unmount a mountpoint."""
-    subprocess.run(['umount', arguments.mountpoint], check=True)
+    subprocess.run(['umount', mountpoint], check=True)
 
 
-def validate_mountpoint(mountpoint):
+def _validate_mountpoint(mountpoint):
-    """Check that the folder is empty, and create it if it doesn't exist"""
+    """Check that the folder is empty, and create it if it doesn't exist."""
     if os.path.exists(mountpoint):
         if _is_mounted(mountpoint):
             raise AlreadyMountedError('Mountpoint %s already mounted' %
@@ -111,27 +81,7 @@ def _is_mounted(mountpoint):
         return False
 
 
-def subcommand_is_mounted(arguments):
+@privileged
+def is_mounted(arguments) -> bool:
     """Print whether a path is already mounted."""
-    print(json.dumps(_is_mounted(arguments.mountpoint)))
+    return _is_mounted(arguments.mountpoint)
-
-
-def read_password():
-    """Read the password from stdin."""
-    if sys.stdin.isatty():
-        return ''
-
-    return ''.join(sys.stdin)
-
-
-def main():
-    """Parse arguments and perform all duties."""
-    arguments = parse_arguments()
-
-    subcommand = arguments.subcommand.replace('-', '_')
-    subcommand_method = globals()['subcommand_' + subcommand]
-    subcommand_method(arguments)
-
-
-if __name__ == '__main__':
-    main()

plinth/modules/backups/repository.py

@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Remote and local Borg backup repositories."""
-Remote and local Borg backup repositories
-"""
 
 import abc
 import contextlib
@@ -19,7 +17,7 @@ from plinth import actions, cfg
 from plinth.errors import ActionError
 from plinth.utils import format_lazy
 
-from . import (_backup_handler, api, errors, get_known_hosts_path,
+from . import (_backup_handler, api, errors, get_known_hosts_path, privileged,
                restore_archive_handler, split_path, store)
 from .schedule import Schedule
 
@@ -213,6 +211,14 @@ class BaseBorgRepository(abc.ABC):
 
         return {}
 
+    @contextlib.contextmanager
+    def _handle_errors(self):
+        """Parse exceptions into more specific ones."""
+        try:
+            yield
+        except Exception as exception:
+            self.reraise_known_error(exception)
+
     def _run(self, cmd, arguments, superuser=True, **kwargs):
         """Run a backups or sshfs action script command."""
         try:
@@ -418,9 +424,8 @@ class SshBorgRepository(BaseBorgRepository):
     @property
     def is_mounted(self):
         """Return whether remote path is mounted locally."""
-        output = self._run('sshfs',
+        with self._handle_errors():
-                           ['is-mounted', '--mountpoint', self._mountpoint])
+            return privileged.is_mounted(self._mountpoint)
-        return json.loads(output)
 
     def initialize(self):
         """Initialize the repository after mounting the target directory."""
@@ -432,22 +437,27 @@ class SshBorgRepository(BaseBorgRepository):
         """Mount the remote path locally using sshfs."""
         if self.is_mounted:
             return
+
         known_hosts_path = get_known_hosts_path()
-        arguments = [
+        kwargs = {'user_known_hosts_file': str(known_hosts_path)}
-            'mount', '--mountpoint', self._mountpoint, '--path', self._path,
+        if 'ssh_password' in self.credentials and self.credentials[
-            '--user-known-hosts-file',
+                'ssh_password']:
-            str(known_hosts_path)
+            kwargs['password'] = self.credentials['ssh_password']
-        ]
+
-        arguments, kwargs = self._append_sshfs_arguments(
+        if 'ssh_keyfile' in self.credentials and self.credentials[
-            arguments, self.credentials)
+                'ssh_keyfile']:
-        self._run('sshfs', arguments, **kwargs)
+            kwargs['ssh_keyfile'] = self.credentials['ssh_keyfile']
+
+        with self._handle_errors():
+            privileged.mount(self._mountpoint, self._path, **kwargs)
 
     def umount(self):
         """Unmount the remote path that was mounted locally using sshfs."""
         if not self.is_mounted:
             return
 
-        self._run('sshfs', ['umount', '--mountpoint', self._mountpoint])
+        with self._handle_errors():
+            privileged.umount(self._mountpoint)
 
     def _umount_ignore_errors(self):
         """Run unmount operation and ignore any exceptions thrown."""
@@ -457,7 +467,7 @@ class SshBorgRepository(BaseBorgRepository):
             logger.warning('Unable to unmount repository', exc_info=exception)
 
     def remove(self):
-        """Remove a repository from the kvstore and delete its mountpoint"""
+        """Remove a repository from the kvstore and delete its mountpoint."""
         self.umount()
         store.delete(self.uuid)
         try:
@@ -471,19 +481,6 @@ class SshBorgRepository(BaseBorgRepository):
         except Exception as err:
             logger.error(err)
 
-    @staticmethod
-    def _append_sshfs_arguments(arguments, credentials):
-        """Add credentials to a run command and kwargs"""
-        kwargs = {}
-
-        if 'ssh_password' in credentials and credentials['ssh_password']:
-            kwargs['input'] = credentials['ssh_password'].encode()
-
-        if 'ssh_keyfile' in credentials and credentials['ssh_keyfile']:
-            arguments += ['--ssh-keyfile', credentials['ssh_keyfile']]
-
-        return (arguments, kwargs)
-
     def _ensure_remote_directory(self):
         """Create remote SSH directory if it does not exist."""
         username, hostname, dir_path = split_path(self.path)