From 491ee376517cded3c9c8d2389e3f9f21daa1a407 Mon Sep 17 00:00:00 2001 From: Igor Scheller Date: Fri, 28 Dec 2018 03:28:33 +0100 Subject: [PATCH] Don't save sessions permanently on api and metrics paths closes #530 (Session on API calls) --- config/app.php | 2 + src/Middleware/RouteDispatcher.php | 4 +- src/Middleware/SessionHandler.php | 59 ++++++++++++++++++ .../SessionHandlerServiceProvider.php | 24 ++++++++ tests/Unit/Middleware/RouteDispatcherTest.php | 3 +- .../SessionHandlerServiceProviderTest.php | 44 +++++++++++++ tests/Unit/Middleware/SessionHandlerTest.php | 61 +++++++++++++++++++ 7 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 src/Middleware/SessionHandler.php create mode 100644 src/Middleware/SessionHandlerServiceProvider.php create mode 100644 tests/Unit/Middleware/SessionHandlerServiceProviderTest.php create mode 100644 tests/Unit/Middleware/SessionHandlerTest.php diff --git a/config/app.php b/config/app.php index 7ba3509e..17fdee11 100644 --- a/config/app.php +++ b/config/app.php @@ -24,6 +24,7 @@ return [ \Engelsystem\Renderer\TwigServiceProvider::class, \Engelsystem\Middleware\RouteDispatcherServiceProvider::class, \Engelsystem\Middleware\RequestHandlerServiceProvider::class, + \Engelsystem\Middleware\SessionHandlerServiceProvider::class, // Additional services \Engelsystem\Mail\MailerServiceProvider::class, @@ -43,6 +44,7 @@ return [ \Engelsystem\Middleware\ErrorHandler::class, \Engelsystem\Middleware\VerifyCsrfToken::class, \Engelsystem\Middleware\RouteDispatcher::class, + \Engelsystem\Middleware\SessionHandler::class, // Handle request \Engelsystem\Middleware\RequestHandler::class, diff --git a/src/Middleware/RouteDispatcher.php b/src/Middleware/RouteDispatcher.php index 24a7906d..c20eba4b 100644 --- a/src/Middleware/RouteDispatcher.php +++ b/src/Middleware/RouteDispatcher.php @@ -50,7 +50,8 @@ class RouteDispatcher implements MiddlewareInterface $path = $request->getPathInfo(); } - $route = $this->dispatcher->dispatch($request->getMethod(), urldecode($path)); + $path = urldecode($path); + $route = $this->dispatcher->dispatch($request->getMethod(), $path); $status = $route[0]; if ($status == FastRouteDispatcher::NOT_FOUND) { @@ -70,6 +71,7 @@ class RouteDispatcher implements MiddlewareInterface $routeHandler = $route[1]; $request = $request->withAttribute('route-request-handler', $routeHandler); + $request = $request->withAttribute('route-request-path', $path); $vars = $route[2]; foreach ($vars as $name => $value) { diff --git a/src/Middleware/SessionHandler.php b/src/Middleware/SessionHandler.php new file mode 100644 index 00000000..8c53b0fd --- /dev/null +++ b/src/Middleware/SessionHandler.php @@ -0,0 +1,59 @@ +paths = $paths; + $this->session = $session; + } + + /** + * @param ServerRequestInterface $request + * @param RequestHandlerInterface $handler + * @return ResponseInterface + */ + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $return = $handler->handle($request); + + $cookies = $request->getCookieParams(); + if ( + $this->session instanceof NativeSessionStorage + && in_array($request->getAttribute('route-request-path'), $this->paths) + && !isset($cookies[$this->session->getName()]) + ) { + $this->destroyNative(); + } + + return $return; + } + + /** + * @return bool + * @codeCoverageIgnore + */ + protected function destroyNative() + { + return session_destroy(); + } +} diff --git a/src/Middleware/SessionHandlerServiceProvider.php b/src/Middleware/SessionHandlerServiceProvider.php new file mode 100644 index 00000000..aefcb674 --- /dev/null +++ b/src/Middleware/SessionHandlerServiceProvider.php @@ -0,0 +1,24 @@ +app + ->when(SessionHandler::class) + ->needs('$paths') + ->give(function () { + return [ + '/api', + '/ical', + '/metrics', + '/shifts-json-export', + '/stats', + ]; + }); + } +} diff --git a/tests/Unit/Middleware/RouteDispatcherTest.php b/tests/Unit/Middleware/RouteDispatcherTest.php index 611d3b7c..d6ed3408 100644 --- a/tests/Unit/Middleware/RouteDispatcherTest.php +++ b/tests/Unit/Middleware/RouteDispatcherTest.php @@ -32,10 +32,11 @@ class RouteDispatcherTest extends TestCase ->with('HEAD', '/foo!bar') ->willReturn([FastRouteDispatcher::FOUND, $handler, ['foo' => 'bar', 'lorem' => 'ipsum']]); - $request->expects($this->exactly(3)) + $request->expects($this->exactly(4)) ->method('withAttribute') ->withConsecutive( ['route-request-handler', $handler], + ['route-request-path', '/foo!bar'], ['foo', 'bar'], ['lorem', 'ipsum'] ) diff --git a/tests/Unit/Middleware/SessionHandlerServiceProviderTest.php b/tests/Unit/Middleware/SessionHandlerServiceProviderTest.php new file mode 100644 index 00000000..bb4f7e92 --- /dev/null +++ b/tests/Unit/Middleware/SessionHandlerServiceProviderTest.php @@ -0,0 +1,44 @@ +createMock(ContextualBindingBuilder::class); + $app = $this->getApp(['when']); + + $app->expects($this->once()) + ->method('when') + ->with(SessionHandler::class) + ->willReturn($bindingBuilder); + + $bindingBuilder->expects($this->once()) + ->method('needs') + ->with('$paths') + ->willReturn($bindingBuilder); + + $bindingBuilder->expects($this->once()) + ->method('give') + ->willReturnCallback(function (callable $callable) { + $paths = $callable(); + + $this->assertTrue(is_array($paths)); + $this->assertTrue(in_array('/metrics', $paths)); + }); + + $serviceProvider = new SessionHandlerServiceProvider($app); + $serviceProvider->register(); + } +} diff --git a/tests/Unit/Middleware/SessionHandlerTest.php b/tests/Unit/Middleware/SessionHandlerTest.php new file mode 100644 index 00000000..4abd71a6 --- /dev/null +++ b/tests/Unit/Middleware/SessionHandlerTest.php @@ -0,0 +1,61 @@ +createMock(NativeSessionStorage::class); + /** @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); + + $handler->expects($this->exactly(2)) + ->method('handle') + ->with($request) + ->willReturn($response); + + $request->expects($this->exactly(2)) + ->method('getCookieParams') + ->willReturnOnConsecutiveCalls([], ['SESSION' => 'BlaFoo']); + + $request->expects($this->exactly(2)) + ->method('getAttribute') + ->with('route-request-path') + ->willReturn('/foo'); + + $sessionStorage->expects($this->exactly(2)) + ->method('getName') + ->willReturn('SESSION'); + + /** @var SessionHandler|MockObject $middleware */ + $middleware = $this->getMockBuilder(SessionHandler::class) + ->setConstructorArgs([$sessionStorage, ['/foo']]) + ->setMethods(['destroyNative']) + ->getMock(); + + $middleware->expects($this->once()) + ->method('destroyNative') + ->willReturn(true); + + $middleware->process($request, $handler); + $middleware->process($request, $handler); + } +}