Backups: Cleanup and improved error handling

- fixes issues as supposed by jvalleroy
- new repositories always get a UUID so they can immediately be fully
  used (mounted, queried etc) also before saving them
- remove test connection page -- errors are shown on form submission
- improved error handling when creating remote repositories

Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

actions/backups

@@ -45,7 +45,7 @@ def parse_arguments():
         'setup', help='Create repository if it does not already exist')
 
     init = subparsers.add_parser('init', help='Initialize a repository')
-    init.add_argument('--encryption', help='Enryption of the repository',
+    init.add_argument('--encryption', help='Encryption of the repository',
                       required=True)
 
     info = subparsers.add_parser('info', help='Show repository information')
@@ -151,7 +151,6 @@ def _extract(archive_path, destination, locations=None, env=None):
     """Extract archive contents."""
     if not env:
         env = dict(os.environ)
-    # TODO: is LANG necessary?
     env['LANG'] = 'C.UTF-8'
 
     prev_dir = os.getcwd()

actions/sshfs

@@ -48,8 +48,8 @@ def parse_arguments():
                                    help='unmount an ssh filesystem')
     umount.add_argument('--mountpoint', help='Mountpoint to unmount',
                         required=True)
-    is_mounted = subparsers.add_parser('is-mounted',
+    is_mounted = subparsers.add_parser(
-                                       help='Check whether an sshfs is mouned')
+        'is-mounted', help='Check whether a mountpoint is mounted')
     is_mounted.add_argument('--mountpoint', help='Mountpoint to check',
                             required=True)
 
@@ -83,7 +83,7 @@ def subcommand_mount(arguments):
 
 
 def subcommand_umount(arguments):
-    """Show repository information."""
+    """Unmount a mountpoint."""
     run(['umount', arguments.mountpoint])
 
 

plinth/modules/backups/errors.py

@@ -26,3 +26,8 @@ class BorgError(PlinthError):
 class BorgRepositoryDoesNotExistError(BorgError):
     """Borg access to a repository works but the repository does not exist"""
     pass
+
+
+class SshfsError(PlinthError):
+    """Generic sshfs errors"""
+    pass

plinth/modules/backups/forms.py

@@ -142,8 +142,9 @@ class AddRepositoryForm(forms.Form):
 
         path = cleaned_data.get("repository")
         credentials = self.get_credentials()
-        self.repository = SshBorgRepository(path=path, credentials=credentials)
         try:
+            self.repository = SshBorgRepository(path=path,
+                                                credentials=credentials)
             self.repository.get_info()
         except BorgRepositoryDoesNotExistError:
             pass

plinth/modules/backups/repository.py

@@ -21,6 +21,7 @@ Remote and local Borg backup repositories
 import json
 import logging
 import os
+from uuid import uuid1
 
 from django.utils.translation import ugettext_lazy as _
 
@@ -30,7 +31,7 @@ from plinth.errors import ActionError
 from . import api, network_storage, _backup_handler, ROOT_REPOSITORY_NAME, \
         ROOT_REPOSITORY_UUID, ROOT_REPOSITORY, restore_archive_handler, \
         zipstream
-from .errors import BorgError, BorgRepositoryDoesNotExistError
+from .errors import BorgError, BorgRepositoryDoesNotExistError, SshfsError
 
 logger = logging.getLogger(__name__)
 
@@ -53,6 +54,17 @@ KNOWN_ERRORS = [{
         "errors": ["not a valid repository", "does not exist"],
         "message": _("Repository not found"),
         "raise_as": BorgRepositoryDoesNotExistError,
+    },
+    {
+        "errors": [("passphrase supplied in BORG_PASSPHRASE or by "
+                    "BORG_PASSCOMMAND is incorrect")],
+        "message": _("Incorrect encryption passphrase"),
+        "raise_as": BorgError,
+    },
+    {
+        "errors": [("Connection reset by peer")],
+        "message": _("SSH access denied"),
+        "raise_as": SshfsError,
     }]
 
 
@@ -192,22 +204,22 @@ class SshBorgRepository(BorgRepository):
         Provide a uuid to instanciate an existing repository,
         or 'ssh_path' and 'credentials' for a new repository.
         """
-        if uuid:
+        is_new_instance = not bool(uuid)
+        if not uuid:
+            uuid = str(uuid1())
         self.uuid = uuid
-            # If all data are given, instanciate right away.
+
         if path and credentials:
             self._path = path
             self.credentials = credentials
         else:
-                self._load_from_kvstore()
+            if is_new_instance:
-        # No uuid given: new instance.
+                # Either a uuid, or both a path and credentials must be given
-        elif path and credentials:
-            self._path = path
-            self.credentials = credentials
-        else:
                 raise ValueError('Invalid arguments.')
+            else:
+                self._load_from_kvstore()
+
         if automount:
-            if self.uuid and not self.is_mounted:
             self.mount()
 
     @property
@@ -273,6 +285,9 @@ class SshBorgRepository(BorgRepository):
         self.uuid = network_storage.update_or_add(storage)
 
     def mount(self):
+        if self.is_mounted:
+            return
+
         arguments = ['mount', '--mountpoint', self.mountpoint, '--path',
                      self._path]
         arguments, kwargs = self._append_sshfs_arguments(arguments,
@@ -280,6 +295,8 @@ class SshBorgRepository(BorgRepository):
         self._run('sshfs', arguments, kwargs=kwargs)
 
     def umount(self):
+        if not self.is_mounted:
+            return
         self._run('sshfs', ['umount', '--mountpoint', self.mountpoint])
 
     def remove_repository(self):

plinth/modules/backups/templates/backups_repository_add.html

@@ -31,10 +31,11 @@
     {{ form|bootstrap }}
 
     <input type="submit" class="btn btn-primary"
-           value="{% trans "Submit" %}"/>
+           value="{% trans "Create Repository" %}"/>
-    <input type="submit" class="btn btn-secondary" value="Test Connection"
+    <a class="abort btn btn-sm btn-default"
-           title="{% trans 'Test Connection to Repository' %}"
+       href="{% url 'backups:index' %}">
-           formaction="{% url 'backups:repository-test' %}" />
+      {% trans "Abort" %}
+    </a>
   </form>
 
 {% endblock %}

plinth/modules/backups/templates/backups_repository_remove.html

@@ -25,11 +25,12 @@
   <h2>{{ title }}</h2>
 
   <p>
-    {% trans "Are you sure that you want to remove the repository" %}<br />
+    {% trans "Are you sure that you want to remove this repository?" %}
-    <b>
-      {{ repository.path }}?
-    </b>
   </p>
+  <p>
+    <b>{{ repository.name }}</b>
+  </p>
+  <p>
     {% blocktrans %}
       The remote repository will not be deleted.
       This just removes the repository from the listing on the backup page, you
@@ -42,9 +43,7 @@
       {% csrf_token %}
 
       <input type="submit" class="btn btn-danger"
-             value="{% blocktrans trimmed with path=repository.path %}
+             value="{% trans "Remove Repository" %}"/>
-                      Remove Repository
-                    {% endblocktrans %}"/>
       <a class="abort btn btn-sm btn-default"
          href="{% url 'backups:index' %}">
         {% trans "Abort" %}

plinth/modules/backups/templates/backups_repository_test.html

@@ -1,33 +0,0 @@
-{% extends "base.html" %}
-{% comment %}
-#
-# This file is part of FreedomBox.
-#
-# This program is free software: you can redistribute it and/or modify
-# it under the terms of the GNU Affero General Public License as
-# published by the Free Software Foundation, either version 3 of the
-# License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU Affero General Public License for more details.
-#
-# You should have received a copy of the GNU Affero General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-#
-{% endcomment %}
-
-{% load bootstrap %}
-{% load i18n %}
-
-{% block content %}
-
-  <h3>{{ title }}</h3>
-
-  {{ message }}
-  <div class="alert alert-warning" role="alert">
-    {{ error }}
-  </div>
-
-{% endblock %}

plinth/modules/backups/urls.py

@@ -23,7 +23,7 @@ from django.conf.urls import url
 from .views import IndexView, CreateArchiveView, AddRepositoryView, \
         DeleteArchiveView, DownloadArchiveView, RemoveRepositoryView, \
         mount_repository, umount_repository, UploadArchiveView, \
-        RestoreArchiveView, RestoreFromUploadView, TestRepositoryView
+        RestoreArchiveView, RestoreFromUploadView
 
 urlpatterns = [
     url(r'^sys/backups/$', IndexView.as_view(), name='index'),
@@ -39,8 +39,6 @@ urlpatterns = [
         RestoreFromUploadView.as_view(), name='restore-from-upload'),
     url(r'^sys/backups/repositories/add$',
         AddRepositoryView.as_view(), name='repository-add'),
-    url(r'^sys/backups/repositories/test/$',
-        TestRepositoryView.as_view(), name='repository-test'),
     url(r'^sys/backups/repositories/delete/(?P<uuid>[^/]+)/$',
         RemoveRepositoryView.as_view(), name='repository-remove'),
     url(r'^sys/backups/repositories/mount/(?P<uuid>[^/]+)/$',

plinth/modules/backups/views.py

@@ -35,14 +35,14 @@ from django.utils.translation import ugettext as _
 from django.utils.translation import ugettext_lazy
 from django.views.generic import View, FormView, TemplateView
 
-from plinth.errors import PlinthError, ActionError
+from plinth.errors import PlinthError
 from plinth.modules import backups, storage
 
 from . import api, forms, SESSION_PATH_VARIABLE, ROOT_REPOSITORY
 from .repository import BorgRepository, SshBorgRepository, get_repository, \
         get_ssh_repositories
 from .decorators import delete_tmp_backup_file
-from .errors import BorgError, BorgRepositoryDoesNotExistError
+from .errors import BorgRepositoryDoesNotExistError
 
 logger = logging.getLogger(__name__)
 
@@ -235,7 +235,6 @@ class RestoreArchiveView(BaseRestoreView):
     def form_valid(self, form):
         """Restore files from the archive on valid form submission."""
         repository = get_repository(self.kwargs['uuid'])
-        import ipdb; ipdb.set_trace()
         repository.restore_archive(self.kwargs['name'],
                                    form.cleaned_data['selected_apps'])
         return super().form_valid(form)
@@ -279,30 +278,6 @@ class AddRepositoryView(SuccessMessageMixin, FormView):
         return super().form_valid(form)
 
 
-class TestRepositoryView(TemplateView):
-    """View to create a new repository."""
-    template_name = 'backups_repository_test.html'
-
-    def post(self, request):
-        # TODO: add support for borg encryption and ssh keyfile
-        context = self.get_context_data()
-        credentials = {
-            'ssh_password': request.POST['backups-ssh_password'],
-        }
-        repository = SshBorgRepository(path=request.POST['backups-repository'],
-                                       credentials=credentials)
-
-        try:
-            repo_info = repository.get_info()
-            context["message"] = repo_info
-        except BorgError as err:
-            context["error"] = str(err)
-        except ActionError as err:
-            context["error"] = str(err)
-
-        return self.render_to_response(context)
-
-
 class RemoveRepositoryView(SuccessMessageMixin, TemplateView):
     """View to delete a repository."""
     template_name = 'backups_repository_remove.html'