From cd8c762a3df4440e69c8fce1d407ef62f6f185a7 Mon Sep 17 00:00:00 2001
From: Matthieu Aubry <mattab@users.noreply.github.com>
Date: Tue, 27 Dec 2016 22:12:10 +1300
Subject: [PATCH] When safe mode is displayed, and Super User was not logged in
 already, let Super User display the full safe mode to troubleshoot further
 (#11082)

* When safe mode is displayed, and Super User was not logged in already, let Super User display the full safe mode to troubleshoot further

* meant for a different PR

* Catch exceptions during CSS/Less compiling

* when there's an error during Twig template processing, or during LESS css compiling, display the safe mode

* Deactivate plugin as super user when authenticated with i_am_super_user

* when user is not logged as Super User, Make deactivate plugin work by forwarding the i_am_super_user URL parameter

* Display plugin version in safe mode fixes https://github.com/piwik/piwik/issues/11043

* Added a text to indicate users to first disable third party plugins

* Updated UI test

* clarify that salt is a secret

* reuse helper method

* minor

* Catching PHP7 errors and making it work on php5
---
 .../UIAssetMerger/StylesheetUIAssetMerger.php |   7 ++-
 .../StylesheetLessCompileException.php        |  13 +++++
 core/FrontController.php                      |  30 +++++++++-
 core/SettingsPiwik.php                        |   4 +-
 libs/upgradephp/upgrade.php                   |   9 +++
 plugins/CorePluginsAdmin/Controller.php       |  53 +++++++++++++++---
 .../CorePluginsAdmin/templates/safemode.twig  |  24 ++++++--
 ...UIIntegrationTest_fatal_error_safemode.png | Bin 131 -> 131 bytes
 8 files changed, 123 insertions(+), 17 deletions(-)
 create mode 100644 core/Exception/StylesheetLessCompileException.php

diff --git a/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php b/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php
index de32191b6b..3d5cad72b6 100644
--- a/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php
+++ b/core/AssetManager/UIAssetMerger/StylesheetUIAssetMerger.php
@@ -13,6 +13,7 @@ use lessc;
 use Piwik\AssetManager\UIAsset;
 use Piwik\AssetManager\UIAssetMerger;
 use Piwik\Common;
+use Piwik\Exception\StylesheetLessCompileException;
 use Piwik\Piwik;
 
 class StylesheetUIAssetMerger extends UIAssetMerger
@@ -41,7 +42,11 @@ class StylesheetUIAssetMerger extends UIAssetMerger
         $concatenatedAssets = $this->getConcatenatedAssets();
 
         $this->lessCompiler->setFormatter('classic');
-        $compiled = $this->lessCompiler->compile($concatenatedAssets);
+        try {
+            $compiled = $this->lessCompiler->compile($concatenatedAssets);
+        } catch(\Exception $e) {
+            throw new StylesheetLessCompileException($e->getMessage());
+        }
 
         foreach ($this->cssAssetsToReplace as $asset) {
             // to fix #10173
diff --git a/core/Exception/StylesheetLessCompileException.php b/core/Exception/StylesheetLessCompileException.php
new file mode 100644
index 0000000000..3e3124eb77
--- /dev/null
+++ b/core/Exception/StylesheetLessCompileException.php
@@ -0,0 +1,13 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ */
+namespace Piwik\Exception;
+
+class StylesheetLessCompileException extends Exception
+{
+}
diff --git a/core/FrontController.php b/core/FrontController.php
index 569f541dfa..c08ad5a6c2 100644
--- a/core/FrontController.php
+++ b/core/FrontController.php
@@ -15,6 +15,7 @@ use Piwik\Container\StaticContainer;
 use Piwik\Exception\AuthenticationFailedException;
 use Piwik\Exception\DatabaseSchemaIsNewerThanCodebaseException;
 use Piwik\Exception\PluginDeactivatedException;
+use Piwik\Exception\StylesheetLessCompileException;
 use Piwik\Http\ControllerResolver;
 use Piwik\Http\Router;
 use Piwik\Plugins\CoreAdminHome\CustomLogo;
@@ -73,11 +74,11 @@ class FrontController extends Singleton
 
     /**
      * @param $lastError
-     * @return mixed|void
+     * @return string
      * @throws AuthenticationFailedException
      * @throws Exception
      */
-    private static function generateSafeModeOutput($lastError)
+    private static function generateSafeModeOutputFromError($lastError)
     {
         Common::sendResponseCode(500);
 
@@ -93,6 +94,20 @@ class FrontController extends Singleton
         return $message;
     }
 
+    /**
+     * @param Exception $e
+     * @return string
+     */
+    private static function generateSafeModeOutputFromException($e)
+    {
+        $error = array(
+            'message' => $e->getMessage(),
+            'file' => $e->getFile(),
+            'line' => $e->getLine()
+        );
+        return self::generateSafeModeOutputFromError($error);
+    }
+
     /**
      * Executes the requested plugin controller method.
      *
@@ -133,6 +148,15 @@ class FrontController extends Singleton
              * @param \Piwik\NoAccessException $exception The exception that was caught.
              */
             Piwik::postEvent('User.isNotAuthorized', array($exception), $pending = true);
+        } catch (\Twig_Error_Runtime $e) {
+            echo $this->generateSafeModeOutputFromException($e);
+            exit;
+        } catch(StylesheetLessCompileException $e) {
+            echo $this->generateSafeModeOutputFromException($e);
+            exit;
+        } catch(\Error $e) {
+            echo $this->generateSafeModeOutputFromException($e);
+            exit;
         }
     }
 
@@ -202,7 +226,7 @@ class FrontController extends Singleton
     {
         $lastError = error_get_last();
         if (!empty($lastError) && $lastError['type'] == E_ERROR) {
-            $message = self::generateSafeModeOutput($lastError);
+            $message = self::generateSafeModeOutputFromError($lastError);
             echo $message;
         }
     }
diff --git a/core/SettingsPiwik.php b/core/SettingsPiwik.php
index 52c4f3e16f..446712703a 100644
--- a/core/SettingsPiwik.php
+++ b/core/SettingsPiwik.php
@@ -20,7 +20,9 @@ class SettingsPiwik
     const OPTION_PIWIK_URL = 'piwikUrl';
 
     /**
-     * Get salt from [General] section
+     * Get salt from [General] section. Should ONLY be used as a seed to create hashes
+     *
+     * NOTE: Keep this salt secret! Never output anywhere or share it etc.
      *
      * @return string
      */
diff --git a/libs/upgradephp/upgrade.php b/libs/upgradephp/upgrade.php
index c6591b70a9..8c29af7c3a 100644
--- a/libs/upgradephp/upgrade.php
+++ b/libs/upgradephp/upgrade.php
@@ -702,3 +702,12 @@ if (!function_exists('dump')) {
 
     }
 }
+
+/**
+ * Need to catch that PHP7 error object on php5
+ */
+if( !class_exists('\Error')) {
+	class Error {
+
+	}
+}
\ No newline at end of file
diff --git a/plugins/CorePluginsAdmin/Controller.php b/plugins/CorePluginsAdmin/Controller.php
index 860cee7404..6d6006fdce 100644
--- a/plugins/CorePluginsAdmin/Controller.php
+++ b/plugins/CorePluginsAdmin/Controller.php
@@ -11,6 +11,7 @@ namespace Piwik\Plugins\CorePluginsAdmin;
 use Exception;
 use Piwik\API\Request;
 use Piwik\Common;
+use Piwik\Config;
 use Piwik\Container\StaticContainer;
 use Piwik\Exception\MissingFilePermissionException;
 use Piwik\Filechecks;
@@ -22,6 +23,7 @@ use Piwik\Plugin;
 use Piwik\Plugins\Marketplace\Marketplace;
 use Piwik\Plugins\Marketplace\Controller as MarketplaceController;
 use Piwik\Plugins\Marketplace\Plugins;
+use Piwik\SettingsPiwik;
 use Piwik\Translation\Translator;
 use Piwik\Url;
 use Piwik\Version;
@@ -296,22 +298,27 @@ class Controller extends Plugin\ControllerAdmin
             return $message;
         }
 
-        if (Common::isPhpCliMode()) { // TODO: I can't find how this will ever get called / safeMode is never set for Console
+        if (Common::isPhpCliMode()) {
             throw new Exception("Error: " . var_export($lastError, true));
         }
-
         $view = new View('@CorePluginsAdmin/safemode');
         $view->lastError   = $lastError;
+        $view->isAllowedToTroubleshootAsSuperUser = $this->isAllowedToTroubleshootAsSuperUser();
         $view->isSuperUser = Piwik::hasUserSuperUserAccess();
         $view->isAnonymousUser = Piwik::isUserIsAnonymous();
         $view->plugins         = $this->pluginManager->loadAllPluginsAndGetTheirInfo();
         $view->deactivateNonce = Nonce::getNonce(static::DEACTIVATE_NONCE);
+        $view->deactivateIAmSuperUserSalt = Common::getRequestVar('i_am_super_user', '', 'string');
         $view->uninstallNonce  = Nonce::getNonce(static::UNINSTALL_NONCE);
         $view->emailSuperUser  = implode(',', Piwik::getAllSuperUserAccessEmailAddresses());
         $view->piwikVersion    = Version::VERSION;
         $view->showVersion     = !Common::getRequestVar('tests_hide_piwik_version', 0);
         $view->pluginCausesIssue = '';
 
+        // When the CSS merger in StylesheetUIAssetMerger throws an exception, safe mode is displayed.
+        // This flag prevents an infinite loop where safemode would try to re-generate the cache buster which requires CSS merger..
+        $view->disableCacheBuster();
+
         if (!empty($lastError['file'])) {
             preg_match('/piwik\/plugins\/(.*)\//', $lastError['file'], $matches);
 
@@ -367,11 +374,13 @@ class Controller extends Plugin\ControllerAdmin
 
     public function deactivate($redirectAfter = true)
     {
-        $pluginName = $this->initPluginModification(static::DEACTIVATE_NONCE);
-        $this->dieIfPluginsAdminIsDisabled();
-
-        $this->pluginManager->deactivatePlugin($pluginName);
-        $this->redirectAfterModification($redirectAfter);
+        if($this->isAllowedToTroubleshootAsSuperUser()) {
+            Piwik::doAsSuperUser(function() use ($redirectAfter) {
+                $this->doDeactivatePlugin($redirectAfter);
+            });
+        } else {
+            $this->doDeactivatePlugin($redirectAfter);
+        }
     }
 
     public function uninstall($redirectAfter = true)
@@ -455,4 +464,34 @@ class Controller extends Plugin\ControllerAdmin
         } catch (Exception $e) {}
     }
 
+    /**
+     * Let Super User troubleshoot in safe mode, even when Login is broken, with this special trick
+     *
+     * @return bool
+     * @throws Exception
+     */
+    protected function isAllowedToTroubleshootAsSuperUser()
+    {
+        $isAllowedToTroubleshootAsSuperUser = false;
+        $salt = SettingsPiwik::getSalt();
+        if (!empty($salt)) {
+            $saltFromRequest = Common::getRequestVar('i_am_super_user', '', 'string');
+            $isAllowedToTroubleshootAsSuperUser = ($salt == $saltFromRequest);
+        }
+        return $isAllowedToTroubleshootAsSuperUser;
+    }
+
+    /**
+     * @param $redirectAfter
+     * @throws Exception
+     */
+    protected function doDeactivatePlugin($redirectAfter)
+    {
+        $pluginName = $this->initPluginModification(static::DEACTIVATE_NONCE);
+        $this->dieIfPluginsAdminIsDisabled();
+
+        $this->pluginManager->deactivatePlugin($pluginName);
+        $this->redirectAfterModification($redirectAfter);
+    }
+
 }
diff --git a/plugins/CorePluginsAdmin/templates/safemode.twig b/plugins/CorePluginsAdmin/templates/safemode.twig
index 440ec91ebc..01dd31d187 100644
--- a/plugins/CorePluginsAdmin/templates/safemode.twig
+++ b/plugins/CorePluginsAdmin/templates/safemode.twig
@@ -28,7 +28,7 @@
 
         <div style="width: 640px">
 
-        {% if not isAnonymousUser %}
+        {% if isAllowedToTroubleshootAsSuperUser or not isAnonymousUser %}
             <p>
                 The following error just broke Piwik{% if showVersion %} (v{{ piwikVersion }}){%  endif %}:
                 <pre>{{ lastError.message }}</pre>
@@ -58,12 +58,13 @@
 
         {% endif %}
 
-        {% if isSuperUser %}
+        {% if isAllowedToTroubleshootAsSuperUser or isSuperUser %}
 
             <h3>Further troubleshooting</h3>
             <p>
                 If this error continues to happen, you may be able to fix this issue by disabling one or more of
-                the Third-Party plugins. You can enable them again in the
+                the Third-Party plugins. If you don't know which plugin is causing this error, we recommend to first disable any plugin not created by "Piwik" and not created by "InnoCraft".
+                You can enable plugin again afterwards in the
                 <a rel="noreferrer" target="_blank" href="index.php?module=CorePluginsAdmin&action=plugins">Plugins</a>
                 or <a target="_blank" href="index.php?module=CorePluginsAdmin&action=themes">Themes</a> page under
                 settings at any time.
@@ -79,7 +80,10 @@
                             {{ pluginName }}
                         </td>
                         <td>
-                            <a href="index.php?module=CorePluginsAdmin&action=deactivate&pluginName={{ pluginName }}&nonce={{ deactivateNonce }}"
+                            {{ plugin.info.version|default('') }}
+                        </td>
+                        <td>
+                            <a href="index.php?module=CorePluginsAdmin&action=deactivate&pluginName={{ pluginName }}&nonce={{ deactivateNonce }}{% if deactivateIAmSuperUserSalt is not empty %}&i_am_super_user={{ deactivateIAmSuperUserSalt }}{% endif %}"
                                target="_blank">deactivate</a>
                         </td>
                     </tr>
@@ -115,7 +119,7 @@
 
         {% elseif isAnonymousUser %}
 
-            <p>Please contact the system administrator, or login to Piwik to learn more.</p>
+            <p>Please contact the system administrator, or <a href="?module={{ loginModule }}">login to Piwik</a> to learn more.</p>
 
         {% else %}
             <p>
@@ -125,6 +129,16 @@
             </p>
         {% endif %}
 
+
+        {% if not isAllowedToTroubleshootAsSuperUser and not isSuperUser %}
+            <p>If you are Super User, but cannot login because of this error, you can still troubleshoot further. Follow these steps:
+                <br/>1) open the config/config.ini.php file and look for the <code>salt</code> value under <code>[General]</code>.
+                <br/>2) edit this current URL you are viewing and add the following text (replacing <code>salt_value_from_config</code> by the <code>salt</code> value from the config file):
+                <br/><br/><code>index.php?i_am_super_user=salt_value_from_config&....</code>
+            </p>
+        {% endif %}
+
+
         </div>
 
     </body>
diff --git a/tests/UI/expected-screenshots/UIIntegrationTest_fatal_error_safemode.png b/tests/UI/expected-screenshots/UIIntegrationTest_fatal_error_safemode.png
index f015ac9c2d49226a8e26ebc39522e594e11b76f0..f0d8e62af8c836fc3736a9cd53b4fee9c6ad2006 100644
GIT binary patch
delta 84
zcmV~$yAgmO3;@uhWeP_yBqU@AhaeyBtnF-Bz>$4#mt8)-dd?ZC3>xG%LorW5pw27Y
ejMy;HK}adUmJ1A%9N9dPTj%=RZyumX%Jl~m;}?_w

delta 84
zcmV~$u@QhU2nEnfn<*UOg9gG74k3ixS=-rW0Y~<|RaW`rg!^#Dk*oxuC3S~c`ZcwC
eC`l$ohcz3c$3C0FSj6Vg$$|RZZ*C+;JJ%oLBNoU2

-- 
GitLab