ttrss: Use privileged decorator for actions

Tests:

- Ignore setting a None domain
- Updated tests to use base class

- Functional tests work
  - Backup/restore works. Database is dumped and restored.
- Initial setup works
- Enabling/disabling works
  - API access is enabled and a valid domain is set when available
- Setting the domain works
  - Configuration is updated in update.php
  - App page show newly set domain
- Not tested: force upgrade of package

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

plinth/modules/ttrss/__init__.py

@@ -1,12 +1,9 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""FreedomBox app to configure Tiny Tiny RSS."""
-FreedomBox app to configure Tiny Tiny RSS.
-"""
 
 from django.urls import reverse_lazy
 from django.utils.translation import gettext_lazy as _
 
-from plinth import actions
 from plinth import app as app_module
 from plinth import cfg, frontpage, menu
 from plinth.daemon import Daemon
@@ -17,7 +14,7 @@ from plinth.modules.users.components import UsersAndGroups
 from plinth.package import Packages, install
 from plinth.utils import Version, format_lazy
 
-from . import manifest
+from . import manifest, privileged
 
 _description = [
     _('Tiny Tiny RSS is a news feed (RSS/Atom) reader and aggregator, '
@@ -97,20 +94,20 @@ class TTRSSApp(app_module.App):
     def enable(self):
         """Enable components and API access."""
         super().enable()
-        actions.superuser_run('ttrss', ['enable-api-access'])
+        privileged.enable_api_access()
 
         # Try to set the domain to one of the available TLS domains
-        domain = get_domain()
+        domain = privileged.get_domain()
         if not domain or domain == 'localhost':
             from plinth.modules import names
             domain = next(names.get_available_tls_domains(), None)
-            set_domain(domain)
+            privileged.set_domain(domain)
 
     def setup(self, old_version):
         """Install and configure the app."""
-        actions.superuser_run('ttrss', ['pre-setup'])
+        privileged.pre_setup()
         super().setup(old_version)
-        actions.superuser_run('ttrss', ['setup'])
+        privileged.setup()
         self.enable()
 
     def force_upgrade(self, packages):
@@ -124,30 +121,19 @@ class TTRSSApp(app_module.App):
             return False
 
         install(['tt-rss'], force_configuration='new')
-        actions.superuser_run('ttrss', ['setup'])
+        privileged.setup()
         return True
 
 
 class TTRSSBackupRestore(BackupRestore):
-    """Component to backup/restore TT-RSS"""
+    """Component to backup/restore TT-RSS."""
 
     def backup_pre(self, packet):
         """Save database contents."""
         super().backup_pre(packet)
-        actions.superuser_run('ttrss', ['dump-database'])
+        privileged.dump_database()
 
     def restore_post(self, packet):
         """Restore database contents."""
         super().restore_post(packet)
-        actions.superuser_run('ttrss', ['restore-database'])
+        privileged.restore_database()
-
-
-def get_domain():
-    """Read TLS domain from tt-rss config file."""
-    return actions.superuser_run('ttrss', ['get-domain']).strip()
-
-
-def set_domain(domain):
-    """Set the TLS domain in tt-rss configuration file."""
-    if domain:
-        actions.superuser_run('ttrss', ['set-domain', domain])

plinth/modules/ttrss/privileged.py

@@ -1,16 +1,14 @@
-#!/usr/bin/python3
 # SPDX-License-Identifier: AGPL-3.0-or-later
-"""
+"""Configure Tiny Tiny RSS."""
-Configuration helper for Tiny Tiny RSS.
-"""
 
-import argparse
 import os
 import subprocess
+from typing import Optional
 
 import augeas
 
 from plinth import action_utils
+from plinth.actions import privileged
 
 CONFIG_FILE = '/etc/tt-rss/config.php'
 DEFAULT_FILE = '/etc/default/tt-rss'
@@ -18,36 +16,15 @@ DATABASE_FILE = '/etc/tt-rss/database.php'
 DB_BACKUP_FILE = '/var/lib/plinth/backups-data/ttrss-database.sql'
 
 
-def parse_arguments():
+@privileged
-    """Return parsed command line arguments as dictionary."""
+def pre_setup():
-    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('setup', help='Setup Tiny Tiny RSS configuration')
-    subparsers.add_parser('enable-api-access', help='Enable Tiny Tiny RSS API')
-    subparsers.add_parser('dump-database', help='Dump database to file')
-    subparsers.add_parser('restore-database',
-                          help='Restore database from file')
-    subparsers.add_parser('get-domain',
-                          help='Get the domain set for Tiny Tiny RSS.')
-    set_domain = subparsers.add_parser(
-        'set-domain', help='Set the domain to be used by Tiny Tiny RSS.')
-    set_domain.add_argument(
-        'domain_name',
-        help='The domain name that will be used by Tiny Tiny RSS.')
-
-    subparsers.required = True
-    return parser.parse_args()
-
-
-def subcommand_pre_setup(_):
     """Preseed debconf values before packages are installed."""
     action_utils.debconf_set_selections(
         ['tt-rss tt-rss/database-type string pgsql'])
 
 
-def subcommand_get_domain(_):
+@privileged
+def get_domain() -> Optional[str]:
     """Get the domain set for Tiny Tiny RSS."""
     aug = load_augeas()
 
@@ -55,12 +32,18 @@ def subcommand_get_domain(_):
     for match in aug.match('/files' + CONFIG_FILE + '/define'):
         if aug.get(match) == 'SELF_URL_PATH':
             url = aug.get(match + '/value').strip("'")
-            print(urlparse(url).netloc)
+            return urlparse(url).netloc
+
+    return None
 
 
-def subcommand_set_domain(args):
+@privileged
+def set_domain(domain_name: Optional[str]):
     """Set the domain to be used by Tiny Tiny RSS."""
-    url = f"'https://{args.domain_name}/tt-rss/'"
+    if not domain_name:
+        return
+
+    url = f"'https://{domain_name}/tt-rss/'"
     aug = load_augeas()
 
     for match in aug.match('/files' + CONFIG_FILE + '/define'):
@@ -70,7 +53,8 @@ def subcommand_set_domain(args):
     aug.save()
 
 
-def subcommand_setup(_):
+@privileged
+def setup():
     """Setup Tiny Tiny RSS configuration."""
     aug = load_augeas()
 
@@ -96,7 +80,8 @@ def subcommand_setup(_):
         action_utils.service_restart('tt-rss')
 
 
-def subcommand_enable_api_access(_):
+@privileged
+def enable_api_access():
     """Enable API access so that tt-rss can be accessed through mobile app."""
     import psycopg2  # Only available post installation
 
@@ -123,14 +108,16 @@ def subcommand_enable_api_access(_):
     connection.close()
 
 
-def subcommand_dump_database(_):
+@privileged
+def dump_database():
     """Dump database to file."""
     os.makedirs(os.path.dirname(DB_BACKUP_FILE), exist_ok=True)
     with open(DB_BACKUP_FILE, 'w', encoding='utf-8') as db_backup_file:
         _run_as_postgres(['pg_dump', 'ttrss'], stdout=db_backup_file)
 
 
-def subcommand_restore_database(_):
+@privileged
+def restore_database():
     """Restore database from file."""
     _run_as_postgres(['dropdb', 'ttrss'])
     _run_as_postgres(['createdb', 'ttrss'])
@@ -155,16 +142,3 @@ def load_augeas():
     aug.set('/augeas/load/Phpvars/incl[last() + 1]', DATABASE_FILE)
     aug.load()
     return aug
-
-
-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/ttrss/tests/test_functional.py

@@ -12,38 +12,25 @@ APP_ID = 'ttrss'
 pytestmark = [pytest.mark.apps, pytest.mark.ttrss, pytest.mark.sso]
 
 
-@pytest.fixture(scope='module', autouse=True)
+class TestTTRSSApp(functional.BaseAppTests):
-def fixture_background(session_browser):
+    """Class to customize basic app tests for TTRSS."""
-    """Login and install the app."""
-    functional.login(session_browser)
-    functional.install(session_browser, APP_ID)
-    yield
-    functional.app_disable(session_browser, APP_ID)
 
+    app_name = 'ttrss'
+    has_service = True
+    has_web = True
 
-def test_enable_disable(session_browser):
+    @pytest.mark.backups
-    """Test enabling the app."""
+    def test_backup_restore(self, session_browser):
-    functional.app_disable(session_browser, APP_ID)
+        """Test backup and restore of app data."""
+        functional.app_enable(session_browser, APP_ID)
+        _subscribe(session_browser)
+        functional.backup_create(session_browser, APP_ID, 'test_ttrss')
 
-    functional.app_enable(session_browser, APP_ID)
+        _unsubscribe(session_browser)
-    assert functional.service_is_running(session_browser, APP_ID)
+        functional.backup_restore(session_browser, APP_ID, 'test_ttrss')
 
-    functional.app_disable(session_browser, APP_ID)
+        assert functional.service_is_running(session_browser, APP_ID)
-    assert functional.service_is_not_running(session_browser, APP_ID)
+        assert _is_subscribed(session_browser)
-
-
-@pytest.mark.backups
-def test_backup_restore(session_browser):
-    """Test backup and restore of app data."""
-    functional.app_enable(session_browser, APP_ID)
-    _subscribe(session_browser)
-    functional.backup_create(session_browser, APP_ID, 'test_ttrss')
-
-    _unsubscribe(session_browser)
-    functional.backup_restore(session_browser, APP_ID, 'test_ttrss')
-
-    assert functional.service_is_running(session_browser, APP_ID)
-    assert _is_subscribed(session_browser)
 
 
 def _ttrss_load_main_interface(browser):

plinth/modules/ttrss/views.py

@@ -1,28 +1,33 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
+"""Django views for Tiny Tiny RSS app."""
 
 from django.contrib import messages
 from django.utils.translation import gettext_lazy as _
 
 from plinth.forms import TLSDomainForm
-from plinth.modules import ttrss
 from plinth.views import AppView
 
+from . import privileged
+
 
 class TTRSSAppView(AppView):
+    """Show TTRSS app main view."""
+
     app_id = 'ttrss'
     form_class = TLSDomainForm
 
     def get_initial(self):
         """Return the values to fill in the form."""
         initial = super().get_initial()
-        initial['domain'] = ttrss.get_domain()
+        initial['domain'] = privileged.get_domain()
         return initial
 
     def form_valid(self, form):
         """Change the domain of TT-RSS app."""
         data = form.cleaned_data
-        if ttrss.get_domain() != data['domain']:
+        old_data = form.initial
-            ttrss.set_domain(data['domain'])
+        if old_data['domain'] != data['domain']:
+            privileged.set_domain(data['domain'])
             messages.success(self.request, _('Configuration updated'))
 
         return super().form_valid(form)

plinth/tests/functional/__init__.py

@@ -44,6 +44,7 @@ _site_url = {
     'cockpit': '/_cockpit/',
     'syncthing': '/syncthing/',
     'rssbridge': '/rss-bridge/',
+    'ttrss': '/tt-rss/',
 }
 
 _sys_modules = [