diff --git a/actions/tor b/actions/tor index b1e779992..6189562f9 100755 --- a/actions/tor +++ b/actions/tor @@ -40,8 +40,6 @@ def parse_arguments(): subparsers.add_parser('get-status', help='Get Tor status in JSON format') configure = subparsers.add_parser('configure', help='Configure Tor') - configure.add_argument('--service', choices=['enable', 'disable'], - help='Configure Tor service') configure.add_argument('--relay', choices=['enable', 'disable'], help='Configure relay') configure.add_argument('--bridge-relay', choices=['enable', 'disable'], @@ -57,6 +55,9 @@ def parse_arguments(): configure.add_argument('--upstream-bridges', help='Set list of upstream bridges to use') + subparsers.add_parser('update-ports', + help='Update firewall ports based on what Tor uses') + subparsers.add_parser('restart', help='Restart Tor') subparsers.required = True @@ -171,9 +172,6 @@ def subcommand_configure(arguments): """Configure Tor.""" aug = augeas_load() - if arguments.service == 'disable': - _disable() - _use_upstream_bridges(arguments.use_upstream_bridges, aug=aug) if arguments.use_upstream_bridges == 'enable': @@ -190,15 +188,17 @@ def subcommand_configure(arguments): elif arguments.hidden_service == 'disable': _disable_hs(aug=aug) - if arguments.service == 'enable': - _enable() - if arguments.apt_transport_tor == 'enable': _enable_apt_transport_tor() elif arguments.apt_transport_tor == 'disable': _disable_apt_transport_tor() +def subcommand_update_ports(_): + """Update firewall ports based on what Tor uses.""" + _update_ports() + + def subcommand_restart(_): """Restart Tor.""" if (action_utils.service_is_enabled('tor@plinth', strict_check=True) diff --git a/plinth/modules/tor/__init__.py b/plinth/modules/tor/__init__.py index 8ab11904a..279f7f8c7 100644 --- a/plinth/modules/tor/__init__.py +++ b/plinth/modules/tor/__init__.py @@ -106,6 +106,19 @@ class TorApp(app_module.App): domain_type='domain-type-tor', name=hostname, services=services) + def enable(self): + """Enable the app and update firewall ports.""" + super().enable() + actions.superuser_run('tor', ['update-ports']) + update_hidden_service_domain() + + def disable(self): + """Disable APT use of Tor before disabling.""" + actions.superuser_run('tor', + ['configure', '--apt-transport-tor', 'disable']) + super().disable() + update_hidden_service_domain() + def diagnose(self): """Run diagnostics and return the results.""" results = super().diagnose() diff --git a/plinth/modules/tor/forms.py b/plinth/modules/tor/forms.py index 422456d68..4267869c9 100644 --- a/plinth/modules/tor/forms.py +++ b/plinth/modules/tor/forms.py @@ -72,7 +72,6 @@ def bridges_validator(bridges): class TorForm(forms.Form): # pylint: disable=W0232 """Tor configuration form.""" - enabled = forms.BooleanField(label=_('Enable Tor'), required=False) use_upstream_bridges = forms.BooleanField( label=_('Use upstream bridges to connect to Tor network'), required=False, help_text=_( diff --git a/plinth/modules/tor/templates/tor.html b/plinth/modules/tor/templates/tor.html index 70fa6a5fc..812cf7c6a 100644 --- a/plinth/modules/tor/templates/tor.html +++ b/plinth/modules/tor/templates/tor.html @@ -8,49 +8,36 @@ {% load static %} {% block status %} - {% if config_running %} -

{% trans "Status" %}

+ {{ block.super }} -

- - {% trans "Tor configuration is being updated" %} -

- {% else %} - {{ block.super }} - - {% if status.hs_enabled %} -
- - - - - - - - - - - - - - - -
{% trans "Onion Service" %}{% trans "Status" %}{% trans "Ports" %}
{{ status.hs_hostname }}{{ status.hs_status }} - {{ status.hs_services|join:', ' }} -
-
- {% endif %} + {% if status.hs_enabled %} +
+ + + + + + + + + + + + + + + +
{% trans "Onion Service" %}{% trans "Status" %}{% trans "Ports" %}
{{ status.hs_hostname }}{{ status.hs_status }} + {{ status.hs_services|join:', ' }} +
+
{% endif %} {% endblock %} {% block internal_zone %} - {% if not config_running %} - {{ block.super }} - {% endif %} + {{ block.super }} {% endblock %} {% block page_js %} - - {% endblock %} diff --git a/plinth/modules/tor/urls.py b/plinth/modules/tor/urls.py index 4b7c31116..aa39a22aa 100644 --- a/plinth/modules/tor/urls.py +++ b/plinth/modules/tor/urls.py @@ -8,5 +8,5 @@ from django.urls import re_path from . import views urlpatterns = [ - re_path(r'^apps/tor/$', views.index, name='index'), + re_path(r'^apps/tor/$', views.TorAppView.as_view(), name='index'), ] diff --git a/plinth/modules/tor/views.py b/plinth/modules/tor/views.py index f4c42d32d..135a6f01b 100644 --- a/plinth/modules/tor/views.py +++ b/plinth/modules/tor/views.py @@ -2,84 +2,94 @@ """ FreedomBox app for configuring Tor. """ -from django.contrib import messages -from django.template.response import TemplateResponse -from django.utils.translation import gettext as _ + +import logging + +from django.utils.translation import gettext_noop +from django.views.generic.edit import FormView from plinth import actions from plinth import app as app_module -from plinth.errors import ActionError +from plinth import operation as operation_module from plinth.modules import tor -from plinth.modules.firewall.components import (Firewall, - get_port_forwarding_info) +from plinth.views import AppView from . import utils as tor_utils from .forms import TorForm config_process = None +logger = logging.getLogger(__name__) -def index(request): - """Serve configuration page.""" - if config_process: - _collect_config_result(request) +class TorAppView(AppView): + """Show Tor app main page.""" - status = tor_utils.get_status() - form = None + app_id = 'tor' + template_name = 'tor.html' + form_class = TorForm + prefix = 'tor' - if request.method == 'POST': - form = TorForm(request.POST, prefix='tor') - # pylint: disable=E1101 - if form.is_valid(): - _apply_changes(request, status, form.cleaned_data) - status = tor_utils.get_status() - form = TorForm(initial=status, prefix='tor') + status = None + + def get_initial(self): + """Return the values to fill in the form.""" + if not self.status: + self.status = tor_utils.get_status() + + initial = super().get_initial() + initial.update(self.status) + return initial + + def get_context_data(self, *args, **kwargs): + """Add additional context data for template.""" + if not self.status: + self.status = tor_utils.get_status() + + context = super().get_context_data(*args, **kwargs) + context['status'] = self.status + return context + + def form_valid(self, form): + """Configure tor app on successful form submission.""" + operation_module.manager.new(self.app_id, + gettext_noop('Updating configuration'), + _apply_changes, + [form.initial, form.cleaned_data], + show_notification=False) + # Skip check for 'Settings unchanged' message by calling grandparent + return super(FormView, self).form_valid(form) + + +def _apply_changes(old_status, new_status): + """Try to apply changes and handle errors.""" + logger.info('tor: applying configuration changes') + exception_to_update = None + message = None + try: + __apply_changes(old_status, new_status) + except Exception as exception: + exception_to_update = exception + message = gettext_noop('Error configuring app: {error}').format( + error=exception) else: - form = TorForm(initial=status, prefix='tor') + message = gettext_noop('Configuration updated.') + + logger.info('tor: configuration changes completed') + operation = operation_module.Operation.get_operation() + operation.on_update(message, exception_to_update) + + +def __apply_changes(old_status, new_status): + """Apply the changes.""" + needs_restart = True + arguments = [] app = app_module.App.get('tor') - return TemplateResponse( - request, 'tor.html', { - 'app_id': 'tor', - 'app_info': app.info, - 'status': status, - 'config_running': bool(config_process), - 'form': form, - 'firewall': app.get_components_of_type(Firewall), - 'has_diagnostics': True, - 'is_enabled': status['enabled'], - 'is_running': status['is_running'], - 'port_forwarding_info': get_port_forwarding_info(app), - 'refresh_page_sec': 3 if bool(config_process) else None, - }) - - -def _apply_changes(request, old_status, new_status): - """Try to apply changes and handle errors.""" - try: - __apply_changes(request, old_status, new_status) - except ActionError as exception: - messages.error( - request, - _('Action error: {0} [{1}] [{2}]').format(exception.args[0], - exception.args[1], - exception.args[2])) - - -def __apply_changes(request, old_status, new_status): - """Apply the changes.""" - global config_process - if config_process: - # Already running a configuration task - return - - needs_restart = False - arguments = [] + is_enabled = app.is_enabled() if old_status['relay_enabled'] != new_status['relay_enabled']: arg_value = 'enable' if new_status['relay_enabled'] else 'disable' arguments.extend(['--relay', arg_value]) - needs_restart = True if old_status['bridge_relay_enabled'] != \ new_status['bridge_relay_enabled']: @@ -87,80 +97,33 @@ def __apply_changes(request, old_status, new_status): if not new_status['bridge_relay_enabled']: arg_value = 'disable' arguments.extend(['--bridge-relay', arg_value]) - needs_restart = True if old_status['hs_enabled'] != new_status['hs_enabled']: arg_value = 'enable' if new_status['hs_enabled'] else 'disable' arguments.extend(['--hidden-service', arg_value]) - needs_restart = True if old_status['apt_transport_tor_enabled'] != \ new_status['apt_transport_tor_enabled']: arg_value = 'disable' - if new_status['enabled'] and new_status['apt_transport_tor_enabled']: + if is_enabled and new_status['apt_transport_tor_enabled']: arg_value = 'enable' arguments.extend(['--apt-transport-tor', arg_value]) + needs_restart = False if old_status['use_upstream_bridges'] != \ new_status['use_upstream_bridges']: - arg_value = 'disable' - if new_status['enabled'] and new_status['use_upstream_bridges']: - arg_value = 'enable' + arg_value = 'enable' if new_status[ + 'use_upstream_bridges'] else 'disable' arguments.extend(['--use-upstream-bridges', arg_value]) - needs_restart = True if old_status['upstream_bridges'] != new_status['upstream_bridges']: arguments.extend( ['--upstream-bridges', new_status['upstream_bridges']]) - needs_restart = True - - if old_status['enabled'] != new_status['enabled']: - arg_value = 'enable' if new_status['enabled'] else 'disable' - arguments.extend(['--service', arg_value]) - # XXX: Perform app enable/disable within the background process - app = app_module.App.get('tor') - if new_status['enabled']: - app.enable() - else: - app.disable() - - config_process = actions.superuser_run('tor', - ['configure'] + arguments, - run_in_background=True) - return if arguments: actions.superuser_run('tor', ['configure'] + arguments) - if not needs_restart: - messages.success(request, _('Configuration updated.')) - if needs_restart and new_status['enabled']: - config_process = actions.superuser_run('tor', ['restart'], - run_in_background=True) - - if not arguments: - messages.info(request, _('Setting unchanged')) - - -def _collect_config_result(request): - """Handle config process completion.""" - global config_process - if not config_process: - return - - return_code = config_process.poll() - - # Config process is not complete yet - if return_code is None: - return - - status = tor_utils.get_status() - - tor.update_hidden_service_domain(status) - - if not return_code: - messages.success(request, _('Configuration updated.')) - else: - messages.error(request, _('An error occurred during configuration.')) - - config_process = None + if needs_restart and is_enabled: + actions.superuser_run('tor', ['restart']) + status = tor_utils.get_status() + tor.update_hidden_service_domain(status) diff --git a/plinth/tests/functional/__init__.py b/plinth/tests/functional/__init__.py index 5760617f2..e588e1dfe 100644 --- a/plinth/tests/functional/__init__.py +++ b/plinth/tests/functional/__init__.py @@ -34,12 +34,9 @@ logger = logging.getLogger(__name__) base_url = config['DEFAULT']['url'] _app_checkbox_id = { - 'tor': 'id_tor-enabled', 'openvpn': 'id_openvpn-enabled', } -_apps_with_loaders = ['tor'] - # unlisted sites just use '/' + site_name as url _site_url = { 'wiki': '/ikiwiki', @@ -228,9 +225,6 @@ def change_checkbox_status(browser, app_name, checkbox_id, submit(browser, form_class='form-configuration') - if app_name in _apps_with_loaders: - wait_for_config_update(browser, app_name) - def wait_for_config_update(browser, app_name): """Wait until the configuration update progress goes away. @@ -241,7 +235,7 @@ def wait_for_config_update(browser, app_name): """ script = 'return (document.readyState == "complete") && ' \ - '(!Boolean(document.querySelector(".running-status.loading")));' + '(!Boolean(document.querySelector(".app-operation")));' while not browser.execute_script(script): time.sleep(0.1) @@ -410,9 +404,6 @@ def _change_app_status(browser, app_name, change_status_to='enabled'): change_checkbox_status(browser, app_name, checkbox_id, change_status_to) - if app_name in _apps_with_loaders: - wait_for_config_update(browser, app_name) - def app_enable(browser, app_name): nav_to_module(browser, app_name)