From fde8cd6c653b39f5146349eace9bf48fef1e1d81 Mon Sep 17 00:00:00 2001 From: Lee Peuker Date: Tue, 27 Feb 2024 08:33:41 +0100 Subject: [PATCH 1/2] Do not set session and cookies for API requests and correctly invalidate session and cookie on login/logout --- public/index.php | 2 -- settings/routes.php | 2 +- src/Domain/User/Service/Authentication.php | 3 ++- .../Web/Middleware/StartSession.php | 21 +++++++++++++++++++ src/Service/Router/RouterService.php | 10 +++++++-- src/Util/SessionWrapper.php | 12 +++++++++++ 6 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 src/HttpController/Web/Middleware/StartSession.php diff --git a/public/index.php b/public/index.php index d345ed981..b8a6dae7f 100644 --- a/public/index.php +++ b/public/index.php @@ -1,7 +1,5 @@ add('POST', '/add-movie-to-watchlist', [Web\WatchlistController::class, 'addMovieToWatchlist'], [Web\Middleware\UserIsAuthenticated::class]); $routes->add('GET', '/fetchMovieRatingByTmdbdId', [Web\Movie\MovieRatingController::class, 'fetchMovieRatingByTmdbdId'], [Web\Middleware\UserIsAuthenticated::class]); - $routerService->addRoutesToRouteCollector($routeCollector, $routes); + $routerService->addRoutesToRouteCollector($routeCollector, $routes, true); } function addApiRoutes(RouterService $routerService, FastRoute\RouteCollector $routeCollector) : void diff --git a/src/Domain/User/Service/Authentication.php b/src/Domain/User/Service/Authentication.php index 8bf7315e9..4eb2c3bf3 100644 --- a/src/Domain/User/Service/Authentication.php +++ b/src/Domain/User/Service/Authentication.php @@ -221,7 +221,8 @@ public function logout() : void public function setAuthenticationCookieAndNewSession(int $userId, string $token, DateTime $expirationDate) : void { - session_regenerate_id(); + $this->sessionWrapper->destroy(); + $this->sessionWrapper->start(); setcookie(self::AUTHENTICATION_COOKIE_NAME, $token, [ 'expires' => (int)$expirationDate->format('U'), 'path' => '/', diff --git a/src/HttpController/Web/Middleware/StartSession.php b/src/HttpController/Web/Middleware/StartSession.php new file mode 100644 index 000000000..033fac9c9 --- /dev/null +++ b/src/HttpController/Web/Middleware/StartSession.php @@ -0,0 +1,21 @@ +sessionWrapper->start(); + + return null; + } +} diff --git a/src/Service/Router/RouterService.php b/src/Service/Router/RouterService.php index 95a1e8eaf..fbdbafe72 100644 --- a/src/Service/Router/RouterService.php +++ b/src/Service/Router/RouterService.php @@ -3,19 +3,25 @@ namespace Movary\Service\Router; use FastRoute\RouteCollector; +use Movary\HttpController\Web; use Movary\Service\Router\Dto\RouteList; class RouterService { - public function addRoutesToRouteCollector(RouteCollector $routeCollector, RouteList $routeList) : void + public function addRoutesToRouteCollector(RouteCollector $routeCollector, RouteList $routeList, bool $isWebRoute = false) : void { foreach ($routeList as $route) { + $middleware = $route->getMiddleware(); + if ($isWebRoute === true) { + $middleware[] = Web\Middleware\StartSession::class; + } + $routeCollector->addRoute( $route->getMethod(), $route->getRoute(), [ 'handler' => $route->getHandler(), - 'middleware' => $route->getMiddleware() + 'middleware' => $middleware ], ); } diff --git a/src/Util/SessionWrapper.php b/src/Util/SessionWrapper.php index 053a3883d..2c3c5ccec 100644 --- a/src/Util/SessionWrapper.php +++ b/src/Util/SessionWrapper.php @@ -6,7 +6,19 @@ class SessionWrapper { public function destroy() : void { + $_SESSION = array(); + + if (ini_get('session.use_cookies')) { + $params = session_get_cookie_params(); + setcookie( + session_name(), '', time() - 42000, + $params['path'], $params['domain'], + $params['secure'], $params['httponly'], + ); + } + session_destroy(); + session_regenerate_id(); } public function find(string $key) : mixed From 26baaca1de310608bc6342bbc467dd795da691f9 Mon Sep 17 00:00:00 2001 From: Lee Peuker Date: Tue, 27 Feb 2024 08:37:44 +0100 Subject: [PATCH 2/2] Fix type issue --- src/Util/SessionWrapper.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Util/SessionWrapper.php b/src/Util/SessionWrapper.php index 2c3c5ccec..057f7ebab 100644 --- a/src/Util/SessionWrapper.php +++ b/src/Util/SessionWrapper.php @@ -9,11 +9,21 @@ public function destroy() : void $_SESSION = array(); if (ini_get('session.use_cookies')) { + $sessionName = session_name(); + if ($sessionName === false) { + throw new \RuntimeException('Could not get session name'); + } + $params = session_get_cookie_params(); + setcookie( - session_name(), '', time() - 42000, - $params['path'], $params['domain'], - $params['secure'], $params['httponly'], + $sessionName, + '', + time() - 42000, + $params['path'], + $params['domain'], + $params['secure'], + $params['httponly'], ); }