From f892843ba5fc64f42ac8d5877cd294fa4845b14d Mon Sep 17 00:00:00 2001 From: Veiko Aasa Date: Thu, 14 Jan 2021 12:44:33 +0200 Subject: [PATCH] syncthing: Create LDAP group name different from system group When installing the syncthing app, create a LDAP group named "syncthing-access" instead of "syncthing", because the app creates the "syncthing" system group to run the syncthing daemon. Duplicate group names can cause some ambiguity as described in #2008. - Rename the existing "syncthing" LDAP and Django group to "syncthing-access". - Update existing web shares to be accessible with new group name - Functional tests: Add tests to check access to the syncthing site with a user in the syncthing-access group and no group. - Functional tests: Scroll to the edit button before clicking. Fixes some test failures on a smaller browser window. Fixes #2008 Tests performed on Debian stable and testing: - Check that the existing "syncthing" group is renamed after upgrade: 1) Without patch applied, install syncthing, create a user in group "syncthing". 2) Apply patch, update Apache2 config file /etc/apache2/conf-available/syncthing-plinth.conf, reload Apache2, restart plinth. 3) Check that the created user is now in the "syncthing-access" group and can access /syncthing site. - Check that the app upgrade succeeds when there are no users in the syncthing group. - Create a web share accessible by the 'syncthing' group. Check that after the upgrade, the share is accessible to a member of syncthing-access group. - All the syncthing app tests pass. Signed-off-by: Veiko Aasa Reviewed-by: Sunil Mohan Adapa --- actions/users | 22 +++++++++++++ plinth/modules/syncthing/__init__.py | 33 +++++++++++++++++-- .../conf-available/syncthing-plinth.conf | 2 +- .../modules/syncthing/tests/syncthing.feature | 12 +++++++ .../syncthing/tests/test_functional.py | 1 + 5 files changed, 66 insertions(+), 4 deletions(-) diff --git a/actions/users b/actions/users index b60cdbb37..5e0656cb8 100755 --- a/actions/users +++ b/actions/users @@ -56,6 +56,12 @@ def parse_arguments(): subparser.add_argument('groupname', help='Name of the LDAP group to create') + subparser = subparsers.add_parser('rename-group', + help='Rename an LDAP group') + subparser.add_argument('old_groupname', + help='Name of the LDAP group to rename') + subparser.add_argument('new_groupname', help='Name of the new LDAP group') + subparser = subparsers.add_parser('remove-group', help='Delete an LDAP group') subparser.add_argument('groupname', @@ -478,6 +484,22 @@ def subcommand_create_group(arguments): flush_cache() +def subcommand_rename_group(arguments): + """Rename an LDAP group. + + Skip if the group to rename from doesn't exist. + """ + old_groupname = arguments.old_groupname + new_groupname = arguments.new_groupname + + if old_groupname == 'admin' or new_groupname == 'admin': + raise argparse.ArgumentTypeError('Can\'t rename the group "admin"') + + if group_exists(old_groupname): + _run(['ldaprenamegroup', old_groupname, new_groupname]) + flush_cache() + + def subcommand_remove_group(arguments): """Remove an LDAP group.""" groupname = arguments.groupname diff --git a/plinth/modules/syncthing/__init__.py b/plinth/modules/syncthing/__init__.py index ffa2fae30..82391bc4b 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 = 3 +version = 4 managed_services = ['syncthing@syncthing'] @@ -36,7 +36,7 @@ _description = [ 'instance of Syncthing that may be used by multiple users. Each ' 'user\'s set of devices may be synchronized with a distinct set of ' 'folders. The web interface on {box_name} is only available for ' - 'users belonging to the "admin" or "syncthing" group.'), + 'users belonging to the "admin" or "syncthing-access" group.'), box_name=_(cfg.box_name)), ] @@ -54,7 +54,9 @@ class SyncthingApp(app_module.App): """Create components for the app.""" super().__init__() - self.groups = {'syncthing': _('Administer Syncthing application')} + self.groups = { + 'syncthing-access': _('Administer Syncthing application') + } info = app_module.Info(app_id=self.app_id, version=version, name=_('Syncthing'), icon_filename='syncthing', @@ -112,4 +114,29 @@ def setup(helper, old_version=None): if old_version == 1 and app.is_enabled(): app.get_component('firewall-syncthing-ports').enable() + if old_version and old_version <= 3: + # rename LDAP and Django group + old_groupname = 'syncthing' + new_groupname = 'syncthing-access' + + actions.superuser_run( + 'users', options=['rename-group', old_groupname, new_groupname]) + + from django.contrib.auth.models import Group + Group.objects.filter(name=old_groupname).update(name=new_groupname) + + # update web shares to have new group name + from plinth.modules import sharing + shares = sharing.list_shares() + for share in shares: + if old_groupname in share['groups']: + new_groups = share['groups'] + new_groups.remove(old_groupname) + new_groups.append(new_groupname) + + name = share['name'] + 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/data/etc/apache2/conf-available/syncthing-plinth.conf b/plinth/modules/syncthing/data/etc/apache2/conf-available/syncthing-plinth.conf index b4cdaddab..6ed5837cd 100644 --- a/plinth/modules/syncthing/data/etc/apache2/conf-available/syncthing-plinth.conf +++ b/plinth/modules/syncthing/data/etc/apache2/conf-available/syncthing-plinth.conf @@ -19,6 +19,6 @@ Include includes/freedombox-single-sign-on.conf ProxyPass http://localhost:8384/ - TKTAuthToken "admin" "syncthing" + TKTAuthToken "admin" "syncthing-access" diff --git a/plinth/modules/syncthing/tests/syncthing.feature b/plinth/modules/syncthing/tests/syncthing.feature index a397eacac..552e91ce7 100644 --- a/plinth/modules/syncthing/tests/syncthing.feature +++ b/plinth/modules/syncthing/tests/syncthing.feature @@ -35,6 +35,18 @@ Scenario: Backup and restore syncthing And I restore the syncthing app data backup with name test_syncthing Then syncthing folder Test should be present +Scenario: User of syncthing-access group can access syncthing site + Given the syncthing application is enabled + And the user syncthinguser in group syncthing-access exists + When I'm logged in as the user syncthinguser + Then the syncthing site should be available + +Scenario: User not of syncthing-access group can't access syncthing site + Given the syncthing application is enabled + And the user nogroupuser exists + When I'm logged in as the user nogroupuser + Then the syncthing site should not be available + Scenario: Disable syncthing application Given the syncthing application is enabled When I disable the syncthing application diff --git a/plinth/modules/syncthing/tests/test_functional.py b/plinth/modules/syncthing/tests/test_functional.py index e927030bc..f398b5edb 100644 --- a/plinth/modules/syncthing/tests/test_functional.py +++ b/plinth/modules/syncthing/tests/test_functional.py @@ -127,6 +127,7 @@ def _remove_folder(browser, folder_name): functional.eventually(lambda: folder.find_by_css('div.collapse.in')) edit_folder_xpath = './/button[contains(@ng-click, "editFolder")]' edit_folder_button = folder.find_by_xpath(edit_folder_xpath).first + edit_folder_button.scroll_to() edit_folder_button.click() # Edit folder dialog