Process login through Slim controller

application/front/ShaarliMiddleware.php

@@ -62,7 +62,9 @@ public function __invoke(Request $request, Response $response, callable $next):
 
             return $response->write($this->container->pageBuilder->render('error'));
         } catch (UnauthorizedException $e) {
-            return $response->withRedirect($this->container->basePath . '/login');
+            $returnUrl = urlencode($this->container->environment['REQUEST_URI']);
+
+            return $response->withRedirect($this->container->basePath . '/login?returnurl=' . $returnUrl);
         } catch (\Throwable $e) {
             // Unknown error encountered
             $this->container->pageBuilder->reset();

application/front/controller/visitor/LoginController.php

@@ -4,8 +4,12 @@
 
 namespace Shaarli\Front\Controller\Visitor;
 
+use Shaarli\Front\Exception\CantLoginException;
 use Shaarli\Front\Exception\LoginBannedException;
+use Shaarli\Front\Exception\WrongTokenException;
 use Shaarli\Render\TemplatePage;
+use Shaarli\Security\CookieManager;
+use Shaarli\Security\SessionManager;
 use Slim\Http\Request;
 use Slim\Http\Response;
 
@@ -19,29 +23,132 @@
  */
 class LoginController extends ShaarliVisitorController
 {
+    /**
+     * GET /login - Display the login page.
+     */
     public function index(Request $request, Response $response): Response
     {
-        if ($this->container->loginManager->isLoggedIn()
-            || $this->container->conf->get('security.open_shaarli', false)
-        ) {
+        try {
+            $this->checkLoginState();
+        } catch (CantLoginException $e) {
             return $this->redirect($response, '/');
         }
 
-        $userCanLogin = $this->container->loginManager->canLogin($request->getServerParams());
-        if ($userCanLogin !== true) {
-            throw new LoginBannedException();
+        if ($request->getParam('login') !== null) {
+            $this->assignView('username', escape($request->getParam('login')));
         }
 
-        if ($request->getParam('username') !== null) {
-            $this->assignView('username', escape($request->getParam('username')));
-        }
+        $returnUrl = $request->getParam('returnurl') ?? $this->container->environment['HTTP_REFERER'] ?? null;
 
         $this
-            ->assignView('returnurl', escape($request->getServerParam('HTTP_REFERER')))
+            ->assignView('returnurl', escape($returnUrl))
             ->assignView('remember_user_default', $this->container->conf->get('privacy.remember_user_default', true))
             ->assignView('pagetitle', t('Login') .' - '. $this->container->conf->get('general.title', 'Shaarli'))
         ;
 
         return $response->write($this->render(TemplatePage::LOGIN));
     }
+
+    /**
+     * POST /login - Process login
+     */
+    public function login(Request $request, Response $response): Response
+    {
+        if (!$this->container->sessionManager->checkToken($request->getParam('token'))) {
+            throw new WrongTokenException();
+        }
+
+        try {
+            $this->checkLoginState();
+        } catch (CantLoginException $e) {
+            return $this->redirect($response, '/');
+        }
+
+        if (!$this->container->loginManager->checkCredentials(
+                $this->container->environment['REMOTE_ADDR'],
+                client_ip_id($this->container->environment),
+                $request->getParam('login'),
+                $request->getParam('password')
+            )
+        ) {
+            $this->container->loginManager->handleFailedLogin($this->container->environment);
+
+            $this->container->sessionManager->setSessionParameter(
+                SessionManager::KEY_ERROR_MESSAGES,
+                [t('Wrong login/password.')]
+            );
+
+            // Call controller directly instead of unnecessary redirection
+            return $this->index($request, $response);
+        }
+
+        $this->container->loginManager->handleSuccessfulLogin($this->container->environment);
+
+        $cookiePath = $this->container->basePath . '/';
+        $expirationTime = $this->saveLongLastingSession($request, $cookiePath);
+        $this->renewUserSession($cookiePath, $expirationTime);
+
+        // Force referer from given return URL
+        $this->container->environment['HTTP_REFERER'] = $request->getParam('returnurl');
+
+        return $this->redirectFromReferer($request, $response, ['login']);
+    }
+
+    /**
+     * Make sure that the user is allowed to login and/or displaying the login page:
+     *   - not already logged in
+     *   - not open shaarli
+     *   - not banned
+     */
+    protected function checkLoginState(): bool
+    {
+        if ($this->container->loginManager->isLoggedIn()
+            || $this->container->conf->get('security.open_shaarli', false)
+        ) {
+            throw new CantLoginException();
+        }
+
+        if (true !== $this->container->loginManager->canLogin($this->container->environment)) {
+            throw new LoginBannedException();
+        }
+
+        return true;
+    }
+
+    /**
+     * @return int Session duration in seconds
+     */
+    protected function saveLongLastingSession(Request $request, string $cookiePath): int
+    {
+        if (empty($request->getParam('longlastingsession'))) {
+            // Standard session expiration (=when browser closes)
+            $expirationTime = 0;
+        } else {
+            // Keep the session cookie even after the browser closes
+            $this->container->sessionManager->setStaySignedIn(true);
+            $expirationTime = $this->container->sessionManager->extendSession();
+        }
+
+        $this->container->cookieManager->setCookieParameter(
+            CookieManager::STAY_SIGNED_IN,
+            $this->container->loginManager->getStaySignedInToken(),
+            $expirationTime,
+            $cookiePath
+        );
+
+        return $expirationTime;
+    }
+
+    protected function renewUserSession(string $cookiePath, int $expirationTime): void
+    {
+        // Send cookie with the new expiration date to the browser
+        $this->container->sessionManager->destroy();
+        $this->container->sessionManager->cookieParameters(
+            $expirationTime,
+            $cookiePath,
+            $this->container->environment['SERVER_NAME']
+        );
+        $this->container->sessionManager->start();
+        $this->container->sessionManager->regenerateId(true);
+    }
 }

application/front/exceptions/CantLoginException.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shaarli\Front\Exception;
+
+class CantLoginException extends \Exception
+{
+
+}

application/security/SessionManager.php

@@ -259,4 +259,34 @@ public function getSavePath(): string
     {
         return $this->savePath;
     }
+
+    /*
+     * Next public functions wrapping native PHP session API.
+     */
+
+    public function destroy(): bool
+    {
+        $this->session = [];
+
+        return session_destroy();
+    }
+
+    public function start(): bool
+    {
+        if (session_status() === PHP_SESSION_ACTIVE) {
+            $this->destroy();
+        }
+
+        return session_start();
+    }
+
+    public function cookieParameters(int $lifeTime, string $path, string $domain): bool
+    {
+        return session_set_cookie_params($lifeTime, $path, $domain);
+    }
+
+    public function regenerateId(bool $deleteOldSession = false): bool
+    {
+        return session_regenerate_id($deleteOldSession);
+    }
 }

index.php

@@ -159,89 +159,6 @@
 
 $loginManager->checkLoginState($clientIpId);
 
-// ------------------------------------------------------------------------------------------
-// Process login form: Check if login/password is correct.
-if (isset($_POST['login'])) {
-    if (! $loginManager->canLogin($_SERVER)) {
-        die(t('I said: NO. You are banned for the moment. Go away.'));
-    }
-    if (isset($_POST['password'])
-        && $sessionManager->checkToken($_POST['token'])
-        && $loginManager->checkCredentials($_SERVER['REMOTE_ADDR'], $clientIpId, $_POST['login'], $_POST['password'])
-    ) {
-        $loginManager->handleSuccessfulLogin($_SERVER);
-
-        $cookiedir = '';
-        if (dirname($_SERVER['SCRIPT_NAME']) != '/') {
-            // Note: Never forget the trailing slash on the cookie path!
-            $cookiedir = dirname($_SERVER["SCRIPT_NAME"]) . '/';
-        }
-
-        if (!empty($_POST['longlastingsession'])) {
-            // Keep the session cookie even after the browser closes
-            $sessionManager->setStaySignedIn(true);
-            $expirationTime = $sessionManager->extendSession();
-
-            setcookie(
-                CookieManager::STAY_SIGNED_IN,
-                $loginManager->getStaySignedInToken(),
-                $expirationTime,
-                WEB_PATH
-            );
-        } else {
-            // Standard session expiration (=when browser closes)
-            $expirationTime = 0;
-        }
-
-        // Send cookie with the new expiration date to the browser
-        session_destroy();
-        session_set_cookie_params($expirationTime, $cookiedir, $_SERVER['SERVER_NAME']);
-        session_start();
-        session_regenerate_id(true);
-
-        // Optional redirect after login:
-        if (isset($_GET['post'])) {
-            $uri = './?post='. urlencode($_GET['post']);
-            foreach (array('description', 'source', 'title', 'tags') as $param) {
-                if (!empty($_GET[$param])) {
-                    $uri .= '&'.$param.'='.urlencode($_GET[$param]);
-                }
-            }
-            header('Location: '. $uri);
-            exit;
-        }
-
-        if (isset($_GET['edit_link'])) {
-            header('Location: ./?edit_link='. escape($_GET['edit_link']));
-            exit;
-        }
-
-        if (isset($_POST['returnurl'])) {
-            // Prevent loops over login screen.
-            if (strpos($_POST['returnurl'], '/login') === false) {
-                header('Location: '. generateLocation($_POST['returnurl'], $_SERVER['HTTP_HOST']));
-                exit;
-            }
-        }
-        header('Location: ./?');
-        exit;
-    } else {
-        $loginManager->handleFailedLogin($_SERVER);
-        $redir = '?username='. urlencode($_POST['login']);
-        if (isset($_GET['post'])) {
-            $redir .= '&post=' . urlencode($_GET['post']);
-            foreach (array('description', 'source', 'title', 'tags') as $param) {
-                if (!empty($_GET[$param])) {
-                    $redir .= '&' . $param . '=' . urlencode($_GET[$param]);
-                }
-            }
-        }
-        // Redirect to login screen.
-        echo '<script>alert("'. t("Wrong login/password.") .'");document.location=\'./login'.$redir.'\';</script>';
-        exit;
-    }
-}
-
 // ------------------------------------------------------------------------------------------
 // Token management for XSRF protection
 // Token should be used in any form which acts on data (create,update,delete,import...).
@@ -283,6 +200,7 @@
     $this->get('/', '\Shaarli\Front\Controller\Visitor\BookmarkListController:index');
     $this->get('/shaare/{hash}', '\Shaarli\Front\Controller\Visitor\BookmarkListController:permalink');
     $this->get('/login', '\Shaarli\Front\Controller\Visitor\LoginController:index')->setName('login');
+    $this->post('/login', '\Shaarli\Front\Controller\Visitor\LoginController:login')->setName('processLogin');
     $this->get('/picture-wall', '\Shaarli\Front\Controller\Visitor\PictureWallController:index');
     $this->get('/tags/cloud', '\Shaarli\Front\Controller\Visitor\TagCloudController:cloud');
     $this->get('/tags/list', '\Shaarli\Front\Controller\Visitor\TagCloudController:list');

tests/front/ShaarliMiddlewareTest.php

@@ -38,6 +38,8 @@ public function setUp(): void
 
         $this->container->loginManager = $this->createMock(LoginManager::class);
 
+        $this->container->environment = ['REQUEST_URI' => 'http://shaarli/subfolder/path'];
+
         $this->middleware = new ShaarliMiddleware($this->container);
     }
 
@@ -127,7 +129,10 @@ public function testMiddlewareExecutionWithUnauthorizedException(): void
         $result = $this->middleware->__invoke($request, $response, $controller);
 
         static::assertSame(302, $result->getStatusCode());
-        static::assertSame('/subfolder/login', $result->getHeader('location')[0]);
+        static::assertSame(
+            '/subfolder/login?returnurl=' . urlencode('http://shaarli/subfolder/path'),
+            $result->getHeader('location')[0]
+        );
     }
 
     /**

tests/front/controller/visitor/FrontControllerMockHelper.php

@@ -80,6 +80,7 @@ protected function createContainer(): void
             'SERVER_NAME' => 'shaarli',
             'SERVER_PORT' => '80',
             'REQUEST_URI' => '/daily-rss',
+            'REMOTE_ADDR' => '1.2.3.4',
         ];
 
         $this->container->basePath = '/subfolder';

tests/front/controller/visitor/LoginControllerTest.php

@@ -7,6 +7,10 @@
 use PHPUnit\Framework\TestCase;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Front\Exception\LoginBannedException;
+use Shaarli\Front\Exception\WrongTokenException;
+use Shaarli\Render\TemplatePage;
+use Shaarli\Security\CookieManager;
+use Shaarli\Security\SessionManager;
 use Slim\Http\Request;
 use Slim\Http\Response;
 
@@ -21,13 +25,25 @@ public function setUp(): void
     {
         $this->createContainer();
 
+        $this->container->cookieManager = $this->createMock(CookieManager::class);
+        $this->container->sessionManager->method('checkToken')->willReturn(true);
+
         $this->controller = new LoginController($this->container);
     }
 
+    /**
+     * Test displaying login form with valid parameters.
+     */
     public function testValidControllerInvoke(): void
     {
         $request = $this->createMock(Request::class);
-        $request->expects(static::once())->method('getServerParam')->willReturn('> referer');
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) {
+                return 'returnurl' === $key ? '> referer' : null;
+            })
+        ;
         $response = new Response();
 
         $assignedVariables = [];
@@ -46,18 +62,32 @@ public function testValidControllerInvoke(): void
 
         static::assertInstanceOf(Response::class, $result);
         static::assertSame(200, $result->getStatusCode());
-        static::assertSame('loginform', (string) $result->getBody());
+        static::assertSame(TemplatePage::LOGIN, (string) $result->getBody());
 
         static::assertSame('> referer', $assignedVariables['returnurl']);
         static::assertSame(true, $assignedVariables['remember_user_default']);
         static::assertSame('Login - Shaarli', $assignedVariables['pagetitle']);
     }
 
+    /**
+     * Test displaying login form with username defined in the request.
+     */
     public function testValidControllerInvokeWithUserName(): void
     {
+        $this->container->environment = ['HTTP_REFERER' => '> referer'];
+
         $request = $this->createMock(Request::class);
-        $request->expects(static::once())->method('getServerParam')->willReturn('> referer');
-        $request->expects(static::exactly(2))->method('getParam')->willReturn('myUser>');
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key, $default) {
+                if ('login' === $key) {
+                    return 'myUser>';
+                }
+
+                return $default;
+            })
+        ;
         $response = new Response();
 
         $assignedVariables = [];
@@ -84,6 +114,9 @@ public function testValidControllerInvokeWithUserName(): void
         static::assertSame('Login - Shaarli', $assignedVariables['pagetitle']);
     }
 
+    /**
+     * Test displaying login page while being logged in.
+     */
     public function testLoginControllerWhileLoggedIn(): void
     {
         $request = $this->createMock(Request::class);
@@ -98,6 +131,9 @@ public function testLoginControllerWhileLoggedIn(): void
         static::assertSame(['/subfolder/'], $result->getHeader('Location'));
     }
 
+    /**
+     * Test displaying login page with open shaarli configured: redirect to homepage.
+     */
     public function testLoginControllerOpenShaarli(): void
     {
         $request = $this->createMock(Request::class);
@@ -119,6 +155,9 @@ public function testLoginControllerOpenShaarli(): void
         static::assertSame(['/subfolder/'], $result->getHeader('Location'));
     }
 
+    /**
+     * Test displaying login page while being banned.
+     */
     public function testLoginControllerWhileBanned(): void
     {
         $request = $this->createMock(Request::class);
@@ -131,4 +170,235 @@ public function testLoginControllerWhileBanned(): void
 
         $this->controller->index($request, $response);
     }
+
+    /**
+     * Test processing login with valid parameters.
+     */
+    public function testProcessLoginWithValidParameters(): void
+    {
+        $parameters = [
+            'login' => 'bob',
+            'password' => 'pass',
+        ];
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters) {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->loginManager->method('canLogin')->willReturn(true);
+        $this->container->loginManager->expects(static::once())->method('handleSuccessfulLogin');
+        $this->container->loginManager
+            ->expects(static::once())
+            ->method('checkCredentials')
+            ->with('1.2.3.4', '1.2.3.4', 'bob', 'pass')
+            ->willReturn(true)
+        ;
+        $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8)));
+
+        $this->container->sessionManager->expects(static::never())->method('extendSession');
+        $this->container->sessionManager->expects(static::once())->method('destroy');
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('cookieParameters')
+            ->with(0, '/subfolder/', 'shaarli')
+        ;
+        $this->container->sessionManager->expects(static::once())->method('start');
+        $this->container->sessionManager->expects(static::once())->method('regenerateId')->with(true);
+
+        $result = $this->controller->login($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/', $result->getHeader('location')[0]);
+    }
+
+    /**
+     * Test processing login with return URL.
+     */
+    public function testProcessLoginWithReturnUrl(): void
+    {
+        $parameters = [
+            'returnurl' => 'http://shaarli/subfolder/admin/shaare',
+        ];
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters) {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->loginManager->method('canLogin')->willReturn(true);
+        $this->container->loginManager->expects(static::once())->method('handleSuccessfulLogin');
+        $this->container->loginManager->expects(static::once())->method('checkCredentials')->willReturn(true);
+        $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8)));
+
+        $result = $this->controller->login($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/admin/shaare', $result->getHeader('location')[0]);
+    }
+
+    /**
+     * Test processing login with remember me session enabled.
+     */
+    public function testProcessLoginLongLastingSession(): void
+    {
+        $parameters = [
+            'longlastingsession' => true,
+        ];
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters) {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->loginManager->method('canLogin')->willReturn(true);
+        $this->container->loginManager->expects(static::once())->method('handleSuccessfulLogin');
+        $this->container->loginManager->expects(static::once())->method('checkCredentials')->willReturn(true);
+        $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8)));
+
+        $this->container->sessionManager->expects(static::once())->method('destroy');
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('cookieParameters')
+            ->with(42, '/subfolder/', 'shaarli')
+        ;
+        $this->container->sessionManager->expects(static::once())->method('start');
+        $this->container->sessionManager->expects(static::once())->method('regenerateId')->with(true);
+        $this->container->sessionManager->expects(static::once())->method('extendSession')->willReturn(42);
+
+        $this->container->cookieManager = $this->createMock(CookieManager::class);
+        $this->container->cookieManager
+            ->expects(static::once())
+            ->method('setCookieParameter')
+            ->willReturnCallback(function (string $name): CookieManager {
+                static::assertSame(CookieManager::STAY_SIGNED_IN, $name);
+
+                return $this->container->cookieManager;
+            })
+        ;
+
+        $result = $this->controller->login($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/', $result->getHeader('location')[0]);
+    }
+
+    /**
+     * Test processing login with invalid credentials
+     */
+    public function testProcessLoginWrongCredentials(): void
+    {
+        $parameters = [
+            'returnurl' => 'http://shaarli/subfolder/admin/shaare',
+        ];
+        $request = $this->createMock(Request::class);
+        $request
+            ->expects(static::atLeastOnce())
+            ->method('getParam')
+            ->willReturnCallback(function (string $key) use ($parameters) {
+                return $parameters[$key] ?? null;
+            })
+        ;
+        $response = new Response();
+
+        $this->container->loginManager->method('canLogin')->willReturn(true);
+        $this->container->loginManager->expects(static::once())->method('handleFailedLogin');
+        $this->container->loginManager->expects(static::once())->method('checkCredentials')->willReturn(false);
+
+        $this->container->sessionManager
+            ->expects(static::once())
+            ->method('setSessionParameter')
+            ->with(SessionManager::KEY_ERROR_MESSAGES, ['Wrong login/password.'])
+        ;
+
+        $result = $this->controller->login($request, $response);
+
+        static::assertSame(200, $result->getStatusCode());
+        static::assertSame(TemplatePage::LOGIN, (string) $result->getBody());
+    }
+
+    /**
+     * Test processing login with wrong token
+     */
+    public function testProcessLoginWrongToken(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->sessionManager = $this->createMock(SessionManager::class);
+        $this->container->sessionManager->method('checkToken')->willReturn(false);
+
+        $this->expectException(WrongTokenException::class);
+
+        $this->controller->login($request, $response);
+    }
+
+    /**
+     * Test processing login with wrong token
+     */
+    public function testProcessLoginAlreadyLoggedIn(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->loginManager->method('isLoggedIn')->willReturn(true);
+        $this->container->loginManager->expects(static::never())->method('handleSuccessfulLogin');
+        $this->container->loginManager->expects(static::never())->method('handleFailedLogin');
+
+        $result = $this->controller->login($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/', $result->getHeader('location')[0]);
+    }
+
+    /**
+     * Test processing login with wrong token
+     */
+    public function testProcessLoginInOpenShaarli(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->conf = $this->createMock(ConfigManager::class);
+        $this->container->conf->method('get')->willReturnCallback(function (string $key, $value) {
+            return 'security.open_shaarli' === $key ? true : $value;
+        });
+
+        $this->container->loginManager->expects(static::never())->method('handleSuccessfulLogin');
+        $this->container->loginManager->expects(static::never())->method('handleFailedLogin');
+
+        $result = $this->controller->login($request, $response);
+
+        static::assertSame(302, $result->getStatusCode());
+        static::assertSame('/subfolder/', $result->getHeader('location')[0]);
+    }
+
+    /**
+     * Test processing login while being banned
+     */
+    public function testProcessLoginWhileBanned(): void
+    {
+        $request = $this->createMock(Request::class);
+        $response = new Response();
+
+        $this->container->loginManager->method('canLogin')->willReturn(false);
+        $this->container->loginManager->expects(static::never())->method('handleSuccessfulLogin');
+        $this->container->loginManager->expects(static::never())->method('handleFailedLogin');
+
+        $this->expectException(LoginBannedException::class);
+
+        $this->controller->login($request, $response);
+    }
 }