backups: Handle errors when adding disk repository

- Simplify repository initialization using inheritance.

- Share the code for saving repository for disk and remote repositories. Error
  messages are properly handled for disk repositories too.

- Move logic to create remote SSH directory to SSH repository class instead of
  views.

- Create a new error for handling borg repository already existing while
  initialization.

- Get information of repository after initializing so that password errors are
  caught early.

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

plinth/modules/backups/errors.py

@@ -20,14 +20,15 @@ from plinth.errors import PlinthError
 
 class BorgError(PlinthError):
     """Generic borg errors"""
-    pass
 
 
 class BorgRepositoryDoesNotExistError(BorgError):
     """Borg access to a repository works but the repository does not exist"""
-    pass
 
 
 class SshfsError(PlinthError):
     """Generic sshfs errors"""
-    pass
+
+
+class BorgRepositoryExists(BorgError):
+    """A repository at target location already exists during initialization."""

plinth/modules/backups/repository.py

@@ -19,6 +19,7 @@ Remote and local Borg backup repositories
 """
 
 import abc
+import contextlib
 import io
 import json
 import logging
@@ -26,54 +27,67 @@ import os
 import re
 from uuid import uuid1
 
+import paramiko
 from django.utils.translation import ugettext_lazy as _
 
 from plinth import actions, cfg
 from plinth.errors import ActionError
 from plinth.utils import format_lazy
 
-from . import (_backup_handler, api, get_known_hosts_path,
+from . import (_backup_handler, api, errors, get_known_hosts_path,
                restore_archive_handler, split_path, store)
-from .errors import BorgError, BorgRepositoryDoesNotExistError, SshfsError
 
 logger = logging.getLogger(__name__)
 
-SUPPORTED_BORG_ENCRYPTION = ['none', 'repokey']
 # known errors that come up when remotely accessing a borg repository
 # 'errors' are error strings to look for in the stacktrace.
-KNOWN_ERRORS = [{
-    'errors': ['subprocess.TimeoutExpired'],
-    'message':
-        _('Connection refused - make sure you provided correct '
-          'credentials and the server is running.'),
-    'raise_as':
-        BorgError,
-},
-                {
-                    'errors': ['Connection refused'],
-                    'message': _('Connection refused'),
-                    'raise_as': BorgError,
-                },
-                {
-                    'errors': [
-                        'not a valid repository', 'does not exist',
-                        'FileNotFoundError'
-                    ],
-                    'message':
-                        _('Repository not found'),
-                    'raise_as':
-                        BorgRepositoryDoesNotExistError,
-                },
-                {
-                    'errors': [('passphrase supplied in .* is incorrect')],
-                    'message': _('Incorrect encryption passphrase'),
-                    'raise_as': BorgError,
-                },
-                {
-                    'errors': [('Connection reset by peer')],
-                    'message': _('SSH access denied'),
-                    'raise_as': SshfsError,
-                }]
+KNOWN_ERRORS = [
+    {
+        'errors': ['subprocess.TimeoutExpired'],
+        'message':
+            _('Connection refused - make sure you provided correct '
+              'credentials and the server is running.'),
+        'raise_as':
+            errors.BorgError,
+    },
+    {
+        'errors': ['Connection refused'],
+        'message': _('Connection refused'),
+        'raise_as': errors.BorgError,
+    },
+    {
+        'errors': [
+            'not a valid repository', 'does not exist', 'FileNotFoundError'
+        ],
+        'message':
+            _('Repository not found'),
+        'raise_as':
+            errors.BorgRepositoryDoesNotExistError,
+    },
+    {
+        'errors': ['passphrase supplied in .* is incorrect'],
+        'message': _('Incorrect encryption passphrase'),
+        'raise_as': errors.BorgError,
+    },
+    {
+        'errors': ['Connection reset by peer'],
+        'message': _('SSH access denied'),
+        'raise_as': errors.SshfsError,
+    },
+    {
+        'errors': ['There is already something at'],
+        'message':
+            _('Repository path is neither empty nor '
+              'is an existing backups repository.'),
+        'raise_as':
+            errors.BorgError,
+    },
+    {
+        'errors': ['A repository already exists at'],
+        'message': None,
+        'raise_as': errors.BorgRepositoryExists,
+    },
+]
 
 
 class BaseBorgRepository(abc.ABC):
@@ -147,7 +161,7 @@ class BaseBorgRepository(abc.ABC):
             repository['mounted'] = self.is_mounted
             if repository['mounted']:
                 repository['archives'] = self.list_archives()
-        except (BorgError, ActionError) as err:
+        except (errors.BorgError, ActionError) as err:
             repository['error'] = str(err)
 
         return repository
@@ -174,13 +188,20 @@ class BaseBorgRepository(abc.ABC):
         archive_path = self._get_archive_path(archive_name)
         self.run(['delete-archive', '--path', archive_path])
 
-    def initialize(self, encryption):
+    def initialize(self):
         """Initialize / create a borg repository."""
-        if encryption not in SUPPORTED_BORG_ENCRYPTION:
-            raise ValueError('Unsupported encryption: %s' % encryption)
+        encryption = 'none'
+        if 'encryption_passphrase' in self.credentials and \
+           self.credentials['encryption_passphrase']:
+            encryption = 'repokey'
 
-        self.run(
-            ['init', '--path', self.borg_path, '--encryption', encryption])
+        try:
+            self.run(
+                ['init', '--path', self.borg_path, '--encryption', encryption])
+        except errors.BorgRepositoryExists:
+            pass
+
+        self.get_info()  # If password is incorrect raise an error early.
 
     @staticmethod
     def _get_encryption_data(credentials):
@@ -380,6 +401,12 @@ class SshBorgRepository(BaseBorgRepository):
                            ['is-mounted', '--mountpoint', self._mountpoint])
         return json.loads(output)
 
+    def initialize(self):
+        """Initialize the repository after mounting the target directory."""
+        self._ensure_remote_directory()
+        self.mount()
+        super().initialize()
+
     def mount(self):
         """Mount the remote path locally using sshfs."""
         if self.is_mounted:
@@ -429,6 +456,41 @@ class SshBorgRepository(BaseBorgRepository):
 
         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)
+        if dir_path == '':
+            dir_path = '.'
+
+        if dir_path[0] == '~':
+            dir_path = '.' + dir_path[1:]
+
+        password = self.credentials['ssh_password']
+
+        # Ensure remote directory exists, check contents
+        # TODO Test with IPv6 connection
+        with _ssh_connection(hostname, username, password) as ssh_client:
+            with ssh_client.open_sftp() as sftp_client:
+                try:
+                    sftp_client.listdir(dir_path)
+                except FileNotFoundError:
+                    logger.info('Directory %s does not exist, creating.',
+                                dir_path)
+                    sftp_client.mkdir(dir_path)
+
+
+@contextlib.contextmanager
+def _ssh_connection(hostname, username, password):
+    """Context manager to create and close an SSH connection."""
+    ssh_client = paramiko.SSHClient()
+    ssh_client.load_host_keys(str(get_known_hosts_path()))
+
+    try:
+        ssh_client.connect(hostname, username=username, password=password)
+        yield ssh_client
+    finally:
+        ssh_client.close()
+
 
 def get_repositories():
     """Get all repositories of a given storage type."""

plinth/modules/backups/views.py

@@ -21,7 +21,6 @@ Views for the backups app.
 import logging
 import os
 import tempfile
-from contextlib import contextmanager
 from datetime import datetime
 from urllib.parse import unquote
 
@@ -39,9 +38,8 @@ from plinth.errors import PlinthError
 from plinth.modules import backups, storage
 
 from . import (SESSION_PATH_VARIABLE, api, forms, get_known_hosts_path,
-               is_ssh_hostkey_verified, split_path)
+               is_ssh_hostkey_verified)
 from .decorators import delete_tmp_backup_file
-from .errors import BorgRepositoryDoesNotExistError
 from .repository import (BorgRepository, SshBorgRepository, get_instance,
                          get_repositories)
 
@@ -265,13 +263,10 @@ class AddRepositoryView(SuccessMessageMixin, FormView):
 
         credentials = {'encryption_passphrase': encryption_passphrase}
         repository = BorgRepository(path, credentials)
-        try:
-            repository.get_info()
-        except BorgRepositoryDoesNotExistError:
-            repository.initialize(encryption)
+        if _save_repository(self.request, repository):
+            return super().form_valid(form)
 
-        repository.save(store_credentials=True, verified=True)
-        return super().form_valid(form)
+        return redirect(reverse_lazy('backups:add-repository'))
 
 
 class AddRemoteRepositoryView(SuccessMessageMixin, FormView):
@@ -346,106 +341,53 @@ class VerifySshHostkeyView(SuccessMessageMixin, FormView):
 
     def get(self, *args, **kwargs):
         """Skip this view if host is already verified."""
-        if is_ssh_hostkey_verified(self._get_repository().hostname):
-            messages.success(self.request, _('SSH host already verified.'))
-            return self._add_remote_repository()
+        if not is_ssh_hostkey_verified(self._get_repository().hostname):
+            return super().get(*args, **kwargs)
 
-        return super().get(*args, **kwargs)
+        messages.success(self.request, _('SSH host already verified.'))
+        if _save_repository(self.request, self._get_repository()):
+            return redirect(reverse_lazy('backups:index'))
+
+        return redirect(reverse_lazy('backups:add-remote-repository'))
 
     def form_valid(self, form):
         """Create and store the repository."""
         ssh_public_key = form.cleaned_data['ssh_public_key']
         self._add_ssh_hostkey(ssh_public_key)
         messages.success(self.request, _('SSH host verified.'))
-        return self._add_remote_repository()
-
-    def _add_remote_repository(self):
-        """On successful verification of host, add repository."""
-        repository = self._get_repository()
-        encryption = 'none'
-        if 'encryption_passphrase' in repository.credentials and \
-           repository.credentials['encryption_passphrase']:
-            encryption = 'repokey'
-
-        try:
-            dir_contents = _list_remote_directory(repository.path,
-                                                  repository.credentials)
-            repository.mount()
-            repository = _create_remote_repository(repository, encryption,
-                                                   dir_contents)
-            repository.save(verified=True)
+        if _save_repository(self.request, self._get_repository()):
             return redirect(reverse_lazy('backups:index'))
-        except paramiko.BadHostKeyException:
-            message = _('SSH host public key could not be verified.')
-        except paramiko.AuthenticationException:
-            message = _('Authentication to remote server failed.')
-        except paramiko.SSHException as exception:
-            message = _('Error establishing connection to server: {}').format(
-                str(exception))
-        except BorgRepositoryDoesNotExistError:
-            message = _('Repository path is neither empty nor '
-                        'is an existing backups repository.')
-        except Exception as exception:
-            message = str(exception)
-            logger.exception('Error adding repository: %s', exception)
 
-        messages.error(self.request, message)
-        messages.error(self.request, _('Repository removed.'))
-        # Remove the repository so that the user can have another go at
-        # creating it.
-        repository.remove()
         return redirect(reverse_lazy('backups:add-remote-repository'))
 
 
-def _list_remote_directory(path, credentials):
-    """List a SSH remote directory. Create if it does not exist. """
-    username, hostname, dir_path = split_path(path)
-    if dir_path == '':
-        dir_path = '.'
-
-    if dir_path[0] == '~':
-        dir_path = '.' + dir_path[1:]
-
-    password = credentials['ssh_password']
-
-    # Ensure remote directory exists, check contents
-    dir_contents = None
-    # TODO Test with IPv6 connection
-    with _ssh_connection(hostname, username, password) as ssh_client:
-        with ssh_client.open_sftp() as sftp_client:
-            try:
-                dir_contents = sftp_client.listdir(dir_path)
-            except FileNotFoundError:
-                logger.info('Directory %s does not exist, creating.', dir_path)
-                sftp_client.mkdir(dir_path)
-
-    return dir_contents
-
-
-def _create_remote_repository(repository, encryption, dir_contents):
-    """Create a Borg repository on remote server if necessary."""
+def _save_repository(request, repository):
+    """Initialize and save a repository. Convert errors to messages."""
     try:
-        repository.get_info()
-    except BorgRepositoryDoesNotExistError:
-        if dir_contents:
-            raise
-
-        repository.initialize(encryption)
-
-    return repository
-
-
-@contextmanager
-def _ssh_connection(hostname, username, password):
-    """Context manager to create and close an SSH connection."""
-    ssh_client = paramiko.SSHClient()
-    ssh_client.load_host_keys(str(get_known_hosts_path()))
+        repository.initialize()
+        repository.save(verified=True)
+        return True
+    except paramiko.BadHostKeyException:
+        message = _('SSH host public key could not be verified.')
+    except paramiko.AuthenticationException:
+        message = _('Authentication to remote server failed.')
+    except paramiko.SSHException as exception:
+        message = _('Error establishing connection to server: {}').format(
+            str(exception))
+    except Exception as exception:
+        message = str(exception)
+        logger.exception('Error adding repository: %s', exception)
 
+    messages.error(request, message)
+    # Remove the repository so that the user can have another go at
+    # creating it.
     try:
-        ssh_client.connect(hostname, username=username, password=password)
-        yield ssh_client
-    finally:
-        ssh_client.close()
+        repository.remove()
+        messages.error(request, _('Repository removed.'))
+    except KeyError:
+        pass
+
+    return False
 
 
 class RemoveRepositoryView(SuccessMessageMixin, TemplateView):