From 8c69858d433feeeedc04872509343dfc67840c82 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Sun, 8 Sep 2024 16:39:26 -0700 Subject: [PATCH] config, names: Move setting hostname from config to names Tests: - Config app description is as expected. - Config form does not show hostname anymore. - Submitting the form with changes works. - Names app has correct link for configuring Local Domain Name. Clicking it takes to page for setting hostname. - Avahi shows the current .local domain correctly in Names app. - Change hostname form shows correct value for current hostname. - Change hostname form sets the value for hostname properly. - Page title is correct. - Validations works. - Pre/post hostname change signals are sent properly - Success message as shown expected - hostnamectl shows the set domain - If domain name is not set, downloaded OpenVPN profile shows hostname. - Unit tests work. - Functional tests on names/config/avahi apps work. Signed-off-by: Sunil Mohan Adapa Reviewed-by: Veiko Aasa --- plinth/modules/avahi/__init__.py | 4 +- plinth/modules/config/__init__.py | 7 +--- plinth/modules/config/forms.py | 17 -------- plinth/modules/config/privileged.py | 9 ----- plinth/modules/config/tests/test_config.py | 28 ------------- .../modules/config/tests/test_functional.py | 11 ------ plinth/modules/config/views.py | 38 +----------------- plinth/modules/names/__init__.py | 34 +++++++++++++++- plinth/modules/names/forms.py | 23 +++++++++++ plinth/modules/names/privileged.py | 10 +++++ plinth/modules/names/tests/test_forms.py | 24 ++++++++++++ plinth/modules/names/tests/test_functional.py | 28 +++++++++++++ plinth/modules/names/urls.py | 2 + plinth/modules/names/views.py | 39 ++++++++++++++++++- plinth/modules/openvpn/views.py | 4 +- plinth/tests/functional/__init__.py | 15 ++++--- 16 files changed, 173 insertions(+), 120 deletions(-) create mode 100644 plinth/modules/names/tests/test_forms.py create mode 100644 plinth/modules/names/tests/test_functional.py diff --git a/plinth/modules/avahi/__init__.py b/plinth/modules/avahi/__init__.py index 8226a0bd4..2db82ba31 100644 --- a/plinth/modules/avahi/__init__.py +++ b/plinth/modules/avahi/__init__.py @@ -7,8 +7,8 @@ from plinth import app as app_module from plinth import cfg, menu from plinth.daemon import Daemon from plinth.modules.backups.components import BackupRestore -from plinth.modules.config import get_hostname from plinth.modules.firewall.components import Firewall +from plinth.modules.names import get_hostname from plinth.modules.names.components import DomainType from plinth.package import Packages from plinth.privileged import service as service_privileged @@ -58,7 +58,7 @@ class AvahiApp(app_module.App): self.add(packages) domain_type = DomainType('domain-type-local', - _('Local Network Domain'), 'config:index', + _('Local Network Domain'), 'names:hostname', can_have_certificate=False) self.add(domain_type) diff --git a/plinth/modules/config/__init__.py b/plinth/modules/config/__init__.py index fb28c948f..dbc54d3d8 100644 --- a/plinth/modules/config/__init__.py +++ b/plinth/modules/config/__init__.py @@ -20,7 +20,7 @@ from . import privileged _description = [ _('Here you can set some general configuration options ' - 'like hostname, domain name, webserver home page etc.') + 'like domain name, webserver home page etc.') ] ADVANCED_MODE_KEY = 'advanced_mode' @@ -101,11 +101,6 @@ def get_domainname(): return '.'.join(fqdn.split('.')[1:]) -def get_hostname(): - """Return the hostname.""" - return socket.gethostname() - - def home_page_url2scid(url): """Return the shortcut ID of the given home page url.""" # url is None when the freedombox-apache-homepage configuration file does diff --git a/plinth/modules/config/forms.py b/plinth/modules/config/forms.py index a35044823..599ad7c82 100644 --- a/plinth/modules/config/forms.py +++ b/plinth/modules/config/forms.py @@ -46,23 +46,6 @@ def get_homepage_choices(): class ConfigurationForm(forms.Form): """Main system configuration form""" - # See: - # https://tools.ietf.org/html/rfc952 - # https://tools.ietf.org/html/rfc1035#section-2.3.1 - # https://tools.ietf.org/html/rfc1123#section-2 - # https://tools.ietf.org/html/rfc2181#section-11 - hostname = forms.CharField( - label=gettext_lazy('Hostname'), help_text=format_lazy( - gettext_lazy( - 'Hostname is the local name by which other devices on the ' - 'local network can reach your {box_name}. It must start and ' - 'end with an alphabet or a digit and have as interior ' - 'characters only alphabets, digits and hyphens. Total ' - 'length must be 63 characters or less.'), - box_name=gettext_lazy(cfg.box_name)), validators=[ - validators.RegexValidator(HOSTNAME_REGEX, - gettext_lazy('Invalid hostname')) - ], strip=True) domainname = forms.CharField( label=gettext_lazy('Domain Name'), help_text=format_lazy( diff --git a/plinth/modules/config/privileged.py b/plinth/modules/config/privileged.py index 953b8d639..17654562c 100644 --- a/plinth/modules/config/privileged.py +++ b/plinth/modules/config/privileged.py @@ -18,15 +18,6 @@ APACHE_HOMEPAGE_CONFIG = os.path.join(APACHE_CONF_ENABLED_DIR, JOURNALD_FILE = pathlib.Path('/etc/systemd/journald.conf.d/50-freedombox.conf') -@privileged -def set_hostname(hostname: str): - """Set system hostname using hostnamectl.""" - subprocess.run( - ['hostnamectl', 'set-hostname', '--transient', '--static', hostname], - check=True) - action_utils.service_restart('avahi-daemon') - - @privileged def set_domainname(domainname: str | None = None): """Set system domainname in /etc/hosts.""" diff --git a/plinth/modules/config/tests/test_config.py b/plinth/modules/config/tests/test_config.py index d03adc412..6ffb11832 100644 --- a/plinth/modules/config/tests/test_config.py +++ b/plinth/modules/config/tests/test_config.py @@ -15,34 +15,6 @@ from plinth.modules.config import (_home_page_scid2url, change_home_page, from plinth.modules.config.forms import ConfigurationForm -def test_hostname_field(): - """Test that hostname field accepts only valid hostnames.""" - valid_hostnames = [ - 'a', '0a', 'a0', 'AAA', '00', '0-0', 'example-hostname', 'example', - '012345678901234567890123456789012345678901234567890123456789012' - ] - invalid_hostnames = [ - '', '-', '-a', 'a-', '.a', 'a.', 'a.a', '?', 'a?a', - '0123456789012345678901234567890123456789012345678901234567890123' - ] - - for hostname in valid_hostnames: - form = ConfigurationForm({ - 'hostname': hostname, - 'domainname': 'example.com', - 'logging_mode': 'volatile' - }) - assert form.is_valid() - - for hostname in invalid_hostnames: - form = ConfigurationForm({ - 'hostname': hostname, - 'domainname': 'example.com', - 'logging_mode': 'volatile' - }) - assert not form.is_valid() - - def test_domainname_field(): """Test that domainname field accepts only valid domainnames.""" valid_domainnames = [ diff --git a/plinth/modules/config/tests/test_functional.py b/plinth/modules/config/tests/test_functional.py index aabebdf2b..1b1c949e4 100644 --- a/plinth/modules/config/tests/test_functional.py +++ b/plinth/modules/config/tests/test_functional.py @@ -19,12 +19,6 @@ def fixture_background(session_browser): functional.login(session_browser) -def test_change_hostname(session_browser): - """Test changing the hostname.""" - functional.set_hostname(session_browser, 'mybox') - assert _get_hostname(session_browser) == 'mybox' - - def test_change_domain_name(session_browser): """Test changing the domain name.""" functional.set_domain_name(session_browser, 'mydomain.example') @@ -45,11 +39,6 @@ def test_change_home_page(session_browser): assert _check_home_page_redirect(session_browser, 'plinth') -def _get_hostname(browser): - functional.nav_to_module(browser, 'config') - return browser.find_by_id('id_hostname').value - - def _get_domain_name(browser): functional.nav_to_module(browser, 'config') return browser.find_by_id('id_domainname').value diff --git a/plinth/modules/config/views.py b/plinth/modules/config/views.py index 7a21b988a..a94b76f8b 100644 --- a/plinth/modules/config/views.py +++ b/plinth/modules/config/views.py @@ -8,8 +8,7 @@ from django.utils.translation import gettext as _ from plinth import views from plinth.modules import config -from plinth.signals import (domain_added, domain_removed, post_hostname_change, - pre_hostname_change) +from plinth.signals import domain_added, domain_removed from . import privileged from .forms import ConfigurationForm @@ -26,7 +25,6 @@ class ConfigAppView(views.AppView): def get_initial(self): """Return the current status.""" return { - 'hostname': config.get_hostname(), 'domainname': config.get_domainname(), 'homepage': config.get_home_page(), 'advanced_mode': config.get_advanced_mode(), @@ -40,17 +38,6 @@ class ConfigAppView(views.AppView): is_changed = False - if old_status['hostname'] != new_status['hostname']: - try: - set_hostname(new_status['hostname']) - except Exception as exception: - messages.error( - self.request, - _('Error setting hostname: {exception}').format( - exception=exception)) - else: - messages.success(self.request, _('Hostname set')) - if old_status['domainname'] != new_status['domainname']: try: set_domainname(new_status['domainname'], @@ -100,29 +87,6 @@ class ConfigAppView(views.AppView): return super().form_valid(form) -def set_hostname(hostname): - """Set machine hostname and send signals before and after.""" - old_hostname = config.get_hostname() - domainname = config.get_domainname() - - # Hostname should be ASCII. If it's unicode but passed our - # valid_hostname check, convert - hostname = str(hostname) - - pre_hostname_change.send_robust(sender='config', old_hostname=old_hostname, - new_hostname=hostname) - - LOGGER.info('Changing hostname to - %s', hostname) - privileged.set_hostname(hostname) - - LOGGER.info('Setting domain name after hostname change - %s', domainname) - privileged.set_domainname(domainname) - - post_hostname_change.send_robust(sender='config', - old_hostname=old_hostname, - new_hostname=hostname) - - def set_domainname(domainname, old_domainname): """Set machine domain name to domainname.""" old_domainname = config.get_domainname() diff --git a/plinth/modules/names/__init__.py b/plinth/modules/names/__init__.py index 9ef138332..b86eb087e 100644 --- a/plinth/modules/names/__init__.py +++ b/plinth/modules/names/__init__.py @@ -4,6 +4,7 @@ FreedomBox app to configure name services. """ import logging +import socket import subprocess from django.utils.translation import gettext_lazy as _ @@ -17,7 +18,8 @@ from plinth.diagnostic_check import (DiagnosticCheck, from plinth.modules.backups.components import BackupRestore from plinth.package import Packages from plinth.privileged import service as service_privileged -from plinth.signals import domain_added, domain_removed +from plinth.signals import (domain_added, domain_removed, post_hostname_change, + pre_hostname_change) from plinth.utils import format_lazy from . import components, manifest, privileged @@ -158,6 +160,36 @@ def on_domain_removed(sender, domain_type, name='', **kwargs): ###################################################### +def get_hostname(): + """Return the hostname.""" + return socket.gethostname() + + +def set_hostname(hostname): + """Set machine hostname and send signals before and after.""" + from plinth.modules import config + from plinth.modules.config import privileged as config_privileged + + old_hostname = get_hostname() + domainname = config.get_domainname() + + # Hostname should be ASCII. If it's unicode but passed our + # valid_hostname check, convert + hostname = str(hostname) + + pre_hostname_change.send_robust(sender='names', old_hostname=old_hostname, + new_hostname=hostname) + + logger.info('Changing hostname to - %s', hostname) + privileged.set_hostname(hostname) + + logger.info('Setting domain name after hostname change - %s', domainname) + config_privileged.set_domainname(domainname) + + post_hostname_change.send_robust(sender='names', old_hostname=old_hostname, + new_hostname=hostname) + + def get_available_tls_domains(): """Return an iterator with all domains able to have a certificate.""" return (domain.name for domain in components.DomainName.list() diff --git a/plinth/modules/names/forms.py b/plinth/modules/names/forms.py index 8f89fa1d9..5337a5099 100644 --- a/plinth/modules/names/forms.py +++ b/plinth/modules/names/forms.py @@ -2,10 +2,14 @@ """Forms for the names app.""" from django import forms +from django.core import validators from django.utils.translation import gettext_lazy as _ +from plinth import cfg from plinth.utils import format_lazy +HOSTNAME_REGEX = r'^[a-zA-Z0-9]([-a-zA-Z0-9]{,61}[a-zA-Z0-9])?$' + class NamesConfigurationForm(forms.Form): """Form to configure names app.""" @@ -64,3 +68,22 @@ class NamesConfigurationForm(forms.Form): 'No.

Do not verify domain name ' 'resolutions.

', allow_markup=True)), ], initial='no') + + +class HostnameForm(forms.Form): + """Form to update system's hostname.""" + # See: + # https://tools.ietf.org/html/rfc952 + # https://tools.ietf.org/html/rfc1035#section-2.3.1 + # https://tools.ietf.org/html/rfc1123#section-2 + # https://tools.ietf.org/html/rfc2181#section-11 + hostname = forms.CharField( + label=_('Hostname'), help_text=format_lazy( + _('Hostname is the local name by which other devices on the local ' + 'network can reach your {box_name}. It must start and end with ' + 'an alphabet or a digit and have as interior characters only ' + 'alphabets, digits and hyphens. Total length must be 63 ' + 'characters or less.'), box_name=_(cfg.box_name)), validators=[ + validators.RegexValidator(HOSTNAME_REGEX, + _('Invalid hostname')) + ], strip=True) diff --git a/plinth/modules/names/privileged.py b/plinth/modules/names/privileged.py index 2870044d8..96deb95a7 100644 --- a/plinth/modules/names/privileged.py +++ b/plinth/modules/names/privileged.py @@ -2,6 +2,7 @@ """Configure Names App.""" import pathlib +import subprocess import augeas @@ -16,6 +17,15 @@ source_fallback_conf = pathlib.Path( '/etc/systemd/resolved.conf.d/freedombox-fallback.conf') +@privileged +def set_hostname(hostname: str): + """Set system hostname using hostnamectl.""" + subprocess.run( + ['hostnamectl', 'set-hostname', '--transient', '--static', hostname], + check=True) + action_utils.service_restart('avahi-daemon') + + @privileged def set_resolved_configuration(dns_fallback: bool | None = None, dns_over_tls: str | None = None, diff --git a/plinth/modules/names/tests/test_forms.py b/plinth/modules/names/tests/test_forms.py new file mode 100644 index 000000000..7faaa98ad --- /dev/null +++ b/plinth/modules/names/tests/test_forms.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +"""Tests for forms in names app.""" + +from ..forms import HostnameForm + + +def test_hostname_field(): + """Test that hostname field accepts only valid hostnames.""" + valid_hostnames = [ + 'a', '0a', 'a0', 'AAA', '00', '0-0', 'example-hostname', 'example', + '012345678901234567890123456789012345678901234567890123456789012' + ] + invalid_hostnames = [ + '', '-', '-a', 'a-', '.a', 'a.', 'a.a', '?', 'a?a', + '0123456789012345678901234567890123456789012345678901234567890123' + ] + + for hostname in valid_hostnames: + form = HostnameForm({'hostname': hostname}) + assert form.is_valid() + + for hostname in invalid_hostnames: + form = HostnameForm({'hostname': hostname}) + assert not form.is_valid() diff --git a/plinth/modules/names/tests/test_functional.py b/plinth/modules/names/tests/test_functional.py new file mode 100644 index 000000000..2e716de96 --- /dev/null +++ b/plinth/modules/names/tests/test_functional.py @@ -0,0 +1,28 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +"""Functional, browser based tests for names app.""" + +import pytest + +from plinth.tests import functional + +pytestmark = [ + pytest.mark.system, pytest.mark.essential, pytest.mark.domain, + pytest.mark.names +] + + +@pytest.fixture(scope='module', autouse=True) +def fixture_background(session_browser): + """Login.""" + functional.login(session_browser) + + +def test_change_hostname(session_browser): + """Test changing the hostname.""" + functional.set_hostname(session_browser, 'mybox') + assert _get_hostname(session_browser) == 'mybox' + + +def _get_hostname(browser): + functional.visit(browser, '/plinth/sys/names/hostname/') + return browser.find_by_id('id_hostname-hostname').value diff --git a/plinth/modules/names/urls.py b/plinth/modules/names/urls.py index 63e6f0787..ad92b66f5 100644 --- a/plinth/modules/names/urls.py +++ b/plinth/modules/names/urls.py @@ -9,4 +9,6 @@ from . import views urlpatterns = [ re_path(r'^sys/names/$', views.NamesAppView.as_view(), name='index'), + re_path(r'^sys/names/hostname/$', views.HostnameView.as_view(), + name='hostname'), ] diff --git a/plinth/modules/names/views.py b/plinth/modules/names/views.py index 72a34ee2e..75bc75169 100644 --- a/plinth/modules/names/views.py +++ b/plinth/modules/names/views.py @@ -4,12 +4,15 @@ FreedomBox app for name services. """ from django.contrib import messages +from django.urls import reverse_lazy from django.utils.translation import gettext_lazy as _ +from django.views.generic.edit import FormView +from plinth.modules import names from plinth.views import AppView from . import components, privileged, resolved -from .forms import NamesConfigurationForm +from .forms import HostnameForm, NamesConfigurationForm class NamesAppView(AppView): @@ -56,6 +59,40 @@ class NamesAppView(AppView): return super().form_valid(form) +class HostnameView(FormView): + """View to update system's hostname.""" + template_name = 'form.html' + form_class = HostnameForm + prefix = 'hostname' + success_url = reverse_lazy('names:index') + + def get_context_data(self, **kwargs): + """Return additional context for rendering the template.""" + context = super().get_context_data(**kwargs) + context['title'] = _('Set Hostname') + return context + + def get_initial(self): + """Return the values to fill in the form.""" + initial = super().get_initial() + initial['hostname'] = names.get_hostname() + return initial + + def form_valid(self, form): + """Apply the form changes.""" + if form.initial['hostname'] != form.cleaned_data['hostname']: + try: + names.set_hostname(form.cleaned_data['hostname']) + messages.success(self.request, _('Configuration updated')) + except Exception as exception: + messages.error( + self.request, + _('Error setting hostname: {exception}').format( + exception=exception)) + + return super().form_valid(form) + + def get_status(): """Get configured services per name.""" domains = components.DomainName.list() diff --git a/plinth/modules/openvpn/views.py b/plinth/modules/openvpn/views.py index 3dad16fc2..3d62d2c36 100644 --- a/plinth/modules/openvpn/views.py +++ b/plinth/modules/openvpn/views.py @@ -5,7 +5,7 @@ import logging from django.http import HttpResponse -from plinth.modules import config +from plinth.modules import config, names from plinth.views import AppView from . import privileged @@ -26,7 +26,7 @@ def profile(request): domainname = config.get_domainname() if not config.get_domainname(): - domainname = config.get_hostname() + domainname = names.get_hostname() profile_string = privileged.get_profile(username, domainname) response = HttpResponse(profile_string, diff --git a/plinth/tests/functional/__init__.py b/plinth/tests/functional/__init__.py index 76b1e8bf8..8072ed4fe 100644 --- a/plinth/tests/functional/__init__.py +++ b/plinth/tests/functional/__init__.py @@ -512,15 +512,18 @@ def running_inside_container(): return result.stdout.decode('utf-8').strip().lower() != 'none' +############################# +# System -> Names utilities # +############################# +def set_hostname(browser, hostname): + visit(browser, '/plinth/sys/names/hostname/') + browser.find_by_id('id_hostname-hostname').fill(hostname) + submit(browser, form_class='form-hostname') + + ############################## # System -> Config utilities # ############################## -def set_hostname(browser, hostname): - nav_to_module(browser, 'config') - browser.find_by_id('id_hostname').fill(hostname) - submit(browser, form_class='form-configuration') - - def set_advanced_mode(browser, mode): nav_to_module(browser, 'config') advanced_mode = browser.find_by_id('id_advanced_mode')