From 585092ca634818ed81a82d3f199ff9e52b862da2 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Fri, 2 Sep 2022 10:43:42 -0700 Subject: [PATCH] actions: Allow nested and top-level actions - Currently, privileged actions are not allowed under top-level plinth module. They are only allowed under each app module. Allow privileged actions under plinth module. - Currently, privileged actions are not allowed under a sub-module of 'privileged' package. They are allowed only in 'privileged' module. Allow sub-modules under 'privileged' package. Tests: - Email app functional tests pass - Functional tests for apps using package and service privileged methods pass Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- actions/actions | 6 ++++- plinth/actions.py | 13 ++++++--- plinth/modules/email/privileged/__init__.py | 2 ++ plinth/tests/test_actions.py | 29 +++++---------------- 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/actions/actions b/actions/actions index 1cebe5eae..c1ce7b1dd 100755 --- a/actions/actions +++ b/actions/actions @@ -59,7 +59,11 @@ def _call(module_name, action_name, arguments): raise SyntaxError('Invalid module name') cfg.read() - import_path = module_loader.get_module_import_path(module_name) + if module_name == 'plinth': + import_path = 'plinth' + else: + import_path = module_loader.get_module_import_path(module_name) + try: module = importlib.import_module(import_path + '.privileged') except ModuleNotFoundError as exception: diff --git a/plinth/actions.py b/plinth/actions.py index cb77b4fc7..68c17c241 100644 --- a/plinth/actions.py +++ b/plinth/actions.py @@ -84,7 +84,6 @@ import os import re import shlex import subprocess -import sys from plinth import cfg from plinth.errors import ActionError @@ -311,5 +310,13 @@ def _check_privileged_action_arguments(func): def _get_privileged_action_module_name(func): """Figure out the module name of a privileged action.""" module_name = func.__module__ - module = sys.modules[module_name] - return module.__package__.rpartition('.')[2] + while module_name: + module_name, _, last = module_name.rpartition('.') + if last == 'privileged': + break + + if not module_name: + raise ValueError('Privileged actions must be placed under a ' + 'package/module named privileged') + + return module_name.rpartition('.')[2] diff --git a/plinth/modules/email/privileged/__init__.py b/plinth/modules/email/privileged/__init__.py index 8591aa98c..999f6efc0 100644 --- a/plinth/modules/email/privileged/__init__.py +++ b/plinth/modules/email/privileged/__init__.py @@ -6,3 +6,5 @@ Provides privileged actions that run as root. from . import aliases, dkim, domain, home, postfix, spam, tls __all__ = ['aliases', 'domain', 'dkim', 'home', 'postfix', 'spam', 'tls'] + +from .aliases import action_setup diff --git a/plinth/tests/test_actions.py b/plinth/tests/test_actions.py index ca7335747..48baa6e31 100644 --- a/plinth/tests/test_actions.py +++ b/plinth/tests/test_actions.py @@ -14,13 +14,11 @@ import shutil import tempfile from unittest.mock import call, patch -import apt_pkg import pytest from plinth import cfg from plinth.actions import _log_command as log_command from plinth.actions import privileged, run, superuser_run -from plinth.errors import ActionError @pytest.fixture(autouse=True) @@ -36,7 +34,6 @@ def actions_test_setup(load_cfg): cfg.actions_dir = str(tmp_directory) actions_dir = pathlib.Path(__file__).parent / '../../actions' - shutil.copy(str(actions_dir / 'packages'), str(tmp_directory)) shutil.copy(str(actions_dir / 'test_path'), str(tmp_directory)) shutil.copy('/bin/echo', str(tmp_directory)) shutil.copy('/usr/bin/id', str(tmp_directory)) @@ -143,24 +140,6 @@ def test_multiple_options_and_output(): assert options == output -@pytest.mark.usefixtures('needs_root') -def test_is_package_manager_busy(): - """Test the behavior of `is-package-manager-busy` in both locked and - unlocked states of the dpkg lock file.""" - - apt_pkg.init() # initialize apt_pkg module - - # In the locked state, the lsof command returns 0. - # Hence no error is thrown. - with apt_pkg.SystemLock(): - superuser_run('packages', ['is-package-manager-busy']) - - # In the unlocked state, the lsof command returns 1. - # An ActionError is raised in this case. - with pytest.raises(ActionError): - superuser_run('packages', ['is-package-manager-busy']) - - @pytest.mark.usefixtures('develop_mode', 'needs_root') def test_action_path(monkeypatch): """Test that in development mode, python action scripts get the @@ -247,14 +226,16 @@ def test_privileged_argument_annotation_check(): privileged(func_valid) +@patch('plinth.actions._get_privileged_action_module_name') @patch('plinth.actions.superuser_run') -def test_privileged_method_call(superuser_run_): +def test_privileged_method_call(superuser_run_, get_module_name): """Test that privileged method calls the superuser action properly.""" def func_with_args(_a: int, _b: str, _c: int = 1, _d: str = 'dval', _e: str = 'eval'): return + get_module_name.return_value = 'tests' superuser_run_.return_value = json.dumps({ 'result': 'success', 'return': 'bar' @@ -269,13 +250,15 @@ def test_privileged_method_call(superuser_run_): [call('actions', ['tests', 'func_with_args'], input=input_.encode())]) +@patch('plinth.actions._get_privileged_action_module_name') @patch('plinth.actions.superuser_run') -def test_privileged_method_exceptions(superuser_run_): +def test_privileged_method_exceptions(superuser_run_, get_module_name): """Test that exceptions on privileged methods are return properly.""" def func_with_exception(): raise TypeError('type error') + get_module_name.return_value = 'tests' superuser_run_.return_value = json.dumps({ 'result': 'exception', 'exception': {