matrixsynapse: Use privileged decorator for actions

Tests:

- Functional tests work
- Initial setup works
- Setup after install works
  - Domain is properly set
  - Configure domains is properly shown in the app page
- Updating TURN configuration works
  - Configuration file is updated
- Enabling/disabling public registration works
  - Configuration file is updated
  - App page show current status
- FAIL: Daemon fails to start when public registration is enabled

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/matrixsynapse/__init__.py

@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app to configure matrix-synapse server."""
-FreedomBox app to configure matrix-synapse server.
-"""
 
 import logging
 import os
@@ -11,7 +9,6 @@ from django.urls import reverse_lazy
 from django.utils.translation import gettext_lazy as _
 from ruamel.yaml.util import load_yaml_guess_indent
 
-from plinth import actions
 from plinth import app as app_module
 from plinth import frontpage, menu
 from plinth.daemon import Daemon
@@ -23,7 +20,7 @@ from plinth.modules.letsencrypt.components import LetsEncrypt
 from plinth.package import Packages, install
 from plinth.utils import format_lazy, is_non_empty_file
 
-from . import manifest
+from . import manifest, privileged
 
 _description = [
     _('<a href="https://matrix.org/docs/guides/faq.html">Matrix</a> is an new '
@@ -41,16 +38,6 @@ _description = [
 
 logger = logging.getLogger(__name__)
 
-CONF_DIR = "/etc/matrix-synapse/conf.d/"
-
-ORIG_CONF_PATH = '/etc/matrix-synapse/homeserver.yaml'
-SERVER_NAME_PATH = CONF_DIR + 'server_name.yaml'
-STATIC_CONF_PATH = CONF_DIR + 'freedombox-static.yaml'
-LISTENERS_CONF_PATH = CONF_DIR + 'freedombox-listeners.yaml'
-REGISTRATION_CONF_PATH = CONF_DIR + 'freedombox-registration.yaml'
-TURN_CONF_PATH = CONF_DIR + 'freedombox-turn.yaml'
-OVERRIDDEN_TURN_CONF_PATH = CONF_DIR + 'turn.yaml'
-
 
 class MatrixSynapseApp(app_module.App):
     """FreedomBox app for Matrix Synapse."""
@@ -122,7 +109,7 @@ class MatrixSynapseApp(app_module.App):
         if old_version and old_version < 6:
             upgrade()
         else:
-            actions.superuser_run('matrixsynapse', ['post-install'])
+            privileged.post_install()
 
         if not old_version:
             self.enable()
@@ -144,14 +131,13 @@ class MatrixSynapseTurnConsumer(TurnConsumer):
 
 def upgrade():
     """Upgrade matrix-synapse configuration to avoid conffile prompt."""
-    public_registration_status = get_public_registration_status()
+    public_registration_status = privileged.public_registration('status')
-    actions.superuser_run('matrixsynapse', ['move-old-conf'])
+    privileged.move_old_conf()
     install(['matrix-synapse'], force_configuration='new', reinstall=True,
             force_missing_configuration=True)
-    actions.superuser_run('matrixsynapse', ['post-install'])
+    privileged.post_install()
     if public_registration_status:
-        actions.superuser_run('matrixsynapse',
+        privileged.public_registration('enable')
-                              ['public-registration', 'enable'])
 
 
 def setup_domain(domain_name):
@@ -159,13 +145,12 @@ def setup_domain(domain_name):
     app = app_module.App.get('matrixsynapse')
     app.get_component('letsencrypt-matrixsynapse').setup_certificates(
         [domain_name])
-    actions.superuser_run('matrixsynapse',
+    privileged.setup(domain_name)
-                          ['setup', '--domain-name', domain_name])
 
 
 def is_setup():
     """Return whether the Matrix Synapse server is setup."""
-    return os.path.exists(SERVER_NAME_PATH)
+    return os.path.exists(privileged.SERVER_NAME_PATH)
 
 
 def get_domains():
@@ -182,7 +167,7 @@ def get_configured_domain_name():
     if not is_setup():
         return None
 
-    with open(SERVER_NAME_PATH, encoding='utf-8') as config_file:
+    with open(privileged.SERVER_NAME_PATH, encoding='utf-8') as config_file:
         config, _, _ = load_yaml_guess_indent(config_file)
 
     return config['server_name']
@@ -190,8 +175,8 @@ def get_configured_domain_name():
 
 def get_turn_configuration() -> (List[str], str, bool):
     """Return TurnConfiguration if setup else empty."""
-    for file_path, managed in ((OVERRIDDEN_TURN_CONF_PATH, False),
+    for file_path, managed in ((privileged.OVERRIDDEN_TURN_CONF_PATH, False),
-                               (TURN_CONF_PATH, True)):
+                               (privileged.TURN_CONF_PATH, True)):
         if is_non_empty_file(file_path):
             with open(file_path, encoding='utf-8') as config_file:
                 config, _, _ = load_yaml_guess_indent(config_file)
@@ -202,13 +187,6 @@ def get_turn_configuration() -> (List[str], str, bool):
     return (TurnConfiguration(), True)
 
 
-def get_public_registration_status() -> bool:
-    """Return whether public registration is enabled."""
-    output = actions.superuser_run('matrixsynapse',
-                                   ['public-registration', 'status'])
-    return output.strip() == 'enabled'
-
-
 def get_certificate_status():
     """Return the status of certificate for the configured domain."""
     app = app_module.App.get('matrixsynapse')
@@ -226,7 +204,4 @@ def update_turn_configuration(config: TurnConfiguration, managed=True,
     if not force and app.needs_setup():
         return
 
-    params = ['configure-turn']
+    privileged.configure_turn(managed, config.to_json())
-    params += ['--managed'] if managed else []
-    actions.superuser_run('matrixsynapse', params,
-                          input=config.to_json().encode())

plinth/modules/matrixsynapse/privileged.py

@@ -1,24 +1,25 @@
-#!/usr/bin/python3
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Configure Matrix-Synapse server."""
-Configuration helper for Matrix-Synapse server.
-"""
 
-import argparse
 import json
 import os
 import pathlib
-import sys
+from typing import Optional
 
 import yaml
 
 from plinth import action_utils
-from plinth.modules.matrixsynapse import (LISTENERS_CONF_PATH, ORIG_CONF_PATH,
+from plinth.actions import privileged
-                                          REGISTRATION_CONF_PATH,
-                                          STATIC_CONF_PATH)
 
-TURN_CONF_PATH = '/etc/matrix-synapse/conf.d/freedombox-turn.yaml'
+CONF_DIR = "/etc/matrix-synapse/conf.d/"
-OVERRIDDEN_TURN_CONF_PATH = '/etc/matrix-synapse/conf.d/turn.yaml'
+
+ORIG_CONF_PATH = '/etc/matrix-synapse/homeserver.yaml'
+SERVER_NAME_PATH = CONF_DIR + 'server_name.yaml'
+STATIC_CONF_PATH = CONF_DIR + 'freedombox-static.yaml'
+LISTENERS_CONF_PATH = CONF_DIR + 'freedombox-listeners.yaml'
+REGISTRATION_CONF_PATH = CONF_DIR + 'freedombox-registration.yaml'
+TURN_CONF_PATH = CONF_DIR + 'freedombox-turn.yaml'
+OVERRIDDEN_TURN_CONF_PATH = CONF_DIR + 'turn.yaml'
 
 STATIC_CONFIG = {
     'max_upload_size':
@@ -40,38 +41,8 @@ STATIC_CONFIG = {
 }
 
 
-def parse_arguments():
+@privileged
-    """Return parsed command line arguments as dictionary"""
+def post_install():
-    parser = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    subparsers.add_parser('post-install', help='Perform post install steps')
-    help_pubreg = 'Enable/Disable/Status public user registration.'
-    pubreg = subparsers.add_parser('public-registration', help=help_pubreg)
-    pubreg.add_argument('command', choices=('enable', 'disable', 'status'),
-                        help=help_pubreg)
-    setup = subparsers.add_parser('setup', help='Set domain name for Matrix')
-    setup.add_argument(
-        '--domain-name',
-        help='The domain name that will be used by Matrix Synapse')
-
-    subparsers.add_parser(
-        'move-old-conf',
-        help='Move old configuration file to backup before reinstall')
-
-    turn = subparsers.add_parser(
-        'configure-turn',
-        help='Configure a TURN server for use with Matrix Synapse')
-    turn.add_argument(
-        '--managed', required=False, default=False, action='store_true',
-        help='Whether configuration is provided by user or auto-managed by '
-        'FreedomBox')
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
-def subcommand_post_install(_):
     """Perform post installation configuration."""
     with open(STATIC_CONF_PATH, 'w', encoding='utf-8') as static_conf_file:
         yaml.dump(STATIC_CONFIG, static_conf_file)
@@ -91,15 +62,19 @@ def subcommand_post_install(_):
         yaml.dump({'listeners': listeners}, listeners_conf_file)
 
 
-def subcommand_setup(arguments):
+@privileged
+def setup(domain_name: str):
     """Configure the domain name for matrix-synapse package."""
-    domain_name = arguments.domain_name
     action_utils.dpkg_reconfigure('matrix-synapse',
                                   {'server-name': domain_name})
 
 
-def subcommand_public_registration(argument):
+@privileged
+def public_registration(command: str) -> Optional[bool]:
     """Enable/Disable/Status public user registration."""
+    if command not in ('enable', 'disable', 'status'):
+        raise ValueError('Invalid command')
+
     try:
         with open(REGISTRATION_CONF_PATH, encoding='utf-8') as reg_conf_file:
             config = yaml.load(reg_conf_file)
@@ -112,25 +87,22 @@ def subcommand_public_registration(argument):
                     orig_config.get('enable_registration', False)
             }
 
-    if argument.command == 'status':
+    if command == 'status':
-        if config['enable_registration']:
+        return bool(config['enable_registration'])
-            print('enabled')
+    elif command == 'enable':
-            return
-        else:
-            print('disabled')
-            return
-    elif argument.command == 'enable':
         config['enable_registration'] = True
-    elif argument.command == 'disable':
+    elif command == 'disable':
         config['enable_registration'] = False
 
     with open(REGISTRATION_CONF_PATH, 'w', encoding='utf-8') as reg_conf_file:
         yaml.dump(config, reg_conf_file)
 
     action_utils.service_try_restart('matrix-synapse')
+    return None
 
 
-def subcommand_move_old_conf(_arguments):
+@privileged
+def move_old_conf():
     """Move old configuration to backup so it can be restored by reinstall."""
     conf_file = pathlib.Path(ORIG_CONF_PATH)
     if conf_file.exists():
@@ -138,8 +110,8 @@ def subcommand_move_old_conf(_arguments):
         conf_file.replace(backup_file)
 
 
-def _set_turn_config(conf_file):
+def _set_turn_config(conf_file, conf):
-    turn_server_config = json.loads(''.join(sys.stdin))
+    turn_server_config = json.loads(conf)
 
     if not turn_server_config['uris']:
         # No valid configuration, remove the configuration file
@@ -161,22 +133,12 @@ def _set_turn_config(conf_file):
         yaml.dump(config, turn_config)
 
 
-def subcommand_configure_turn(arguments):
+@privileged
+def configure_turn(managed: bool, conf: str):
     """Set parameters for the STUN/TURN server to use with Matrix Synapse."""
-    if arguments.managed:
+    if managed:
-        _set_turn_config(TURN_CONF_PATH)
+        _set_turn_config(TURN_CONF_PATH, conf)
     else:
-        _set_turn_config(OVERRIDDEN_TURN_CONF_PATH)
+        _set_turn_config(OVERRIDDEN_TURN_CONF_PATH, conf)
 
     action_utils.service_try_restart('matrix-synapse')
-
-
-def main():
-    arguments = parse_arguments()
-    sub_command = arguments.subcommand.replace('-', '_')
-    sub_command_method = globals()['subcommand_' + sub_command]
-    sub_command_method(arguments)
-
-
-if __name__ == '__main__':
-    main()

plinth/modules/matrixsynapse/tests/test_turn_config.py

@@ -9,8 +9,10 @@ import pytest
 
 from plinth.modules import matrixsynapse
 from plinth.modules.coturn.components import TurnConfiguration
+from plinth.modules.matrixsynapse import privileged
 
-actions_name = 'matrixsynapse'
+pytestmark = pytest.mark.usefixtures('mock_privileged')
+privileged_modules_to_mock = ['plinth.modules.matrixsynapse.privileged']
 
 
 @pytest.fixture(name='managed_turn_conf_file')
@@ -28,29 +30,27 @@ def fixture_overridden_turn_conf_file(tmp_path):
 
 
 @pytest.fixture(autouse=True)
-def fixture_set_paths(actions_module, capsys, managed_turn_conf_file,
+def fixture_set_paths(managed_turn_conf_file, overridden_turn_conf_file):
-                      overridden_turn_conf_file):
     """Run actions with custom root path."""
-    actions_module.TURN_CONF_PATH = managed_turn_conf_file
+    privileged.TURN_CONF_PATH = managed_turn_conf_file
-    actions_module.OVERRIDDEN_TURN_CONF_PATH = overridden_turn_conf_file
+    privileged.OVERRIDDEN_TURN_CONF_PATH = overridden_turn_conf_file
-    with patch('plinth.action_utils.service_try_restart'):
+    with patch('plinth.privileged.service.try_restart'):
         yield
 
 
 @pytest.fixture(name='test_configuration', autouse=True)
-def fixture_test_configuration(call_action, managed_turn_conf_file,
+def fixture_test_configuration(managed_turn_conf_file,
                                overridden_turn_conf_file):
     """Use a separate Matrix Synapse configuration for tests.
 
-    Overrides TURN configuration files and patches actions.superuser_run
+    Overrides TURN configuration files.
-    with the fixture call_action
     """
-    with (patch('plinth.modules.matrixsynapse.TURN_CONF_PATH',
+    matrixsynapse = 'plinth.modules.matrixsynapse'
+    with (patch(f'{matrixsynapse}.privileged.TURN_CONF_PATH',
                 managed_turn_conf_file),
-          patch('plinth.modules.matrixsynapse.OVERRIDDEN_TURN_CONF_PATH',
+          patch(f'{matrixsynapse}.privileged.OVERRIDDEN_TURN_CONF_PATH',
                 overridden_turn_conf_file),
-          patch('plinth.modules.matrixsynapse.is_setup', return_value=True),
+          patch(f'{matrixsynapse}.is_setup', return_value=True),
-          patch('plinth.actions.superuser_run', call_action),
           patch('plinth.app.App.get') as app_get):
         app = Mock()
         app_get.return_value = app
@@ -71,14 +71,12 @@ updated_coturn_configuration = TurnConfiguration(
     'aiP02OsbkC7BUeKvKzhAsTZ8MEwMd3yTwpr2uvbOxgWe51AGyOlj6WGuCyqj7iaO')
 
 
-def _set_managed_configuration(monkeypatch, config=coturn_configuration):
+def _set_managed_configuration(config=coturn_configuration):
-    monkeypatch.setattr('sys.stdin', config.to_json())
     matrixsynapse.update_turn_configuration(config)
 
 
-def _set_overridden_configuration(monkeypatch,
+def _set_overridden_configuration(
                                   config=overridden_configuration):
-    monkeypatch.setattr('sys.stdin', config.to_json())
     matrixsynapse.update_turn_configuration(config, managed=False)
 
 
@@ -90,30 +88,30 @@ def _assert_conf(expected_configuration, expected_managed):
     assert managed == expected_managed
 
 
-def test_managed_turn_server_configuration(monkeypatch):
+def test_managed_turn_server_configuration():
     """Test setting and getting managed TURN server configuration."""
-    _set_managed_configuration(monkeypatch)
+    _set_managed_configuration()
     _assert_conf(coturn_configuration, True)
 
 
-def test_overridden_turn_server_configuration(monkeypatch):
+def test_overridden_turn_server_configuration():
     """Test setting and getting overridden TURN sever configuration."""
-    _set_overridden_configuration(monkeypatch)
+    _set_overridden_configuration()
     _assert_conf(overridden_configuration, False)
 
 
-def test_revert_to_managed_turn_server_configuration(monkeypatch):
+def test_revert_to_managed_turn_server_configuration():
     """Test setting and getting overridden TURN sever configuration."""
     # Had to do all 3 operations because all fixtures were function-scoped
-    _set_managed_configuration(monkeypatch)
+    _set_managed_configuration()
-    _set_overridden_configuration(monkeypatch)
+    _set_overridden_configuration()
-    _set_overridden_configuration(monkeypatch, TurnConfiguration())
+    _set_overridden_configuration(TurnConfiguration())
     _assert_conf(coturn_configuration, True)
 
 
-def test_coturn_configuration_update_after_admin_override(monkeypatch):
+def test_coturn_configuration_update_after_admin_override():
     """Test that overridden conf prevails even if managed conf is updated."""
-    _set_managed_configuration(monkeypatch)
+    _set_managed_configuration()
-    _set_overridden_configuration(monkeypatch)
+    _set_overridden_configuration()
-    _set_managed_configuration(monkeypatch, updated_coturn_configuration)
+    _set_managed_configuration(updated_coturn_configuration)
     _assert_conf(overridden_configuration, False)

plinth/modules/matrixsynapse/views.py

@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Views for the Matrix Synapse module."""
-Views for the Matrix Synapse module.
-"""
 
 from django.contrib import messages
 from django.shortcuts import redirect
@@ -9,19 +7,19 @@ from django.urls import reverse_lazy
 from django.utils.translation import gettext_lazy as _
 from django.views.generic import FormView
 
-from plinth import actions
 from plinth import app as app_module
 from plinth.forms import DomainSelectionForm
 from plinth.modules import matrixsynapse, names
 from plinth.modules.coturn.components import TurnConfiguration
 from plinth.views import AppView
 
-from . import get_public_registration_status, get_turn_configuration
+from . import get_turn_configuration, privileged
 from .forms import MatrixSynapseForm
 
 
 class SetupView(FormView):
     """Show matrix-synapse setup page."""
+
     template_name = 'matrix-synapse-pre-setup.html'
     form_class = DomainSelectionForm
     success_url = reverse_lazy('matrixsynapse:index')
@@ -46,6 +44,7 @@ class SetupView(FormView):
 
 class MatrixSynapseAppView(AppView):
     """Show matrix-synapse service page."""
+
     app_id = 'matrixsynapse'
     template_name = 'matrix-synapse.html'
     form_class = MatrixSynapseForm
@@ -69,21 +68,24 @@ class MatrixSynapseAppView(AppView):
         initial = super().get_initial()
         config, managed = get_turn_configuration()
         initial.update({
-            'enable_public_registration': get_public_registration_status(),
+            'enable_public_registration':
-            'enable_managed_turn': managed,
+                privileged.public_registration('status'),
-            'turn_uris': '\n'.join(config.uris),
+            'enable_managed_turn':
-            'shared_secret': config.shared_secret
+                managed,
+            'turn_uris':
+                '\n'.join(config.uris),
+            'shared_secret':
+                config.shared_secret
         })
         return initial
 
     @staticmethod
     def _handle_public_registrations(new_config):
+
         if new_config['enable_public_registration']:
-            actions.superuser_run('matrixsynapse',
+            privileged.public_registration('enable')
-                                  ['public-registration', 'enable'])
         else:
-            actions.superuser_run('matrixsynapse',
+            privileged.public_registration('disable')
-                                  ['public-registration', 'disable'])
 
     @staticmethod
     def _handle_turn_configuration(old_config, new_config):