refs #4053 use nonces for all plugin operations (even for deactivate and uninstall)

4 files changed, 38 insertions, 39 deletions

diff --git a/plugins/CorePluginsAdmin/Controller.php b/plugins/CorePluginsAdmin/Controller.php
index 706a16c8d8..f5ee37c693 100644
--- a/plugins/CorePluginsAdmin/Controller.php
+++ b/plugins/CorePluginsAdmin/Controller.php
@@ -28,27 +28,23 @@ use Piwik\PluginsManager;
  */
 class Controller extends \Piwik\Controller\Admin
 {
+    const UPDATE_NONCE     = 'CorePluginsAdmin.updatePlugin';
+    const INSTALL_NONCE    = 'CorePluginsAdmin.installPlugin';
+    const ACTIVATE_NONCE   = 'CorePluginsAdmin.activatePlugin';
+    const DEACTIVATE_NONCE = 'CorePluginsAdmin.deactivatePlugin';
+    const UNINSTALL_NONCE  = 'CorePluginsAdmin.uninstallPlugin';
+
     private $validSortMethods  = array('popular', 'newest', 'alpha');
     private $defaultSortMethod = 'popular';
 
     private function createUpdateOrInstallView($template, $nonceName)
     {
-        Piwik::checkUserIsSuperUser();
+        $pluginName = $this->initPluginModification($nonceName);
 
         $view = $this->configureView('@CorePluginsAdmin/' . $template);
 
-        $pluginName = Common::getRequestVar('pluginName', null, 'string');
-        $nonce      = Common::getRequestVar('nonce', null, 'string');
-
         $view->plugin = array('name' => $pluginName);
 
-        if (!Nonce::verifyNonce('CorePluginsAdmin.' . $nonceName, $nonce)) {
-            $view->errorMessage = Piwik_Translate('General_ExceptionNonceMismatch');
-            return $view;
-        }
-
-        Nonce::discardNonce('CorePluginsAdmin.' . $nonceName);
-
         try {
             $pluginInstaller = new PluginInstaller($pluginName);
             $pluginInstaller->installOrUpdatePluginFromMarketplace();
@@ -66,13 +62,13 @@ class Controller extends \Piwik\Controller\Admin
 
     public function updatePlugin()
     {
-        $view = $this->createUpdateOrInstallView('updatePlugin', 'updatePlugin');
+        $view = $this->createUpdateOrInstallView('updatePlugin', static::UPDATE_NONCE);
         echo $view->render();
     }
 
     public function installPlugin()
     {
-        $view = $this->createUpdateOrInstallView('installPlugin', 'installPlugin');
+        $view = $this->createUpdateOrInstallView('installPlugin', static::INSTALL_NONCE);
         $view->nonce = Nonce::getNonce('CorePluginsAdmin.activatePlugin');
 
         echo $view->render();
@@ -110,8 +106,8 @@ class Controller extends \Piwik\Controller\Admin
 
         $view->query   = $query;
         $view->sort    = $sort;
-        $view->installNonce = Nonce::getNonce('CorePluginsAdmin.installPlugin');
-        $view->updateNonce  = Nonce::getNonce('CorePluginsAdmin.updatePlugin');
+        $view->installNonce = Nonce::getNonce(static::INSTALL_NONCE);
+        $view->updateNonce  = Nonce::getNonce(static::UPDATE_NONCE);
         $view->isSuperUser  = Piwik::isUserIsSuperUser();
 
         return $view;
@@ -141,7 +137,6 @@ class Controller extends \Piwik\Controller\Admin
 
         $activated  = Common::getRequestVar('activated', false, 'integer', $_GET);
         $pluginName = Common::getRequestVar('pluginName', '', 'string');
-        $pluginName = strip_tags($pluginName);
 
         $view = $this->configureView('@CorePluginsAdmin/' . $template);
 
@@ -150,9 +145,11 @@ class Controller extends \Piwik\Controller\Admin
             $view->activatedPluginName = $pluginName;
         }
 
-        $view->updateNonce   = Nonce::getNonce('CorePluginsAdmin.updatePlugin');
-        $view->activateNonce = Nonce::getNonce('CorePluginsAdmin.activatePlugin');
-        $view->pluginsInfo   = $this->getPluginsInfo($themesOnly);
+        $view->updateNonce     = Nonce::getNonce(static::UPDATE_NONCE);
+        $view->activateNonce   = Nonce::getNonce(static::ACTIVATE_NONCE);
+        $view->uninstallNonce  = Nonce::getNonce(static::UNINSTALL_NONCE);
+        $view->deactivateNonce = Nonce::getNonce(static::DEACTIVATE_NONCE);
+        $view->pluginsInfo     = $this->getPluginsInfo($themesOnly);
 
         $marketplace = new Marketplace();
         $view->pluginsHavingUpdate = $marketplace->getPluginsHavingUpdate($themesOnly);
@@ -228,7 +225,7 @@ class Controller extends \Piwik\Controller\Admin
 
     public function deactivate($redirectAfter = true)
     {
-        $pluginName = $this->initPluginModification();
+        $pluginName = $this->initPluginModification(static::DEACTIVATE_NONCE);
         \Piwik\PluginsManager::getInstance()->deactivatePlugin($pluginName);
         $this->redirectAfterModification($redirectAfter);
     }
@@ -240,26 +237,25 @@ class Controller extends \Piwik\Controller\Admin
         }
     }
 
-    protected function initPluginModification()
+    protected function initPluginModification($nonceName)
     {
         Piwik::checkUserIsSuperUser();
-        $this->checkTokenInUrl();
+
+        $nonce = Common::getRequestVar('nonce', null, 'string');
+
+        if (!Nonce::verifyNonce($nonceName, $nonce)) {
+            throw new \Exception(Piwik_Translate('General_ExceptionNonceMismatch'));
+        }
+
+        Nonce::discardNonce($nonceName);
+
         $pluginName = Common::getRequestVar('pluginName', null, 'string');
         return $pluginName;
     }
 
     public function activate($redirectAfter = true)
     {
-        Piwik::checkUserIsSuperUser();
-
-        $pluginName = Common::getRequestVar('pluginName', null, 'string');
-        $nonce      = Common::getRequestVar('nonce', null, 'string');
-
-        if (!Nonce::verifyNonce('CorePluginsAdmin.activatePlugin', $nonce)) {
-            throw new \Exception(Piwik_Translate('General_ExceptionNonceMismatch'));
-        }
-
-        Nonce::discardNonce('CorePluginsAdmin.activatePlugin');
+        $pluginName = $this->initPluginModification(static::ACTIVATE_NONCE);
 
         \Piwik\PluginsManager::getInstance()->activatePlugin($pluginName);
 
@@ -278,8 +274,10 @@ class Controller extends \Piwik\Controller\Admin
 
     public function uninstall($redirectAfter = true)
     {
-        $pluginName = $this->initPluginModification();
+        $pluginName  = $this->initPluginModification(static::UNINSTALL_NONCE);
+
         $uninstalled = \Piwik\PluginsManager::getInstance()->uninstallPlugin($pluginName);
+
         if (!$uninstalled) {
             $path = Filesystem::getPathToPiwikRoot() . '/plugins/' . $pluginName . '/';
             $messagePermissions = Filechecks::getErrorMessageMissingPermissions($path);
@@ -289,6 +287,7 @@ class Controller extends \Piwik\Controller\Admin
             $exitMessage = $messageIntro . "<br/><br/>" . $messagePermissions;
             Piwik_ExitWithMessage($exitMessage, $optionalTrace = false, $optionalLinks = false, $optionalLinkBack = true);
         }
+
         $this->redirectAfterModification($redirectAfter);
     }
 
diff --git a/plugins/CorePluginsAdmin/templates/macros.twig b/plugins/CorePluginsAdmin/templates/macros.twig
index 73e9c778ce..8a52bb4787 100644
--- a/plugins/CorePluginsAdmin/templates/macros.twig
+++ b/plugins/CorePluginsAdmin/templates/macros.twig
@@ -41,7 +41,7 @@
 
 {% endmacro %}
 
-{% macro tablePlugins(pluginsInfo, token_auth, activateNonce, isTheme) %}
+{% macro tablePlugins(pluginsInfo, activateNonce, deactivateNonce, uninstallNonce, isTheme) %}
 
 <div class='entityContainer'>
     <table class="dataTable entityTable">
@@ -84,15 +84,15 @@
                             {{ 'CorePluginsAdmin_Active'|translate }}
                         {% else %}
                             {{ 'CorePluginsAdmin_Inactive'|translate }} <br/>
-                            - {% if plugin.uninstallable %}<a href='index.php?module=CorePluginsAdmin&action=uninstall&pluginName={{ name }}&token_auth={{
-                        token_auth }}'>uninstall</a>{% endif %}
+                            - {% if plugin.uninstallable %}<a href='index.php?module=CorePluginsAdmin&action=uninstall&pluginName={{ name }}&nonce={{
+                        uninstallNonce }}'>uninstall</a>{% endif %}
                         {% endif %}
                     </td>
 
                     <td class="togl action-links">
                         {% if plugin.invalid is not defined %}
                             {% if plugin.activated %}
-                                <a href='index.php?module=CorePluginsAdmin&action=deactivate&pluginName={{ name }}&token_auth={{ token_auth }}'>{{ 'CorePluginsAdmin_Deactivate'|translate }}</a>
+                                <a href='index.php?module=CorePluginsAdmin&action=deactivate&pluginName={{ name }}&nonce={{ deactivateNonce }}'>{{ 'CorePluginsAdmin_Deactivate'|translate }}</a>
                             {% else %}
                                 <a href='index.php?module=CorePluginsAdmin&action=activate&pluginName={{ name }}&nonce={{  activateNonce }}'>{{ 'CorePluginsAdmin_Activate'|translate }}</a>
                             {% endif %}
diff --git a/plugins/CorePluginsAdmin/templates/plugins.twig b/plugins/CorePluginsAdmin/templates/plugins.twig
index 7fa22d01cb..c87114c5a1 100644
--- a/plugins/CorePluginsAdmin/templates/plugins.twig
+++ b/plugins/CorePluginsAdmin/templates/plugins.twig
@@ -21,7 +21,7 @@
 
     <p>{{ 'CorePluginsAdmin_MainDescription'|translate }}</p>
 
-    {{ plugins.tablePlugins(pluginsInfo, token_auth, activateNonce, false) }}
+    {{ plugins.tablePlugins(pluginsInfo, activateNonce, deactivateNonce, uninstallNonce, false) }}
 
 </div>
 {% endblock %}
\ No newline at end of file
diff --git a/plugins/CorePluginsAdmin/templates/themes.twig b/plugins/CorePluginsAdmin/templates/themes.twig
index c88e3d8f5f..599e5fb91c 100644
--- a/plugins/CorePluginsAdmin/templates/themes.twig
+++ b/plugins/CorePluginsAdmin/templates/themes.twig
@@ -21,7 +21,7 @@
 
     <p>{{ 'CorePluginsAdmin_ThemesDescription'|translate }}</p>
 
-    {{ plugins.tablePlugins(pluginsInfo, token_auth, activateNonce, true) }}
+    {{ plugins.tablePlugins(pluginsInfo, activateNonce, deactivateNonce, uninstallNonce, true) }}
 
 </div>
 {% endblock %}