mediawiki: Use privileged decorator for actions

Tests:

- Functional tests works (when libpam-tmpdir is removed)
- Initial setup works
  - Website is accessible
  - sqlite file is created
  - Database update is triggered
- Changing skin/admin password/public registrations/private mode/site name works
  - Configuration file is updated
  - App page shows the current value
  - Website is reflects the correct value
  - When private mode is enabled, public registrations are automatically
    disabled

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

plinth/modules/mediawiki/__init__.py

@@ -1,14 +1,11 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app to configure MediaWiki."""
-FreedomBox app to configure MediaWiki.
-"""
 
 import re
 from urllib.parse import urlparse
 
 from django.utils.translation import gettext_lazy as _
 
-from plinth import actions
 from plinth import app as app_module
 from plinth import frontpage, menu
 from plinth.daemon import Daemon
@@ -17,7 +14,7 @@ from plinth.modules.backups.components import BackupRestore
 from plinth.modules.firewall.components import Firewall
 from plinth.package import Packages
 
-from . import manifest
+from . import manifest, privileged
 
 _description = [
     _('MediaWiki is the wiki engine that powers Wikipedia and other WikiMedia '
@@ -96,8 +93,8 @@ class MediaWikiApp(app_module.App):
     def setup(self, old_version):
         """Install and configure the app."""
         super().setup(old_version)
-        actions.superuser_run('mediawiki', ['setup'])
+        privileged.setup()
-        actions.superuser_run('mediawiki', ['update'])
+        privileged.update()
         self.enable()
 
 
@@ -107,20 +104,7 @@ class Shortcut(frontpage.Shortcut):
     def enable(self):
         """When enabled, check if MediaWiki is in private mode."""
         super().enable()
-        self.login_required = is_private_mode_enabled()
+        self.login_required = privileged.private_mode('status')
-
-
-def is_public_registration_enabled():
-    """Return whether public registration is enabled."""
-    output = actions.superuser_run('mediawiki',
-                                   ['public-registrations', 'status'])
-    return output.strip() == 'enabled'
-
-
-def is_private_mode_enabled():
-    """Return whether private mode is enabled or disabled."""
-    output = actions.superuser_run('mediawiki', ['private-mode', 'status'])
-    return output.strip() == 'enabled'
 
 
 def _get_config_value_in_file(setting_name, config_file):
@@ -144,11 +128,6 @@ def get_default_skin():
     return _get_config_value('$wgDefaultSkin')
 
 
-def set_default_skin(skin):
-    """Set the value of the default skin."""
-    actions.superuser_run('mediawiki', ['set-default-skin', skin])
-
-
 def get_server_url():
     """Return the value of the server URL."""
     server_url = _get_config_value('$wgServer')
@@ -161,15 +140,9 @@ def set_server_url(domain):
     if domain.endswith('.onion'):
         protocol = 'http'
 
-    actions.superuser_run('mediawiki',
+    privileged.set_server_url(f'{protocol}://{domain}')
-                          ['set-server-url', f'{protocol}://{domain}'])
 
 
 def get_site_name():
     """Return the value of MediaWiki's site name."""
     return _get_config_value('$wgSitename') or 'Wiki'
-
-
-def set_site_name(site_name):
-    """Set the value of $wgSitename."""
-    actions.superuser_run('mediawiki', ['set-site-name', site_name])

plinth/modules/mediawiki/privileged.py

@@ -1,15 +1,12 @@
-#!/usr/bin/python3
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Configure MediaWiki."""
-Configuration helper for MediaWiki.
-"""
 
-import argparse
 import os
 import subprocess
-import sys
 import tempfile
+from typing import Optional
 
+from plinth.actions import privileged
 from plinth.utils import generate_password
 
 MAINTENANCE_SCRIPTS_DIR = "/usr/share/mediawiki/maintenance"
@@ -17,49 +14,6 @@ CONF_FILE = '/etc/mediawiki/FreedomBoxSettings.php'
 LOCAL_SETTINGS_CONF = '/etc/mediawiki/LocalSettings.php'
 
 
-def parse_arguments():
-    """Return parsed command line arguments as dictionary."""
-    parser = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    subparsers.add_parser('setup', help='Setup MediaWiki')
-    subparsers.add_parser('update', help='Run MediaWiki update script')
-
-    help_pub_reg = 'Enable/Disable/Status public user registration.'
-    pub_reg = subparsers.add_parser('public-registrations', help=help_pub_reg)
-    pub_reg.add_argument('command', choices=('enable', 'disable', 'status'),
-                         help=help_pub_reg)
-
-    help_private_mode = 'Enable/Disable/Status private mode.'
-    private_mode = subparsers.add_parser('private-mode',
-                                         help=help_private_mode)
-    private_mode.add_argument('command',
-                              choices=('enable', 'disable', 'status'),
-                              help=help_private_mode)
-
-    change_password = subparsers.add_parser('change-password',
-                                            help='Change user password')
-    change_password.add_argument('--username', default='admin',
-                                 help='name of the MediaWiki user')
-    change_password.add_argument('--password',
-                                 help='new password for the MediaWiki user')
-
-    default_skin = subparsers.add_parser('set-default-skin',
-                                         help='Set the default skin')
-    default_skin.add_argument('skin', help='name of the skin')
-
-    server_url = subparsers.add_parser(
-        'set-server-url', help='Set the value of $wgServer for this server')
-    server_url.add_argument('server_url', help='value of $wgServer')
-
-    site_name = subparsers.add_parser(
-        'set-site-name', help='Set the value of $wgSitename for this Wiki')
-    site_name.add_argument('site_name', help='value of $wgSitename')
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
 def _get_php_command():
     """Return the PHP command that should be used on CLI.
 
@@ -82,7 +36,8 @@ def _get_php_command():
     return f'php{version}'
 
 
-def subcommand_setup(_):
+@privileged
+def setup():
     """Run the installer script to create database and configuration file."""
     data_dir = '/var/lib/mediawiki-db/'
     if not os.path.exists(data_dir):
@@ -103,10 +58,10 @@ def subcommand_setup(_):
             ])
     subprocess.run(['chmod', '-R', 'o-rwx', data_dir], check=True)
     subprocess.run(['chown', '-R', 'www-data:www-data', data_dir], check=True)
-    include_custom_config()
+    _include_custom_config()
 
 
-def include_custom_config():
+def _include_custom_config():
     """Include FreedomBox specific configuration in LocalSettings.php."""
     with open(LOCAL_SETTINGS_CONF, 'r', encoding='utf-8') as conf_file:
         lines = conf_file.readlines()
@@ -133,26 +88,30 @@ def include_custom_config():
         conf_file.writelines(lines)
 
 
-def subcommand_change_password(arguments):
+@privileged
-    """Change the password for a given user"""
+def change_password(username: str, password: str):
-    new_password = ''.join(sys.stdin)
+    """Change the password for a given user."""
     change_password_script = os.path.join(MAINTENANCE_SCRIPTS_DIR,
                                           'changePassword.php')
 
     subprocess.check_call([
-        _get_php_command(), change_password_script, '--user',
+        _get_php_command(), change_password_script, '--user', username,
-        arguments.username, '--password', new_password
+        '--password', password
     ])
 
 
-def subcommand_update(_):
+@privileged
+def update():
     """Run update.php maintenance script when version upgrades happen."""
     update_script = os.path.join(MAINTENANCE_SCRIPTS_DIR, 'update.php')
     subprocess.check_call([_get_php_command(), update_script, '--quick'])
 
 
-def subcommand_public_registrations(arguments):
+@privileged
+def public_registrations(command: str) -> Optional[bool]:
     """Enable or Disable public registrations for MediaWiki."""
+    if command not in ('enable', 'disable', 'status'):
+        raise ValueError('Invalid command')
 
     with open(CONF_FILE, 'r', encoding='utf-8') as conf_file:
         lines = conf_file.readlines()
@@ -160,28 +119,31 @@ def subcommand_public_registrations(arguments):
     def is_pub_reg_line(line):
         return line.startswith("$wgGroupPermissions['*']['createaccount']")
 
-    if arguments.command == 'status':
+    if command == 'status':
         conf_lines = list(filter(is_pub_reg_line, lines))
-        if conf_lines:
+        return bool(conf_lines and 'true' in conf_lines[0])
-            print('enabled' if 'true' in conf_lines[0] else 'disabled')
+
-        else:
+    with open(CONF_FILE, 'w', encoding='utf-8') as conf_file:
-            print('disabled')
+        for line in lines:
-    else:
+            if is_pub_reg_line(line):
-        with open(CONF_FILE, 'w', encoding='utf-8') as conf_file:
+                words = line.split()
-            for line in lines:
+                if command == 'enable':
-                if is_pub_reg_line(line):
+                    words[-1] = 'true;'
-                    words = line.split()
-                    if arguments.command == 'enable':
-                        words[-1] = 'true;'
-                    else:
-                        words[-1] = 'false;'
-                    conf_file.write(" ".join(words) + '\n')
                 else:
-                    conf_file.write(line)
+                    words[-1] = 'false;'
+                conf_file.write(" ".join(words) + '\n')
+            else:
+                conf_file.write(line)
+
+    return None
 
 
-def subcommand_private_mode(arguments):
+@privileged
-    """Enable or Disable Private mode for wiki"""
+def private_mode(command: str):
+    """Enable or Disable Private mode for wiki."""
+    if command not in ('enable', 'disable', 'status'):
+        raise ValueError('Invalid command')
+
     with open(CONF_FILE, 'r', encoding='utf-8') as conf_file:
         lines = conf_file.readlines()
 
@@ -189,25 +151,22 @@ def subcommand_private_mode(arguments):
         return line.startswith("$wgGroupPermissions['*']['read']")
 
     read_conf_lines = list(filter(is_read_line, lines))
-    if arguments.command == 'status':
+    if command == 'status':
-        if read_conf_lines and 'false' in read_conf_lines[0]:
+        return (read_conf_lines and 'false' in read_conf_lines[0])
-            print('enabled')
-        else:
-            print('disabled')
-    else:
-        with open(CONF_FILE, 'w', encoding='utf-8') as conf_file:
-            conf_value = 'false;' if arguments.command == 'enable' else 'true;'
-            for line in lines:
-                if is_read_line(line):
-                    words = line.split()
-                    words[-1] = conf_value
-                    conf_file.write(" ".join(words) + '\n')
-                else:
-                    conf_file.write(line)
 
-            if not read_conf_lines:
+    with open(CONF_FILE, 'w', encoding='utf-8') as conf_file:
-                conf_file.write("$wgGroupPermissions['*']['read'] = " +
+        conf_value = 'false;' if command == 'enable' else 'true;'
-                                conf_value + '\n')
+        for line in lines:
+            if is_read_line(line):
+                words = line.split()
+                words[-1] = conf_value
+                conf_file.write(" ".join(words) + '\n')
+            else:
+                conf_file.write(line)
+
+        if not read_conf_lines:
+            conf_file.write("$wgGroupPermissions['*']['read'] = " +
+                            conf_value + '\n')
 
 
 def _update_setting(setting_name, setting_line):
@@ -229,31 +188,20 @@ def _update_setting(setting_name, setting_line):
         conf_file.writelines(lines)
 
 
-def subcommand_set_default_skin(arguments):
+@privileged
+def set_default_skin(skin: str):
     """Set a default skin."""
-    skin = arguments.skin
     _update_setting('$wgDefaultSkin ', f'$wgDefaultSkin = "{skin}";\n')
 
 
-def subcommand_set_server_url(arguments):
+@privileged
+def set_server_url(server_url: str):
     """Set the value of $wgServer for this MediaWiki server."""
     # This is a required setting from MediaWiki 1.34
-    _update_setting('$wgServer', f'$wgServer = "{arguments.server_url}";\n')
+    _update_setting('$wgServer', f'$wgServer = "{server_url}";\n')
 
 
-def subcommand_set_site_name(arguments):
+@privileged
+def set_site_name(site_name: str):
     """Set the value of $wgSitename for this MediaWiki server."""
-    _update_setting('$wgSitename', f'$wgSitename = "{arguments.site_name}";\n')
+    _update_setting('$wgSitename', f'$wgSitename = "{site_name}";\n')
-
-
-def main():
-    """Parse arguments and perform all duties."""
-    arguments = parse_arguments()
-
-    subcommand = arguments.subcommand.replace('-', '_')
-    subcommand_method = globals()['subcommand_' + subcommand]
-    subcommand_method(arguments)
-
-
-if __name__ == '__main__':
-    main()

plinth/modules/mediawiki/tests/test_functional.py

@@ -25,9 +25,21 @@ class TestMediawikiApp(functional.BaseAppTests):
         """Setup the app."""
         functional.login(session_browser)
         functional.install(session_browser, 'mediawiki')
+        functional.app_enable(session_browser, 'mediawiki')
         _set_domain(session_browser)
+        _set_admin_password(session_browser, 'whatever123')
 
-    def test_public_registrations(self, session_browser):
+    @pytest.fixture(name='no_login')
+    def fixture_no_login(self, session_browser):
+        """Ensure logout from MediaWiki."""
+        _logout(session_browser)
+
+    @pytest.fixture(name='login')
+    def fixture_login(self, session_browser):
+        """Ensure login to MediaWiki."""
+        _login_with_credentials(session_browser, 'admin', 'whatever123')
+
+    def test_public_registrations(self, session_browser, no_login):
         """Test enabling public registrations."""
         _enable_public_registrations(session_browser)
         _verify_create_account_link(session_browser)
@@ -35,7 +47,7 @@ class TestMediawikiApp(functional.BaseAppTests):
         _disable_public_registrations(session_browser)
         _verify_no_create_account_link(session_browser)
 
-    def test_private_mode(self, session_browser):
+    def test_private_mode(self, session_browser, no_login):
         """Test enabling private mode."""
         _enable_private_mode(session_browser)
         _verify_no_create_account_link(session_browser)
@@ -44,7 +56,8 @@ class TestMediawikiApp(functional.BaseAppTests):
         _disable_private_mode(session_browser)
         _verify_anonymous_reads_edits_link(session_browser)
 
-    def test_private_mode_public_registrations(self, session_browser):
+    def test_private_mode_public_registrations(self, session_browser,
+                                               no_login):
         """Test interactive between private mode and public registrations.
 
         Requires JS."""
@@ -58,25 +71,18 @@ class TestMediawikiApp(functional.BaseAppTests):
         _enable_public_registrations(session_browser)
         _verify_create_account_link(session_browser)
 
-    def test_upload_files(self, session_browser):
+    def test_upload_images(self, session_browser, login):
-        """Test that logged in user can see upload files option.
-
-        Requires JS."""
-        _set_admin_password(session_browser, 'whatever123')
-        _login_with_credentials(session_browser, 'admin', 'whatever123')
-
-    def test_upload_images(self, session_browser):
         """Test uploading an image."""
         _upload_image(session_browser, 'admin', 'whatever123', 'noise.png')
         assert _image_exists(session_browser, 'Noise.png')
 
-    def test_upload_svg_image(self, session_browser):
+    def test_upload_svg_image(self, session_browser, login):
         """Test uploading an SVG image."""
         _upload_image(session_browser, 'admin', 'whatever123',
                       'apps-background.svg')
         assert _image_exists(session_browser, 'Apps-background.svg')
 
-    def test_backup_restore(self, session_browser):
+    def test_backup_restore(self, session_browser, login):
         """Test backup and restore of pages and images."""
         if not _image_exists(session_browser, 'Noise.png'):
             _upload_image(session_browser, 'admin', 'whatever123', 'Noise.png')
@@ -147,19 +153,19 @@ def _verify_no_create_account_link(browser):
         lambda: not _is_create_account_available(browser))
 
 
-def _is_anonymouse_read_allowed(browser):
+def _is_anonymous_read_allowed(browser):
     """Load the main page and check if anonymous reading is allowed."""
     functional.visit(browser, '/mediawiki')
     return browser.is_element_present_by_id('ca-nstab-main')
 
 
 def _verify_anonymous_reads_edits_link(browser):
-    assert functional.eventually(_is_anonymouse_read_allowed, args=[browser])
+    assert functional.eventually(_is_anonymous_read_allowed, args=[browser])
 
 
 def _verify_no_anonymous_reads_edits_link(browser):
     assert functional.eventually(
-        lambda: not _is_anonymouse_read_allowed(browser))
+        lambda: not _is_anonymous_read_allowed(browser))
     assert browser.is_element_present_by_id('ca-nstab-special')
 
 
@@ -179,6 +185,13 @@ def _login_with_credentials(browser, username, password):
                                  args=['t-upload'])
 
 
+def _logout(browser):
+    """Logout from MediaWiki."""
+    functional.visit(browser, '/mediawiki/Special:UserLogout')
+    if browser.find_by_css('#bodyContent form'):
+        functional.submit(browser, form_class='oo-ui-formLayout')
+
+
 def _upload_image(browser, username, password, image, ignore_warnings=True):
     """Upload an image to MediaWiki. Idempotent."""
     functional.visit(browser, '/mediawiki')

plinth/modules/mediawiki/tests/test_settings.py

@@ -10,15 +10,17 @@ from unittest.mock import patch
 import pytest
 
 from plinth.modules import mediawiki
+from plinth.modules.mediawiki import privileged
 
-actions_name = 'mediawiki'
+pytestmark = pytest.mark.usefixtures('mock_privileged')
 current_directory = pathlib.Path(__file__).parent
+privileged_modules_to_mock = ['plinth.modules.mediawiki.privileged']
 
 
 @pytest.fixture(autouse=True)
-def fixture_setup_configuration(actions_module, conf_file):
+def fixture_setup_configuration(conf_file):
     """Set configuration file path in actions module."""
-    actions_module.CONF_FILE = conf_file
+    privileged.CONF_FILE = conf_file
 
 
 @pytest.fixture(name='conf_file')
@@ -34,12 +36,11 @@ def fixture_conf_file(tmp_path):
 
 
 @pytest.fixture(name='test_configuration', autouse=True)
-def fixture_test_configuration(call_action, conf_file):
+def fixture_test_configuration(conf_file):
     """Use a separate MediaWiki configuration for tests.
 
     Uses local FreedomBoxStaticSettings.php, a temp version of
-    FreedomBoxSettings.php and patches actions.superuser_run with the fixture
+    FreedomBoxSettings.php
-    call_action
 
     """
     data_directory = pathlib.Path(__file__).parent.parent / 'data'
@@ -47,8 +48,7 @@ def fixture_test_configuration(call_action, conf_file):
                              mediawiki.STATIC_CONFIG_FILE.split('/')[-1])
     with patch('plinth.modules.mediawiki.STATIC_CONFIG_FILE',
                static_config_file), \
-            patch('plinth.modules.mediawiki.USER_CONFIG_FILE', conf_file), \
+         patch('plinth.modules.mediawiki.USER_CONFIG_FILE', conf_file):
-            patch('plinth.actions.superuser_run', call_action):
         yield
 
 
@@ -56,7 +56,7 @@ def test_default_skin():
     """Test getting and setting the default skin."""
     assert mediawiki.get_default_skin() == 'timeless'
     new_skin = 'vector'
-    mediawiki.set_default_skin(new_skin)
+    privileged.set_default_skin(new_skin)
     assert mediawiki.get_default_skin() == new_skin
 
 
@@ -72,5 +72,5 @@ def test_site_name():
     """Test getting and setting $wgSitename."""
     assert mediawiki.get_site_name() == 'Wiki'
     new_site_name = 'My MediaWiki'
-    mediawiki.set_site_name(new_site_name)
+    privileged.set_site_name(new_site_name)
     assert mediawiki.get_site_name() == new_site_name

plinth/modules/mediawiki/views.py

@@ -1,21 +1,16 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app for configuring MediaWiki."""
-FreedomBox app for configuring MediaWiki.
-"""
 
 import logging
 
 from django.contrib import messages
 from django.utils.translation import gettext as _
 
-from plinth import actions
 from plinth import app as app_module
 from plinth import views
-from plinth.errors import ActionError
 from plinth.modules import mediawiki
 
-from . import (get_default_skin, get_server_url, get_site_name,
+from . import get_default_skin, get_server_url, get_site_name, privileged
-               is_private_mode_enabled, is_public_registration_enabled)
 from .forms import MediaWikiForm
 
 logger = logging.getLogger(__name__)
@@ -23,6 +18,7 @@ logger = logging.getLogger(__name__)
 
 class MediaWikiAppView(views.AppView):
     """App configuration page."""
+
     app_id = 'mediawiki'
     form_class = MediaWikiForm
     template_name = 'mediawiki.html'
@@ -31,11 +27,16 @@ class MediaWikiAppView(views.AppView):
         """Return the values to fill in the form."""
         initial = super().get_initial()
         initial.update({
-            'enable_public_registrations': is_public_registration_enabled(),
+            'enable_public_registrations':
-            'enable_private_mode': is_private_mode_enabled(),
+                privileged.public_registrations('status'),
-            'default_skin': get_default_skin(),
+            'enable_private_mode':
-            'domain': get_server_url(),
+                privileged.private_mode('status'),
-            'site_name': get_site_name()
+            'default_skin':
+                get_default_skin(),
+            'domain':
+                get_server_url(),
+            'site_name':
+                get_site_name()
         })
         return initial
 
@@ -49,10 +50,9 @@ class MediaWikiAppView(views.AppView):
 
         if new_config['password']:
             try:
-                actions.superuser_run('mediawiki', ['change-password'],
+                privileged.change_password('admin', new_config['password'])
-                                      input=new_config['password'].encode())
                 messages.success(self.request, _('Password updated'))
-            except ActionError as exception:
+            except Exception as exception:
                 logger.exception('Failed to update password: %s', exception)
                 messages.error(
                     self.request,
@@ -63,8 +63,7 @@ class MediaWikiAppView(views.AppView):
             # note action public-registration restarts, if running now
             if new_config['enable_public_registrations']:
                 if not new_config['enable_private_mode']:
-                    actions.superuser_run('mediawiki',
+                    privileged.public_registrations('enable')
-                                          ['public-registrations', 'enable'])
                     messages.success(self.request,
                                      _('Public registrations enabled'))
                 else:
@@ -72,21 +71,19 @@ class MediaWikiAppView(views.AppView):
                         self.request, 'Public registrations ' +
                         'cannot be enabled when private mode is enabled')
             else:
-                actions.superuser_run('mediawiki',
+                privileged.public_registrations('disable')
-                                      ['public-registrations', 'disable'])
                 messages.success(self.request,
                                  _('Public registrations disabled'))
 
         if is_changed('enable_private_mode'):
             if new_config['enable_private_mode']:
-                actions.superuser_run('mediawiki', ['private-mode', 'enable'])
+                privileged.private_mode('enable')
                 messages.success(self.request, _('Private mode enabled'))
                 if new_config['enable_public_registrations']:
                     # If public registrations are enabled, then disable it
-                    actions.superuser_run('mediawiki',
+                    privileged.public_registrations('disable')
-                                          ['public-registrations', 'disable'])
             else:
-                actions.superuser_run('mediawiki', ['private-mode', 'disable'])
+                privileged.private_mode('disable')
                 messages.success(self.request, _('Private mode disabled'))
 
             app = app_module.App.get('mediawiki')
@@ -94,7 +91,7 @@ class MediaWikiAppView(views.AppView):
             shortcut.login_required = new_config['enable_private_mode']
 
         if is_changed('default_skin'):
-            mediawiki.set_default_skin(new_config['default_skin'])
+            privileged.set_default_skin(new_config['default_skin'])
             messages.success(self.request, _('Default skin changed'))
 
         if is_changed('domain'):
@@ -102,7 +99,7 @@ class MediaWikiAppView(views.AppView):
             messages.success(self.request, _('Domain name updated'))
 
         if is_changed('site_name'):
-            mediawiki.set_site_name(new_config['site_name'])
+            privileged.set_site_name(new_config['site_name'])
             messages.success(self.request, _('Site name updated'))
 
         return super().form_valid(form)