Process plugins administration page through Slim controllers

13 files changed, 312 insertions, 62 deletions

diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php
index a4fd6a0c..ba91fe8b 100644
--- a/application/container/ContainerBuilder.php
+++ b/application/container/ContainerBuilder.php
@@ -88,7 +88,12 @@ class ContainerBuilder
         };
 
         $container['pluginManager'] = function (ShaarliContainer $container): PluginManager {
-            return new PluginManager($container->conf);
+            $pluginManager = new PluginManager($container->conf);
+
+            // FIXME! Configuration is already injected
+            $pluginManager->load($container->conf->get('general.enabled_plugins'));
+
+            return $pluginManager;
         };
 
         $container['formatterFactory'] = function (ShaarliContainer $container): FormatterFactory {
diff --git a/application/front/controller/admin/PluginsController.php b/application/front/controller/admin/PluginsController.php
new file mode 100644
index 00000000..d5ec91f0
--- /dev/null
+++ b/application/front/controller/admin/PluginsController.php
@@ -0,0 +1,98 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Admin;
+
+use Exception;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+/**
+ * Class PluginsController
+ *
+ * Slim controller used to handle Shaarli plugins configuration page (display + save new config).
+ */
+class PluginsController extends ShaarliAdminController
+{
+    /**
+     * GET /admin/plugins - Displays the configuration page
+     */
+    public function index(Request $request, Response $response): Response
+    {
+        $pluginMeta = $this->container->pluginManager->getPluginsMeta();
+
+        // Split plugins into 2 arrays: ordered enabled plugins and disabled.
+        $enabledPlugins = array_filter($pluginMeta, function ($v) {
+            return ($v['order'] ?? false) !== false;
+        });
+        $enabledPlugins = load_plugin_parameter_values($enabledPlugins, $this->container->conf->get('plugins', []));
+        uasort(
+            $enabledPlugins,
+            function ($a, $b) {
+                return $a['order'] - $b['order'];
+            }
+        );
+        $disabledPlugins = array_filter($pluginMeta, function ($v) {
+            return ($v['order'] ?? false) === false;
+        });
+
+        $this->assignView('enabledPlugins', $enabledPlugins);
+        $this->assignView('disabledPlugins', $disabledPlugins);
+        $this->assignView(
+            'pagetitle',
+            t('Plugin Administration') .' - '. $this->container->conf->get('general.title', 'Shaarli')
+        );
+
+        return $response->write($this->render('pluginsadmin'));
+    }
+
+    /**
+     * POST /admin/plugins - Update Shaarli's configuration
+     */
+    public function save(Request $request, Response $response): Response
+    {
+        $this->checkToken($request);
+
+        try {
+            $parameters = $request->getParams() ?? [];
+
+            $this->executeHooks($parameters);
+
+            if (isset($parameters['parameters_form'])) {
+                unset($parameters['parameters_form']);
+                foreach ($parameters as $param => $value) {
+                    $this->container->conf->set('plugins.'. $param, escape($value));
+                }
+            } else {
+                $this->container->conf->set('general.enabled_plugins', save_plugin_config($parameters));
+            }
+
+            $this->container->conf->write($this->container->loginManager->isLoggedIn());
+            $this->container->history->updateSettings();
+
+            $this->saveSuccessMessage(t('Setting successfully saved.'));
+        } catch (Exception $e) {
+            $this->saveErrorMessage(
+                t('ERROR while saving plugin configuration: ') . PHP_EOL . $e->getMessage()
+            );
+        }
+
+        return $this->redirect($response, '/admin/plugins');
+    }
+
+    /**
+     * @param mixed[] $data Variables passed to the template engine
+     *
+     * @return mixed[] Template data after active plugins render_picwall hook execution.
+     */
+    protected function executeHooks(array $data): array
+    {
+        $this->container->pluginManager->executeHooks(
+            'save_plugin_parameters',
+            $data
+        );
+
+        return $data;
+    }
+}
diff --git a/application/plugin/PluginManager.php b/application/plugin/PluginManager.php
index f7b24a8e..591a9180 100644
--- a/application/plugin/PluginManager.php
+++ b/application/plugin/PluginManager.php
@@ -16,7 +16,7 @@ class PluginManager
      *
      * @var array $authorizedPlugins
      */
-    private $authorizedPlugins;
+    private $authorizedPlugins = [];
 
     /**
      * List of loaded plugins.
diff --git a/doc/md/Translations.md b/doc/md/Translations.md
index df86f4d4..dd42bf30 100644
--- a/doc/md/Translations.md
+++ b/doc/md/Translations.md
@@ -43,7 +43,7 @@ http://<replace_domain>/admin/export
 http://<replace_domain>/admin/import
 http://<replace_domain>/login
 http://<replace_domain>/picture-wall
-http://<replace_domain>/?do=pluginadmin
+http://<replace_domain>/admin/plugins
 http://<replace_domain>/tags/cloud
 http://<replace_domain>/tags/list
 ```
diff --git a/index.php b/index.php
index 47fef3ed..1571df60 100644
--- a/index.php
+++ b/index.php
@@ -584,60 +584,14 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
 
     // Plugin administration page
     if ($targetPage == Router::$PAGE_PLUGINSADMIN) {
-        $pluginMeta = $pluginManager->getPluginsMeta();
-
-        // Split plugins into 2 arrays: ordered enabled plugins and disabled.
-        $enabledPlugins = array_filter($pluginMeta, function ($v) {
-            return $v['order'] !== false;
-        });
-        // Load parameters.
-        $enabledPlugins = load_plugin_parameter_values($enabledPlugins, $conf->get('plugins', array()));
-        uasort(
-            $enabledPlugins,
-            function ($a, $b) {
-                return $a['order'] - $b['order'];
-            }
-        );
-        $disabledPlugins = array_filter($pluginMeta, function ($v) {
-            return $v['order'] === false;
-        });
-
-        $PAGE->assign('enabledPlugins', $enabledPlugins);
-        $PAGE->assign('disabledPlugins', $disabledPlugins);
-        $PAGE->assign('pagetitle', t('Plugin administration') .' - '. $conf->get('general.title', 'Shaarli'));
-        $PAGE->renderPage('pluginsadmin');
+        header('Location: ./admin/plugins');
         exit;
     }
 
     // Plugin administration form action
     if ($targetPage == Router::$PAGE_SAVE_PLUGINSADMIN) {
-        try {
-            if (isset($_POST['parameters_form'])) {
-                $pluginManager->executeHooks('save_plugin_parameters', $_POST);
-                unset($_POST['parameters_form']);
-                foreach ($_POST as $param => $value) {
-                    $conf->set('plugins.'. $param, escape($value));
-                }
-            } else {
-                $conf->set('general.enabled_plugins', save_plugin_config($_POST));
-            }
-            $conf->write($loginManager->isLoggedIn());
-            $history->updateSettings();
-        } catch (Exception $e) {
-            error_log(
-                'ERROR while saving plugin configuration:.' . PHP_EOL .
-                $e->getMessage()
-            );
-
-            // TODO: do not handle exceptions/errors in JS.
-            echo '<script>alert("'
-                . $e->getMessage()
-                .'");document.location=\'./?do='
-                . Router::$PAGE_PLUGINSADMIN
-                .'\';</script>';
-            exit;
-        }
-        header('Location: ./?do='. Router::$PAGE_PLUGINSADMIN);
+        // This route is no longer supported in legacy mode
+        header('Location: ./admin/plugins');
         exit;
     }
 
@@ -1022,6 +976,8 @@ $app->group('', function () {
     $this->post('/admin/export', '\Shaarli\Front\Controller\Admin\ExportController:export');
     $this->get('/admin/import', '\Shaarli\Front\Controller\Admin\ImportController:index');
     $this->post('/admin/import', '\Shaarli\Front\Controller\Admin\ImportController:import');
+    $this->get('/admin/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:index');
+    $this->post('/admin/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:save');
 
     $this->get('/links-per-page', '\Shaarli\Front\Controller\Admin\SessionFilterController:linksPerPage');
     $this->get('/visibility/{visibility}', '\Shaarli\Front\Controller\Admin\SessionFilterController:visibility');
diff --git a/plugins/wallabag/README.md b/plugins/wallabag/README.md
index ea21a519..c53a04d9 100644
--- a/plugins/wallabag/README.md
+++ b/plugins/wallabag/README.md
@@ -21,7 +21,7 @@ The directory structure should look like:
 
 To enable the plugin, you can either:
 
-  * enable it in the plugins administration page (`?do=pluginadmin`). 
+  * enable it in the plugins administration page (`/admin/plugins`).
   * add `wallabag` to your list of enabled plugins in `data/config.json.php` (`general.enabled_plugins` section).
 
 ### Configuration
diff --git a/tests/front/controller/admin/PluginsControllerTest.php b/tests/front/controller/admin/PluginsControllerTest.php
new file mode 100644
index 00000000..700a0df2
--- /dev/null
+++ b/tests/front/controller/admin/PluginsControllerTest.php
@@ -0,0 +1,190 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller\Admin;
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\Config\ConfigManager;
+use Shaarli\Front\Exception\WrongTokenException;
+use Shaarli\Security\SessionManager;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+class PluginsControllerTest extends TestCase
+{
+    use FrontAdminControllerMockHelper;
+
+    /** @var PluginsController */
+    protected $controller;
+
+    public function setUp(): void
+    {
+        $this->createContainer();
+
+        $this->controller = new PluginsController($this->container);
+    }
+
+    /**
+     * Test displaying plugins admin page
+     */
+    public function testIndex(): void
+    {
+        $assignedVariables = [];
+        $this->assignTemplateVars($assignedVariables);
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $data = [
+            'plugin1' => ['order' => 2, 'other' => 'field'],
+            'plugin2' => ['order' => 1],
+            'plugin3' => ['order' => false, 'abc' => 'def'],
+            'plugin4' => [],
+        ];
+
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('getPluginsMeta')
+            ->willReturn($data);
+
+        $result = $this->controller->index($request, $response);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame('pluginsadmin', (string) $result->getBody());
+
+        static::assertSame('Plugin Administration - Shaarli', $assignedVariables['pagetitle']);
+        static::assertSame(
+            ['plugin2' => $data['plugin2'], 'plugin1' => $data['plugin1']],
+            $assignedVariables['enabledPlugins']
+        );
+        static::assertSame(
+            ['plugin3' => $data['plugin3'], 'plugin4' => $data['plugin4']],
+            $assignedVariables['disabledPlugins']
+        );
+    }
+
+    /**
+     * Test save plugins admin page
+     */
+    public function testSaveEnabledPlugins(): void
+    {
+        $parameters = [
+            'plugin1' => 'on',
+            'order_plugin1' => '2',
+            'plugin2' => 'on',
+        ];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParams')
+            ->willReturnCallback(function () use ($parameters): array {
+                return $parameters;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('executeHooks')
+            ->with('save_plugin_parameters', $parameters)
+        ;
+        $this->container->conf
+            ->expects(static::atLeastOnce())
+            ->method('set')
+            ->with('general.enabled_plugins', ['plugin1', 'plugin2'])
+        ;
+
+        $result = $this->controller->save($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/admin/plugins'], $result->getHeader('location'));
+    }
+
+    /**
+     * Test save plugin parameters
+     */
+    public function testSavePluginParameters(): void
+    {
+        $parameters = [
+            'parameters_form' => true,
+            'parameter1' => 'blip',
+            'parameter2' => 'blop',
+        ];
+
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParams')
+            ->willReturnCallback(function () use ($parameters): array {
+                return $parameters;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->pluginManager
+            ->expects(static::once())
+            ->method('executeHooks')
+            ->with('save_plugin_parameters', $parameters)
+        ;
+        $this->container->conf
+            ->expects(static::atLeastOnce())
+            ->method('set')
+            ->withConsecutive(['plugins.parameter1', 'blip'], ['plugins.parameter2', 'blop'])
+        ;
+
+        $result = $this->controller->save($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/admin/plugins'], $result->getHeader('location'));
+    }
+
+    /**
+     * Test save plugin parameters - error encountered
+     */
+    public function testSaveWithError(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->conf = $this->createMock(ConfigManager::class);
+        $this->container->conf
+            ->expects(static::atLeastOnce())
+            ->method('write')
+            ->willThrowException(new \Exception($message = 'error message'))
+        ;
+
+        $this->container->sessionManager = $this->createMock(SessionManager::class);
+        $this->container->sessionManager->method('checkToken')->willReturn(true);
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(
+                SessionManager::KEY_ERROR_MESSAGES,
+                ['ERROR while saving plugin configuration: ' . PHP_EOL . $message]
+            )
+        ;
+
+        $result = $this->controller->save($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/admin/plugins'], $result->getHeader('location'));
+    }
+
+    /**
+     * Test save plugin parameters - wrong token
+     */
+    public function testSaveWrongToken(): void
+    {
+        $this->container->sessionManager = $this->createMock(SessionManager::class);
+        $this->container->sessionManager->method('checkToken')->willReturn(false);
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->expectException(WrongTokenException::class);
+
+        $this->controller->save($request, $response);
+    }
+}
diff --git a/tpl/default/includes.html b/tpl/default/includes.html
index 102314d5..227f9b52 100644
--- a/tpl/default/includes.html
+++ b/tpl/default/includes.html
@@ -12,10 +12,10 @@
   <link type="text/css" rel="stylesheet" href="{$asset_path}/css/markdown.min.css?v={$version_hash}#" />
 {/if}
 {loop="$plugins_includes.css_files"}
-  <link type="text/css" rel="stylesheet" href="{$value}?v={$version_hash}#"/>
+  <link type="text/css" rel="stylesheet" href="{$base_path}/{$value}?v={$version_hash}#"/>
 {/loop}
 {if="is_file('data/user.css')"}
-  <link type="text/css" rel="stylesheet" href="data/user.css#" />
+  <link type="text/css" rel="stylesheet" href="{$base_path}/data/user.css#" />
 {/if}
 <link rel="search" type="application/opensearchdescription+xml" href="{$base_path}/open-search#"
       title="Shaarli search - {$shaarlititle}" />
diff --git a/tpl/default/page.footer.html b/tpl/default/page.footer.html
index d72917de..51bdb2f0 100644
--- a/tpl/default/page.footer.html
+++ b/tpl/default/page.footer.html
@@ -25,7 +25,7 @@
 {/loop}
 
 {loop="$plugins_footer.js_files"}
-        <script src="{$value}#"></script>
+        <script src="{$base_path}/{$value}#"></script>
 {/loop}
 
 <div id="js-translations" class="hidden">
diff --git a/tpl/default/pluginsadmin.html b/tpl/default/pluginsadmin.html
index 1536c311..05d13556 100644
--- a/tpl/default/pluginsadmin.html
+++ b/tpl/default/pluginsadmin.html
@@ -16,7 +16,7 @@
   <div class="clear"></div>
 </noscript>
 
-<form method="POST" action="{$base_path}/?do=save_pluginadmin" name="pluginform" id="pluginform" class="pluginform-container">
+<form method="POST" action="{$base_path}/admin/plugins" name="pluginform" id="pluginform" class="pluginform-container">
   <div class="pure-g">
     <div class="pure-u-lg-1-8 pure-u-1-24"></div>
     <div class="pure-u-lg-3-4 pure-u-22-24 page-form page-form-complete">
@@ -127,7 +127,7 @@
   <input type="hidden" name="token" value="{$token}">
 </form>
 
-<form action="{$base_path}/?do=save_pluginadmin" method="POST">
+<form action="{$base_path}/admin/plugins" method="POST">
   <div class="pure-g">
     <div class="pure-u-lg-1-8 pure-u-1-24"></div>
     <div class="pure-u-lg-3-4 pure-u-22-24 page-form page-form-light">
@@ -173,6 +173,7 @@
       </section>
     </div>
   </div>
+  <input type="hidden" name="token" value="{$token}">
 </form>
 
 {include="page.footer"}
diff --git a/tpl/default/tools.html b/tpl/default/tools.html
index 045defc9..31f33a09 100644
--- a/tpl/default/tools.html
+++ b/tpl/default/tools.html
@@ -16,7 +16,7 @@
       </a>
     </div>
     <div class="tools-item">
-      <a href="{$base_path}/?do=pluginadmin" title="{'Enable, disable and configure plugins'|t}">
+      <a href="{$base_path}/admin/plugins" title="{'Enable, disable and configure plugins'|t}">
         <span class="pure-button pure-u-lg-2-3 pure-u-3-4">{'Plugin administration'|t}</span>
       </a>
     </div>
diff --git a/tpl/vintage/pluginsadmin.html b/tpl/vintage/pluginsadmin.html
index c94dc211..a04c77c2 100644
--- a/tpl/vintage/pluginsadmin.html
+++ b/tpl/vintage/pluginsadmin.html
@@ -16,7 +16,7 @@
 </noscript>
 
 <div id="pluginsadmin">
-  <form action="{$base_path}/?do=save_pluginadmin" method="POST">
+  <form action="{$base_path}/admin/plugins" method="POST">
     <section id="enabled_plugins">
       <h1>Enabled Plugins</h1>
 
@@ -88,7 +88,7 @@
     </section>
   </form>
 
-  <form action="{$base_path}/?do=save_pluginadmin" method="POST">
+  <form action="{$base_path}/admin/plugins" method="POST">
     <section id="plugin_parameters">
       <h1>Enabled Plugin Parameters</h1>
 
diff --git a/tpl/vintage/tools.html b/tpl/vintage/tools.html
index 95f89d8c..1125bba9 100644
--- a/tpl/vintage/tools.html
+++ b/tpl/vintage/tools.html
@@ -7,7 +7,7 @@
         <div id="toolsdiv">
                 <a href="{$base_path}/admin/configure"><b>Configure your Shaarli</b><span>: Change Title, timezone...</span></a>
                 <br><br>
-                <a href="{$base_path}/?do=pluginadmin"><b>Plugin administration</b><span>: Enable, disable and configure plugins.</span></a>
+                <a href="{$base_path}/admin/plugins"><b>Plugin administration</b><span>: Enable, disable and configure plugins.</span></a>
     <br><br>
                 {if="!$openshaarli"}<a href="{$base_path}/admin/password"><b>Change password</b><span>: Change your password.</span></a>
     <br><br>{/if}