Merge pull request #12949 from nupplaphil/bug/site_env_view

Disable setting fields in case we use environment variables

.woodpecker/.phpunit.yml

@@ -77,7 +77,7 @@ pipeline:
     when:
       matrix:
         PHP_MAJOR_VERSION: 7.4
-        PHP_VERSION: 7.4.18
+        PHP_VERSION: 7.4.33
       repo:
         - friendica/friendica
     settings:

src/Core/Config/Capability/IManageConfigValues.php

@@ -58,6 +58,16 @@ interface IManageConfigValues
 	 */
 	public function get(string $cat, string $key = null, $default_value = null);
 
+	/**
+	 * Returns true, if the current config can be changed
+	 *
+	 * @param string $cat The category of the configuration value
+	 * @param string $key The configuration key to query
+	 *
+	 * @return bool true, if writing is possible
+	 */
+	public function isWritable(string $cat, string $key): bool;
+
 	/**
 	 * Sets a configuration value for system config
 	 *

src/Core/Config/Model/DatabaseConfig.php

@@ -80,6 +80,12 @@ class DatabaseConfig implements IManageConfigValues
 		return $this->cache->get($cat, $key) ?? $default_value;
 	}
 
+	/** {@inheritDoc} */
+	public function isWritable(string $cat, string $key): bool
+	{
+		return $this->cache->getSource($cat, $key) < Cache::SOURCE_ENV;
+	}
+
 	/** {@inheritDoc} */
 	public function set(string $cat, string $key, $value): bool
 	{

src/Core/Config/Model/ReadOnlyFileConfig.php

@@ -68,6 +68,12 @@ class ReadOnlyFileConfig implements IManageConfigValues
 		return $this->configCache->get($cat, $key) ?? $default_value;
 	}
 
+	/** {@inheritDoc} */
+	public function isWritable(string $cat, string $key): bool
+	{
+		return $this->configCache->getSource($cat, $key) < Cache::SOURCE_ENV;
+	}
+
 	/** {@inheritDoc} */
 	public function set(string $cat, string $key, $value): bool
 	{

src/Core/Config/ValueObject/Cache.php

@@ -31,6 +31,15 @@ use ParagonIE\HiddenString\HiddenString;
  */
 class Cache
 {
+	/** @var int[] A list of valid config source  */
+	const VALID_SOURCES = [
+		self::SOURCE_STATIC,
+		self::SOURCE_FILE,
+		self::SOURCE_DATA,
+		self::SOURCE_ENV,
+		self::SOURCE_FIX,
+	];
+
 	/** @var int Indicates that the cache entry is a default value - Lowest Priority */
 	const SOURCE_STATIC = 0;
 	/** @var int Indicates that the cache entry is set by file - Low Priority */

src/Module/Admin/Logs/Settings.php

@@ -48,9 +48,15 @@ class Settings extends BaseAdmin
 			return;
 		}
 
+		if (DI::config()->isWritable('system', 'logfile')) {
 			DI::config()->set('system', 'logfile', $logfile);
+		}
+		if (DI::config()->isWritable('system', 'debugging')) {
 			DI::config()->set('system', 'debugging', $debugging);
+		}
+		if (DI::config()->isWritable('system', 'loglevel')) {
 			DI::config()->set('system', 'loglevel', $loglevel);
+		}
 
 		DI::baseUrl()->redirect('admin/logs');
 	}
@@ -82,9 +88,9 @@ class Settings extends BaseAdmin
 			'$clear' => DI::l10n()->t('Clear'),
 			'$logname' => DI::config()->get('system', 'logfile'),
 			// see /help/smarty3-templates#1_1 on any Friendica node
-			'$debugging' => ['debugging', DI::l10n()->t("Enable Debugging"), DI::config()->get('system', 'debugging'), ""],
-			'$logfile' => ['logfile', DI::l10n()->t("Log file"), DI::config()->get('system', 'logfile'), DI::l10n()->t("Must be writable by web server. Relative to your Friendica top-level directory.")],
-			'$loglevel' => ['loglevel', DI::l10n()->t("Log level"), DI::config()->get('system', 'loglevel'), "", $log_choices],
+			'$debugging' => ['debugging', DI::l10n()->t('Enable Debugging'), DI::config()->get('system', 'debugging'), !DI::config()->isWritable('system', 'debugging') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : '', !DI::config()->isWritable('system', 'debugging') ? 'disabled' : ''],
+			'$logfile' => ['logfile', DI::l10n()->t('Log file'), DI::config()->get('system', 'logfile'), DI::l10n()->t('Must be writable by web server. Relative to your Friendica top-level directory.') . (!DI::config()->isWritable('system', 'logfile') ? '<br>' . DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : ''), '', !DI::config()->isWritable('system', 'logfile') ? 'disabled' : ''],
+			'$loglevel' => ['loglevel', DI::l10n()->t("Log level"), DI::config()->get('system', 'loglevel'), !DI::config()->isWritable('system', 'loglevel') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : '', $log_choices, !DI::config()->isWritable('system', 'loglevel') ? 'disabled' : ''],
 			'$form_security_token' => self::getFormSecurityToken("admin_logs"),
 			'$phpheader' => DI::l10n()->t("PHP logging"),
 			'$phphint' => DI::l10n()->t("To temporarily enable logging of PHP errors and warnings you can prepend the following to the index.php file of your installation. The filename set in the 'error_log' line is relative to the friendica top-level directory and must be writeable by the web server. The option '1' for 'log_errors' and 'display_errors' is to enable these options, set to '0' to disable them."),

src/Module/Admin/Site.php

@@ -165,7 +165,9 @@ class Site extends BaseAdmin
 		$transactionConfig->set('system', 'poco_discovery'        , $poco_discovery);
 		$transactionConfig->set('system', 'poco_local_search'     , $poco_local_search);
 		$transactionConfig->set('system', 'nodeinfo'              , $nodeinfo);
+		if (DI::config()->isWritable('config', 'sitename')) {
 			$transactionConfig->set('config', 'sitename', $sitename);
+		}
 		$transactionConfig->set('config', 'sender_email'          , $sender_email);
 		$transactionConfig->set('system', 'suppress_tags'         , $suppress_tags);
 		$transactionConfig->set('system', 'shortcut_icon'         , $shortcut_icon);
@@ -188,7 +190,9 @@ class Site extends BaseAdmin
 		} else {
 			$transactionConfig->set('config', 'info', $additional_info);
 		}
+		if (DI::config()->isWritable('system', 'language')) {
 			$transactionConfig->set('system', 'language', $language);
+		}
 		$transactionConfig->set('system', 'theme', $theme);
 		Theme::install($theme);
 
@@ -413,7 +417,7 @@ class Site extends BaseAdmin
 			'$relocate_cmd'      => DI::l10n()->t('(Friendica directory)# bin/console relocate https://newdomain.com'),
 
 			// name, label, value, help string, extra data...
-			'$sitename'         => ['sitename', DI::l10n()->t('Site name'), DI::config()->get('config', 'sitename'), ''],
+			'$sitename'         => ['sitename', DI::l10n()->t('Site name'), DI::config()->get('config', 'sitename'), !DI::config()->isWritable('config', 'sitename') ? DI::l10n()->t('<strong>Read-only</strong> because it is set by an environment variable') : '', '', !DI::config()->isWritable('config', 'sitename') ? 'disabled' : ''],
 			'$sender_email'     => ['sender_email', DI::l10n()->t('Sender Email'), DI::config()->get('config', 'sender_email'), DI::l10n()->t('The email address your server shall use to send notification emails from.'), '', '', 'email'],
 			'$system_actor_name' => ['system_actor_name', DI::l10n()->t('Name of the system actor'), User::getActorName(), DI::l10n()->t("Name of the internal system account that is used to perform ActivityPub requests. This must be an unused username. If set, this can't be changed again.")],
 			'$banner'           => ['banner', DI::l10n()->t('Banner/Logo'), $banner, ''],
@@ -421,7 +425,7 @@ class Site extends BaseAdmin
 			'$shortcut_icon'    => ['shortcut_icon', DI::l10n()->t('Shortcut icon'), DI::config()->get('system', 'shortcut_icon'), DI::l10n()->t('Link to an icon that will be used for browsers.')],
 			'$touch_icon'       => ['touch_icon', DI::l10n()->t('Touch icon'), DI::config()->get('system', 'touch_icon'), DI::l10n()->t('Link to an icon that will be used for tablets and mobiles.')],
 			'$additional_info'  => ['additional_info', DI::l10n()->t('Additional Info'), $additional_info, DI::l10n()->t('For public servers: you can add additional information here that will be listed at %s/servers.', Search::getGlobalDirectory())],
-			'$language'         => ['language', DI::l10n()->t('System language'), DI::config()->get('system', 'language'), '', $lang_choices],
+			'$language'         => ['language', DI::l10n()->t('System language'), DI::config()->get('system', 'language'), !DI::config()->isWritable('system', 'language') ? DI::l10n()->t("<strong>Read-only</strong> because it is set by an environment variable") : '', $lang_choices, !DI::config()->isWritable('system', 'language') ? 'disabled' : ''],
 			'$theme'            => ['theme', DI::l10n()->t('System theme'), DI::config()->get('system', 'theme'), DI::l10n()->t('Default system theme - may be over-ridden by user profiles - <a href="%s" id="cnftheme">Change default theme settings</a>', DI::baseUrl() . '/admin/themes'), $theme_choices],
 			'$theme_mobile'     => ['theme_mobile', DI::l10n()->t('Mobile system theme'), DI::config()->get('system', 'mobile-theme', '---'), DI::l10n()->t('Theme for mobile devices'), $theme_choices_mobile],
 			'$force_ssl'        => ['force_ssl', DI::l10n()->t('Force SSL'), DI::config()->get('system', 'force_ssl'), DI::l10n()->t('Force all Non-SSL requests to SSL - Attention: on some systems it could lead to endless loops.')],

src/Module/Admin/Storage.php

@@ -76,7 +76,7 @@ class Storage extends BaseAdmin
 			}
 		}
 
-		if (!empty($_POST['submit_save_set'])) {
+		if (!empty($_POST['submit_save_set']) && DI::config()->isWritable('storage', 'name') ) {
 			try {
 				$newstorage = DI::storageManager()->getWritableStorageByName($storagebackend);
 
@@ -145,6 +145,8 @@ class Storage extends BaseAdmin
 			'$save_reload'           => DI::l10n()->t('Save & Reload'),
 			'$noconfig'              => DI::l10n()->t('This backend doesn\'t have custom settings'),
 			'$form_security_token'   => self::getFormSecurityToken("admin_storage"),
+			'$storagebackend_ro_txt' => !DI::config()->isWritable('storage', 'name') ? DI::l10n()->t('Changing the current backend is prohibited because it is set by an environment variable') : '',
+			'$is_writable'           => DI::config()->isWritable('storage', 'name'),
 			'$storagebackend'        => $current_storage_backend instanceof ICanWriteToStorage ? $current_storage_backend::getName() : DI::l10n()->t('Database (legacy)'),
 			'$availablestorageforms' => $available_storage_forms,
 		]);

tests/Util/VFSTrait.php

@@ -65,7 +65,7 @@ trait VFSTrait
 	 * @param string $sourceFilePath The filename of the config file
 	 * @param bool   $static         True, if the folder `static` instead of `config` should be used
 	 */
-	protected function setConfigFile(string $sourceFilePath, bool $static = false, string $targetFileName = null)
+	public function setConfigFile(string $sourceFilePath, bool $static = false, string $targetFileName = null)
 	{
 		$file = dirname(__DIR__) . DIRECTORY_SEPARATOR .
 			'..' . DIRECTORY_SEPARATOR .

tests/src/Core/Config/ConfigTest.php

@@ -566,4 +566,74 @@ class ConfigTest extends DatabaseTest
 		$config->set('test', 'it', $value);
 		self:self::assertEquals($assertion, $config->get('test', 'it'));
 	}
+
+	public function dataEnv(): array
+	{
+		$data = [
+			'config' => [
+				'admin_email' => 'value1',
+				'timezone' => 'value2',
+				'language' => 'value3',
+				'sitename' => 'value',
+			],
+			'system' => [
+				'url' => 'value1a',
+				'debugging' => true,
+				'logfile' => 'value4',
+				'loglevel' => 'notice',
+				'proflier' => true,
+			],
+			'proxy'  => [
+				'trusted_proxies' => 'value5',
+			],
+		];
+
+		return [
+			'empty' => [
+				'data'   => $data,
+				'server' => [],
+				'assertDisabled' => [],
+			],
+			'mixed' => [
+				'data'   => $data,
+				'server' => [
+					'FRIENDICA_ADMIN_MAIL' => 'test@friendica.local',
+					'FRIENDICA_DEBUGGING' => true,
+				],
+				'assertDisabled' => [
+					'config' => [
+						'admin_email' => true,
+					],
+					'system' => [
+						'debugging' => true,
+					],
+				],
+			],
+		];
+	}
+
+	/**
+	 * Tests if environment variables can change the permission to write a config key
+	 *
+	 * @dataProvider dataEnv
+	 */
+	public function testIsWritable(array $data, array $server, array $assertDisabled)
+	{
+		$this->setConfigFile('static' . DIRECTORY_SEPARATOR . 'env.config.php', true);
+		$this->loadDirectFixture($this->configToDbArray($data), $this->getDbInstance());
+
+		$configFileManager = new ConfigFileManager($this->root->url(), $this->root->url() . '/config/', $this->root->url() . '/static/', $server);
+		$configFileManager->setupCache($this->configCache);
+		$config = new DatabaseConfig($this->getDbInstance(), $this->configCache);
+
+		foreach ($data as $category => $keyvalues) {
+			foreach ($keyvalues as $key => $value) {
+				if (empty($assertDisabled[$category][$key])) {
+					static::assertTrue($config->isWritable($category, $key), sprintf('%s.%s is not true', $category, $key));
+				} else {
+					static::assertFalse($config->isWritable($category, $key), sprintf('%s.%s is not false', $category, $key));
+				}
+			}
+		}
+	}
 }

view/templates/admin/storage.tpl

@@ -2,6 +2,7 @@
 	<h1>{{$title}} - {{$page}}</h1>
 
 		<h2>{{$label_current}}: <b>{{$storagebackend}}</b></h2>
+		{{$storagebackend_ro_txt nofilter}}
 
 		<h2>{{$label_config}}</h2>
 
@@ -19,12 +20,14 @@
 
 		{{if $storage.form}}
 		<input type="submit" name="submit_save" value="{{$save}}"/>
+        {{if $is_writable}}
 				{{if $storage.active}}
 		<input type="submit" name="submit_save_set" value="{{$save_reload}}"/>
 				{{else}}
 		<input type="submit" name="submit_save_set" value="{{$save_use}}"/>
 				{{/if}}
-		{{else}}
+		{{/if}}
+		{{elseif $is_writable}}
 		<br /><input type="submit" name="submit_save_set" {{if $storage.active}}disabled="disabled"{{/if}} value="{{$use}}"/>
 		{{/if}}
 	</form>

view/templates/field_select.tpl

@@ -1,7 +1,7 @@
 
 	<div class="field select">
 		<label for="id_{{$field.0}}">{{$field.1}}</label>
-		<select name="{{$field.0}}" id="id_{{$field.0}}" aria-describedby="{{$field.0}}_tip">
+		<select name="{{$field.0}}" id="id_{{$field.0}}" aria-describedby="{{$field.0}}_tip" {{$field.5 nofilter}}>
 	{{foreach $field.4 as $opt=>$val}}
 			<option value="{{$opt}}" dir="auto"{{if $opt==$field.2}} selected="selected"{{/if}}>{{$val}}</option>
 	{{/foreach}}

view/theme/frio/templates/admin/storage.tpl

@@ -5,6 +5,9 @@
 
 	<div class="well well-lg">
         	{{$label_current}}: <b>{{$storagebackend}}</b>
+			{{if $storagebackend_ro_txt}}
+			<br><i>{{$storagebackend_ro_txt nofilter}}</i>
+			{{/if}}
 	</div>
 
 		<h2>{{$label_config}}</h2>
@@ -33,12 +36,14 @@
 				<div class="panel-footer">
 					{{if $storage.form}}
 					<input type="submit" name="submit_save" class="btn btn-primary" value="{{$save}}"/>
+	                    {{if $is_writable}}
 							{{if $storage.active}}
 					<input type="submit" name="submit_save_set" class="btn btn-primary" value="{{$save_reload}}"/>
 							{{else}}
 					<input type="submit" name="submit_save_set" class="btn btn-primary" value="{{$save_use}}"/>
 							{{/if}}
-					{{else}}
+						{{/if}}
+					{{elseif $is_writable}}
 					<input type="submit" name="submit_save_set" class="btn btn-primary" {{if $storage.active}}disabled="disabled"{{/if}} value="{{$use}}"/>
 					{{/if}}
 				</div>

view/theme/frio/templates/field_select.tpl

@@ -1,7 +1,7 @@
 
 	<div class="form-group field select">
 		<label for="id_{{$field.0}}">{{$field.1}}</label>
-		<select name="{{$field.0}}" id="id_{{$field.0}}" class="form-control" aria-describedby="{{$field.0}}_tip">
+		<select name="{{$field.0}}" id="id_{{$field.0}}" class="form-control" aria-describedby="{{$field.0}}_tip" {{$field.5 nofilter}}>
 	{{foreach $field.4 as $opt=>$val}}
 			<option value="{{$opt}}" {{if $opt==$field.2}}selected="selected"{{/if}}>{{$val}}</option>
 	{{/foreach}}