From 8e698987de35ffcb75b49f477dbf7b4656d5c061 Mon Sep 17 00:00:00 2001 From: Veiko Aasa Date: Fri, 17 Jan 2020 13:15:27 +0200 Subject: [PATCH] deluge: Allow to set a download directory - add directory selection form to the app configuration page - add debian-deluged user to the freedombox-share group - storage: new validator parameter check-creatable (because deluged is able to create subdirectories) Signed-off-by: Veiko Aasa Reviewed-by: James Valleroy --- actions/deluge | 34 ++++++++++ actions/storage | 29 ++++++-- plinth/modules/deluge/__init__.py | 5 +- plinth/modules/deluge/forms.py | 37 +++++++++++ plinth/modules/deluge/urls.py | 10 +-- plinth/modules/deluge/views.py | 69 ++++++++++++++++++++ plinth/modules/storage/forms.py | 10 ++- plinth/modules/storage/tests/test_storage.py | 18 ++++- 8 files changed, 195 insertions(+), 17 deletions(-) create mode 100644 plinth/modules/deluge/forms.py create mode 100644 plinth/modules/deluge/views.py diff --git a/actions/deluge b/actions/deluge index a3f7436a9..18585bd22 100755 --- a/actions/deluge +++ b/actions/deluge @@ -20,6 +20,7 @@ Configuration helper for BitTorrent web client. """ import argparse +import json import os import shutil import subprocess @@ -80,6 +81,16 @@ def parse_arguments(): subparsers.add_parser('setup', help='Setup deluge') + subparsers.add_parser('get-configuration', + help='Return the current configuration') + + subparser = subparsers.add_parser('set-configuration', + help='Set the configuration parameter') + subparser.add_argument('parameter', + help='Name of the configuration parameter') + subparser.add_argument('value', + help='Value of the configuration parameter') + subparsers.required = True return parser.parse_args() @@ -141,6 +152,29 @@ def _set_deluged_daemon_options(): aug.save() +def subcommand_get_configuration(_): + """Return the current deluged configuration in JSON format.""" + deluged_conf_file = os.path.join(DELUGE_CONF_DIR, 'core.conf') + if config is None: + script = 'from deluge import config;\ + conf = config.Config(filename="{0}");\ + print(conf["download_location"])'.format(deluged_conf_file) + output = subprocess.check_output(['python2', '-c', script]).decode() + download_location = output.strip() + else: + conf = config.Config(filename=deluged_conf_file) + download_location = conf["download_location"] + + print(json.dumps({'download_location': download_location})) + + +def subcommand_set_configuration(arguments): + """Set the deluged configuration.""" + if arguments.parameter != 'download_location': + return + _set_configuration('core.conf', arguments.parameter, arguments.value) + + def subcommand_setup(_): """Perform initial setup for deluge.""" diff --git a/actions/storage b/actions/storage index 1147d5104..181f431f7 100755 --- a/actions/storage +++ b/actions/storage @@ -56,6 +56,9 @@ def parse_arguments(): help='Validate a directory') subparser.add_argument('--path', help='Path of the directory', required=True) + subparser.add_argument('--check-creatable', required=False, default=False, + action='store_true', + help='Check that the directory is creatable') subparser.add_argument('--check-writable', required=False, default=False, action='store_true', help='Check that the directory is writable') @@ -329,14 +332,32 @@ def subcommand_usage_info(_): def subcommand_validate_directory(arguments): """Validate a directory""" directory = arguments.path - if not os.path.exists(directory): - print('ValidationError: 1') + + def part_exists(path): + """Returns part of the path that exists.""" + if not path or os.path.exists(path): + return path + return part_exists(os.path.dirname(path)) + + if arguments.check_creatable: + directory = part_exists(directory) + if not directory: + directory = '.' + else: + if not os.path.exists(directory): + # doesn't exist + print('ValidationError: 1') + if not os.path.isdir(directory): + # is not a directory print('ValidationError: 2') if not os.access(directory, os.R_OK): + # is not readable print('ValidationError: 3') - if arguments.check_writable and not os.access(directory, os.W_OK): - print('ValidationError: 4') + if arguments.check_writable or arguments.check_creatable: + if not os.access(directory, os.W_OK): + # is not writable + print('ValidationError: 4') def main(): diff --git a/plinth/modules/deluge/__init__.py b/plinth/modules/deluge/__init__.py index 320e46c0d..5e3981d3c 100644 --- a/plinth/modules/deluge/__init__.py +++ b/plinth/modules/deluge/__init__.py @@ -26,11 +26,11 @@ from plinth import frontpage, menu from plinth.daemon import Daemon from plinth.modules.apache.components import Webserver from plinth.modules.firewall.components import Firewall -from plinth.modules.users import register_group +from plinth.modules.users import register_group, add_user_to_share_group from .manifest import backup, clients # noqa, pylint: disable=unused-import -version = 5 +version = 6 managed_services = ['deluged', 'deluge-web'] @@ -109,4 +109,5 @@ def setup(helper, old_version=None): """Install and configure the module.""" helper.install(managed_packages) helper.call('post', actions.superuser_run, 'deluge', ['setup']) + add_user_to_share_group(reserved_usernames[0]) helper.call('post', app.enable) diff --git a/plinth/modules/deluge/forms.py b/plinth/modules/deluge/forms.py new file mode 100644 index 000000000..b786a803e --- /dev/null +++ b/plinth/modules/deluge/forms.py @@ -0,0 +1,37 @@ +# +# 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 . +# +""" +Forms for Deluge app. +""" + +from django.utils.translation import ugettext_lazy as _ + +from plinth.modules.deluge import reserved_usernames +from plinth.modules.storage.forms import (DirectorySelectForm, + DirectoryValidator) + + +class DelugeForm(DirectorySelectForm): + """Deluge configuration form""" + + def __init__(self, *args, **kw): + validator = DirectoryValidator(username=reserved_usernames[0], + check_creatable=True) + super(DelugeForm, self).__init__( + title=_('Download directory'), + default='/var/lib/deluged/Downloads/', validator=validator, *args, + **kw) diff --git a/plinth/modules/deluge/urls.py b/plinth/modules/deluge/urls.py index 4da1fa6dc..8b4a72fb6 100644 --- a/plinth/modules/deluge/urls.py +++ b/plinth/modules/deluge/urls.py @@ -20,14 +20,8 @@ URLs for the Deluge module. from django.conf.urls import url -from plinth.modules import deluge -from plinth.views import AppView +from .views import DelugeAppView urlpatterns = [ - url( - r'^apps/deluge/$', - AppView.as_view(name=deluge.name, description=deluge.description, - clients=deluge.clients, app_id='deluge', - manual_page=deluge.manual_page, - icon_filename=deluge.icon_filename), name='index'), + url(r'^apps/deluge/$', DelugeAppView.as_view(), name='index') ] diff --git a/plinth/modules/deluge/views.py b/plinth/modules/deluge/views.py new file mode 100644 index 000000000..1a15149a9 --- /dev/null +++ b/plinth/modules/deluge/views.py @@ -0,0 +1,69 @@ +# +# 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 . +# +""" +Django views for Deluge. +""" + +import json +import os + +from django.contrib import messages +from django.utils.translation import ugettext as _ + +from plinth import actions, views +from plinth.modules import deluge + +from .forms import DelugeForm + + +class DelugeAppView(views.AppView): + """Serve configuration page.""" + clients = deluge.clients + name = deluge.name + description = deluge.description + diagnostics_module_name = 'deluge' + form_class = DelugeForm + app_id = 'deluge' + manual_page = deluge.manual_page + icon_filename = deluge.icon_filename + + def get_initial(self): + """Get current Deluge server settings.""" + status = super().get_initial() + configuration = json.loads( + actions.superuser_run('deluge', ['get-configuration'])) + status['storage_path'] = os.path.normpath( + configuration['download_location']) + return status + + def form_valid(self, form): + """Apply the changes submitted in the form.""" + old_status = form.initial + new_status = form.cleaned_data + + # don't change the configuration if the application was disabled + if new_status['is_enabled'] or not old_status['is_enabled']: + if old_status['storage_path'] != new_status['storage_path']: + new_configuration = [ + 'download_location', new_status['storage_path'] + ] + + actions.superuser_run( + 'deluge', ['set-configuration'] + new_configuration) + messages.success(self.request, _('Configuration updated')) + + return super().form_valid(form) diff --git a/plinth/modules/storage/forms.py b/plinth/modules/storage/forms.py index a22e48195..87dd85d1c 100644 --- a/plinth/modules/storage/forms.py +++ b/plinth/modules/storage/forms.py @@ -60,14 +60,18 @@ def is_module_enabled(name): class DirectoryValidator: username = None check_writable = False + check_creatable = False add_user_to_share_group = False service_to_restart = None - def __init__(self, username=None, check_writable=None): + def __init__(self, username=None, check_writable=None, + check_creatable=None): if username is not None: self.username = username if check_writable is not None: self.check_writable = check_writable + if check_creatable is not None: + self.check_creatable = check_creatable def __call__(self, value): """Validate a directory.""" @@ -75,7 +79,9 @@ class DirectoryValidator: raise ValidationError(_('Invalid directory name.'), 'invalid') command = ['validate-directory', '--path', value] - if self.check_writable: + if self.check_creatable: + command.append('--check-creatable') + elif self.check_writable: command.append('--check-writable') if self.username: diff --git a/plinth/modules/storage/tests/test_storage.py b/plinth/modules/storage/tests/test_storage.py index 3e8e10461..9e4a99974 100644 --- a/plinth/modules/storage/tests/test_storage.py +++ b/plinth/modules/storage/tests/test_storage.py @@ -247,11 +247,14 @@ class TestActions: subprocess.run(command, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=True) - def assert_validate_directory(self, path, error, check_writable=False): + def assert_validate_directory(self, path, error, check_writable=False, + check_creatable=False): """Perform directory validation checks.""" action_command = ['storage', 'validate-directory', '--path', path] if check_writable: action_command += ['--check-writable'] + if check_creatable: + action_command += ['--check-creatable'] proc = self.call_action(action_command, stderr=subprocess.PIPE, stdout=subprocess.PIPE) output = proc.stdout.decode() @@ -291,3 +294,16 @@ class TestActions: """Test that directory writable validation returns expected output.""" self.assert_validate_directory(directory['path'], directory['error'], check_writable=True) + + @pytest.mark.usefixtures('needs_not_root') + @pytest.mark.parametrize('directory', [{ + 'path': '/var/lib/plinth_storage_test_not_exists', + 'error': '4' + }, { + 'path': '/tmp/plint_storage_test_not_exists', + 'error': '' + }]) + def test_validate_directory_creatable(self, directory): + """Test that directory creatable validation returns expected output.""" + self.assert_validate_directory(directory['path'], directory['error'], + check_creatable=True)