Move all admin controller into a dedicated group

Also handle authentication check in a new middleware for the admin group.

application/front/ShaarliAdminMiddleware.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Shaarli\Front;
+
+use Slim\Http\Request;
+use Slim\Http\Response;
+
+/**
+ * Middleware used for controller requiring to be authenticated.
+ * It extends ShaarliMiddleware, and just make sure that the user is authenticated.
+ * Otherwise, it redirects to the login page.
+ */
+class ShaarliAdminMiddleware extends ShaarliMiddleware
+{
+    public function __invoke(Request $request, Response $response, callable $next): Response
+    {
+        $this->initBasePath($request);
+
+        if (true !== $this->container->loginManager->isLoggedIn()) {
+            $returnUrl = urlencode($this->container->environment['REQUEST_URI']);
+
+            return $response->withRedirect($this->container->basePath . '/login?returnurl=' . $returnUrl);
+        }
+
+        return parent::__invoke($request, $response, $next);
+    }
+}

application/front/ShaarliMiddleware.php

@@ -40,7 +40,7 @@ public function __construct(ShaarliContainer $container)
      */
     public function __invoke(Request $request, Response $response, callable $next): Response
     {
-        $this->container->basePath = rtrim($request->getUri()->getBasePath(), '/');
+        $this->initBasePath($request);
 
         try {
             if (!is_file($this->container->conf->getConfigFileExt())
@@ -125,4 +125,14 @@ protected function checkOpenShaarli(Request $request, Response $response, callab
 
         return true;
     }
+
+    /**
+     * Initialize the URL base path if it hasn't been defined yet.
+     */
+    protected function initBasePath(Request $request): void
+    {
+        if (null === $this->container->basePath) {
+            $this->container->basePath = rtrim($request->getUri()->getBasePath(), '/');
+        }
+    }
 }

application/front/controller/admin/SessionFilterController.php

@@ -17,7 +17,7 @@
 class SessionFilterController extends ShaarliAdminController
 {
     /**
-     * GET /visibility: allows to display only public or only private bookmarks in linklist
+     * GET /admin/visibility: allows to display only public or only private bookmarks in linklist
      */
     public function visibility(Request $request, Response $response, array $args): Response
     {
@@ -46,16 +46,5 @@ public function visibility(Request $request, Response $response, array $args): R
         return $this->redirectFromReferer($request, $response, ['visibility']);
     }
 
-    /**
-     * GET /untagged-only: allows to display only bookmarks without any tag
-     */
-    public function untaggedOnly(Request $request, Response $response): Response
-    {
-        $this->container->sessionManager->setSessionParameter(
-            SessionManager::KEY_UNTAGGED_ONLY,
-            empty($this->container->sessionManager->getSessionParameter(SessionManager::KEY_UNTAGGED_ONLY))
-        );
 
-        return $this->redirectFromReferer($request, $response, ['untaggedonly', 'untagged-only']);
-    }
 }

application/front/controller/admin/ShaarliAdminController.php

@@ -22,15 +22,6 @@
  */
 abstract class ShaarliAdminController extends ShaarliVisitorController
 {
-    public function __construct(ShaarliContainer $container)
-    {
-        parent::__construct($container);
-
-        if (true !== $this->container->loginManager->isLoggedIn()) {
-            throw new UnauthorizedException();
-        }
-    }
-
     /**
      * Any persistent action to the config or data store must check the XSRF token validity.
      */

application/front/controller/visitor/PublicSessionFilterController.php

@@ -30,4 +30,17 @@ public function linksPerPage(Request $request, Response $response): Response
 
         return $this->redirectFromReferer($request, $response, ['linksperpage'], ['nb']);
     }
+
+    /**
+     * GET /untagged-only: allows to display only bookmarks without any tag
+     */
+    public function untaggedOnly(Request $request, Response $response): Response
+    {
+        $this->container->sessionManager->setSessionParameter(
+            SessionManager::KEY_UNTAGGED_ONLY,
+            empty($this->container->sessionManager->getSessionParameter(SessionManager::KEY_UNTAGGED_ONLY))
+        );
+
+        return $this->redirectFromReferer($request, $response, ['untaggedonly', 'untagged-only']);
+    }
 }

application/legacy/LegacyController.php

@@ -67,7 +67,7 @@ protected function login(Request $request, Response $response): Response
     /** Legacy route: ?do=logout */
     protected function logout(Request $request, Response $response): Response
     {
-        return $this->redirect($response, '/logout');
+        return $this->redirect($response, '/admin/logout');
     }
 
     /** Legacy route: ?do=picwall */

index.php

@@ -95,39 +95,41 @@
     $this->get('/add-tag/{newTag}', '\Shaarli\Front\Controller\Visitor\TagController:addTag');
     $this->get('/remove-tag/{tag}', '\Shaarli\Front\Controller\Visitor\TagController:removeTag');
     $this->get('/links-per-page', '\Shaarli\Front\Controller\Visitor\PublicSessionFilterController:linksPerPage');
+    $this->get('/untagged-only', '\Shaarli\Front\Controller\Admin\PublicSessionFilterController:untaggedOnly');
+})->add('\Shaarli\Front\ShaarliMiddleware');
 
-    /* -- LOGGED IN -- */
+$app->group('/admin', function () {
     $this->get('/logout', '\Shaarli\Front\Controller\Admin\LogoutController:index');
-    $this->get('/admin/tools', '\Shaarli\Front\Controller\Admin\ToolsController:index');
+    $this->get('/tools', '\Shaarli\Front\Controller\Admin\ToolsController:index');
-    $this->get('/admin/password', '\Shaarli\Front\Controller\Admin\PasswordController:index');
+    $this->get('/password', '\Shaarli\Front\Controller\Admin\PasswordController:index');
-    $this->post('/admin/password', '\Shaarli\Front\Controller\Admin\PasswordController:change');
+    $this->post('/password', '\Shaarli\Front\Controller\Admin\PasswordController:change');
-    $this->get('/admin/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:index');
+    $this->get('/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:index');
-    $this->post('/admin/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:save');
+    $this->post('/configure', '\Shaarli\Front\Controller\Admin\ConfigureController:save');
-    $this->get('/admin/tags', '\Shaarli\Front\Controller\Admin\ManageTagController:index');
+    $this->get('/tags', '\Shaarli\Front\Controller\Admin\ManageTagController:index');
-    $this->post('/admin/tags', '\Shaarli\Front\Controller\Admin\ManageTagController:save');
+    $this->post('/tags', '\Shaarli\Front\Controller\Admin\ManageTagController:save');
-    $this->get('/admin/add-shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:addShaare');
+    $this->get('/add-shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:addShaare');
-    $this->get('/admin/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayCreateForm');
+    $this->get('/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayCreateForm');
-    $this->get('/admin/shaare/{id:[0-9]+}', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayEditForm');
+    $this->get('/shaare/{id:[0-9]+}', '\Shaarli\Front\Controller\Admin\ManageShaareController:displayEditForm');
-    $this->post('/admin/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:save');
+    $this->post('/shaare', '\Shaarli\Front\Controller\Admin\ManageShaareController:save');
-    $this->get('/admin/shaare/delete', '\Shaarli\Front\Controller\Admin\ManageShaareController:deleteBookmark');
+    $this->get('/shaare/delete', '\Shaarli\Front\Controller\Admin\ManageShaareController:deleteBookmark');
-    $this->get('/admin/shaare/visibility', '\Shaarli\Front\Controller\Admin\ManageShaareController:changeVisibility');
+    $this->get('/shaare/visibility', '\Shaarli\Front\Controller\Admin\ManageShaareController:changeVisibility');
-    $this->get('/admin/shaare/{id:[0-9]+}/pin', '\Shaarli\Front\Controller\Admin\ManageShaareController:pinBookmark');
+    $this->get('/shaare/{id:[0-9]+}/pin', '\Shaarli\Front\Controller\Admin\ManageShaareController:pinBookmark');
     $this->patch(
-        '/admin/shaare/{id:[0-9]+}/update-thumbnail',
+        '/shaare/{id:[0-9]+}/update-thumbnail',
         '\Shaarli\Front\Controller\Admin\ThumbnailsController:ajaxUpdate'
     );
-    $this->get('/admin/export', '\Shaarli\Front\Controller\Admin\ExportController:index');
+    $this->get('/export', '\Shaarli\Front\Controller\Admin\ExportController:index');
-    $this->post('/admin/export', '\Shaarli\Front\Controller\Admin\ExportController:export');
+    $this->post('/export', '\Shaarli\Front\Controller\Admin\ExportController:export');
-    $this->get('/admin/import', '\Shaarli\Front\Controller\Admin\ImportController:index');
+    $this->get('/import', '\Shaarli\Front\Controller\Admin\ImportController:index');
-    $this->post('/admin/import', '\Shaarli\Front\Controller\Admin\ImportController:import');
+    $this->post('/import', '\Shaarli\Front\Controller\Admin\ImportController:import');
-    $this->get('/admin/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:index');
+    $this->get('/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:index');
-    $this->post('/admin/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:save');
+    $this->post('/plugins', '\Shaarli\Front\Controller\Admin\PluginsController:save');
-    $this->get('/admin/token', '\Shaarli\Front\Controller\Admin\TokenController:getToken');
+    $this->get('/token', '\Shaarli\Front\Controller\Admin\TokenController:getToken');
-    $this->get('/admin/thumbnails', '\Shaarli\Front\Controller\Admin\ThumbnailsController:index');
+    $this->get('/thumbnails', '\Shaarli\Front\Controller\Admin\ThumbnailsController:index');
 
     $this->get('/visibility/{visibility}', '\Shaarli\Front\Controller\Admin\SessionFilterController:visibility');
-    $this->get('/untagged-only', '\Shaarli\Front\Controller\Admin\SessionFilterController:untaggedOnly');
+})->add('\Shaarli\Front\ShaarliAdminMiddleware');
-})->add('\Shaarli\Front\ShaarliMiddleware');
+
 
 // REST API routes
 $app->group('/api/v1', function () {

tests/front/ShaarliAdminMiddlewareTest.php

@@ -0,0 +1,100 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front;
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\Config\ConfigManager;
+use Shaarli\Container\ShaarliContainer;
+use Shaarli\Security\LoginManager;
+use Shaarli\Updater\Updater;
+use Slim\Http\Request;
+use Slim\Http\Response;
+use Slim\Http\Uri;
+
+class ShaarliAdminMiddlewareTest extends TestCase
+{
+    protected const TMP_MOCK_FILE = '.tmp';
+
+    /** @var ShaarliContainer */
+    protected $container;
+
+    /** @var ShaarliMiddleware  */
+    protected $middleware;
+
+    public function setUp(): void
+    {
+        $this->container = $this->createMock(ShaarliContainer::class);
+
+        touch(static::TMP_MOCK_FILE);
+
+        $this->container->conf = $this->createMock(ConfigManager::class);
+        $this->container->conf->method('getConfigFileExt')->willReturn(static::TMP_MOCK_FILE);
+
+        $this->container->loginManager = $this->createMock(LoginManager::class);
+        $this->container->updater = $this->createMock(Updater::class);
+
+        $this->container->environment = ['REQUEST_URI' => 'http://shaarli/subfolder/path'];
+
+        $this->middleware = new ShaarliAdminMiddleware($this->container);
+    }
+
+    public function tearDown(): void
+    {
+        unlink(static::TMP_MOCK_FILE);
+    }
+
+    /**
+     * Try to access an admin controller while logged out -> redirected to login page.
+     */
+    public function testMiddlewareWhileLoggedOut(): void
+    {
+        $this->container->loginManager->expects(static::once())->method('isLoggedIn')->willReturn(false);
+
+        $request = $this->createMock(Request::class);
+        $request->method('getUri')->willReturnCallback(function (): Uri {
+            $uri = $this->createMock(Uri::class);
+            $uri->method('getBasePath')->willReturn('/subfolder');
+
+            return $uri;
+        });
+
+        $response = new Response();
+
+        /** @var Response $result */
+        $result = $this->middleware->__invoke($request, $response, function () {});
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(
+            '/subfolder/login?returnurl=' . urlencode('http://shaarli/subfolder/path'),
+            $result->getHeader('location')[0]
+        );
+    }
+
+    /**
+     * Process controller while logged in.
+     */
+    public function testMiddlewareWhileLoggedIn(): void
+    {
+        $this->container->loginManager->method('isLoggedIn')->willReturn(true);
+
+        $request = $this->createMock(Request::class);
+        $request->method('getUri')->willReturnCallback(function (): Uri {
+            $uri = $this->createMock(Uri::class);
+            $uri->method('getBasePath')->willReturn('/subfolder');
+
+            return $uri;
+        });
+
+        $response = new Response();
+        $controller = function (Request $request, Response $response): Response {
+            return $response->withStatus(418); // I'm a tea pot
+        };
+
+        /** @var Response $result */
+        $result = $this->middleware->__invoke($request, $response, $controller);
+
+        static::assertSame(418, $result->getStatusCode());
+    }
+}

tests/front/ShaarliMiddlewareTest.php

@@ -43,7 +43,7 @@ public function setUp(): void
         $this->middleware = new ShaarliMiddleware($this->container);
     }
 
-    public function tearDown()
+    public function tearDown(): void
     {
         unlink(static::TMP_MOCK_FILE);
     }

tests/front/controller/admin/SessionFilterControllerTest.php

@@ -174,55 +174,4 @@ public function testVisibilityLoggedOut(): void
         static::assertSame(302, $result->getStatusCode());
         static::assertSame(['/subfolder/controller/?searchtag=abc'], $result->getHeader('location'));
     }
-
-    /**
-     * Untagged only - valid call
-     */
-    public function testUntaggedOnly(): void
-    {
-        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc'];
-
-        $request = $this->createMock(Request::class);
-        $response = new Response();
-
-        $this->container->sessionManager
-            ->expects(static::once())
-            ->method('setSessionParameter')
-            ->with(SessionManager::KEY_UNTAGGED_ONLY, true)
-        ;
-
-        $result = $this->controller->untaggedOnly($request, $response);
-
-        static::assertInstanceOf(Response::class, $result);
-        static::assertSame(302, $result->getStatusCode());
-        static::assertSame(['/subfolder/controller/?searchtag=abc'], $result->getHeader('location'));
-    }
-
-    /**
-     * Untagged only - toggle off
-     */
-    public function testUntaggedOnlyToggleOff(): void
-    {
-        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc'];
-
-        $request = $this->createMock(Request::class);
-        $response = new Response();
-
-        $this->container->sessionManager
-            ->method('getSessionParameter')
-            ->with(SessionManager::KEY_UNTAGGED_ONLY)
-            ->willReturn(true)
-        ;
-        $this->container->sessionManager
-            ->expects(static::once())
-            ->method('setSessionParameter')
-            ->with(SessionManager::KEY_UNTAGGED_ONLY, false)
-        ;
-
-        $result = $this->controller->untaggedOnly($request, $response);
-
-        static::assertInstanceOf(Response::class, $result);
-        static::assertSame(302, $result->getStatusCode());
-        static::assertSame(['/subfolder/controller/?searchtag=abc'], $result->getHeader('location'));
-    }
 }

tests/front/controller/admin/ShaarliAdminControllerTest.php

@@ -5,9 +5,7 @@
 namespace Shaarli\Front\Controller\Admin;
 
 use PHPUnit\Framework\TestCase;
-use Shaarli\Front\Exception\UnauthorizedException;
 use Shaarli\Front\Exception\WrongTokenException;
-use Shaarli\Security\LoginManager;
 use Shaarli\Security\SessionManager;
 use Slim\Http\Request;
 
@@ -52,19 +50,6 @@ public function saveErrorMessage(string $message): void
         };
     }
 
-    /**
-     * Creating an instance of an admin controller while logged out should raise an exception.
-     */
-    public function testInstantiateWhileLoggedOut(): void
-    {
-        $this->expectException(UnauthorizedException::class);
-
-        $this->container->loginManager = $this->createMock(LoginManager::class);
-        $this->container->loginManager->method('isLoggedIn')->willReturn(false);
-
-        $this->controller = new class($this->container) extends ShaarliAdminController {};
-    }
-
     /**
      * Trigger controller's checkToken with a valid token.
      */

tests/front/controller/visitor/PublicSessionFilterControllerTest.php

@@ -68,4 +68,55 @@ public function testLinksPerPageNotValid(): void
         static::assertSame(302, $result->getStatusCode());
         static::assertSame(['/subfolder/'], $result->getHeader('location'));
     }
+
+    /**
+     * Untagged only - valid call
+     */
+    public function testUntaggedOnly(): void
+    {
+        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc'];
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_UNTAGGED_ONLY, true)
+        ;
+
+        $result = $this->controller->untaggedOnly($request, $response);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/controller/?searchtag=abc'], $result->getHeader('location'));
+    }
+
+    /**
+     * Untagged only - toggle off
+     */
+    public function testUntaggedOnlyToggleOff(): void
+    {
+        $this->container->environment = ['HTTP_REFERER' => 'http://shaarli/subfolder/controller/?searchtag=abc'];
+
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->sessionManager
+            ->method('getSessionParameter')
+            ->with(SessionManager::KEY_UNTAGGED_ONLY)
+            ->willReturn(true)
+        ;
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_UNTAGGED_ONLY, false)
+        ;
+
+        $result = $this->controller->untaggedOnly($request, $response);
+
+        static::assertInstanceOf(Response::class, $result);
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame(['/subfolder/controller/?searchtag=abc'], $result->getHeader('location'));
+    }
 }

tests/legacy/LegacyControllerTest.php

@@ -73,8 +73,8 @@ public function getProcessProvider(): array
             ['addlink', [], '/login', false],
             ['login', [], '/login', true],
             ['login', [], '/login', false],
-            ['logout', [], '/logout', true],
+            ['logout', [], '/admin/logout', true],
-            ['logout', [], '/logout', false],
+            ['logout', [], '/admin/logout', false],
             ['picwall', [], '/picture-wall', false],
             ['picwall', [], '/picture-wall', true],
             ['tagcloud', [], '/tags/cloud', false],

tpl/default/linklist.paging.html

@@ -6,10 +6,10 @@
           {'Filters'|t}
         </span>
         {if="$is_logged_in"}
-        <a href="{$base_path}/visibility/private" aria-label="{'Only display private links'|t}" title="{'Only display private links'|t}"
+        <a href="{$base_path}/admin/visibility/private" aria-label="{'Only display private links'|t}" title="{'Only display private links'|t}"
            class="{if="$visibility==='private'"}filter-on{else}filter-off{/if}"
         ><i class="fa fa-user-secret" aria-hidden="true"></i></a>
-        <a href="{$base_path}/visibility/public" aria-label="{'Only display public links'|t}" title="{'Only display public links'|t}"
+        <a href="{$base_path}/admin/visibility/public" aria-label="{'Only display public links'|t}" title="{'Only display public links'|t}"
            class="{if="$visibility==='public'"}filter-on{else}filter-off{/if}"
         ><i class="fa fa-globe" aria-hidden="true"></i></a>
         {/if}

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="{$base_path}/logout" class="pure-menu-link">{'Logout'|t}</a>
+            <a href="{$base_path}/admin/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="{$base_path}/logout" class="pure-menu-link" aria-label="{'Logout'|t}" title="{'Logout'|t}">
+              <a href="{$base_path}/admin/logout" class="pure-menu-link" aria-label="{'Logout'|t}" title="{'Logout'|t}">
                 <i class="fa fa-sign-out" aria-hidden="true"></i>
               </a>
             </li>

tpl/vintage/linklist.paging.html

@@ -1,7 +1,7 @@
 <div class="paging">
 {if="$is_logged_in"}
     <div class="paging_privatelinks">
-      <a href="{$base_path}/visibility/private">
+      <a href="{$base_path}/admin/isibility/private">
 		{if="$visibility=='private'"}
       <img src="{$asset_path}/img/private_16x16_active.png#" width="16" height="16" title="Click to see all links" alt="Click to see all links">
     {else}

tpl/vintage/page.header.html

@@ -18,7 +18,7 @@
 {else}
 <li><a href="{$titleLink}" class="nomobile">Home</a></li>
     {if="$is_logged_in"}
-    <li><a href="{$base_path}/logout">Logout</a></li>
+    <li><a href="{$base_path}/admin/logout">Logout</a></li>
     <li><a href="{$base_path}/admin/tools">Tools</a></li>
     <li><a href="{$base_path}/admin/add-shaare">Add link</a></li>
     {elseif="$openshaarli"}