Fix plugins might not be updated when updating core (#16322)

* Fix plugins might not be updated when updating core

* dont invalidate all caches as it could have side effects re template etc

* dont invalidate cache

* fix error if marketplace is not enabled, fallback to direct dispatch if climulti doesnt work

* show update log

* fix test

6 files changed, 107 insertions, 38 deletions

diff --git a/core/FrontController.php b/core/FrontController.php
index c7aae14c5a..e59d2b359a 100644
--- a/core/FrontController.php
+++ b/core/FrontController.php
@@ -153,7 +153,7 @@ class FrontController extends Singleton
         if (self::$enableDispatch === false) {
             return;
         }
-
+        
         $filter = new Router();
         $redirection = $filter->filterUrl(Url::getCurrentUrl());
         if ($redirection !== null) {
diff --git a/plugins/CoreUpdater/Controller.php b/plugins/CoreUpdater/Controller.php
index de4dc20532..207bf9f329 100644
--- a/plugins/CoreUpdater/Controller.php
+++ b/plugins/CoreUpdater/Controller.php
@@ -20,7 +20,6 @@ use Piwik\Filesystem;
 use Piwik\Http;
 use Piwik\Option;
 use Piwik\Piwik;
-use Piwik\Plugin;
 use Piwik\Plugin\Manager as PluginManager;
 use Piwik\Plugins\LanguagesManager\LanguagesManager;
 use Piwik\Plugins\Marketplace\Plugins;
@@ -173,23 +172,41 @@ class Controller extends \Piwik\Plugin\Controller
         return $view->render();
     }
 
-    public function oneClickUpdatePartTwo()
+    public function oneClickUpdatePartTwo($sendHeader = true)
     {
-        Json::sendHeaderJSON();
+        if ($sendHeader) {
+            Json::sendHeaderJSON();
+        }
 
-        $messages = [];
+        $task = "Couldn't update Marketplace plugins.";
+
+        $nonce = Common::getRequestVar('nonce', '', 'string');
+        if (empty($nonce)) {
+            return json_encode(['No token. ' . $task]);
+        }
+        $value = Option::get('NonceOneClickUpdatePartTwo');
+        if (empty($value)) {
+            return json_encode(['Invalid token. ' . $task]);
+        }
+        $value = json_decode($value, true);
+
+        if (empty($value['nonce'])
+            || empty($value['ttl'])
+            || time() > (int) $value['ttl']
+            || $nonce !== $value['nonce']) {
+            return json_encode(['Invalid nonce or nonce expired. ' . $task]);
+        }
 
         try {
-            Piwik::checkUserHasSuperUserAccess();
             $messages = $this->updater->oneClickUpdatePartTwo();
         } catch (UpdaterException $e) {
             $messages = $e->getUpdateLogMessages();
             $messages[] = $e->getMessage();
         } catch (Exception $e) {
-            $messages[] = $e->getMessage();
+            $messages = [$e->getMessage()];
         }
 
-        echo json_encode($messages);
+        return json_encode($messages);
     }
 
     public function oneClickResults()
@@ -205,10 +222,14 @@ class Controller extends \Piwik\Plugin\Controller
         } elseif ($error) {
             $view = new View('@CoreUpdater/updateHttpError');
             $view->error = $error;
-            $view->feedbackMessages = safe_unserialize(Common::unsanitizeInputValue(Common::getRequestVar('messages', '', 'string', $_POST)));
         } else {
             $view = new View('@CoreUpdater/updateSuccess');
         }
+        $messages = safe_unserialize(Common::unsanitizeInputValue(Common::getRequestVar('messages', '', 'string', $_POST)));
+        if (!is_array($messages)) {
+            $messages = array();
+        }
+        $view->feedbackMessages = $messages;
 
         $this->addCustomLogoInfo($view);
         $this->setBasicVariablesView($view);
diff --git a/plugins/CoreUpdater/Updater.php b/plugins/CoreUpdater/Updater.php
index abda0c8b4c..c190f562f9 100644
--- a/plugins/CoreUpdater/Updater.php
+++ b/plugins/CoreUpdater/Updater.php
@@ -10,9 +10,14 @@ namespace Piwik\Plugins\CoreUpdater;
 
 use Exception;
 use Piwik\ArchiveProcessor\Rules;
+use Piwik\Cache as PiwikCache;
+use Piwik\CliMulti;
+use Piwik\Common;
 use Piwik\Container\StaticContainer;
+use Piwik\Context;
 use Piwik\Filechecks;
 use Piwik\Filesystem;
+use Piwik\FrontController;
 use Piwik\Http;
 use Piwik\Option;
 use Piwik\Plugin\Manager as PluginManager;
@@ -23,7 +28,6 @@ use Piwik\Plugins\Marketplace\Marketplace;
 use Piwik\SettingsServer;
 use Piwik\Translation\Translator;
 use Piwik\Unzip;
-use Piwik\Url;
 use Piwik\Version;
 
 class Updater
@@ -124,15 +128,42 @@ class Updater
             throw new UpdaterException($e, $messages);
         }
 
-        $partTwoUrl = Url::getCurrentUrlWithoutQueryString() . Url::getCurrentQueryStringWithParametersModified([
-            'action' => 'oneClickUpdatePartTwo',
-        ]);
-
-        $response = Http::sendHttpRequest($partTwoUrl, 300);
-        $response = @json_decode($response, $assoc = true);
-
-        if (!empty($response)) {
-            $messages = array_merge($messages, $response);
+        $validFor10Minutes = time() + (60 * 10);
+        $nonce = Common::generateUniqId();
+        Option::set('NonceOneClickUpdatePartTwo', json_encode(['nonce' => $nonce, 'ttl' => $validFor10Minutes]));
+
+        $cliMulti = new CliMulti();
+        $responses = $cliMulti->request(['?module=CoreUpdater&action=oneClickUpdatePartTwo&nonce=' . $nonce]);
+
+        if (!empty($responses)) {
+            $responseCliMulti = array_shift($responses);
+            $responseCliMulti = @json_decode($responseCliMulti, $assoc = true);
+            if (is_array($responseCliMulti)) {
+                // we expect a json encoded array response from oneClickUpdatePartTwo. Otherwise something went wrong.
+                $messages = array_merge($messages, $responseCliMulti);
+            } else {
+                // there was likely an error eg such as an invalid ssl certificate... let's try executing it directly
+                // in case this works. For explample $response is in this case not an array but a string because the "communcation"
+                // with the controller went wrong: "Got invalid response from API request: https://ABC/?module=CoreUpdater&action=oneClickUpdatePartTwo&nonce=ABC. Response was \'curl_exec: SSL certificate problem: unable to get local issuer certificate. Hostname requested was: ABC"
+                try {
+                    $response = null;
+                    Context::executeWithQueryParameters(array('nonce' => $nonce), function () use (&$response) {
+                        $response = FrontController::getInstance()->dispatch('CoreUpdater', 'oneClickUpdatePartTwo', array($sendHeader = false));
+                    });
+                    if (!empty($response)) {
+                        $response = @json_decode($response, $assoc = true);
+                        if (!empty($response) && is_array($response)) {
+                            $messages = array_merge($messages, $response);
+                        }
+                    }
+                } catch (Exception $e) {
+                    // ignore any error should this fail too. this might be the case eg if
+                    // the user upgrades from one major version to another major version
+                    if (is_string($responseCliMulti)) {
+                        $messages[] = $responseCliMulti; // show why the original request failed eg invalid ssl certificate
+                    }
+                }
+            }
         }
 
         try {
@@ -151,6 +182,12 @@ class Updater
     {
         $messages = [];
 
+        if (!Marketplace::isMarketplaceEnabled()) {
+            $messages[] = 'Marketplace is disabled. Not updating any plugins.';
+            // prevent error Entry "Piwik\Plugins\Marketplace\Api\Client" cannot be resolved: Entry "Piwik\Plugins\Marketplace\Api\Service" cannot be resolved
+            return $messages;
+        }
+
         $newVersion = Version::VERSION;
 
         // we also need to make sure to create a new instance here as otherwise we would change the "global"
@@ -163,23 +200,20 @@ class Updater
         ));
 
         try {
-
-            if (Marketplace::isMarketplaceEnabled()) {
-                $messages[] = $this->translator->translate('CoreUpdater_CheckingForPluginUpdates');
-                $pluginManager = PluginManager::getInstance();
-                $pluginManager->loadAllPluginsAndGetTheirInfo();
-                $loadedPlugins = $pluginManager->getLoadedPlugins();
-
-                $marketplaceClient->clearAllCacheEntries();
-                $pluginsWithUpdate = $marketplaceClient->checkUpdates($loadedPlugins);
-
-                foreach ($pluginsWithUpdate as $pluginWithUpdate) {
-                    $pluginName = $pluginWithUpdate['name'];
-                    $messages[] = $this->translator->translate('CoreUpdater_UpdatingPluginXToVersionY',
-                        array($pluginName, $pluginWithUpdate['version']));
-                    $pluginInstaller = new PluginInstaller($marketplaceClient);
-                    $pluginInstaller->installOrUpdatePluginFromMarketplace($pluginName);
-                }
+            $messages[] = $this->translator->translate('CoreUpdater_CheckingForPluginUpdates');
+            $pluginManager = PluginManager::getInstance();
+            $pluginManager->loadAllPluginsAndGetTheirInfo();
+            $loadedPlugins = $pluginManager->getLoadedPlugins();
+
+            $marketplaceClient->clearAllCacheEntries();
+            $pluginsWithUpdate = $marketplaceClient->checkUpdates($loadedPlugins);
+
+            foreach ($pluginsWithUpdate as $pluginWithUpdate) {
+                $pluginName = $pluginWithUpdate['name'];
+                $messages[] = $this->translator->translate('CoreUpdater_UpdatingPluginXToVersionY',
+                    array($pluginName, $pluginWithUpdate['version']));
+                $pluginInstaller = new PluginInstaller($marketplaceClient);
+                $pluginInstaller->installOrUpdatePluginFromMarketplace($pluginName);
             }
         } catch (MarketplaceApi\Exception $e) {
             // there is a problem with the connection to the server, ignore for now
diff --git a/plugins/CoreUpdater/lang/en.json b/plugins/CoreUpdater/lang/en.json
index bc703116b8..25e417d8b5 100644
--- a/plugins/CoreUpdater/lang/en.json
+++ b/plugins/CoreUpdater/lang/en.json
@@ -7,6 +7,7 @@
         "DisablingIncompatiblePlugins": "Disabling incompatible plugins: %s",
         "DownloadingUpdateFromX": "Downloading update from %s",
         "DownloadX": "Download %s",
+        "UpdateLog": "Update log",
         "EmptyDatabaseError": "Database %s is empty. You must edit or remove your Matomo configuration file.",
         "ErrorDIYHelp": "If you are an advanced user and encounter an error in the database upgrade:",
         "ErrorDIYHelp_1": "identify and correct the source of the problem (e.g., memory_limit or max_execution_time)",
diff --git a/plugins/CoreUpdater/templates/updateSuccess.twig b/plugins/CoreUpdater/templates/updateSuccess.twig
index 7a1c118e79..e9edaf79ff 100644
--- a/plugins/CoreUpdater/templates/updateSuccess.twig
+++ b/plugins/CoreUpdater/templates/updateSuccess.twig
@@ -36,6 +36,19 @@
             </div>
         </div>
 
+        {% if feedbackMessages is defined and feedbackMessages is not empty %}
+        <h2>{{ 'CoreUpdater_UpdateLog'|translate }}</h2>
+        <div class="row">
+            <div class="col s12">
+                <pre style="margin-top: 0;"><code>
+                    {%- for message in feedbackMessages %}
+&#10003; {{ message }}
+{% endfor -%}
+                </code></pre>
+            </div>
+        </div>
+        {% endif %}
+
     </div>
 
     <div class="footer">
diff --git a/tests/UI/expected-screenshots/OneClickUpdate_update_success.png b/tests/UI/expected-screenshots/OneClickUpdate_update_success.png
index 6e3c494ebb..7e34156f05 100644
--- a/tests/UI/expected-screenshots/OneClickUpdate_update_success.png
+++ b/tests/UI/expected-screenshots/OneClickUpdate_update_success.png
@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:73e1130c2f80bfab81a0e430d1cece5b2b26b4e5d4c9d11f47ecca1e2940b9c3
-size 224408
+oid sha256:480f606ff26a4021d6296f014155d8c295e96510ab9ab072973a9dbf26d26c48
+size 244116