From 8e47af2b3620c920116ec056173277c039163ec1 Mon Sep 17 00:00:00 2001
From: ArthurHoaro <arthur@hoa.ro>
Date: Thu, 23 Jan 2020 21:52:03 +0100
Subject: [PATCH] Process logout through Slim controller

---
 application/container/ContainerBuilder.php    | 20 ++++++-
 application/container/ShaarliContainer.php    |  3 +
 .../front/controllers/LogoutController.php    | 31 ++++++++++
 index.php                                     |  7 +--
 tests/container/ContainerBuilderTest.php      | 10 +++-
 .../front/controller/LogoutControllerTest.php | 60 +++++++++++++++++++
 tpl/default/page.header.html                  |  4 +-
 tpl/vintage/page.header.html                  |  6 +-
 8 files changed, 128 insertions(+), 13 deletions(-)
 create mode 100644 application/front/controllers/LogoutController.php
 create mode 100644 tests/front/controller/LogoutControllerTest.php

diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php
index 99c12334..c5c4a2c3 100644
--- a/application/container/ContainerBuilder.php
+++ b/application/container/ContainerBuilder.php
@@ -11,6 +11,7 @@ use Shaarli\Formatter\FormatterFactory;
 use Shaarli\History;
 use Shaarli\Plugin\PluginManager;
 use Shaarli\Render\PageBuilder;
+use Shaarli\Render\PageCacheManager;
 use Shaarli\Security\LoginManager;
 use Shaarli\Security\SessionManager;
 
@@ -34,19 +35,30 @@ class ContainerBuilder
     /** @var LoginManager */
     protected $login;
 
-    public function __construct(ConfigManager $conf, SessionManager $session, LoginManager $login)
-    {
+    /** @var string */
+    protected $webPath;
+
+    public function __construct(
+        ConfigManager $conf,
+        SessionManager $session,
+        LoginManager $login,
+        string $webPath
+    ) {
         $this->conf = $conf;
         $this->session = $session;
         $this->login = $login;
+        $this->webPath = $webPath;
     }
 
     public function build(): ShaarliContainer
     {
         $container = new ShaarliContainer();
+
         $container['conf'] = $this->conf;
         $container['sessionManager'] = $this->session;
         $container['loginManager'] = $this->login;
+        $container['webPath'] = $this->webPath;
+
         $container['plugins'] = function (ShaarliContainer $container): PluginManager {
             return new PluginManager($container->conf);
         };
@@ -81,6 +93,10 @@ class ContainerBuilder
             return new FormatterFactory($container->conf, $container->loginManager->isLoggedIn());
         };
 
+        $container['pageCacheManager'] = function (ShaarliContainer $container): PageCacheManager {
+            return new PageCacheManager($container->conf->get('resource.page_cache'));
+        };
+
         return $container;
     }
 }
diff --git a/application/container/ShaarliContainer.php b/application/container/ShaarliContainer.php
index fdf2f77f..af62e574 100644
--- a/application/container/ShaarliContainer.php
+++ b/application/container/ShaarliContainer.php
@@ -10,6 +10,7 @@ use Shaarli\Formatter\FormatterFactory;
 use Shaarli\History;
 use Shaarli\Plugin\PluginManager;
 use Shaarli\Render\PageBuilder;
+use Shaarli\Render\PageCacheManager;
 use Shaarli\Security\LoginManager;
 use Shaarli\Security\SessionManager;
 use Slim\Container;
@@ -20,11 +21,13 @@ use Slim\Container;
  * @property ConfigManager            $conf
  * @property SessionManager           $sessionManager
  * @property LoginManager             $loginManager
+ * @property string                   $webPath
  * @property History                  $history
  * @property BookmarkServiceInterface $bookmarkService
  * @property PageBuilder              $pageBuilder
  * @property PluginManager            $pluginManager
  * @property FormatterFactory         $formatterFactory
+ * @property PageCacheManager         $pageCacheManager
  */
 class ShaarliContainer extends Container
 {
diff --git a/application/front/controllers/LogoutController.php b/application/front/controllers/LogoutController.php
new file mode 100644
index 00000000..aba078c3
--- /dev/null
+++ b/application/front/controllers/LogoutController.php
@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller;
+
+use Shaarli\Security\LoginManager;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+/**
+ * Class LogoutController
+ *
+ * Slim controller used to logout the user.
+ * It invalidates page cache and terminate the user session. Then it redirects to the homepage.
+ *
+ * @package Front\Controller
+ */
+class LogoutController extends ShaarliController
+{
+    public function index(Request $request, Response $response): Response
+    {
+        $this->container->pageCacheManager->invalidateCaches();
+        $this->container->sessionManager->logout();
+
+        // TODO: switch to a simple Cookie manager allowing to check the session, and create mocks.
+        setcookie(LoginManager::$STAY_SIGNED_IN_COOKIE, 'false', 0, $this->container->webPath);
+
+        return $response->withRedirect('./');
+    }
+}
diff --git a/index.php b/index.php
index 73d9e022..57e13d4e 100644
--- a/index.php
+++ b/index.php
@@ -604,10 +604,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
     }
     // -------- User wants to logout.
     if (isset($_SERVER['QUERY_STRING']) && startsWith($_SERVER['QUERY_STRING'], 'do=logout')) {
-        invalidateCaches($conf->get('resource.page_cache'));
-        $sessionManager->logout();
-        setcookie(LoginManager::$STAY_SIGNED_IN_COOKIE, 'false', 0, WEB_PATH);
-        header('Location: ?');
+        header('Location: ./logout');
         exit;
     }
 
@@ -1894,7 +1891,7 @@ if (isset($_SERVER['QUERY_STRING']) && startsWith($_SERVER['QUERY_STRING'], 'do=
     exit;
 }
 
-$containerBuilder = new ContainerBuilder($conf, $sessionManager, $loginManager);
+$containerBuilder = new ContainerBuilder($conf, $sessionManager, $loginManager, WEB_PATH);
 $container = $containerBuilder->build();
 $app = new App($container);
 
diff --git a/tests/container/ContainerBuilderTest.php b/tests/container/ContainerBuilderTest.php
index cc2eb37f..65647249 100644
--- a/tests/container/ContainerBuilderTest.php
+++ b/tests/container/ContainerBuilderTest.php
@@ -10,6 +10,7 @@ use Shaarli\Config\ConfigManager;
 use Shaarli\Formatter\FormatterFactory;
 use Shaarli\History;
 use Shaarli\Render\PageBuilder;
+use Shaarli\Render\PageCacheManager;
 use Shaarli\Security\LoginManager;
 use Shaarli\Security\SessionManager;
 
@@ -35,7 +36,12 @@ class ContainerBuilderTest extends TestCase
         $this->loginManager = $this->createMock(LoginManager::class);
         $this->loginManager->method('isLoggedIn')->willReturn(true);
 
-        $this->containerBuilder = new ContainerBuilder($this->conf, $this->sessionManager, $this->loginManager);
+        $this->containerBuilder = new ContainerBuilder(
+            $this->conf,
+            $this->sessionManager,
+            $this->loginManager,
+            'UT web path'
+        );
     }
 
     public function testBuildContainer(): void
@@ -45,9 +51,11 @@ class ContainerBuilderTest extends TestCase
         static::assertInstanceOf(ConfigManager::class, $container->conf);
         static::assertInstanceOf(SessionManager::class, $container->sessionManager);
         static::assertInstanceOf(LoginManager::class, $container->loginManager);
+        static::assertSame('UT web path', $container->webPath);
         static::assertInstanceOf(History::class, $container->history);
         static::assertInstanceOf(BookmarkServiceInterface::class, $container->bookmarkService);
         static::assertInstanceOf(PageBuilder::class, $container->pageBuilder);
         static::assertInstanceOf(FormatterFactory::class, $container->formatterFactory);
+        static::assertInstanceOf(PageCacheManager::class, $container->pageCacheManager);
     }
 }
diff --git a/tests/front/controller/LogoutControllerTest.php b/tests/front/controller/LogoutControllerTest.php
new file mode 100644
index 00000000..d9ca1c25
--- /dev/null
+++ b/tests/front/controller/LogoutControllerTest.php
@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Controller;
+
+/** Override PHP builtin setcookie function in the local namespace to mock it... more or less */
+if (!function_exists('Shaarli\Front\Controller\setcookie')) {
+    function setcookie(string $name, string $value): void {
+        $_COOKIE[$name] = $value;
+    }
+}
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\Container\ShaarliContainer;
+use Shaarli\Render\PageCacheManager;
+use Shaarli\Security\LoginManager;
+use Shaarli\Security\SessionManager;
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+class LogoutControllerTest extends TestCase
+{
+    /** @var ShaarliContainer */
+    protected $container;
+
+    /** @var LogoutController */
+    protected $controller;
+
+    public function setUp(): void
+    {
+        $this->container = $this->createMock(ShaarliContainer::class);
+        $this->controller = new LogoutController($this->container);
+
+        setcookie(LoginManager::$STAY_SIGNED_IN_COOKIE, $cookie = 'hi there');
+    }
+
+    public function testValidControllerInvoke(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $pageCacheManager = $this->createMock(PageCacheManager::class);
+        $pageCacheManager->expects(static::once())->method('invalidateCaches');
+        $this->container->pageCacheManager = $pageCacheManager;
+
+        $sessionManager = $this->createMock(SessionManager::class);
+        $sessionManager->expects(static::once())->method('logout');
+        $this->container->sessionManager = $sessionManager;
+
+        static::assertSame('hi there', $_COOKIE[LoginManager::$STAY_SIGNED_IN_COOKIE]);
+
+        $result = $this->controller->index($request, $response);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertContains('./', $result->getHeader('Location'));
+        static::assertSame('false', $_COOKIE[LoginManager::$STAY_SIGNED_IN_COOKIE]);
+    }
+}
diff --git a/tpl/default/page.header.html b/tpl/default/page.header.html
index 116265a5..2086aeb0 100644
--- a/tpl/default/page.header.html
+++ b/tpl/default/page.header.html
@@ -56,7 +56,7 @@
         </li>
         {if="$is_logged_in"}
           <li class="pure-menu-item pure-u-lg-0 shaarli-menu-mobile" id="shaarli-menu-mobile-logout">
-            <a href="./?do=logout" class="pure-menu-link">{'Logout'|t}</a>
+            <a href="/logout" class="pure-menu-link">{'Logout'|t}</a>
           </li>
         {else}
           <li class="pure-menu-item pure-u-lg-0 shaarli-menu-mobile" id="shaarli-menu-mobile-login">
@@ -88,7 +88,7 @@
             </li>
           {else}
             <li class="pure-menu-item" id="shaarli-menu-desktop-logout">
-              <a href="./?do=logout" class="pure-menu-link" aria-label="{'Logout'|t}" title="{'Logout'|t}">
+              <a href="/logout" class="pure-menu-link" aria-label="{'Logout'|t}" title="{'Logout'|t}">
                 <i class="fa fa-sign-out" aria-hidden="true"></i>
               </a>
             </li>
diff --git a/tpl/vintage/page.header.html b/tpl/vintage/page.header.html
index ce938421..8b9db353 100644
--- a/tpl/vintage/page.header.html
+++ b/tpl/vintage/page.header.html
@@ -18,9 +18,9 @@
 {else}
 <li><a href="{$titleLink}" class="nomobile">Home</a></li>
     {if="$is_logged_in"}
-    <li><a href="./?do=logout">Logout</a></li>
-    <li><a href="./?do=tools">Tools</a></li>
-    <li><a href="./?do=addlink">Add link</a></li>
+    <li><a href="/logout">Logout</a></li>
+    <li><a href="?do=tools">Tools</a></li>
+    <li><a href="?do=addlink">Add link</a></li>
     {elseif="$openshaarli"}
     <li><a href="./?do=tools">Tools</a></li>
     <li><a href="./?do=addlink">Add link</a></li>
-- 
2.41.0