When Piwik config file becomes un-readable, make it clear that the file still exists but is not readable (#10331)

* * In messages, make it more clear whether config file exists and/or is (not) readable
* When the config file exists but is not readable, provide users the choice between making the file readable, or starting a fresh install
* As long as Piwik is not installed, piwik.php should not return 500 when config file is not readable
* Fixes #10283
* UI tests: updated message when config file is not found

13 files changed, 168 insertions, 49 deletions

diff --git a/core/Application/Kernel/EnvironmentValidator.php b/core/Application/Kernel/EnvironmentValidator.php
index 321be28ef1..bfdb7c4460 100644
--- a/core/Application/Kernel/EnvironmentValidator.php
+++ b/core/Application/Kernel/EnvironmentValidator.php
@@ -9,7 +9,9 @@
 namespace Piwik\Application\Kernel;
 
 use Piwik\Common;
+use Piwik\Filechecks;
 use Piwik\Piwik;
+use Piwik\SettingsPiwik;
 use Piwik\SettingsServer;
 use Piwik\Translation\Translator;
 
@@ -37,11 +39,28 @@ class EnvironmentValidator
 
     public function validate()
     {
-        $inTrackerRequest = SettingsServer::isTrackerApiRequest();
-        $inConsole = Common::isPhpCliMode();
-
         $this->checkConfigFileExists($this->settingsProvider->getPathGlobal());
-        $this->checkConfigFileExists($this->settingsProvider->getPathLocal(), $startInstaller = !$inTrackerRequest && !$inConsole);
+
+        if(SettingsPiwik::isPiwikInstalled()) {
+            $this->checkConfigFileExists($this->settingsProvider->getPathLocal(), $startInstaller = false);
+            return;
+        }
+
+        $startInstaller = true;
+
+        if(SettingsServer::isTrackerApiRequest()) {
+            // if Piwik is not installed yet, the piwik.php should do nothing and not return an error
+            throw new \Exception("As Piwik is not installed yet, the Tracking API will now exit without error.");
+        }
+
+        if(Common::isPhpCliMode()) {
+            // in CLI, do not start/redirect to installer, simply output the exception at the top
+            $startInstaller = false;
+        }
+
+        // Start the installation when config file not found
+        $this->checkConfigFileExists($this->settingsProvider->getPathLocal(), $startInstaller);
+
     }
 
     /**
@@ -55,25 +74,69 @@ class EnvironmentValidator
             return;
         }
 
-        $message = $this->translator->translate('General_ExceptionConfigurationFileNotFound', array($path));
-        if (Common::isPhpCliMode()) {
-            $message .= "\n" . $this->translator->translate('General_ExceptionConfigurationFileNotFound2', array($path, get_current_user()));
-        }
+        $message = $this->getSpecificMessageWhetherFileExistsOrNot($path);
 
         $exception = new \Exception($message);
 
         if ($startInstaller) {
-            /**
-             * Triggered when the configuration file cannot be found or read, which usually
-             * means Piwik is not installed yet.
-             *
-             * This event can be used to start the installation process or to display a custom error message.
-             *
-             * @param \Exception $exception The exception that was thrown by `Config::getInstance()`.
-             */
-            Piwik::postEvent('Config.NoConfigurationFile', array($exception), $pending = true);
+            $this->startInstallation($exception);
         } else {
             throw $exception;
         }
     }
+
+    /**
+     * @param $exception
+     */
+    private function startInstallation($exception)
+    {
+        /**
+         * Triggered when the configuration file cannot be found or read, which usually
+         * means Piwik is not installed yet.
+         *
+         * This event can be used to start the installation process or to display a custom error message.
+         *
+         * @param \Exception $exception The exception that was thrown by `Config::getInstance()`.
+         */
+        Piwik::postEvent('Config.NoConfigurationFile', array($exception), $pending = true);
+    }
+
+    /**
+     * @param $path
+     * @return string
+     */
+    private function getMessageWhenFileExistsButNotReadable($path)
+    {
+        $format = " \n<b>» %s </b>";
+        if(Common::isPhpCliMode()) {
+            $format = "\n » %s \n";
+        }
+
+        return sprintf($format,
+            $this->translator->translate('General_ExceptionConfigurationFilePleaseCheckReadableByUser',
+                array($path, Filechecks::getUser())));
+    }
+
+    /**
+     * @param $path
+     * @return string
+     */
+    private function getSpecificMessageWhetherFileExistsOrNot($path)
+    {
+        if (!file_exists($path)) {
+            $message = $this->translator->translate('General_ExceptionConfigurationFileNotFound', array($path));
+            if (Common::isPhpCliMode()) {
+                $message .= $this->getMessageWhenFileExistsButNotReadable($path);
+            }
+        } else {
+            $message = $this->translator->translate('General_ExceptionConfigurationFileExistsButNotReadable',
+                array($path));
+            $message .= $this->getMessageWhenFileExistsButNotReadable($path);
+        }
+
+        if (Common::isPhpCliMode()) {
+            $message = "\n" . $message;
+        }
+        return $message;
+    }
 }
diff --git a/core/Db.php b/core/Db.php
index a4b114d634..b906790397 100644
--- a/core/Db.php
+++ b/core/Db.php
@@ -57,6 +57,12 @@ class Db
         return self::$connection;
     }
 
+    /**
+     * Returns an array with the Database connection information.
+     *
+     * @param array|null $dbConfig
+     * @return array
+     */
     public static function getDatabaseConfig($dbConfig = null)
     {
         $config = Config::getInstance();
diff --git a/core/Db/Adapter.php b/core/Db/Adapter.php
index bea69e1d04..df84b4ae50 100644
--- a/core/Db/Adapter.php
+++ b/core/Db/Adapter.php
@@ -69,7 +69,7 @@ class Adapter
     {
         $className = 'Piwik\Db\Adapter\\' . str_replace(' ', '\\', ucwords(str_replace(array('_', '\\'), ' ', strtolower($adapterName))));
         if (!class_exists($className)) {
-            throw new \Exception("Adapter $adapterName is not valid.");
+            throw new \Exception(sprintf("Adapter '%s' is not valid. Maybe check that your Piwik configuration files in config/*.ini.php are readable by the webserver.", $adapterName));
         }
         return $className;
     }
diff --git a/core/Filechecks.php b/core/Filechecks.php
index bf3e36f1d7..417b13bbca 100644
--- a/core/Filechecks.php
+++ b/core/Filechecks.php
@@ -220,12 +220,18 @@ class Filechecks
         return $user . ':' . $group;
     }
 
-    private static function getUser()
+    public static function getUser()
     {
-        if (!function_exists('shell_exec')) {
-            return 'www-data';
+        if (function_exists('shell_exec')) {
+            return trim(shell_exec('whoami'));
         }
-        return trim(shell_exec('whoami'));
+
+        $currentUser = get_current_user();
+        if(!empty($currentUser)) {
+            return $currentUser;
+        }
+
+        return 'www-data';
     }
 
     /**
@@ -237,7 +243,7 @@ class Filechecks
     private static function getMakeWritableCommand($realpath)
     {
         if (SettingsServer::isWindows()) {
-            return "<code>cacls $realpath /t /g " . get_current_user() . ":f</code><br />\n";
+            return "<code>cacls $realpath /t /g " . self::getUser() . ":f</code><br />\n";
         }
         return "<code>chmod -R 0755 $realpath</code><br />";
     }
diff --git a/core/FrontController.php b/core/FrontController.php
index f394aaf4f7..31ef714b98 100644
--- a/core/FrontController.php
+++ b/core/FrontController.php
@@ -71,6 +71,28 @@ class FrontController extends Singleton
     private $initialized = false;
 
     /**
+     * @param $lastError
+     * @return mixed|void
+     * @throws AuthenticationFailedException
+     * @throws Exception
+     */
+    private static function generateSafeModeOutput($lastError)
+    {
+        Common::sendResponseCode(500);
+
+        $controller = FrontController::getInstance();
+        try {
+            $controller->init();
+            $message = $controller->dispatch('CorePluginsAdmin', 'safemode', array($lastError));
+        } catch(Exception $e) {
+            // may fail in safe mode (eg. global.ini.php not found)
+            $message = sprintf("Piwik encoutered an error: %s (which lead to: %s)", $lastError['message'], $e->getMessage());
+        }
+
+        return $message;
+    }
+
+    /**
      * Executes the requested plugin controller method.
      *
      * @throws Exception|\Piwik\PluginDeactivatedException in case the plugin doesn't exist, the action doesn't exist,
@@ -179,12 +201,7 @@ class FrontController extends Singleton
     {
         $lastError = error_get_last();
         if (!empty($lastError) && $lastError['type'] == E_ERROR) {
-            Common::sendResponseCode(500);
-
-            $controller = FrontController::getInstance();
-            $controller->init();
-            $message = $controller->dispatch('CorePluginsAdmin', 'safemode', array($lastError));
-
+            $message = self::generateSafeModeOutput($lastError);
             echo $message;
         }
     }
diff --git a/core/Piwik.php b/core/Piwik.php
index 7d2193d485..930b64cf03 100644
--- a/core/Piwik.php
+++ b/core/Piwik.php
@@ -77,11 +77,18 @@ class Piwik
     {
         Common::sendHeader('Content-Type: text/html; charset=utf-8');
 
-        $output = "<style>a{color:red;}</style>\n" .
-            "<div style='color:red;font-size:120%'>" .
-            "<p><img src='plugins/Morpheus/images/error_medium.png' style='vertical-align:middle; float:left;padding:20px' />" .
+        $message = str_replace("\n", "<br/>", $message);
+
+        $output = "<html><body>".
+            "<style>a{color:red;}</style>\n" .
+            "<div style='color:red;font-size:120%; width:100%;margin: 30px;'>" .
+            " <div style='width: 50px; float: left;'><img src='plugins/Morpheus/images/error_medium.png' /></div>" .
+            "  <div style='margin-left: 70px; min-width: 950px;'>" .
             $message .
-            "</p></div>";
+            "  </div>" .
+            " </div>" .
+            "</div>".
+            "</body></html>";
         print($output);
         exit;
     }
diff --git a/core/SettingsPiwik.php b/core/SettingsPiwik.php
index b0df27b2b6..c0b2d8e177 100644
--- a/core/SettingsPiwik.php
+++ b/core/SettingsPiwik.php
@@ -210,7 +210,7 @@ class SettingsPiwik
         $config = Config::getInstance()->getLocalPath();
         $exists = file_exists($config);
 
-        // Piwik is installed if the config file is found
+        // Piwik is not installed if the config file is not found
         if (!$exists) {
             return false;
         }
diff --git a/lang/en.json b/lang/en.json
index 67e812bd9f..965c5ab1bd 100644
--- a/lang/en.json
+++ b/lang/en.json
@@ -140,7 +140,8 @@
         "ExceptionContactSupportGeneric": "If you still have this issue please %1$scontact your Piwik administrator%2$s for assistance. ",
         "ExceptionCheckUserHasSuperUserAccessOrIsTheUser": "The user has to be either a Super User or the user '%s' itself.",
         "ExceptionConfigurationFileNotFound": "The configuration file {%s} has not been found or could not be read.",
-        "ExceptionConfigurationFileNotFound2": "If the file exists, please check that %1$s is readable by the user '%2$s'.",
+        "ExceptionConfigurationFileExistsButNotReadable": "The config file %s seems to exist but Piwik could not read it.",
+        "ExceptionConfigurationFilePleaseCheckReadableByUser": "Please check that %1$s is readable by the user '%2$s'.",
         "ExceptionDatabaseVersion": "Your %1$s version is %2$s but Piwik requires at least %3$s.",
         "ExceptionDatabaseVersionNewerThanCodebase": "Your Piwik codebase is running the old version %1$s and we have detected that your Piwik Database has already been upgraded to the newer version %2$s.",
         "ExceptionDatabaseVersionNewerThanCodebaseWait": "Maybe your Piwik administrators are currently finishing the upgrade process. Please try again in a few minutes.",
diff --git a/piwik.php b/piwik.php
index f3e0ba934a..c1af709e2c 100644
--- a/piwik.php
+++ b/piwik.php
@@ -49,7 +49,6 @@ require_once PIWIK_INCLUDE_PATH . '/core/Tracker/Cache.php';
 require_once PIWIK_INCLUDE_PATH . '/core/Tracker/Request.php';
 require_once PIWIK_INCLUDE_PATH . '/core/Cookie.php';
 
-// TODO should move to Tracker application class later. currently needed for environment validation.
 SettingsServer::setIsTrackerApiRequest();
 
 $environment = new \Piwik\Application\Environment('tracker');
diff --git a/plugins/Installation/Installation.php b/plugins/Installation/Installation.php
index 59a0b02e64..6deb2732f0 100644
--- a/plugins/Installation/Installation.php
+++ b/plugins/Installation/Installation.php
@@ -105,7 +105,7 @@ class Installation extends \Piwik\Plugin
         if ($this->isAllowedAction($action)) {
             echo FrontController::getInstance()->dispatch('Installation', $action, array($message));
         } else {
-            Piwik::exitWithErrorMessage(Piwik::translate('Installation_NoConfigFound'));
+            Piwik::exitWithErrorMessage($this->getMessageToInviteUserToInstallPiwik($message));
         }
 
         exit;
@@ -127,4 +127,22 @@ class Installation extends \Piwik\Plugin
         return in_array($action, array_keys($controller->getInstallationSteps()))
                 || $isActionWhiteListed;
     }
+
+    /**
+     * @param $message
+     * @return string
+     */
+    private function getMessageToInviteUserToInstallPiwik($message)
+    {
+        $messageWhenPiwikSeemsNotInstalled =
+            $message .
+            "\n<br/>" .
+            Piwik::translate('Installation_NoConfigFileFound') .
+            "<br/><b>» " .
+            Piwik::translate('Installation_YouMayInstallPiwikNow', array("<a href='index.php'>", "</a></b>")) .
+            "<br/><small>" .
+            Piwik::translate('Installation_IfPiwikInstalledBeforeTablesCanBeKept') .
+            "</small>";
+        return $messageWhenPiwikSeemsNotInstalled;
+    }
 }
diff --git a/plugins/Installation/lang/en.json b/plugins/Installation/lang/en.json
index 0296e5f108..c67066b201 100644
--- a/plugins/Installation/lang/en.json
+++ b/plugins/Installation/lang/en.json
@@ -36,7 +36,9 @@
         "NfsFilesystemWarning": "Your server is using an NFS filesystem.",
         "NfsFilesystemWarningSuffixAdmin": "This means Piwik will be extremely slow when using file based sessions.",
         "NfsFilesystemWarningSuffixInstall": "Using file based sessions on NFS is extremely slow, so Piwik will use database sessions. If you have many concurrent dashboard users, you may need to increase the maximum number of client connections to the database server.",
-        "NoConfigFound": "The Piwik configuration file couldn't be found and you are trying to access a Piwik page.<br \/><b>  » You can <a href='index.php'>install Piwik now<\/a><\/b><br \/><small>If you installed Piwik before and have some tables in your DB, don't worry, you can reuse the same tables and keep your existing data!<\/small>",
+        "NoConfigFileFound": "The Piwik configuration file couldn't be found and you are trying to access a Piwik page.",
+        "YouMayInstallPiwikNow": "You may %1$sinstall Piwik now%2$s",
+        "IfPiwikInstalledBeforeTablesCanBeKept": "If you installed Piwik before and have some tables in your DB, don't worry, you can reuse the same tables and keep your existing data!",
         "Optional": "Optional",
         "Password": "Password",
         "PasswordDoNotMatch": "password do not match",
diff --git a/tests/PHPUnit/System/EnvironmentValidationTest.php b/tests/PHPUnit/System/EnvironmentValidationTest.php
index c283cbbd8f..f96032f36d 100644
--- a/tests/PHPUnit/System/EnvironmentValidationTest.php
+++ b/tests/PHPUnit/System/EnvironmentValidationTest.php
@@ -46,25 +46,24 @@ class EnvironmentValidationTest extends SystemTestCase
         $this->simulateAbsentConfigFile('global.ini.php');
 
         $output = $this->triggerPiwikFrom($entryPoint);
+
         $this->assertOutputContainsConfigFileMissingError('global.ini.php', $output);
+
     }
 
-    public function getEntryPointsThatErrorWithNoLocal()
+    public function test_NoLocalConfigFile_TriggersError_inTracker()
     {
-        return array(
-            array('tracker'),
-            array('console')
-        );
+        $this->simulateAbsentConfigFile('config.ini.php');
+
+        $output = $this->triggerPiwikFrom('tracker');
+        $this->assertContains('As Piwik is not installed yet, the Tracking API will now exit without error', $output);
     }
 
-    /**
-     * @dataProvider getEntryPointsThatErrorWithNoLocal
-     */
-    public function test_NoLocalConfigFile_TriggersError($entryPoint)
+    public function test_NoLocalConfigFile_TriggersError_inConsole()
     {
         $this->simulateAbsentConfigFile('config.ini.php');
 
-        $output = $this->triggerPiwikFrom($entryPoint);
+        $output = $this->triggerPiwikFrom('console');
         $this->assertOutputContainsConfigFileMissingError('config.ini.php', $output);
     }
 
@@ -104,6 +103,7 @@ class EnvironmentValidationTest extends SystemTestCase
         $this->simulateBadConfigFile($configFile);
 
         $output = $this->triggerPiwikFrom($entryPoint);
+
         $this->assertOutputContainsBadConfigFileError($output);
     }
 
@@ -123,7 +123,7 @@ class EnvironmentValidationTest extends SystemTestCase
 
     private function assertOutputContainsConfigFileMissingError($fileName, $output)
     {
-        $this->assertRegExp("/The configuration file \\{.*\\/" . preg_quote($fileName) . "\\} has not been found or could not be read\\./", $output);
+        $this->assertRegExp("/.*The configuration file \\{.*\\/" . preg_quote($fileName) . "\\} has not been found or could not be read\\..*/", $output, "Output did not contain the expected exception for $fileName --- Output was --- $output");
     }
 
     private function assertOutputContainsBadConfigFileError($output)
diff --git a/tests/UI/expected-ui-screenshots b/tests/UI/expected-ui-screenshots
-Subproject 5f35b2563d576762993c6f39f6e7830e3742910
+Subproject e18616b9c745778a543725e5cb8a15beaa1dcc1