bepasty: Use privileged decorator for actions

Tests:

- Functional tests
- Initial setup
  - Sets the domain to freedombox.local (SITENAME)
  - Default permissions are set to read
  - Three passwords with varying permissions are create by default
- Current configuration is retrieved properly (default permissions, passwords)
- Adding passwords works, they are list as expected
  - With or without comment
- Removing password works
- Setting default permissions works
- Untested:
  - Upgrade from version 1

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

plinth/modules/bepasty/__init__.py

@@ -1,13 +1,8 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app for bepasty."""
-FreedomBox app for bepasty.
-"""
-
-import json
 
 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.modules.apache.components import Uwsgi, Webserver
@@ -15,7 +10,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 = [
     _('bepasty is a web application that allows large files to be uploaded '
@@ -97,38 +92,10 @@ class BepastyApp(app_module.App):
     def setup(self, old_version):
         """Install and configure the app."""
         super().setup(old_version)
-        actions.superuser_run('bepasty',
+        privileged.setup('freedombox.local')
-                              ['setup', '--domain-name', 'freedombox.local'])
         self.enable()
-        if old_version == 1 and not get_configuration().get(
+        if old_version == 1 and not privileged.get_configuration().get(
                 'DEFAULT_PERMISSIONS'):
             # Upgrade to a better default only if user hasn't changed the
             # value.
-            set_default_permissions('read')
+            privileged.set_default(['read'])
-
-
-def get_configuration():
-    """Get a full configuration including passwords and defaults."""
-    output = actions.superuser_run('bepasty', ['get-configuration'])
-    return json.loads(output)
-
-
-def add_password(permissions, comment=None):
-    """Generate a password with given permissions."""
-    command = ['add-password', '--permissions'] + permissions
-    if comment:
-        command += ['--comment', comment]
-
-    actions.superuser_run('bepasty', command)
-
-
-def remove_password(password):
-    """Remove a password and its permissions."""
-    actions.superuser_run('bepasty', ['remove-password'],
-                          input=password.encode())
-
-
-def set_default_permissions(permissions):
-    """Set default permissions."""
-    perm = permissions.split()
-    actions.superuser_run('bepasty', ['set-default', '--permissions'] + perm)

plinth/modules/bepasty/privileged.py

@@ -1,10 +1,6 @@
-#!/usr/bin/python3
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Configuration helper for bepasty."""
-Configuration helper for bepasty.
-"""
 
-import argparse
 import collections
 import grp
 import json
@@ -15,11 +11,12 @@ import secrets
 import shutil
 import string
 import subprocess
-import sys
+from typing import Optional
 
 import augeas
 
 from plinth import action_utils
+from plinth.actions import privileged
 from plinth.modules import bepasty
 
 DATA_DIR = '/var/lib/bepasty'
@@ -29,42 +26,6 @@ PASSWORD_LENGTH = 20
 CONF_FILE = pathlib.Path('/etc/bepasty-freedombox.conf')
 
 
-def parse_arguments():
-    """Return parsed command line arguments as dictionary."""
-    parser = argparse.ArgumentParser()
-    subparsers = parser.add_subparsers(dest='subcommand', help='Sub command')
-
-    setup = subparsers.add_parser(
-        'setup', help='Perform post-installation operations for bepasty')
-    setup.add_argument('--domain-name', required=True,
-                       help='The domain name that will be used by bepasty')
-
-    subparsers.add_parser('get-configuration', help='Get all configuration')
-
-    add_password = subparsers.add_parser(
-        'add-password', help='Generate a password with given permissions')
-    add_password.add_argument(
-        '--permissions', nargs='+',
-        help='Any number of permissions from the set: {}'.format(', '.join(
-            bepasty.PERMISSIONS.keys())))
-    add_password.add_argument(
-        '--comment', required=False,
-        help='A comment for the password and its permissions')
-
-    subparsers.add_parser('remove-password',
-                          help='Remove a password and its permissions')
-
-    set_default = subparsers.add_parser('set-default',
-                                        help='Set default permissions')
-    set_default.add_argument(
-        '--permissions', nargs='*',
-        help='Any number of permissions from the set: {}'.format(', '.join(
-            bepasty.PERMISSIONS.keys())))
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
 def _augeas_load():
     """Initialize Augeas."""
     aug = augeas.Augeas(flags=augeas.Augeas.NO_LOAD +
@@ -104,7 +65,8 @@ def conf_file_write(conf):
     aug.save()
 
 
-def subcommand_setup(arguments):
+@privileged
+def setup(domain_name: str):
     """Post installation actions for bepasty."""
     # Create bepasty group if needed.
     try:
@@ -135,7 +97,7 @@ def subcommand_setup(arguments):
                 'the original configuration format is supported. Each line '
                 'should be in KEY = VALUE format. VALUE must be a JSON '
                 'encoded string.',
-            'SITENAME': arguments.domain_name,
+            'SITENAME': domain_name,
             'STORAGE_FILESYSTEM_DIRECTORY': '/var/lib/bepasty',
             'SECRET_KEY': secrets.token_hex(64),
             'PERMISSIONS': {
@@ -155,29 +117,30 @@ def subcommand_setup(arguments):
         shutil.chown(CONF_FILE, user='bepasty', group='bepasty')
 
 
-def subcommand_get_configuration(_):
+@privileged
+def get_configuration() -> dict[str, object]:
     """Get default permissions, passwords, permissions and comments."""
-    conf = conf_file_read()
+    return conf_file_read()
-    print(json.dumps(conf))
 
 
-def subcommand_add_password(arguments):
+@privileged
+def add_password(permissions: list[str], comment: Optional[str] = None):
     """Generate a password with given permissions."""
     conf = conf_file_read()
-    permissions = _format_permissions(arguments.permissions)
+    permissions = _format_permissions(permissions)
     password = _generate_password()
     conf['PERMISSIONS'][password] = permissions
-    if arguments.comment:
+    if comment:
-        conf['PERMISSION_COMMENTS'][password] = arguments.comment
+        conf['PERMISSION_COMMENTS'][password] = comment
 
     conf_file_write(conf)
     action_utils.service_try_restart('uwsgi')
 
 
-def subcommand_remove_password(_arguments):
+@privileged
+def remove_password(password: str):
     """Remove a password and its permissions."""
     conf = conf_file_read()
-    password = ''.join(sys.stdin)
     if password in conf['PERMISSIONS']:
         del conf['PERMISSIONS'][password]
 
@@ -187,9 +150,10 @@ def subcommand_remove_password(_arguments):
     action_utils.service_try_restart('uwsgi')
 
 
-def subcommand_set_default(arguments):
+@privileged
+def set_default(permissions: list[str]):
     """Set default permissions."""
-    conf = {'DEFAULT_PERMISSIONS': _format_permissions(arguments.permissions)}
+    conf = {'DEFAULT_PERMISSIONS': _format_permissions(permissions)}
     conf_file_write(conf)
     action_utils.service_try_restart('uwsgi')
 
@@ -204,16 +168,3 @@ def _generate_password():
     """Generate a random password."""
     alphabet = string.ascii_letters + string.digits
     return ''.join(secrets.choice(alphabet) for _ in range(PASSWORD_LENGTH))
-
-
-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/bepasty/views.py

@@ -1,7 +1,5 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Views for the bepasty app."""
-Views for the bepasty app.
-"""
 
 from django.contrib import messages
 from django.contrib.messages.views import SuccessMessageMixin
@@ -11,10 +9,9 @@ from django.utils.translation import gettext_lazy as _
 from django.views.decorators.http import require_POST
 from django.views.generic import FormView
 
-from plinth.errors import ActionError
-from plinth.modules import bepasty
 from plinth.views import AppView
 
+from . import privileged
 from .forms import AddPasswordForm, SetDefaultPermissionsForm
 
 # i18n for permission comments
@@ -27,6 +24,7 @@ PERMISSION_COMMENTS_STRINGS = {
 
 class BepastyView(AppView):
     """Serve configuration page."""
+
     app_id = 'bepasty'
     form_class = SetDefaultPermissionsForm
     template_name = 'bepasty.html'
@@ -39,7 +37,7 @@ class BepastyView(AppView):
     def _get_configuration(self):
         """Return the current configuration."""
         if not self.conf:
-            self.conf = bepasty.get_configuration()
+            self.conf = privileged.get_configuration()
 
         return self.conf
 
@@ -85,10 +83,10 @@ class BepastyView(AppView):
 
         if old_data['default_permissions'] != form_data['default_permissions']:
             try:
-                bepasty.set_default_permissions(
+                privileged.set_default(
-                    form_data['default_permissions'])
+                    form_data['default_permissions'].split(' '))
                 messages.success(self.request, _('Configuration updated.'))
-            except ActionError:
+            except Exception:
                 messages.error(self.request,
                                _('An error occurred during configuration.'))
 
@@ -112,17 +110,14 @@ class AddPasswordView(SuccessMessageMixin, FormView):
 
     def form_valid(self, form):
         """Add the password on valid form submission."""
-        _add_password(form.cleaned_data)
+        form_data = form.cleaned_data
+        privileged.add_password(form_data['permissions'], form_data['comment'])
         return super().form_valid(form)
 
 
-def _add_password(form_data):
-    bepasty.add_password(form_data['permissions'], form_data['comment'])
-
-
 @require_POST
 def remove(request, password):
     """View to remove a password."""
-    bepasty.remove_password(password)
+    privileged.remove_password(password)
     messages.success(request, _('Password deleted.'))
     return redirect(reverse_lazy('bepasty:index'))