feat: make sure config file is writable

matomo-org · Oct 11, 2024 · 6989fcc · 6989fcc
1 parent 20b148e
commit 6989fcc
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 8 deletions.
diff --git a/core/Config.php b/core/Config.php
@@ -451,6 +451,16 @@ public function getConfigNotWritableException()
         return new MissingFilePermissionException(Piwik::translate('General_ConfigFileIsNotWritable', array("(" . $path . ")", "")));
     }
 
+    /**
+     * @throws MissingFilePermissionException If config file is not writable.
+     */
+    public function checkConfigIsWritable()
+    {
+        if (!$this->isFileWritable()) {
+            throw $this->getConfigNotWritableException();
+        }
+    }
+
     /**
      * Convenience method for setting settings in a single section. Will set them in a new array first
      * to be compatible with certain PHP versions.

diff --git a/core/Plugin/ControllerAdmin.php b/core/Plugin/ControllerAdmin.php
@@ -14,6 +14,7 @@
 use Piwik\Container\StaticContainer;
 use Piwik\Date;
 use Piwik\Development;
+use Piwik\Exception\MissingFilePermissionException;
 use Piwik\Menu\MenuAdmin;
 use Piwik\Menu\MenuTop;
 use Piwik\Notification;
@@ -202,14 +203,11 @@ private static function notifyIfDevelopmentModeOnButNotInstalledThroughGit()
      */
     public static function displayWarningIfConfigFileNotWritable()
     {
-        $isConfigFileWritable = PiwikConfig::getInstance()->isFileWritable();
-
-        if (!$isConfigFileWritable) {
-            $exception = PiwikConfig::getInstance()->getConfigNotWritableException();
-            $message = $exception->getMessage();
-
-            $notification = new Notification($message);
-            $notification->raw     = true;
+        try {
+            PiwikConfig::getInstance()->checkConfigIsWritable();
+        } catch (MissingFilePermissionException $exception) {
+            $notification = new Notification($exception->getMessage());
+            $notification->raw = true;
             $notification->context = Notification::CONTEXT_WARNING;
             Notification\Manager::notify('ControllerAdmin_ConfigNotWriteable', $notification);
         }

diff --git a/plugins/CorePluginsAdmin/Commands/InstallPlugin.php b/plugins/CorePluginsAdmin/Commands/InstallPlugin.php
@@ -9,6 +9,7 @@
 
 namespace Piwik\Plugins\CorePluginsAdmin\Commands;
 
+use Piwik\Config as PiwikConfig;
 use Piwik\Container\StaticContainer;
 use Piwik\Plugin\ConsoleCommand;
 use Piwik\Plugin\Manager;
@@ -29,6 +30,8 @@ protected function configure(): void
 
     protected function doExecute(): int
     {
+        PiwikConfig::getInstance()->checkConfigIsWritable();
+
         $input = $this->getInput();
         $output = $this->getOutput();
         $pluginManager = Manager::getInstance();

diff --git a/tests/PHPUnit/Unit/ConfigTest.php b/tests/PHPUnit/Unit/ConfigTest.php
@@ -12,6 +12,7 @@
 use PHPUnit\Framework\TestCase;
 use Piwik\Application\Kernel\GlobalSettingsProvider;
 use Piwik\Config;
+use Piwik\Exception\MissingFilePermissionException;
 
 class DumpConfigTestMockIniFileChain extends Config\IniFileChain
 {
@@ -562,4 +563,27 @@ public function testSanityCheckFails()
         $this->assertTrue($config->sanityCheck($userFile, $correctContent));
         $this->assertSame($userFile, $expectedPath);
     }
+
+    public function testCheckConfigIsWritableNotThrowsExceptionWhenWritable()
+    {
+        $this->assertTrue($this->checkConfigIsWritable('Config'));
+    }
+
+    public function testCheckConfigIsWritableThrowsExceptionWhenNotWritable()
+    {
+        $this->expectException(MissingFilePermissionException::class);
+        $this->expectExceptionMessage('ConfigFileIsNotWritable');
+        $this->checkConfigIsWritable('ConfigNotExists');
+    }
+
+    private function checkConfigIsWritable(string $directory): bool
+    {
+        $userFile = PIWIK_INCLUDE_PATH . "/tests/resources/$directory/config.ini.php";
+        $globalFile = PIWIK_INCLUDE_PATH . "/tests/resources/$directory/global.ini.php";
+        $commonFile = PIWIK_INCLUDE_PATH . "/tests/resources/$directory/common.config.ini.php";
+
+        $config = new Config(new GlobalSettingsProvider($globalFile, $userFile, $commonFile));
+        $config->checkConfigIsWritable();
+        return true;
+    }
 }