diff --git a/plinth/modules/backups/__init__.py b/plinth/modules/backups/__init__.py index 249930f58..357aa3c7b 100644 --- a/plinth/modules/backups/__init__.py +++ b/plinth/modules/backups/__init__.py @@ -18,7 +18,6 @@ FreedomBox app to manage backup archives. """ -import errno import json import os @@ -26,6 +25,7 @@ from django.utils.text import get_valid_filename from django.utils.translation import ugettext_lazy as _ from plinth import actions +from plinth.errors import PlinthError from plinth.menu import main_menu from plinth.modules import storage @@ -127,6 +127,16 @@ def get_export_locations(): return locations +def get_location_path(label, locations=None): + """Returns the location path given a disk label""" + if locations is None: + locations = get_export_locations() + for (location_path, location_label) in locations: + if location_label == label: + return location_path + raise PlinthError("Could not find path of location %s" % label) + + def get_export_files(): """Return a dict of exported backup archives found in storage locations.""" locations = get_export_locations() @@ -145,14 +155,8 @@ def get_archive_path(location, archive_name): def find_exported_archive(disk_label, archive_name): """Return the full path for the exported archive file.""" - locations = get_export_locations() - for (location_path, location_name) in locations: - if location_name == disk_label: - return get_archive_path(location_path, archive_name) - - - raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), - archive_name) + location_path = get_location_path(disk_label) + return get_archive_path(location_path, archive_name) def get_export_apps(filename): diff --git a/plinth/modules/backups/forms.py b/plinth/modules/backups/forms.py index b23fb8a3f..f97cf8534 100644 --- a/plinth/modules/backups/forms.py +++ b/plinth/modules/backups/forms.py @@ -26,7 +26,7 @@ from django.core.validators import FileExtensionValidator from django.utils.translation import ugettext_lazy as _ from . import backups as backups_api -from . import get_export_locations, get_archive_path +from . import get_export_locations, get_archive_path, get_location_path class CreateArchiveForm(forms.Form): @@ -93,16 +93,21 @@ class UploadForm(forms.Form): def __init__(self, *args, **kwargs): """Initialize the form with location choices.""" super().__init__(*args, **kwargs) - # TODO: write a test that assures the format of get_export_locations - self.fields['location'].choices = get_export_locations() + locations = get_export_locations() + # users should only be able to select a location name -- don't + # provide paths as a form input for security reasons + location_labels = [(location[1], location[1]) for location in locations] + self.fields['location'].choices = location_labels def clean(self): - """Check that the uploaded file does not exist""" + """Check that the uploaded file does not yet exist.""" cleaned_data = super().clean() file = cleaned_data.get('file') - location = cleaned_data.get('location') + location_label = cleaned_data.get('location') + location_path = get_location_path(location_label) + # if other errors occured before, 'file' won't be in cleaned_data if (file and file.name): - filepath = get_archive_path(location, file.name) + filepath = get_archive_path(location_path, file.name) if os.path.exists(filepath): raise forms.ValidationError("File %s already exists" % file.name) else: diff --git a/plinth/modules/backups/tests/test_backups.py b/plinth/modules/backups/tests/test_backups.py index b03501499..e2e6b82c6 100644 --- a/plinth/modules/backups/tests/test_backups.py +++ b/plinth/modules/backups/tests/test_backups.py @@ -19,13 +19,18 @@ Tests for backups module. """ import collections +from django.core.files.uploadedfile import SimpleUploadedFile import unittest from unittest.mock import call, patch, MagicMock + +from plinth.errors import PlinthError from plinth.module_loader import load_modules from ..backups import validate, Packet, backup_apps, restore_apps, \ get_all_apps_for_backup, _get_apps_in_order, _get_manifests, \ _lockdown_apps, _shutdown_services, _restore_services +from .. import get_export_locations, get_location_path +from ..forms import UploadForm def _get_test_manifest(name): @@ -46,8 +51,8 @@ def _get_test_manifest(name): }) -class TestBackups(unittest.TestCase): - """Test cases for backups module.""" +class TestBackupProcesses(unittest.TestCase): + """Test cases for backup processes""" def test_packet_process_manifests(self): """Test that directories/files are collected from manifests.""" @@ -83,6 +88,12 @@ class TestBackups(unittest.TestCase): assert isinstance(apps, list) # apps may be empty, if no apps supporting backup are installed. + def test_export_locations(self): + """Check get_export_locations returns a list of tuples of length 2.""" + locations = get_export_locations() + assert(len(locations)) + assert(len(locations[0]) == 2) + def test__get_apps_in_order(self): """Test that apps are listed in correct dependency order.""" load_modules() @@ -148,3 +159,34 @@ class TestBackups(unittest.TestCase): 'app_name': 'b', 'app': None, 'was_running': False} _restore_services(original_state) run.assert_called_once_with('service', ['start', 'a']) + + +class TestBackupModule(unittest.TestCase): + """Tests of the backups django module, like views or forms.""" + + def test_get_location_path(self): + """Test the 'get_location_path' method""" + locations = [('/var/www', 'dummy location'), ('/etc', 'dangerous')] + location = get_location_path('dummy location', locations) + self.assertEquals(location, locations[0][0]) + # verify that an unknown location raises an error + with self.assertRaises(PlinthError): + get_location_path('unknown location', locations) + + def test_file_upload(self): + locations = get_export_locations() + location_name = locations[0][1] + post_data = {'location': location_name} + + # posting a video should fail + video_file = SimpleUploadedFile("video.mp4", b"file_content", + content_type="video/mp4") + form = UploadForm(post_data, {'file': video_file}) + self.assertFalse(form.is_valid()) + + # posting an archive file should work + archive_file = SimpleUploadedFile("backup.tar.gz", b"file_content", + content_type="application/gzip") + form = UploadForm(post_data, {'file': archive_file}) + form.is_valid() + self.assertTrue(form.is_valid()) diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py index 754e5deaf..f86ce49cb 100644 --- a/plinth/modules/backups/views.py +++ b/plinth/modules/backups/views.py @@ -138,7 +138,7 @@ class UploadArchiveView(SuccessMessageMixin, FormView): prefix = 'backups' template_name = 'backups_upload.html' success_url = reverse_lazy('backups:index') - success_message = _('Archive uploaded.') + success_message = _('Backup file uploaded.') def get_context_data(self, **kwargs): """Return additional context for rendering the template."""