From 6c50a6ccceecf54850e62c312ab2397b84d89ab4 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 18 Jan 2020 17:50:11 +0100 Subject: [PATCH] Render login page through Slim controller --- application/container/ContainerBuilder.php | 77 ++++++++ application/container/ShaarliContainer.php | 28 +++ application/front/ShaarliMiddleware.php | 57 ++++++ .../front/controllers/LoginController.php | 46 +++++ .../front/controllers/ShaarliController.php | 31 ++++ .../front/exceptions/LoginBannedException.php | 15 ++ .../front/exceptions/ShaarliException.php | 23 +++ application/render/PageBuilder.php | 17 ++ application/security/SessionManager.php | 6 + assets/default/scss/shaarli.scss | 13 +- composer.json | 4 + index.php | 66 +++---- tests/container/ContainerBuilderTest.php | 49 +++++ tests/front/ShaarliMiddlewareTest.php | 70 +++++++ .../front/controller/LoginControllerTest.php | 173 ++++++++++++++++++ tpl/default/404.html | 2 +- tpl/default/error.html | 22 +++ tpl/default/loginform.html | 60 +++--- tpl/vintage/error.html | 25 +++ tpl/vintage/loginform.html | 44 ++--- 20 files changed, 728 insertions(+), 100 deletions(-) create mode 100644 application/container/ContainerBuilder.php create mode 100644 application/container/ShaarliContainer.php create mode 100644 application/front/ShaarliMiddleware.php create mode 100644 application/front/controllers/LoginController.php create mode 100644 application/front/controllers/ShaarliController.php create mode 100644 application/front/exceptions/LoginBannedException.php create mode 100644 application/front/exceptions/ShaarliException.php create mode 100644 tests/container/ContainerBuilderTest.php create mode 100644 tests/front/ShaarliMiddlewareTest.php create mode 100644 tests/front/controller/LoginControllerTest.php create mode 100644 tpl/default/error.html create mode 100644 tpl/vintage/error.html diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php new file mode 100644 index 00000000..ff29825c --- /dev/null +++ b/application/container/ContainerBuilder.php @@ -0,0 +1,77 @@ +conf = $conf; + $this->session = $session; + $this->login = $login; + } + + public function build(): ShaarliContainer + { + $container = new ShaarliContainer(); + $container['conf'] = $this->conf; + $container['sessionManager'] = $this->session; + $container['loginManager'] = $this->login; + $container['plugins'] = function (ShaarliContainer $container): PluginManager { + return new PluginManager($container->conf); + }; + + $container['history'] = function (ShaarliContainer $container): History { + return new History($container->conf->get('resource.history')); + }; + + $container['bookmarkService'] = function (ShaarliContainer $container): BookmarkServiceInterface { + return new BookmarkFileService( + $container->conf, + $container->history, + $container->loginManager->isLoggedIn() + ); + }; + + $container['pageBuilder'] = function (ShaarliContainer $container): PageBuilder { + return new PageBuilder( + $container->conf, + $container->sessionManager->getSession(), + $container->bookmarkService, + $container->sessionManager->generateToken(), + $container->loginManager->isLoggedIn() + ); + }; + + return $container; + } +} diff --git a/application/container/ShaarliContainer.php b/application/container/ShaarliContainer.php new file mode 100644 index 00000000..f5483d5e --- /dev/null +++ b/application/container/ShaarliContainer.php @@ -0,0 +1,28 @@ +container = $container; + } + + /** + * Middleware execution: + * - execute the controller + * - return the response + * + * In case of error, the error template will be displayed with the exception message. + * + * @param Request $request Slim request + * @param Response $response Slim response + * @param callable $next Next action + * + * @return Response response. + */ + public function __invoke(Request $request, Response $response, callable $next) + { + try { + $response = $next($request, $response); + } catch (ShaarliException $e) { + $this->container->pageBuilder->assign('message', $e->getMessage()); + if ($this->container->conf->get('dev.debug', false)) { + $this->container->pageBuilder->assign( + 'stacktrace', + nl2br(get_class($this) .': '. $e->getTraceAsString()) + ); + } + + $response = $response->withStatus($e->getCode()); + $response = $response->write($this->container->pageBuilder->render('error')); + } + + return $response; + } +} diff --git a/application/front/controllers/LoginController.php b/application/front/controllers/LoginController.php new file mode 100644 index 00000000..47fa3ee3 --- /dev/null +++ b/application/front/controllers/LoginController.php @@ -0,0 +1,46 @@ +ci->loginManager->isLoggedIn() || $this->ci->conf->get('security.open_shaarli', false)) { + return $response->withRedirect('./'); + } + + $userCanLogin = $this->ci->loginManager->canLogin($request->getServerParams()); + if ($userCanLogin !== true) { + throw new LoginBannedException(); + } + + if ($request->getParam('username') !== null) { + $this->assignView('username', escape($request->getParam('username'))); + } + + $this + ->assignView('returnurl', escape($request->getServerParam('HTTP_REFERER'))) + ->assignView('remember_user_default', $this->ci->conf->get('privacy.remember_user_default', true)) + ->assignView('pagetitle', t('Login') .' - '. $this->ci->conf->get('general.title', 'Shaarli')) + ; + + return $response->write($this->ci->pageBuilder->render('loginform')); + } +} diff --git a/application/front/controllers/ShaarliController.php b/application/front/controllers/ShaarliController.php new file mode 100644 index 00000000..2a166c3c --- /dev/null +++ b/application/front/controllers/ShaarliController.php @@ -0,0 +1,31 @@ +ci = $ci; + } + + /** + * Assign variables to RainTPL template through the PageBuilder. + * + * @param mixed $value Value to assign to the template + */ + protected function assignView(string $name, $value): self + { + $this->ci->pageBuilder->assign($name, $value); + + return $this; + } +} diff --git a/application/front/exceptions/LoginBannedException.php b/application/front/exceptions/LoginBannedException.php new file mode 100644 index 00000000..b31a4a14 --- /dev/null +++ b/application/front/exceptions/LoginBannedException.php @@ -0,0 +1,15 @@ +tpl->draw($page); } + /** + * Render a specific page as string (using a template file). + * e.g. $pb->render('picwall'); + * + * @param string $page Template filename (without extension). + * + * @return string Processed template content + */ + public function render(string $page): string + { + if ($this->tpl === false) { + $this->initialize(); + } + + return $this->tpl->draw($page, true); + } + /** * Render a 404 page (uses the template : tpl/404.tpl) * usage: $PAGE->render404('The link was deleted') diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php index b8b8ab8d..994fcbe5 100644 --- a/application/security/SessionManager.php +++ b/application/security/SessionManager.php @@ -196,4 +196,10 @@ class SessionManager } return true; } + + /** @return array Local reference to the global $_SESSION array */ + public function getSession(): array + { + return $this->session; + } } diff --git a/assets/default/scss/shaarli.scss b/assets/default/scss/shaarli.scss index cd5dd9e6..28acb4b4 100644 --- a/assets/default/scss/shaarli.scss +++ b/assets/default/scss/shaarli.scss @@ -1236,8 +1236,19 @@ form { color: $dark-grey; } -.page404-container { +.pageError-container { color: $dark-grey; + + h2 { + margin: 70px 0 25px 0; + } + + pre { + text-align: left; + margin: 0 20%; + padding: 20px 0; + line-height: 0.7em; + } } // EDIT LINK diff --git a/composer.json b/composer.json index ada06a74..6b670fa2 100644 --- a/composer.json +++ b/composer.json @@ -48,9 +48,13 @@ "Shaarli\\Bookmark\\Exception\\": "application/bookmark/exception", "Shaarli\\Config\\": "application/config/", "Shaarli\\Config\\Exception\\": "application/config/exception", + "Shaarli\\Container\\": "application/container", "Shaarli\\Exceptions\\": "application/exceptions", "Shaarli\\Feed\\": "application/feed", "Shaarli\\Formatter\\": "application/formatter", + "Shaarli\\Front\\": "application/front", + "Shaarli\\Front\\Controller\\": "application/front/controllers", + "Shaarli\\Front\\Exception\\": "application/front/exceptions", "Shaarli\\Http\\": "application/http", "Shaarli\\Legacy\\": "application/legacy", "Shaarli\\Netscape\\": "application/netscape", diff --git a/index.php b/index.php index 76ad3696..7da8c22f 100644 --- a/index.php +++ b/index.php @@ -61,29 +61,31 @@ require_once 'application/FileUtils.php'; require_once 'application/TimeZone.php'; require_once 'application/Utils.php'; -use \Shaarli\ApplicationUtils; -use Shaarli\Bookmark\BookmarkServiceInterface; -use \Shaarli\Bookmark\Exception\BookmarkNotFoundException; +use Shaarli\ApplicationUtils; use Shaarli\Bookmark\Bookmark; -use Shaarli\Bookmark\BookmarkFilter; use Shaarli\Bookmark\BookmarkFileService; -use \Shaarli\Config\ConfigManager; -use \Shaarli\Feed\CachedPage; -use \Shaarli\Feed\FeedBuilder; +use Shaarli\Bookmark\BookmarkFilter; +use Shaarli\Bookmark\BookmarkServiceInterface; +use Shaarli\Bookmark\Exception\BookmarkNotFoundException; +use Shaarli\Config\ConfigManager; +use Shaarli\Container\ContainerBuilder; +use Shaarli\Feed\CachedPage; +use Shaarli\Feed\FeedBuilder; use Shaarli\Formatter\BookmarkMarkdownFormatter; use Shaarli\Formatter\FormatterFactory; -use \Shaarli\History; -use \Shaarli\Languages; -use \Shaarli\Netscape\NetscapeBookmarkUtils; -use \Shaarli\Plugin\PluginManager; -use \Shaarli\Render\PageBuilder; -use \Shaarli\Render\ThemeUtils; -use \Shaarli\Router; -use \Shaarli\Security\LoginManager; -use \Shaarli\Security\SessionManager; -use \Shaarli\Thumbnailer; -use \Shaarli\Updater\Updater; -use \Shaarli\Updater\UpdaterUtils; +use Shaarli\History; +use Shaarli\Languages; +use Shaarli\Netscape\NetscapeBookmarkUtils; +use Shaarli\Plugin\PluginManager; +use Shaarli\Render\PageBuilder; +use Shaarli\Render\ThemeUtils; +use Shaarli\Router; +use Shaarli\Security\LoginManager; +use Shaarli\Security\SessionManager; +use Shaarli\Thumbnailer; +use Shaarli\Updater\Updater; +use Shaarli\Updater\UpdaterUtils; +use Slim\App; // Ensure the PHP version is supported try { @@ -594,19 +596,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM // -------- Display login form. if ($targetPage == Router::$PAGE_LOGIN) { - if ($conf->get('security.open_shaarli')) { - header('Location: ?'); - exit; - } // No need to login for open Shaarli - if (isset($_GET['username'])) { - $PAGE->assign('username', escape($_GET['username'])); - } - $PAGE->assign('returnurl', (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']):'')); - // add default state of the 'remember me' checkbox - $PAGE->assign('remember_user_default', $conf->get('privacy.remember_user_default')); - $PAGE->assign('user_can_login', $loginManager->canLogin($_SERVER)); - $PAGE->assign('pagetitle', t('Login') .' - '. $conf->get('general.title', 'Shaarli')); - $PAGE->renderPage('loginform'); + header('Location: ./login'); exit; } // -------- User wants to logout. @@ -1930,11 +1920,9 @@ if (isset($_SERVER['QUERY_STRING']) && startsWith($_SERVER['QUERY_STRING'], 'do= exit; } -$container = new \Slim\Container(); -$container['conf'] = $conf; -$container['plugins'] = $pluginManager; -$container['history'] = $history; -$app = new \Slim\App($container); +$containerBuilder = new ContainerBuilder($conf, $sessionManager, $loginManager); +$container = $containerBuilder->build(); +$app = new App($container); // REST API routes $app->group('/api/v1', function () { @@ -1953,6 +1941,10 @@ $app->group('/api/v1', function () { $this->get('/history', '\Shaarli\Api\Controllers\HistoryController:getHistory')->setName('getHistory'); })->add('\Shaarli\Api\ApiMiddleware'); +$app->group('', function () { + $this->get('/login', '\Shaarli\Front\Controller\LoginController:index')->setName('login'); +})->add('\Shaarli\Front\ShaarliMiddleware'); + $response = $app->run(true); // Hack to make Slim and Shaarli router work together: diff --git a/tests/container/ContainerBuilderTest.php b/tests/container/ContainerBuilderTest.php new file mode 100644 index 00000000..9b97ed6d --- /dev/null +++ b/tests/container/ContainerBuilderTest.php @@ -0,0 +1,49 @@ +conf = new ConfigManager('tests/utils/config/configJson'); + $this->sessionManager = $this->createMock(SessionManager::class); + $this->loginManager = $this->createMock(LoginManager::class); + + $this->containerBuilder = new ContainerBuilder($this->conf, $this->sessionManager, $this->loginManager); + } + + public function testBuildContainer(): void + { + $container = $this->containerBuilder->build(); + + static::assertInstanceOf(ConfigManager::class, $container->conf); + static::assertInstanceOf(SessionManager::class, $container->sessionManager); + static::assertInstanceOf(LoginManager::class, $container->loginManager); + static::assertInstanceOf(History::class, $container->history); + static::assertInstanceOf(BookmarkServiceInterface::class, $container->bookmarkService); + static::assertInstanceOf(PageBuilder::class, $container->pageBuilder); + } +} diff --git a/tests/front/ShaarliMiddlewareTest.php b/tests/front/ShaarliMiddlewareTest.php new file mode 100644 index 00000000..80974f37 --- /dev/null +++ b/tests/front/ShaarliMiddlewareTest.php @@ -0,0 +1,70 @@ +container = $this->createMock(ShaarliContainer::class); + $this->middleware = new ShaarliMiddleware($this->container); + } + + public function testMiddlewareExecution(): void + { + $request = $this->createMock(Request::class); + $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::assertInstanceOf(Response::class, $result); + static::assertSame(418, $result->getStatusCode()); + } + + public function testMiddlewareExecutionWithException(): void + { + $request = $this->createMock(Request::class); + $response = new Response(); + $controller = function (): void { + $exception = new LoginBannedException(); + + throw new $exception; + }; + + $pageBuilder = $this->createMock(PageBuilder::class); + $pageBuilder->method('render')->willReturnCallback(function (string $message): string { + return $message; + }); + $this->container->pageBuilder = $pageBuilder; + + $conf = $this->createMock(ConfigManager::class); + $this->container->conf = $conf; + + /** @var Response $result */ + $result = $this->middleware->__invoke($request, $response, $controller); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(401, $result->getStatusCode()); + static::assertContains('error', (string) $result->getBody()); + } +} diff --git a/tests/front/controller/LoginControllerTest.php b/tests/front/controller/LoginControllerTest.php new file mode 100644 index 00000000..ddcfe154 --- /dev/null +++ b/tests/front/controller/LoginControllerTest.php @@ -0,0 +1,173 @@ +container = $this->createMock(ShaarliContainer::class); + $this->controller = new LoginController($this->container); + } + + public function testValidControllerInvoke(): void + { + $this->createValidContainerMockSet(); + + $request = $this->createMock(Request::class); + $request->expects(static::once())->method('getServerParam')->willReturn('> referer'); + $response = new Response(); + + $assignedVariables = []; + $this->container->pageBuilder + ->expects(static::exactly(3)) + ->method('assign') + ->willReturnCallback(function ($key, $value) use (&$assignedVariables) { + $assignedVariables[$key] = $value; + + return $this; + }) + ; + + $result = $this->controller->index($request, $response); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(200, $result->getStatusCode()); + static::assertSame('loginform', (string) $result->getBody()); + + static::assertSame('> referer', $assignedVariables['returnurl']); + static::assertSame(true, $assignedVariables['remember_user_default']); + static::assertSame('Login - Shaarli', $assignedVariables['pagetitle']); + } + + public function testValidControllerInvokeWithUserName(): void + { + $this->createValidContainerMockSet(); + + $request = $this->createMock(Request::class); + $request->expects(static::once())->method('getServerParam')->willReturn('> referer'); + $request->expects(static::exactly(2))->method('getParam')->willReturn('myUser>'); + $response = new Response(); + + $assignedVariables = []; + $this->container->pageBuilder + ->expects(static::exactly(4)) + ->method('assign') + ->willReturnCallback(function ($key, $value) use (&$assignedVariables) { + $assignedVariables[$key] = $value; + + return $this; + }) + ; + + $result = $this->controller->index($request, $response); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(200, $result->getStatusCode()); + static::assertSame('loginform', (string) $result->getBody()); + + static::assertSame('myUser>', $assignedVariables['username']); + static::assertSame('> referer', $assignedVariables['returnurl']); + static::assertSame(true, $assignedVariables['remember_user_default']); + static::assertSame('Login - Shaarli', $assignedVariables['pagetitle']); + } + + public function testLoginControllerWhileLoggedIn(): void + { + $request = $this->createMock(Request::class); + $response = new Response(); + + $loginManager = $this->createMock(LoginManager::class); + $loginManager->expects(static::once())->method('isLoggedIn')->willReturn(true); + $this->container->loginManager = $loginManager; + + $result = $this->controller->index($request, $response); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./'], $result->getHeader('Location')); + } + + public function testLoginControllerOpenShaarli(): void + { + $this->createValidContainerMockSet(); + + $request = $this->createMock(Request::class); + $response = new Response(); + + $conf = $this->createMock(ConfigManager::class); + $conf->method('get')->willReturnCallback(function (string $parameter, $default) { + if ($parameter === 'security.open_shaarli') { + return true; + } + return $default; + }); + $this->container->conf = $conf; + + $result = $this->controller->index($request, $response); + + static::assertInstanceOf(Response::class, $result); + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['./'], $result->getHeader('Location')); + } + + public function testLoginControllerWhileBanned(): void + { + $this->createValidContainerMockSet(); + + $request = $this->createMock(Request::class); + $response = new Response(); + + $loginManager = $this->createMock(LoginManager::class); + $loginManager->method('isLoggedIn')->willReturn(false); + $loginManager->method('canLogin')->willReturn(false); + $this->container->loginManager = $loginManager; + + $this->expectException(LoginBannedException::class); + + $this->controller->index($request, $response); + } + + protected function createValidContainerMockSet(): void + { + // User logged out + $loginManager = $this->createMock(LoginManager::class); + $loginManager->method('isLoggedIn')->willReturn(false); + $loginManager->method('canLogin')->willReturn(true); + $this->container->loginManager = $loginManager; + + // Config + $conf = $this->createMock(ConfigManager::class); + $conf->method('get')->willReturnCallback(function (string $parameter, $default) { + return $default; + }); + $this->container->conf = $conf; + + // PageBuilder + $pageBuilder = $this->createMock(PageBuilder::class); + $pageBuilder + ->method('render') + ->willReturnCallback(function (string $template): string { + return $template; + }) + ; + $this->container->pageBuilder = $pageBuilder; + } +} diff --git a/tpl/default/404.html b/tpl/default/404.html index 472566a6..1bc46c63 100644 --- a/tpl/default/404.html +++ b/tpl/default/404.html @@ -6,7 +6,7 @@