syncthing: Fixes issues with enabling and setup

- On enable, don't just run syncthing, actually enable it.

- Make setup step idempotent. Run setup during enable as it is
  idempotent.

- Perform user/group creation properly during setup.

- Provide better description.

- Modify the application label and icon for accuracy of the service
  provided.

- Fix problem with showing service in firewall.

actions/syncthing

@@ -21,6 +21,10 @@ Configuration helper for Syncthing.
 """
 
 import argparse
+import grp
+import os
+import pwd
+import shutil
 import subprocess
 
 from plinth import action_utils
@@ -31,49 +35,47 @@ def parse_arguments():
     parser = argparse.ArgumentParser()
     subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
 
-    subparsers.add_parser('pre-setup', help='Perform pre-setup operations')
+    subparsers.add_parser('enable', help='Enable Syncthing')
-    subparsers.add_parser('setup', help='Setup Syncthing configuration')
+    subparsers.add_parser('disable', help='Disable Syncthing')
-    subparsers.add_parser('enable', help='Enable Syncthing site')
-    subparsers.add_parser('disable', help='Disable Syncthing site')
 
     return parser.parse_args()
 
 
-def subcommand_pre_setup(_):
+def setup():
     """Actions to be performed before installing Syncthing"""
-    subprocess.check_call(
+    data_dir = '/var/lib/syncthing'
-        ['adduser', '--system', '--disabled-login', 'syncthing'])
 
+    # Create syncthing group if needed.
+    try:
+        grp.getgrnam('syncthing')
+    except KeyError:
+        subprocess.run(['addgroup', '--system', 'syncthing'], check=True)
 
-def subcommand_setup(_):
+    # Create syncthing user is needed.
-    """Setup Syncthing configuration."""
+    try:
-    subprocess.check_call(
+        pwd.getpwnam('syncthing')
-        ['cp', '/lib/systemd/system/syncthing@.service', '/etc/systemd'])
+    except KeyError:
-    service_enable()
+        subprocess.run(['adduser', '--system', '--ingroup', 'syncthing',
-    action_utils.webserver_enable('syncthing-plinth')
+                        '--home', '/var/lib/syncthing',
+                        '--gecos', 'Syncthing file synchronization server',
+                        'syncthing'], check=True)
 
-
+    if not os.path.exists(data_dir):
-def service_enable():
+        os.makedirs(data_dir, mode=0o750)
-    """Start the Syncthing service as plinth user"""
+        shutil.chown(data_dir, user='syncthing', group='syncthing')
-    subprocess.check_call(
-        ['systemctl', 'start', 'syncthing@syncthing.service'])
-
-
-def service_disable():
-    """Stop the Syncthing service as plinth user"""
-    subprocess.check_call(['systemctl', 'stop', 'syncthing@syncthing.service'])
 
 
 def subcommand_enable(_):
     """Enable web configuration and reload."""
-    service_enable()
+    setup()
+    action_utils.service_enable('syncthing@syncthing')
     action_utils.webserver_enable('syncthing-plinth')
 
 
 def subcommand_disable(_):
     """Disable web configuration and reload."""
     action_utils.webserver_disable('syncthing-plinth')
-    service_disable()
+    action_utils.service_disable('syncthing@syncthing')
 
 
 def main():

plinth/modules/syncthing/__init__.py

@@ -26,30 +26,43 @@ from plinth import action_utils
 from plinth import cfg
 from plinth import frontpage
 from plinth import service as service_module
+from plinth.utils import format_lazy
+
 
 version = 1
 
 depends = ['apps']
 
-managed_services = ['syncthing@syncthing.service']
+managed_services = ['syncthing']
 
 managed_packages = ['syncthing']
 
-title = _('Personal Cloud (Syncthing)')
+title = _('File Synchronization (Syncthing)')
 
 description = [
-    _('Syncthing is a file synchronization program that can sync files '
+    _('Syncthing is an application that can synchronizes files across '
-      'between multiple devices. Syncthing enables this FreedomBox to be '
+      'multiple devices.  Creation, modification and deletion of files on one '
-      'used as a cloud server for storing and sharing files'),
+      'device will automatically be replicated to other devices.'),
+    format_lazy(
+        _('Running Synching on {box_name}, provides an extra synchronization '
+          'point for your data that is available most of the time so that '
+          'your devices synchronize more often.  On {box_name}, a single '
+          'instance of Syncthing runs and may be used by many users.  Each '
+          'user\'s set of devices many be synchronized with a distinct set '
+          'of folders.  Web interface is only available for users '
+          'belonging to the "admin" group.'), box_name=_(cfg.box_name)),
     _('When enabled, Syncthing will be available from <a href="/syncthing/">'
-      '/syncthing</a> path on the web server.'),
+      '/syncthing</a> web interface. Desktop and mobile clients are also '
+      '<a href="https://syncthing.net/">available.</a>'),
 ]
 
+service = None
+
 
 def init():
     """Intialize the module."""
     menu = cfg.main_menu.get('apps:index')
-    menu.add_urlname(title, 'glyphicon-cloud', 'syncthing:index')
+    menu.add_urlname(title, 'glyphicon-refresh', 'syncthing:index')
 
     global service
     setup_helper = globals()['setup_helper']
@@ -61,7 +74,8 @@ def init():
             is_external=True,
             is_enabled=is_enabled,
             enable=enable,
-            disable=disable)
+            disable=disable,
+            is_running=is_running)
 
         if is_enabled():
             add_shortcut()
@@ -69,9 +83,8 @@ def init():
 
 def setup(helper, old_version=None):
     """Install and configure the module."""
-    helper.call('pre', actions.superuser_run, 'syncthing', ['pre-setup'])
     helper.install(managed_packages)
-    helper.call('post', actions.superuser_run, 'syncthing', ['setup'])
+    helper.call('post', actions.superuser_run, 'syncthing', ['enable'])
     global service
     if service is None:
         service = service_module.Service(
@@ -81,7 +94,8 @@ def setup(helper, old_version=None):
             is_external=True,
             is_enabled=is_enabled,
             enable=enable,
-            disable=disable)
+            disable=disable,
+            is_running=is_running)
     helper.call('post', service.notify_enabled, None, True)
     helper.call('post', add_shortcut)
 
@@ -92,6 +106,11 @@ def add_shortcut():
         'syncthing', title, url='/syncthing/', login_required=True)
 
 
+def is_running():
+    """Return whether the service is running."""
+    return action_utils.service_is_running('syncthing@syncthing')
+
+
 def is_enabled():
     """Return whether the module is enabled."""
     return (action_utils.service_is_enabled('syncthing@syncthing') and