From 23c0fae36fb8159bcf8b95bae98555201146457e Mon Sep 17 00:00:00 2001 From: Igor Scheller Date: Mon, 3 Sep 2018 15:33:13 +0100 Subject: [PATCH] Added csrf middleware --- config/app.php | 1 + includes/pages/admin_user.php | 3 + includes/sys_form.php | 13 +- resources/assets/js/vendor.js | 4 + resources/views/errors/419.twig | 7 + resources/views/layouts/app.twig | 1 + src/Http/Response.php | 12 +- src/Http/SessionServiceProvider.php | 7 + src/Middleware/VerifyCsrfToken.php | 90 ++++++++++++ src/Renderer/Twig/Extensions/Csrf.php | 48 +++++++ src/Renderer/TwigServiceProvider.php | 4 +- .../Unit/Http/SessionServiceProviderTest.php | 21 ++- tests/Unit/Middleware/VerifyCsrfTokenTest.php | 128 ++++++++++++++++++ .../Renderer/Twig/Extensions/CsrfTest.php | 63 +++++++++ 14 files changed, 395 insertions(+), 7 deletions(-) create mode 100644 resources/views/errors/419.twig create mode 100644 src/Middleware/VerifyCsrfToken.php create mode 100644 src/Renderer/Twig/Extensions/Csrf.php create mode 100644 tests/Unit/Middleware/VerifyCsrfTokenTest.php create mode 100644 tests/Unit/Renderer/Twig/Extensions/CsrfTest.php diff --git a/config/app.php b/config/app.php index 214acb1c..77b1e874 100644 --- a/config/app.php +++ b/config/app.php @@ -40,6 +40,7 @@ return [ // The application code \Engelsystem\Middleware\ErrorHandler::class, + \Engelsystem\Middleware\VerifyCsrfToken::class, \Engelsystem\Middleware\RouteDispatcher::class, // Handle request diff --git a/includes/pages/admin_user.php b/includes/pages/admin_user.php index 958563a0..3894e724 100644 --- a/includes/pages/admin_user.php +++ b/includes/pages/admin_user.php @@ -44,6 +44,7 @@ function admin_user() $html .= '
' . "\n"; + $html .= form_csrf(); $html .= '' . "\n"; $html .= '' . "\n"; $html .= '
' . "\n"; @@ -105,6 +106,7 @@ function admin_user() $html .= 'Hier kannst Du das Passwort dieses Engels neu setzen:' . "\n"; + $html .= form_csrf(); $html .= '' . "\n"; $html .= ' ' . "\n"; $html .= ' ' . "\n"; @@ -135,6 +137,7 @@ function admin_user() $html .= 'Hier kannst Du die Benutzergruppen des Engels festlegen:' . "\n"; + $html .= form_csrf(); $html .= '
Passwort' . '
Wiederholung' . '
'; $groups = DB::select(' diff --git a/includes/sys_form.php b/includes/sys_form.php index a1b78b70..07a61dbb 100644 --- a/includes/sys_form.php +++ b/includes/sys_form.php @@ -407,7 +407,18 @@ function form_element($label, $input, $for = '') */ function form($elements, $action = '') { - return '' . join($elements) . ''; + return '' + . form_csrf() + . join($elements) + . ''; +} + +/** + * @return string + */ +function form_csrf() +{ + return form_hidden('_token', session()->get('_token')); } /** diff --git a/resources/assets/js/vendor.js b/resources/assets/js/vendor.js index dd6b66b3..f9cddad6 100644 --- a/resources/assets/js/vendor.js +++ b/resources/assets/js/vendor.js @@ -14,3 +14,7 @@ require('./moment-countdown'); $(function () { moment.locale($('html').attr('lang')); }); + +$.ajaxSetup({ + headers: {'X-CSRF-TOKEN': $('meta[name="csrf-token"]').attr('content')} +}); diff --git a/resources/views/errors/419.twig b/resources/views/errors/419.twig new file mode 100644 index 00000000..dcfec022 --- /dev/null +++ b/resources/views/errors/419.twig @@ -0,0 +1,7 @@ +{% extends "errors/default.twig" %} + +{% block title %}{{ __("Authentication expired") }}{% endblock %} + +{% block content %} +
{{ __("The provided CSRF token is invalid or has expired") }}
+{% endblock %} diff --git a/resources/views/layouts/app.twig b/resources/views/layouts/app.twig index fcbcc665..dc02e3ed 100644 --- a/resources/views/layouts/app.twig +++ b/resources/views/layouts/app.twig @@ -7,6 +7,7 @@ + diff --git a/src/Http/Response.php b/src/Http/Response.php index 4edf644a..58cd7662 100644 --- a/src/Http/Response.php +++ b/src/Http/Response.php @@ -96,7 +96,7 @@ class Response extends SymfonyResponse implements ResponseInterface /** * Return an instance with the rendered content. * - * THis method retains the immutability of the message and returns + * This method retains the immutability of the message and returns * an instance with the updated status and headers * * @param string $view @@ -111,6 +111,14 @@ class Response extends SymfonyResponse implements ResponseInterface throw new \InvalidArgumentException('Renderer not defined'); } - return $this->create($this->view->render($view, $data), $status, $headers); + $new = clone $this; + $new->setContent($this->view->render($view, $data)); + $new->setStatusCode($status, ($status == $this->getStatusCode() ? $this->statusText : null)); + + foreach ($headers as $key => $values) { + $new = $new->withAddedHeader($key, $values); + } + + return $new; } } diff --git a/src/Http/SessionServiceProvider.php b/src/Http/SessionServiceProvider.php index c2e09624..4d779aa6 100644 --- a/src/Http/SessionServiceProvider.php +++ b/src/Http/SessionServiceProvider.php @@ -5,7 +5,9 @@ namespace Engelsystem\Http; use Engelsystem\Config\Config; use Engelsystem\Container\ServiceProvider; use Engelsystem\Http\SessionHandlers\DatabaseHandler; +use Illuminate\Support\Str; use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage; use Symfony\Component\HttpFoundation\Session\Storage\SessionStorageInterface; @@ -21,6 +23,11 @@ class SessionServiceProvider extends ServiceProvider $session = $this->app->make(Session::class); $this->app->instance(Session::class, $session); $this->app->instance('session', $session); + $this->app->bind(SessionInterface::class, Session::class); + + if (!$session->has('_token')) { + $session->set('_token', Str::random(42)); + } /** @var Request $request */ $request = $this->app->get('request'); diff --git a/src/Middleware/VerifyCsrfToken.php b/src/Middleware/VerifyCsrfToken.php new file mode 100644 index 00000000..cc0c1fbc --- /dev/null +++ b/src/Middleware/VerifyCsrfToken.php @@ -0,0 +1,90 @@ +session = $session; + } + + /** + * Verify csrf tokens + * + * @param ServerRequestInterface $request + * @param RequestHandlerInterface $handler + * @return ResponseInterface + */ + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + if ( + $this->isReading($request) + || $this->tokensMatch($request) + ) { + return $handler->handle($request); + } + + return $this->notAuthorizedResponse(); + } + + /** + * @param ServerRequestInterface $request + * @return bool + */ + protected function isReading(ServerRequestInterface $request): bool + { + return in_array( + $request->getMethod(), + ['GET', 'HEAD', 'OPTIONS'] + ); + } + + /** + * @param ServerRequestInterface $request + * @return bool + */ + protected function tokensMatch(ServerRequestInterface $request): bool + { + $token = null; + $body = $request->getParsedBody(); + $header = $request->getHeader('X-CSRF-TOKEN'); + + if (is_array($body) && isset($body['_token'])) { + $token = $body['_token']; + } + + if (!empty($header)) { + $header = array_shift($header); + } + + $token = $token ?: $header; + $sessionToken = $this->session->get('_token'); + + return is_string($token) + && is_string($sessionToken) + && hash_equals($sessionToken, $token); + } + + /** + * @return ResponseInterface + * @codeCoverageIgnore + */ + protected function notAuthorizedResponse(): ResponseInterface + { + // The 419 code is used as "Page Expired" to differentiate from a 401 (not authorized) + return response()->withStatus(419, 'Authentication Token Mismatch'); + } +} diff --git a/src/Renderer/Twig/Extensions/Csrf.php b/src/Renderer/Twig/Extensions/Csrf.php new file mode 100644 index 00000000..9f77df80 --- /dev/null +++ b/src/Renderer/Twig/Extensions/Csrf.php @@ -0,0 +1,48 @@ +session = $session; + } + + /** + * @return TwigFunction[] + */ + public function getFunctions() + { + return [ + new TwigFunction('csrf', [$this, 'getCsrfField'], ['is_safe' => ['html']]), + new TwigFunction('csrf_token', [$this, 'getCsrfToken']), + ]; + } + + /** + * @return string + */ + public function getCsrfField() + { + return sprintf('', $this->getCsrfToken()); + } + + /** + * @return string + */ + public function getCsrfToken() + { + return $this->session->get('_token'); + } +} diff --git a/src/Renderer/TwigServiceProvider.php b/src/Renderer/TwigServiceProvider.php index 49a0eb90..57ebe9e5 100644 --- a/src/Renderer/TwigServiceProvider.php +++ b/src/Renderer/TwigServiceProvider.php @@ -4,9 +4,10 @@ namespace Engelsystem\Renderer; use Engelsystem\Config\Config as EngelsystemConfig; use Engelsystem\Container\ServiceProvider; -use Engelsystem\Renderer\Twig\Extensions\Authentication; use Engelsystem\Renderer\Twig\Extensions\Assets; +use Engelsystem\Renderer\Twig\Extensions\Authentication; use Engelsystem\Renderer\Twig\Extensions\Config; +use Engelsystem\Renderer\Twig\Extensions\Csrf; use Engelsystem\Renderer\Twig\Extensions\Globals; use Engelsystem\Renderer\Twig\Extensions\Legacy; use Engelsystem\Renderer\Twig\Extensions\Session; @@ -23,6 +24,7 @@ class TwigServiceProvider extends ServiceProvider 'assets' => Assets::class, 'authentication' => Authentication::class, 'config' => Config::class, + 'csrf' => Csrf::class, 'globals' => Globals::class, 'session' => Session::class, 'legacy' => Legacy::class, diff --git a/tests/Unit/Http/SessionServiceProviderTest.php b/tests/Unit/Http/SessionServiceProviderTest.php index dd0e538f..70e751f3 100644 --- a/tests/Unit/Http/SessionServiceProviderTest.php +++ b/tests/Unit/Http/SessionServiceProviderTest.php @@ -9,6 +9,7 @@ use Engelsystem\Http\SessionServiceProvider; use Engelsystem\Test\Unit\ServiceProviderTest; use PHPUnit_Framework_MockObject_MockObject as MockObject; use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage; use Symfony\Component\HttpFoundation\Session\Storage\SessionStorageInterface as StorageInterface; @@ -104,8 +105,16 @@ class SessionServiceProviderTest extends ServiceProviderTest ['driver' => 'pdo', 'name' => 'foobar'] ); - $this->setExpects($app, 'bind', [StorageInterface::class, 'session.storage'], null, $this->atLeastOnce()); + $app->expects($this->atLeastOnce()) + ->method('bind') + ->withConsecutive( + [StorageInterface::class, 'session.storage'], + [SessionInterface::class, Session::class] + ); + $this->setExpects($request, 'setSession', [$session], null, $this->atLeastOnce()); + $this->setExpects($session, 'has', ['_token'], false, $this->atLeastOnce()); + $this->setExpects($session, 'set', ['_token'], null, $this->atLeastOnce()); $this->setExpects($session, 'start', null, null, $this->atLeastOnce()); $serviceProvider->register(); @@ -142,10 +151,16 @@ class SessionServiceProviderTest extends ServiceProviderTest [Session::class, $session], ['session', $session] ); + $app->expects($this->atLeastOnce()) + ->method('bind') + ->withConsecutive( + [StorageInterface::class, 'session.storage'], + [SessionInterface::class, Session::class] + ); - $this->setExpects($app, 'bind', [StorageInterface::class, 'session.storage']); $this->setExpects($app, 'get', ['request'], $request); $this->setExpects($request, 'setSession', [$session]); + $this->setExpects($session, 'has', ['_token'], true); $this->setExpects($session, 'start'); $serviceProvider = new SessionServiceProvider($app); @@ -160,7 +175,7 @@ class SessionServiceProviderTest extends ServiceProviderTest $sessionStorage = $this->getMockForAbstractClass(StorageInterface::class); return $this->getMockBuilder(Session::class) ->setConstructorArgs([$sessionStorage]) - ->setMethods(['start']) + ->setMethods(['start', 'has', 'set']) ->getMock(); } diff --git a/tests/Unit/Middleware/VerifyCsrfTokenTest.php b/tests/Unit/Middleware/VerifyCsrfTokenTest.php new file mode 100644 index 00000000..60280c5b --- /dev/null +++ b/tests/Unit/Middleware/VerifyCsrfTokenTest.php @@ -0,0 +1,128 @@ +getMockForAbstractClass(ServerRequestInterface::class); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + /** @var ResponseInterface|MockObject $response */ + $response = $this->getMockForAbstractClass(ResponseInterface::class); + + $handler->expects($this->exactly(2)) + ->method('handle') + ->with($request) + ->willReturn($response); + + /** @var VerifyCsrfToken|MockObject $middleware */ + $middleware = $this->getMockBuilder(VerifyCsrfToken::class) + ->disableOriginalConstructor() + ->setMethods(['notAuthorizedResponse', 'tokensMatch']) + ->getMock(); + + $middleware->expects($this->exactly(1)) + ->method('notAuthorizedResponse') + ->willReturn($response); + + $middleware->expects($this->exactly(2)) + ->method('tokensMatch') + ->willReturnOnConsecutiveCalls(true, false); + + // Results in true, false, false + $request->expects($this->exactly(3)) + ->method('getMethod') + ->willReturnOnConsecutiveCalls('GET', 'POST', 'DELETE'); + + $middleware->process($request, $handler); + $middleware->process($request, $handler); + $middleware->process($request, $handler); + } + + /** + * @covers \Engelsystem\Middleware\VerifyCsrfToken::__construct + * @covers \Engelsystem\Middleware\VerifyCsrfToken::tokensMatch + */ + public function testTokensMatch() + { + /** @var ServerRequestInterface|MockObject $request */ + $request = $this->getMockForAbstractClass(ServerRequestInterface::class); + /** @var RequestHandlerInterface|MockObject $handler */ + $handler = $this->getMockForAbstractClass(RequestHandlerInterface::class); + /** @var ResponseInterface|MockObject $response */ + $response = $this->getMockForAbstractClass(ResponseInterface::class); + /** @var ResponseInterface|MockObject $noAuthResponse */ + $noAuthResponse = $this->getMockForAbstractClass(ResponseInterface::class); + /** @var SessionInterface|MockObject $session */ + $session = $this->getMockForAbstractClass(SessionInterface::class); + + /** @var VerifyCsrfToken|MockObject $middleware */ + $middleware = $this->getMockBuilder(VerifyCsrfToken::class) + ->setConstructorArgs([$session]) + ->setMethods(['isReading', 'notAuthorizedResponse']) + ->getMock(); + + $middleware->expects($this->atLeastOnce()) + ->method('isReading') + ->willReturn(false); + $middleware->expects($this->exactly(1)) + ->method('notAuthorizedResponse') + ->willReturn($noAuthResponse); + + $handler->expects($this->exactly(3)) + ->method('handle') + ->willReturn($response); + + $request->expects($this->exactly(4)) + ->method('getParsedBody') + ->willReturnOnConsecutiveCalls( + null, + null, + ['_token' => 'PostFooToken'], + ['_token' => 'PostBarToken'] + ); + $request->expects($this->exactly(4)) + ->method('getHeader') + ->with('X-CSRF-TOKEN') + ->willReturnOnConsecutiveCalls( + [], + ['HeaderFooToken'], + [], + ['HeaderBarToken'] + ); + + $session->expects($this->exactly(4)) + ->method('get') + ->with('_token') + ->willReturnOnConsecutiveCalls( + 'NotAvailableToken', + 'HeaderFooToken', + 'PostFooToken', + 'PostBarToken' + ); + + // Not tokens + $this->assertEquals($noAuthResponse, $middleware->process($request, $handler)); + // Header token + $this->assertEquals($response, $middleware->process($request, $handler)); + // POST token + $this->assertEquals($response, $middleware->process($request, $handler)); + // Header and POST tokens + $this->assertEquals($response, $middleware->process($request, $handler)); + } +} diff --git a/tests/Unit/Renderer/Twig/Extensions/CsrfTest.php b/tests/Unit/Renderer/Twig/Extensions/CsrfTest.php new file mode 100644 index 00000000..644e6d50 --- /dev/null +++ b/tests/Unit/Renderer/Twig/Extensions/CsrfTest.php @@ -0,0 +1,63 @@ +createMock(SessionInterface::class); + + $extension = new Csrf($session); + $functions = $extension->getFunctions(); + + $this->assertExtensionExists('csrf', [$extension, 'getCsrfField'], $functions, ['is_safe' => ['html']]); + $this->assertExtensionExists('csrf_token', [$extension, 'getCsrfToken'], $functions); + } + + /** + * @covers \Engelsystem\Renderer\Twig\Extensions\Csrf::getCsrfField + */ + public function testGetCsrfField() + { + /** @var Csrf|MockObject $extension */ + $extension = $this->getMockBuilder(Csrf::class) + ->disableOriginalConstructor() + ->setMethods(['getCsrfToken']) + ->getMock(); + + $extension->expects($this->once()) + ->method('getCsrfToken') + ->willReturn('SomeRandomCsrfToken'); + + $this->assertEquals( + '', + $extension->getCsrfField() + ); + } + + /** + * @covers \Engelsystem\Renderer\Twig\Extensions\Csrf::__construct + * @covers \Engelsystem\Renderer\Twig\Extensions\Csrf::getCsrfToken + */ + public function testGetCsrfToken() + { + /** @var SessionInterface|MockObject $session */ + $session = $this->createMock(SessionInterface::class); + $session->expects($this->once()) + ->method('get') + ->with('_token') + ->willReturn('SomeOtherCsrfToken'); + + $extension = new Csrf($session); + $this->assertEquals('SomeOtherCsrfToken', $extension->getCsrfToken()); + } +}