diff --git a/conftest.py b/conftest.py index d892c6120..c250213c8 100644 --- a/conftest.py +++ b/conftest.py @@ -55,7 +55,7 @@ def fixture_load_cfg(): cfg_file = test_data_dir / 'etc' / 'plinth' / 'plinth.config' cfg.read(str(cfg_file), str(root_dir)) yield cfg - cfg.read() + cfg.read(str(cfg_file), str(root_dir)) @pytest.fixture(name='develop_mode') diff --git a/data/etc/plinth/plinth.config b/data/etc/plinth/plinth.config deleted file mode 100644 index f1bd618fe..000000000 --- a/data/etc/plinth/plinth.config +++ /dev/null @@ -1,36 +0,0 @@ -[Path] -# directory locations -file_root = /usr/share/plinth -config_dir = /etc/plinth -data_dir = /var/lib/plinth -server_dir = /plinth -actions_dir = /usr/share/plinth/actions -doc_dir = /usr/share/freedombox -custom_static_dir = /var/www/plinth/custom/static - -# file locations -store_file = %(data_dir)s/plinth.sqlite3 - -[Network] -host = 127.0.0.1 -port = 8000 - -# Enable the following only if Plinth is behind a proxy server. The -# proxy server should properly clean and the following HTTP headers: -# X-Forwarded-For -# X-Forwarded-Host -# X-Forwarded-Proto -# If you enable these unnecessarily, this will lead to serious security -# problems. For more information, see -# https://docs.djangoproject.com/en/1.7/ref/settings/ -# -# These are enabled by default in Plinth because the default -# configuration allows only connections from localhost -# -# Leave the values blank to disable -use_x_forwarded_for = True -use_x_forwarded_host = True -secure_proxy_ssl_header = HTTP_X_FORWARDED_PROTO - -[Misc] -box_name = FreedomBox diff --git a/debian/freedombox.maintscript b/debian/freedombox.maintscript index 9349c7399..2ccd25060 100644 --- a/debian/freedombox.maintscript +++ b/debian/freedombox.maintscript @@ -10,3 +10,4 @@ rm_conffile /etc/plinth/modules-enabled/udiskie 0.39.0~ rm_conffile /etc/plinth/modules-enabled/restore 20.1~ rm_conffile /etc/plinth/modules-enabled/repro 20.1~ rm_conffile /etc/apt/preferences.d/50freedombox3.pref 20.5~ +rm_conffile /etc/plinth/plinth.config 20.12~ diff --git a/plinth/cfg.py b/plinth/cfg.py index c270d2b1b..0494cd810 100644 --- a/plinth/cfg.py +++ b/plinth/cfg.py @@ -1,4 +1,7 @@ # SPDX-License-Identifier: AGPL-3.0-or-later +""" +Configuration parser and default values for configuration options. +""" import configparser import logging @@ -6,28 +9,46 @@ import os logger = logging.getLogger(__name__) -box_name = None +# [Path] section root = None -file_root = None -config_dir = None -data_dir = None -custom_static_dir = None -store_file = None -actions_dir = None -doc_dir = None -host = None -port = None -use_x_forwarded_for = False -use_x_forwarded_host = False -secure_proxy_ssl_header = None +file_root = '/usr/share/plinth' +config_dir = '/etc/plinth' +data_dir = '/var/lib/plinth' +custom_static_dir = '/var/www/plinth/custom/static' +store_file = data_dir + '/plinth.sqlite3' +actions_dir = '/usr/share/plinth/actions' +doc_dir = '/usr/share/freedombox' +server_dir = '/plinth' + +# [Network] section +host = '127.0.0.1' +port = 8000 + +# Enable the following only if Plinth is behind a proxy server. The +# proxy server should properly clean and the following HTTP headers: +# X-Forwarded-For +# X-Forwarded-Host +# X-Forwarded-Proto +# If you enable these unnecessarily, this will lead to serious security +# problems. For more information, see +# https://docs.djangoproject.com/en/1.7/ref/settings/ +# +# These are enabled by default in FreedomBox because the default +# configuration allows only connections from localhost +# +# Leave the values blank to disable +use_x_forwarded_for = True +use_x_forwarded_host = True +secure_proxy_ssl_header = 'HTTP_X_FORWARDED_PROTO' + +# [Misc] section +box_name = 'FreedomBox' + +# Other globals develop = False -server_dir = '/' config_file = None -DEFAULT_CONFIG_FILE = '/etc/plinth/plinth.config' -DEFAULT_ROOT = '/' - def get_fallback_config_paths(): """Get config paths of the current source code folder""" @@ -54,8 +75,8 @@ def read(config_path=None, root_directory=None): config_path, root_directory = get_config_paths() if not os.path.isfile(config_path): - msg = 'No plinth.config file could be found on path: %s' % config_path - raise FileNotFoundError(msg) + # Ignore missing configuration files + return global config_file # pylint: disable-msg=invalid-name,global-statement config_file = config_path @@ -87,9 +108,8 @@ def read(config_path=None, root_directory=None): try: value = parser.get(section, name) except (configparser.NoSectionError, configparser.NoOptionError): - logger.error('Configuration does not contain option: %s.%s', - section, name) - raise + # Use default values for any missing keys in configuration + continue else: if datatype == 'int': value = int(value) diff --git a/plinth/tests/data/plinth.config.with_missing_options b/plinth/tests/data/plinth.config.with_missing_options index 684b415d1..f66c8f4d3 100644 --- a/plinth/tests/data/plinth.config.with_missing_options +++ b/plinth/tests/data/plinth.config.with_missing_options @@ -1,5 +1,5 @@ [Misc] -box_name = FreedomBox +box_name = FreedomBoxTestMissingOptions [Path] diff --git a/plinth/tests/data/plinth.config.with_missing_sections b/plinth/tests/data/plinth.config.with_missing_sections index fbb473246..1d76ef7a8 100644 --- a/plinth/tests/data/plinth.config.with_missing_sections +++ b/plinth/tests/data/plinth.config.with_missing_sections @@ -1,2 +1,2 @@ [Misc] -box_name = FreedomBox +box_name = FreedomBoxTestMissingSections diff --git a/plinth/tests/test_cfg.py b/plinth/tests/test_cfg.py index f33c2c586..0fab9bb89 100644 --- a/plinth/tests/test_cfg.py +++ b/plinth/tests/test_cfg.py @@ -6,6 +6,7 @@ Test module for configuration module. import configparser import logging import os +from unittest.mock import patch import pytest @@ -23,54 +24,33 @@ logging.disable(logging.CRITICAL) pytestmark = pytest.mark.usefixtures('load_cfg') -@pytest.fixture(name='test_config_file') -def fixture_test_config_file(load_cfg): - """Test fixture to return the configuration file path""" - return cfg.get_config_paths()[0] - - -@pytest.fixture(name='test_config_dir') -def fixture_test_config_dir(load_cfg): - """Test fixture to return the configuration file directory.""" - return cfg.get_config_paths()[1] - - -def test_read_default_config_file(test_config_dir, test_config_file): +def test_read_default_config_file(): """Verify that the default config file can be read correctly.""" + config_file, config_dir = cfg.get_fallback_config_paths() + # Read the plinth.config file directly - parser = configparser.ConfigParser(defaults={'root': test_config_dir}) - parser.read(test_config_file) + parser = configparser.ConfigParser(defaults={'root': config_dir}) + parser.read(config_file) # Read the plinth.config file via the cfg module - cfg.read(test_config_file, test_config_dir) + cfg.read(config_file, config_dir) # Compare the two results compare_configurations(parser) -def test_read_primary_config_file(): +@patch('plinth.cfg.get_config_paths') +def test_read_primary_config_file(get_config_paths): """Verify that the primary config file is used by default.""" - original_config_path = cfg.DEFAULT_CONFIG_FILE - original_root_directory = cfg.DEFAULT_ROOT - expected_config_path = CONFIG_FILE_WITH_MISSING_OPTIONS root_directory = 'x-default-root' expected_root_directory = os.path.realpath(root_directory) + get_config_paths.return_value = (expected_config_path, root_directory) - try: - cfg.DEFAULT_CONFIG_FILE = expected_config_path - cfg.DEFAULT_ROOT = root_directory - # reading the config file will fail, but still cfg.root and - # cfg.config_file will be set for parsing the config file - try: - cfg.read() - except configparser.NoOptionError: - pass - assert cfg.config_file == expected_config_path - assert cfg.root == expected_root_directory - finally: - cfg.DEFAULT_CONFIG_FILE = original_config_path - cfg.DEFAULT_ROOT = original_root_directory + cfg.read() + + assert cfg.config_file == expected_config_path + assert cfg.root == expected_root_directory def test_read_fallback_config_file(): @@ -86,20 +66,19 @@ def test_read_fallback_config_file(): def test_read_missing_config_file(): """Verify that an exception is raised when there's no config file.""" - with pytest.raises(FileNotFoundError): - cfg.read('x-non-existant-file', 'x-root-directory') + cfg.read('x-non-existant-file', 'x-root-directory') -def test_read_config_file_with_missing_sections(test_config_dir): +def test_read_config_file_with_missing_sections(): """Verify that missing configuration sections can be detected.""" - with pytest.raises(configparser.NoSectionError): - cfg.read(CONFIG_FILE_WITH_MISSING_SECTIONS, test_config_dir) + cfg.read(CONFIG_FILE_WITH_MISSING_SECTIONS, TEST_CONFIG_DIR) + assert cfg.box_name == 'FreedomBoxTestMissingSections' -def test_read_config_file_with_missing_options(test_config_dir): +def test_read_config_file_with_missing_options(): """Verify that missing configuration options can be detected.""" - with pytest.raises(configparser.NoOptionError): - cfg.read(CONFIG_FILE_WITH_MISSING_OPTIONS, test_config_dir) + cfg.read(CONFIG_FILE_WITH_MISSING_OPTIONS, TEST_CONFIG_DIR) + assert cfg.box_name == 'FreedomBoxTestMissingOptions' def compare_configurations(parser):