From 77face68b0131f9ebea4e68a1d788fe9d691467e Mon Sep 17 00:00:00 2001 From: Veiko Aasa Date: Sun, 17 Jan 2021 17:22:53 +0200 Subject: [PATCH] syncthing: Hide unnecessary security warning - Hide the warning 'The Syncthing admin interface is configured to allow remote access without a password. ... '. This warning is unnecessary because we are using authentication through Apache. - Hide the usage reporting notification by declining reporting, if the user has not made a choice yet. - Call add_user_to_share_group() in setup() before starting syncthing to avoid another syncthing daemon restart. - Add a functional test for hidden notification messages. - Functional tests: Improve the method to check if the javascript loading process is complete. Closes #1581 Tests performed: - The syncthing app is installed successfully on Debian testing - All syncthing tests pass on Debian stable and testing Signed-off-by: Veiko Aasa Reviewed-by: Sunil Mohan Adapa --- actions/syncthing | 62 ++++++++++++++++--- plinth/modules/syncthing/__init__.py | 8 ++- .../modules/syncthing/tests/syncthing.feature | 6 ++ .../syncthing/tests/test_functional.py | 41 ++++++------ 4 files changed, 86 insertions(+), 31 deletions(-) diff --git a/actions/syncthing b/actions/syncthing index ce531f191..200dbb117 100755 --- a/actions/syncthing +++ b/actions/syncthing @@ -10,6 +10,14 @@ import os import pwd import shutil import subprocess +import time + +import augeas + +from plinth import action_utils + +DATA_DIR = '/var/lib/syncthing' +CONF_FILE = DATA_DIR + '/.config/syncthing/config.xml' def parse_arguments(): @@ -19,14 +27,23 @@ def parse_arguments(): subparsers.add_parser('setup', help='Setup Syncthing') + subparsers.add_parser('setup-config', help='Setup Syncthing configuration') + subparsers.required = True return parser.parse_args() +def augeas_load(): + """Initialize Augeas.""" + aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD + + augeas.Augeas.NO_MODL_AUTOLOAD) + aug.add_transform('Xml.lns', CONF_FILE) + aug.load() + return aug + + def subcommand_setup(_): """Actions to be performed before installing Syncthing""" - data_dir = '/var/lib/syncthing' - # Create syncthing group if needed. try: grp.getgrnam('syncthing') @@ -39,13 +56,44 @@ def subcommand_setup(_): except KeyError: subprocess.run([ 'adduser', '--system', '--ingroup', 'syncthing', '--home', - '/var/lib/syncthing', '--gecos', - 'Syncthing file synchronization server', 'syncthing' + DATA_DIR, '--gecos', 'Syncthing file synchronization server', + 'syncthing' ], check=True) - if not os.path.exists(data_dir): - os.makedirs(data_dir, mode=0o750) - shutil.chown(data_dir, user='syncthing', group='syncthing') + if not os.path.exists(DATA_DIR): + os.makedirs(DATA_DIR, mode=0o750) + shutil.chown(DATA_DIR, user='syncthing', group='syncthing') + + +def subcommand_setup_config(_): + """Make configuration changes.""" + # wait until the configuration file is created by the syncthing daemon + timeout = 300 + while timeout > 0: + if os.path.exists(CONF_FILE): + break + timeout = timeout - 1 + time.sleep(1) + + aug = augeas_load() + + # disable authentication missing notification as FreedomBox itself + # provides authentication + auth_conf = ('/configuration/options/unackedNotificationID' + '[#text="authenticationUserAndPassword"]') + conf_changed = bool(aug.remove('/files' + CONF_FILE + auth_conf)) + + # disable usage reporting notification by declining reporting + # if the user has not made a choice yet + usage_conf = '/configuration/options/urAccepted/#text' + if aug.get('/files' + CONF_FILE + usage_conf) == '0': + aug.set('/files' + CONF_FILE + usage_conf, '-1') + conf_changed = True + + aug.save() + + if conf_changed: + action_utils.service_try_restart('syncthing@syncthing') def main(): diff --git a/plinth/modules/syncthing/__init__.py b/plinth/modules/syncthing/__init__.py index 82391bc4b..d01878b3f 100644 --- a/plinth/modules/syncthing/__init__.py +++ b/plinth/modules/syncthing/__init__.py @@ -18,7 +18,7 @@ from plinth.utils import format_lazy from . import manifest -version = 4 +version = 5 managed_services = ['syncthing@syncthing'] @@ -108,9 +108,13 @@ def setup(helper, old_version=None): """Install and configure the module.""" helper.install(managed_packages) helper.call('post', actions.superuser_run, 'syncthing', ['setup']) + add_user_to_share_group(SYSTEM_USER, managed_services[0]) + if not old_version: helper.call('post', app.enable) + helper.call('post', actions.superuser_run, 'syncthing', ['setup-config']) + if old_version == 1 and app.is_enabled(): app.get_component('firewall-syncthing-ports').enable() @@ -138,5 +142,3 @@ def setup(helper, old_version=None): sharing.remove_share(name) sharing.add_share(name, share['path'], new_groups, share['is_public']) - - add_user_to_share_group(SYSTEM_USER, managed_services[0]) diff --git a/plinth/modules/syncthing/tests/syncthing.feature b/plinth/modules/syncthing/tests/syncthing.feature index 552e91ce7..c252ac29d 100644 --- a/plinth/modules/syncthing/tests/syncthing.feature +++ b/plinth/modules/syncthing/tests/syncthing.feature @@ -13,6 +13,12 @@ Scenario: Enable syncthing application When I enable the syncthing application Then the syncthing service should be running +Scenario: Authentication and usage reporting notifications not shown + Given the syncthing application is enabled + When I access syncthing application + Then the usage reporting notification is not shown + And the authentication notification is not shown + Scenario: Add a syncthing folder Given the syncthing application is enabled And syncthing folder Test is not present diff --git a/plinth/modules/syncthing/tests/test_functional.py b/plinth/modules/syncthing/tests/test_functional.py index f398b5edb..cfbce4f55 100644 --- a/plinth/modules/syncthing/tests/test_functional.py +++ b/plinth/modules/syncthing/tests/test_functional.py @@ -3,6 +3,8 @@ Functional, browser based tests for syncthing app. """ +import time + from pytest_bdd import given, parsers, scenarios, then, when from plinth.tests import functional @@ -37,6 +39,19 @@ def syncthing_remove_folder(session_browser, folder_name): _remove_folder(session_browser, folder_name) +@then('the usage reporting notification is not shown') +def syncthing_assert_usage_report_notification_not_shown(session_browser): + _load_main_interface(session_browser) + assert session_browser.find_by_id('ur').visible is False + + +@then('the authentication notification is not shown') +def syncthing_assert_authentication_notification_not_shown(session_browser): + _load_main_interface(session_browser) + assert bool(session_browser.find_by_css( + '#authenticationUserAndPassword *')) is False + + @then(parsers.parse('syncthing folder {folder_name:w} should be present')) def syncthing_assert_folder_present(session_browser, folder_name): assert _folder_is_present(session_browser, folder_name) @@ -63,28 +78,12 @@ def _load_main_interface(browser): functional.eventually(service_is_available) # Wait for javascript loading process to complete - browser.execute_script(''' - document.is_ui_online = false; - var old_console_log = console.log; - console.log = function(message) { - old_console_log.apply(null, arguments); - if (message == 'UIOnline') { - document.is_ui_online = true; - console.log = old_console_log; - } - }; - ''') - functional.eventually( - lambda: browser.evaluate_script('document.is_ui_online'), timeout=5) + functional.eventually(lambda: browser.evaluate_script( + 'angular.element("[ng-controller=SyncthingController]").scope()' + '.thisDevice().name')) - # Dismiss the Usage Reporting consent dialog - functional.eventually(browser.find_by_id, ['ur']) - usage_reporting = browser.find_by_id('ur').first - functional.eventually(lambda: usage_reporting.visible, timeout=2) - if usage_reporting.visible: - yes_xpath = './/button[contains(@ng-click, "declineUR")]' - usage_reporting.find_by_xpath(yes_xpath).first.click() - functional.eventually(lambda: not usage_reporting.visible) + # Give browser additional time to setup site + time.sleep(1) def _folder_is_present(browser, folder_name):