diff --git a/docs/docs/plugins/install.md b/docs/docs/plugins/install.md index 3d18d58850..6f2f9f5fd6 100644 --- a/docs/docs/plugins/install.md +++ b/docs/docs/plugins/install.md @@ -74,6 +74,9 @@ Enter the package name into the form as shown below. You can add a path and a ve {{ image("plugin/plugin_install_txt.png", "Plugin.txt file") }} +!!! info "Superuser Required" + Only users with superuser privileges can manage plugins via the web interface. + #### Local Directory Custom plugins can be placed in the `data/plugins/` directory, where they will be automatically discovered. This can be useful for developing and testing plugins, but can prove more difficult in production (e.g. when using Docker). diff --git a/src/backend/InvenTree/InvenTree/settings.py b/src/backend/InvenTree/InvenTree/settings.py index 8fa1e571db..7c617ff47d 100644 --- a/src/backend/InvenTree/InvenTree/settings.py +++ b/src/backend/InvenTree/InvenTree/settings.py @@ -201,6 +201,11 @@ PLUGINS_INSTALL_DISABLED = get_boolean_setting( 'INVENTREE_PLUGIN_NOINSTALL', 'plugin_noinstall', False ) +if not PLUGINS_ENABLED: + PLUGINS_INSTALL_DISABLED = ( + True # If plugins are disabled, also disable installation + ) + PLUGIN_FILE = config.get_plugin_file() # Plugin test settings diff --git a/src/backend/InvenTree/plugin/api.py b/src/backend/InvenTree/plugin/api.py index 91580062b3..9093d6e7f4 100644 --- a/src/backend/InvenTree/plugin/api.py +++ b/src/backend/InvenTree/plugin/api.py @@ -210,6 +210,7 @@ class PluginInstall(CreateAPI): queryset = PluginConfig.objects.none() serializer_class = PluginSerializers.PluginConfigInstallSerializer + permission_classes = [InvenTree.permissions.IsSuperuserOrSuperScope] def create(self, request, *args, **kwargs): """Install a plugin via the API.""" diff --git a/src/backend/InvenTree/plugin/installer.py b/src/backend/InvenTree/plugin/installer.py index b1fb2de03f..acf6584a8d 100644 --- a/src/backend/InvenTree/plugin/installer.py +++ b/src/backend/InvenTree/plugin/installer.py @@ -236,8 +236,8 @@ def install_plugin(url=None, packagename=None, user=None, version=None): user: Optional user performing the installation version: Optional version specifier """ - if user and not user.is_staff: - raise ValidationError(_('Only staff users can administer plugins')) + if user and not user.is_superuser: + raise ValidationError(_('Only superuser accounts can administer plugins')) if settings.PLUGINS_INSTALL_DISABLED: raise ValidationError(_('Plugin installation is disabled')) @@ -269,6 +269,13 @@ def install_plugin(url=None, packagename=None, user=None, version=None): if version: full_pkg = f'{full_pkg}=={version}' + if not full_pkg: + raise ValidationError(_('No package name or URL provided for installation')) + + # Sanitize the package name for installation + if any(c in full_pkg for c in ';&|`$()'): + raise ValidationError(_('Invalid characters in package name or URL')) + install_name.append(full_pkg) ret = {} @@ -333,6 +340,9 @@ def uninstall_plugin(cfg: plugin.models.PluginConfig, user=None, delete_config=T """ from plugin.registry import registry + if user and not user.is_superuser: + raise ValidationError(_('Only superuser accounts can administer plugins')) + if settings.PLUGINS_INSTALL_DISABLED: raise ValidationError(_('Plugin uninstalling is disabled')) diff --git a/src/backend/InvenTree/plugin/serializers.py b/src/backend/InvenTree/plugin/serializers.py index 5fbff385b4..b12d0d3590 100644 --- a/src/backend/InvenTree/plugin/serializers.py +++ b/src/backend/InvenTree/plugin/serializers.py @@ -165,6 +165,9 @@ class PluginConfigInstallSerializer(serializers.Serializer): version = data.get('version', None) user = self.context['request'].user + if not user or not user.is_superuser: + raise ValidationError(_('Only superuser accounts can administer plugins')) + return install_plugin( url=url, packagename=packagename, version=version, user=user ) @@ -266,10 +269,13 @@ class PluginUninstallSerializer(serializers.Serializer): """Uninstall the specified plugin.""" from plugin.installer import uninstall_plugin + user = self.context['request'].user + + if not user or not user.is_superuser: + raise ValidationError(_('Only superuser accounts can administer plugins')) + return uninstall_plugin( - instance, - user=self.context['request'].user, - delete_config=validated_data.get('delete_config', True), + instance, user=user, delete_config=validated_data.get('delete_config', True) ) diff --git a/src/backend/InvenTree/plugin/test_api.py b/src/backend/InvenTree/plugin/test_api.py index ca6c312670..820c5c96fa 100644 --- a/src/backend/InvenTree/plugin/test_api.py +++ b/src/backend/InvenTree/plugin/test_api.py @@ -63,6 +63,21 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): """Test the plugin install command.""" url = reverse('api-plugin-install') + # Requires superuser permissions + self.user.is_superuser = False + self.user.save() + + self.post( + url, + {'confirm': True, 'packagename': self.PKG_NAME}, + expected_code=403, + max_query_time=30, + ) + + # Provide superuser permissions + self.user.is_superuser = True + self.user.save() + # invalid package name data = self.post( url, @@ -209,7 +224,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): test_plg.refresh_from_db() self.assertTrue(test_plg.is_active()) - def test_pluginCfg_delete(self): + def test_plugin_config_delete(self): """Test deleting a config.""" test_plg = self.plugin_confs.first() assert test_plg is not None diff --git a/src/backend/InvenTree/plugin/test_plugin.py b/src/backend/InvenTree/plugin/test_plugin.py index 2d7c3eba04..c918eadc3d 100644 --- a/src/backend/InvenTree/plugin/test_plugin.py +++ b/src/backend/InvenTree/plugin/test_plugin.py @@ -11,11 +11,14 @@ from typing import Optional from unittest import mock from unittest.mock import patch +from django.contrib.auth.models import User +from django.core.exceptions import ValidationError from django.test import TestCase, override_settings import plugin.templatetags.plugin_extras as plugin_tags from InvenTree.unit_test import PluginRegistryMixin, TestQueryMixin from plugin import InvenTreePlugin, PluginMixinEnum +from plugin.installer import install_plugin from plugin.registry import registry from plugin.samples.integration.another_sample import ( NoIntegrationPlugin, @@ -326,6 +329,9 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase): def test_broken_samples(self): """Test that the broken samples trigger reloads.""" + # Reset the registry to a known state + registry.errors = {} + # In the base setup there are no errors self.assertEqual(len(registry.errors), 0) @@ -686,3 +692,40 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase): self.assertTrue(cfg.is_builtin()) self.assertFalse(cfg.is_package()) self.assertFalse(cfg.is_sample()) + + +class InstallerTests(TestCase): + """Tests for the plugin installer code.""" + + def test_plugin_install_errors(self): + """Test error handling for plugin installation.""" + # No data provided + with self.assertRaises(ValidationError) as e: + install_plugin() + + self.assertIn( + 'No package name or URL provided for installation', str(e.exception) + ) + + # Invalid package name + for pkg in [ + 'invalid;name', + 'invalid&name', + 'invalid|name', + 'invalid`name', + 'invalid$(name)', + ]: + with self.assertRaises(ValidationError) as e: + install_plugin(packagename=pkg) + + self.assertIn('Invalid characters in package name or URL', str(e.exception)) + + # Non superuser account + user = User.objects.create(username='my-user', is_superuser=False) + + with self.assertRaises(ValidationError) as e: + install_plugin(user=user, packagename='some-package') + + self.assertIn( + 'Only superuser accounts can administer plugins', str(e.exception) + ) diff --git a/src/frontend/src/tables/plugin/PluginListTable.tsx b/src/frontend/src/tables/plugin/PluginListTable.tsx index 711d61f987..580414ffaa 100644 --- a/src/frontend/src/tables/plugin/PluginListTable.tsx +++ b/src/frontend/src/tables/plugin/PluginListTable.tsx @@ -220,7 +220,6 @@ export default function PluginListTable() { // Uninstall an installed plugin // Must be inactive, not a builtin, not a sample, and installed as a package hidden: - !user.isSuperuser() || record.active || record.is_builtin || record.is_mandatory || @@ -244,8 +243,7 @@ export default function PluginListTable() { record.is_builtin || record.is_mandatory || record.is_sample || - record.is_installed || - !user.isSuperuser(), + record.is_installed, title: t`Delete`, tooltip: t`Delete selected plugin configuration`, color: 'red', @@ -355,7 +353,12 @@ export default function PluginListTable() { // Custom table actions const tableActions = useMemo(() => { - if (!user.isSuperuser() || !server.plugins_enabled) { + if ( + !user.isSuperuser() || + !server.plugins_enabled || + server.plugins_install_disabled + ) { + // Prevent installation if plugins are disabled or user is not superuser return []; } @@ -376,7 +379,6 @@ export default function PluginListTable() { setPluginPackage(''); installPluginModal.open(); }} - disabled={server.plugins_install_disabled || false} /> ]; }, [user, server]);