chore(core): site secret now reliably gets a factory if not in config

This fixes the /serve-file handler.
Elgg · Dec 18, 2017 · 3fdc3c0 · 3fdc3c0
1 parent 3e77ce5
commit 3fdc3c0
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 8 deletions.
diff --git a/engine/classes/Elgg/Di/ServiceProvider.php b/engine/classes/Elgg/Di/ServiceProvider.php
@@ -449,6 +449,8 @@ public function __construct(Config $config) {
 			return \ElggSession::fromDatabase($c->config, $c->db);
 		});
 
+		$this->initSiteSecret($config);
+
 		$this->setClassName('urlSigner', \Elgg\Security\UrlSigner::class);
 
 		$this->setFactory('simpleCache', function(ServiceProvider $c) {
@@ -525,6 +527,26 @@ public function __construct(Config $config) {
 		$this->setClassName('widgets', \Elgg\WidgetsService::class);
 	}
 
+	/**
+	 * Extract the site secret from config or set up its factory
+	 *
+	 * @param Config $config Elgg Config
+	 * @return void
+	 */
+	protected function initSiteSecret(Config $config) {
+		// Try the config, because if it's there we want to remove it to isolate who can see it.
+		$secret = SiteSecret::fromConfig($config);
+		if ($secret) {
+			$this->setValue('siteSecret', $secret);
+			$config->elgg_config_set_secret = true;
+			return;
+		}
+
+		$this->setFactory('siteSecret', function (ServiceProvider $c) {
+			return SiteSecret::fromDatabase($c->configTable);
+		});
+	}
+
 	/**
 	 * Validate, normalize, fill in missing values, and lock some
 	 *
@@ -613,14 +635,6 @@ public function initConfig(Config $config, ServiceProvider $sp) {
 		unset($config->dbuser);
 		unset($config->dbpass);
 
-		// If the site secret is in the settings file, let's move it to that component
-		// right away to keep this value out of config.
-		$secret = SiteSecret::fromConfig($config);
-		if ($secret) {
-			$sp->setValue('siteSecret', $secret);
-			$config->elgg_config_set_secret = true;
-		}
-
 		$config->boot_complete = false;
 	}
 }
diff --git a/engine/tests/phpunit/unit/Elgg/Di/ServiceProviderUnitTest.php b/engine/tests/phpunit/unit/Elgg/Di/ServiceProviderUnitTest.php
@@ -2,6 +2,8 @@
 
 namespace Elgg\Di;
 
+use Elgg\Config;
+use Elgg\Database\SiteSecret;
 use phpDocumentor\Reflection\DocBlock;
 use Zend\Mail\Transport\InMemory;
 
@@ -19,6 +21,30 @@ public function down() {
 
 	}
 
+	public function testCanExtractSiteSecretFromConfig() {
+		$config = new Config([
+			SiteSecret::CONFIG_KEY => md5('bar'),
+		]);
+		$sp = new ServiceProvider($config);
+
+		$this->assertEmpty($config->{SiteSecret::CONFIG_KEY});
+
+		$this->assertInstanceOf(SiteSecret::class, $sp->siteSecret);
+		$this->assertEquals(md5('bar'), $sp->siteSecret->get());
+	}
+
+	public function testSetsBackupSiteSecretFactory() {
+		$config_table = _elgg_services()->configTable;
+		$config_table->set(SiteSecret::CONFIG_KEY, md5('foo'));
+
+		$config = new Config();
+		$sp = new ServiceProvider($config);
+		$sp->setValue('configTable', $config_table);
+
+		$this->assertInstanceOf(SiteSecret::class, $sp->siteSecret);
+		$this->assertEquals(md5('foo'), $sp->siteSecret->get());
+	}
+
 	/**
 	 * @dataProvider servicesListProvider
 	 */